Fix: Pilots from ammo-detonated OpFor units missing in AAR (#6497)#8857
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes an AAR prisoner-capture tracking gap where OpFor units destroyed via ammo detonation (i.e., “devastated” entities) were not being processed in ResolveScenarioTracker.processGame(), causing their pilots/crew to be absent from the capturable personnel list.
Changes:
- Extend
ResolveScenarioTracker.processGame()to process enemy devastated entities by recording ejected crews and tracking completely destroyed enemy units for prisoner capture. - Add a unit test to ensure enemy devastated entities are added to
devastatedEnemyUnitsand registered insalvageStatus.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| MekHQ/src/mekhq/campaign/ResolveScenarioTracker.java | Adds processing for enemy devastated entities so crews can be considered for prisoner capture (mirrors MUL-loading behavior). |
| MekHQ/unittests/mekhq/campaign/ResolveScenarioTrackerTest.java | Introduces a regression test validating devastated enemy units are tracked for downstream prisoner-capture processing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8857 +/- ##
============================================
+ Coverage 13.33% 13.42% +0.09%
- Complexity 7966 8007 +41
============================================
Files 1309 1310 +1
Lines 169452 169507 +55
Branches 25511 25524 +13
============================================
+ Hits 22588 22761 +173
+ Misses 144642 144479 -163
- Partials 2222 2267 +45 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
IllianiBird
left a comment
There was a problem hiding this comment.
This looks right but this area of the code is notoriously cranky so I recommend flagging for testing with the Test Pilots when it's merged.
Summary
Enemy pilots whose units were destroyed by ammo detonation were absent from the capturable personnel list in the After Action Report (AAR).
Root Cause
In
ResolveScenarioTracker.processGame(), entities are categorized into three post-battle groups:getEntities()) — enemy units correctly added topotentialSalvagegetGraveyardEntities()) — enemy units correctly added topotentialSalvagegetDevastatedEntities()) — enemy units not processed at allUnits destroyed by ammo explosion are flagged as
REMOVE_DEVASTATEDand go into the devastated category. Since there was noelse ifbranch for enemy devastated entities, their crews were never fed intoprocessPrisonerCapture(), making those pilots invisible in the AAR.The alternate code path (
loadUnitsAndPilots(), used when loading from MUL files) already handled this correctly, with an explicit comment: "completely destroyed units (such as from an ammo explosion) need to be kept track of, as mekwarriors may eject from them, etc."Fix
Added an
else ifbranch for enemy devastated entities inprocessGame()that:EjectedCrewinstances toenemyEjections(matching the graveyard handling pattern)TestUnitfor other enemy units and adds them todevastatedEnemyUnits, allowingprocessPrisonerCapture()to generate capturable personnel from their crews (similar to loadUnitsAndPilots())Testing
Couldn't repro with the original campaign in the bug. I migrated it to 50.06, then 50.11 and then open in my local environment, but the save game attached for the battle does not have the conditions to repro the issue. The other saves attached to the bug are missing the campaign information to be usable.
So added
ResolveScenarioTrackerTestwith a test that mocksPostGameResolution.getDevastatedEntities()returning an enemy entity and verifies it ends up indevastatedEnemyUnitsandsalvageStatus. The test fails without the fix and passes with it.Added also a second test to test the ejection code path based on code review feedback.