Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Issue 6524: Prevent weapon-malfunction-crits from being unjammed via Unjam RAC (movement phase) & prevent NPE if unit unjams rapid-fire weapon and flees #6528

Merged
merged 2 commits into from
Feb 9, 2025

Conversation

psikomonkie
Copy link
Collaborator

Two for one bonus special!

Fixes #6524

There are two issues at play here:

  • An entity is trying to unjam an RAC on the same turn that its fled the map. Once an entity flees, it is null, so the unjam step cannot be properly processed.
    • This is handled by adding a null entity check in resolveUnjam.
    • I suspect a similar bug will occur for other cases in TWGameManager::resolveAllButWeaponAttacks. I considered putting the null entity check there, but several of these actions still need to be processed even if the entity has fled, like clearing a minefield or activating an AP pod. I think we should handle these cases as they occur.
    • This only became an issue when we enabled fleeing at the end of movement. Prior to that a unit would never be null before processing its unjam step.
  • Tanks were able to unjam "weapon malfunction" crits (TW p. 195) in the movement phases using the "Unjam RAC" which is meant to be used for the RAC or other rapid fire weapons if the setting is enabled.
    • Tank weapons that are jammed because of that are added to a special list that only tanks have. When checking if a weapon can be unjammed in the manner of rapid-fire weapons we first check if the weapon was jammed from a weapon malfunction crit - if it was we can't unjam it using this action.

Copy link

codecov bot commented Feb 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 28.56%. Comparing base (a4f835c) to head (5f53114).
Report is 118 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6528      +/-   ##
============================================
- Coverage     28.56%   28.56%   -0.01%     
- Complexity    14495    14496       +1     
============================================
  Files          2815     2815              
  Lines        277388   277405      +17     
  Branches      49002    49001       -1     
============================================
+ Hits          79226    79227       +1     
- Misses       193446   193461      +15     
- Partials       4716     4717       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@Sleet01 Sleet01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@HammerGS HammerGS merged commit 7108464 into MegaMek:master Feb 9, 2025
6 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Issue] Uncaught java.lang.NullPointerException at beginning of firing phase
3 participants