Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix 6434 and 3890: pilot rolls reported before all attacks #6439

Conversation

Sleet01
Copy link
Collaborator

@Sleet01 Sleet01 commented Jan 26, 2025

This set of fixes cleans up some of what appear to mistakes in how certain critical effects and PSR results are reported.

The issue is that a number of functions within TWGameManager.java used to write directly to vPhaseReport (the class member, not the various identically named local Vector instances, thanks for that) or use the addReport() function to the same effect, while most of the rest of the code has been updated to pass a per-Phase Vector of reports around, or compose one from the results of calling all the various handling functions.

This fix should clear up two specific instances of Pilot Skill checks appearing in the phase report log prior to the attacks that actually caused them.

NOTE: there are likely many more such functions within TWGameManager.java that need to be examined, and possibly updated:

  • Any function that can be called within the Movement Phase and the Attack Handling Phase; that
  • returns a non-Vector result, and/or
  • calls addReport or vPhaseReport.addAll() on the class member itself.

Unfortunately, there are some 700+ such locations to check within the class.

Prior behaviour:
Image
Image

Updated behaviour:
Screenshot_20250125_223526

Testing:

Close #3890 #6434

Copy link

codecov bot commented Jan 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 28.46%. Comparing base (732e576) to head (a0765d0).
Report is 17 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6439      +/-   ##
============================================
- Coverage     28.46%   28.46%   -0.01%     
  Complexity    14199    14199              
============================================
  Files          2797     2798       +1     
  Lines        274834   274846      +12     
  Branches      48632    48624       -8     
============================================
+ Hits          78231    78234       +3     
- Misses       192531   192541      +10     
+ Partials       4072     4071       -1     

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

@Sleet01 Sleet01 requested a review from SJuliez January 26, 2025 07:54
@Sleet01 Sleet01 force-pushed the Fix_6434_Pilot_Rolls_reported_before_all_attacks branch from f89d00d to a0765d0 Compare January 26, 2025 19:32
@Sleet01 Sleet01 merged commit 5510a28 into MegaMek:master Jan 26, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Squadron fighters being destroyed before damage is reported about their squadron 0.49.9
2 participants