[Issue] Results of VTOL Pilot hit playing out before the pilot hit actually happens.

[RaozSpaz]

Brief Description *

The first image is at the start of the weapon phase. Showing the VTOL crashing due to a failed piloting roll. The second image is at the very end of the weapon phase where the pilot finally gets hit.

There is another issue going on here but I will post that in another issue.

3. Steps to Reproduce

No response

Attach Files

No response

Severity *

Medium (Gameplay Limitation): Non-core functionality is impaired, providing a suboptimal but playable experience.

MegaMek Suite Version *

50.03 Nightly

Operating System *

Windows 10

Java Version *

17.0.12

Final Verification

• I confirm this is a single, unique issue that hasn't been reported before
• I have included all necessary information and files to help reproduce this issue
• I have asked on MegaMek Discord about this issue
• I have confirmed this issue is being opened on the correct repository: MegaMek, MegaMekLab, or MekHQ

[RaozSpaz]

Something I noticed to add to this issue. The 'crash' does seem to be processing with the information from the attack having played out. This can be noted by the frontal armor being reduced to 25 from the Manticore attack, and then reduced to 20 at the start of the crash.
Not sure if that information will be helpful to figuring out the problem.

[Sleet01]

I haven't taken a look yet, but from my recollection this is because, when an attack causes a critical, we jump into the critical processing code prior to writing the report for the attack itself. That is, our processing looks something like:

• See if attack hit
• • See if attack did damage
• • • See if attack did critical
• • • • Process critical effect
• • • • • Process additional damage from critical effect
• • • • • Print results of critical effect damage
• • • • Print results of critical effect
• Print attack hit
• • Print attack damage results

Likely because the critical effect code is self-contained. What we may want to do is either:

1. delay processing the critical until after the damage is printed (we should already have rolled it at this point)
2. refactor the critical effect code to return a report rather than writing it.

#1 would be the simplest but most brittle; #2 would give us more flexibility but require a lot of work.

[Sleet01]

...or maybe not?
At least from inspecting the code, it looks like we are maintaining ordering in all the calls to apply critical effects, so that report should only appear after the attack and damage which actually caused it.

@RaozSpaz I don't suppose you have any saves from this game?

[RaozSpaz]

Dangit. No. I knew I was forgetting something.

[gsparks3]

Potentially (?) related to a similar issue with fighters inside squadrons being destroyed showing up at the top of the report: #3890

[Sleet01]

I think see the mechanism that is causing Pilot Roll reports (not PSRs! Not any other rolls! Just Pilots!)

The weird thing is that it looks like it should still put the reports in the proper order, but I think I see a way to ensure they're never added out of order.

[RaozSpaz]

Trying to replicate the Pilot hit effect XD Got an engine crit which forced me to the dirt and that rolled properly. And was placed properly.

[RaozSpaz]

Save.zip

Got it.
The folder includes the custom unit being used for the test, the log, and an autosave right before and right after the attack. Not certain if it will actually help you any [since its all RNG anyways] but there you go.

[Sleet01]

JMFGDCIAMFB! This bug!

The issue here is, in the process of handling just Inferno warhead damage, we jump into applyTankCrit() (natch, since Infernos crit Tanks, q.e.d.), which then calls the function, doSkillCheckInPlace(). I suspect this may be a mistake, because the function itself is commented,

 * Do a piloting skill check while standing still (during the movement
 * phase).

which is absolutely not what we are doing at this point.

At any rate, the insidious thing is that this function, since it is meant to be called during the movement phase when each entity's movement is evaluated linearly and in order, writes directly to the phase report instead of compiling its updates and returning them to be added to the complete turn report (as all the attack handling code does).
But when we do this during attack handling, prior to writing all the attack reports... it shoots to the top of the (currently empty) phase report.

I've made some minimal updates to ensure this doesn't happen, and possibly also make displacement reports cleaner (they look broken in the same way).
As the lord Cthulhu is my witness, I hope to never touch the reporting code again... until I help re-write it as a small Python script >_<;

[Sleet01]

Got it. The folder includes the custom unit being used for the test, the log, and an autosave right before and right after the attack. Not certain if it will actually help you any [since its all RNG anyways] but there you go.

This, by the way, was an immense help because I was able to repro the bug more reliably by dumping the team MULs from this save, doubling them, and then just running masses of test runs one after another. Thanks again!

[RaozSpaz]

Got it. The folder includes the custom unit being used for the test, the log, and an autosave right before and right after the attack. Not certain if it will actually help you any [since its all RNG anyways] but there you go.

This, by the way, was an immense help because I was able to repro the bug more reliably by dumping the team MULs from this save, doubling them, and then just running masses of test runs one after another. Thanks again!

No problem. Glad it helped.

[Sleet01]

Closed by merge of #6439