bugfix: Revert death module consolidation regressions

[Stubbjax]

This change is a follow-up to #118 and reverts death module consolidation for objects that did not originally have a DestroyDie module defined, as well as several InstantDeathBehavior modules that were not gated by RequiredStatus. As a result, the following notable behaviours are restored:

• The Aurora Alpha's fuel-air bomb deals damage on its initial impact
• The Repair Bay tech building leaves rubble when destroyed
• The Nuclear Bunker building (located in the corners of Defcon 6) leaves rubble when destroyed
• Fake Arms Dealers, fake Barracks, fake Command Centers and fake Supply Stashes deal damage when detonated

Most of the other affected objects are cinematic and never die in the first place or have unused data. Note that the RebuildHoleBehavior module destroys its object in its onDie method, making any death modules superfluous, and the BridgeBehavior module similarly handles its own death behaviour.

This also means that several objects originally missing a DestroyDie module that were indirectly fixed by #118 (e.g. crushing an empty Troop Crawler would leave an indestructible hulk) are now broken again, though these can be fixed separately.

[xezon]

Is there a technical explanation for why these bugs occur? Maybe there is a code bug in the new module or are the misbehavior warranted?

[Stubbjax]

Is there a technical explanation for why these bugs occur? Maybe there is a code bug in the new module or are the misbehavior warranted?

The objects affected by this change did not originally have a DestroyDie module, which meant that the objects (typically) kept existing past their death. This is most notable with the destroyed buildings, which we want to persist in a 'rubble' state rather than being deleted from the world upon destruction.

The Aurora Bomb is set up in a funky way where the damage is only applied when the bomb impacts the ground rather than the target, so deleting it upon impacting the target caused it to never apply its initial impact damage.

[xezon]

So if I understand right InstantDeathBehavior acts differently depending on a DestroyDie module?

This is quite a bit confusing to keep track of. To me it sounds like InstantDeathBehavior is inferior to the other death modules then because of this dependency. Perhaps we should not use this module then?

[Stubbjax]

So if I understand right InstantDeathBehavior acts differently depending on a DestroyDie module?

This is quite a bit confusing to keep track of. To me it sounds like InstantDeathBehavior is inferior to the other death modules then because of this dependency. Perhaps we should not use this module then?

The original intent was to combine DestroyDie + FXListDie + CreateObjectDie. However, I mistakenly combined any combination when at least two existed. This meant that FXListDie and CreateObjectDie ended up being combined into an InstantDeathBehavior, which was erroneous as it effectively added the DestroyDie's behaviour.

[xezon]

Should we revert all of them to restore exact original behavior? To me it sounds like something else might be overlooked if we just revert part of it.

[Stubbjax]

Should we revert all of them to restore exact original behavior? To me it sounds like something else might be overlooked if we just revert part of it.

I effectively reverted the original change and then recombined everything using the new rule where a DestroyDie module is a prerequisite for generating an InstantDeathBehavior module. This PR makes up the difference.

I counted the number of objects with either a DestroyDie or InstantDeathBehavior module before and after each change. The counts are as follows:

PR #117 = 1,222
PR #118 = 1,267
PR #172 = 1,221*

This effectively means that no InstantDeathBehavior modules have been introduced where there wasn't originally a DestroyDie module as of this change.

*The singular 1,222 → 1,221 object discrepancy is caused by Nuke_ChinaCarpetBomb, in which its SlowDeathBehavior absorbed the FireWeaponWhenDeadBehavior, FXListDie, and DestroyDie modules as it defines no DeathTypes or any other fields that would preclude them from consolidation. (The original SlowDeathBehavior really should have been a CreateObjectDie module.) This is the only such case. We can streamline the respective SlowDeathBehavior into an InstantDeathBehavior in a follow-up change if desired, as the behaviour between the two modules is identical in this case.

What else do you think could be overlooked?

[xezon]

I lost the track here and do not understand. So the original issue was that the differences between the various death effect modules was not well understood and it is unclear to me if they are now 100% understood. The safest strategy would be to revert all death module consolidations to the original state, which will restore 100% original behavior.

How confident are we that the current change with a partly reverted consolidations restores 100% original functionality? Is the confidence 100%, if yes, why, and if not, why not play it safe and revert its entirety?

[xezon]

I asked Claude Opus:

Q:

Breakdown similarities and differences of modules CreateObjectDie, DestroyDie, FireWeaponWhenDeadBehavior, FXListDie in relation to InstantDeathBehavior. In particular show under what circumstances the former 4 modules can be consolidated into a InstantDeathBehavior module.

A:

Die Module Consolidation: CreateObjectDie, DestroyDie, FireWeaponWhenDeadBehavior, FXListDie vs InstantDeathBehavior

Analysis based on the GeneralsMD tree. The Generals tree is identical except where noted.

Sources consulted: the five modules, plus DieModule / DieMuxData, Object::onDie,
ObjectCreationList nuggets, FXList::doFXObj / doFXPos, and GameLogicRandomValue.

1. What they all share

Module	Base classes	Module data
DestroyDie (DestroyDie.h:39)	DieModule	inherits DieModuleData (no own fields)
CreateObjectDie (CreateObjectDie.h:59)	DieModule	CreateObjectDieModuleData
FXListDie (FXListDie.h:76)	DieModule + UpgradeMux	FXListDieModuleData
FireWeaponWhenDeadBehavior (FireWeaponWhenDeadBehavior.h:72)	BehaviorModule + UpgradeMux + DieModuleInterface	FireWeaponWhenDeadBehaviorModuleData (holds its own DieMuxData)
InstantDeathBehavior (InstantDeathBehavior.h:61)	DieModule	InstantDeathBehaviorModuleData

All five are driven from the same place — Object.cpp:4697-4702 walks the behavior list in
INI declaration order and calls onDie on every module exposing getDie().

All five gate on the identical DieMuxData field set (DeathTypes, VeterancyLevels,
ExemptStatus, RequiredStatus) via DieModule.cpp:66. FireWeaponWhenDeadBehavior gets
them by embedding DieMuxData directly, since it is not a DieModule.

None of them persist per-instance state except the two UpgradeMux ones, which xfer their
upgrade flag.

So InstantDeathBehavior really is the union of the other four's core actions, in a fixed
order — FX -> OCL -> weapon -> destroy (InstantDeathBehavior.cpp:122-196).

2. Where the semantics diverge

a) All-of vs one-of

FX, OCL and Weapon on InstantDeathBehavior are vectors, and exactly one entry per
category is picked at random each death. Two FXListDie modules both play their FX; one
InstantDeathBehavior with two FX entries plays one of them. Collapsing N modules into N
list entries is a behavior change unless N = 1.

b) FX invocation

InstantDeathBehavior always calls FXList::doFXObj(fxl, obj, nullptr). That equals
FXListDie with OrientToObject = Yes (the default) and a null damage dealer.
OrientToObject = No is not representable — it routes to doFXPos(pos) with a null transform
matrix and a different shroud test (FXList.cpp:806-833). FX nuggets that consume the
secondary position also lose the killer.

c) OCL invocation

CreateObjectDie resolves damageInfo->in.m_sourceID and passes the damage dealer as the OCL
secondary (CreateObjectDie.cpp:96-98); InstantDeathBehavior passes nullptr.
FireWeaponNugget, AttackNugget and DeliverPayloadNugget all bail out with
DEBUG_CRASH("You must have a primary and secondary source for this effect") on a null
secondary. GenericObjectCreationNugget (ObjectCreation / Debris) and
ApplyRandomForceNugget ignore it and are safe.

d) CreateObjectDie extras

TransferPreviousHealth and TransferSelection (Zero Hour only — Generals has neither)
operate on the object returned by the OCL: subdual + health transfer, attacker retargeting,
and re-selection. InstantDeathBehavior discards the return value entirely, so neither is
expressible.

e) Under-construction guard

FireWeaponWhenDeadBehavior refuses to fire while OBJECT_STATUS_UNDER_CONSTRUCTION is set,
so cancelling a scaffold does not detonate it. InstantDeathBehavior only got that guard
behind #if !RETAIL_COMPATIBLE_CRC (InstantDeathBehavior.cpp:162-166) — in a
retail-compatible build the consolidated version will fire on a cancelled scaffold.

f) Upgrade gating

FXListDie and FireWeaponWhenDeadBehavior are UpgradeMux: StartsActive, TriggeredBy,
ConflictsWith, RequiresAllTriggers, plus a conflicting-mask test against both the
object's and the controlling player's completed upgrade masks. InstantDeathBehavior has none
of this. Note the differing defaults: FXListDie starts active (m_initiallyActive = TRUE,
the "847 cases" hack), FireWeaponWhenDeadBehavior starts inactive.

g) Destruction is unconditional

Only DestroyDie and InstantDeathBehavior call TheGameLogic->destroyObject. Folding an
FXListDie / CreateObjectDie / FireWeaponWhenDeadBehavior into an InstantDeathBehavior
adds a destroy the object previously did not have at that point.

h) AI dead-state handshake

InstantDeathBehavior uniquely calls markAsDead() — which also sets
setEffectivelyDead(TRUE) and wakes the AI — and skips if isAiInDeadState(). Under
RETAIL_COMPATIBLE_CRC that check runs first, so on an object with an AIUpdate whose AI is
already dead the whole module (FX, OCL, weapon) is skipped; in modern builds the check moved to
the end so only the destroy is guarded. None of the other four touch AI state.

i) RNG stream

In a RETAIL_COMPATIBLE_CRC build GameLogicRandomValue(0, 0) still draws from the logic seed
(delta == 1); in modern builds the lo >= hi early-out returns without drawing
(RandomValue.cpp:268-290). So even a single-entry consolidation perturbs the logic RNG stream
in retail-compat builds — a replay/CRC hazard, not just cosmetic.

j) Ordering

Four modules occupy four slots in the behavior list; the consolidated module occupies one.
destroyObject runs the onDestroy modules immediately, sets OBJECT_STATUS_DESTROYED and
clears the AI locomotor/path before deferring the actual delete, so later die modules in the
list still run but observe a destroyed object.

3. Consolidation checklist

CreateObjectDie + DestroyDie + FireWeaponWhenDeadBehavior + FXListDie -> one
InstantDeathBehavior is behavior-preserving only when all of these hold:

1. Identical die filter across every module being merged — same DeathTypes,
   VeterancyLevels, ExemptStatus, RequiredStatus. The merged module has exactly one
   filter.
2. At most one module of each kind, so there is no all-of -> one-of regression.
3. A DestroyDie is among them with that same filter, and no SlowDeathBehavior /
   KeepObjectDie-style handler needs the object to survive onDie. If there is no
   DestroyDie, do not consolidate — you would be adding destruction.
4. FXListDie: OrientToObject = Yes, StartsActive = Yes, no TriggeredBy /
   ConflictsWith, and its FXList does not rely on the secondary (source) object.
5. CreateObjectDie: TransferPreviousHealth = No, TransferSelection = No, and the OCL
   contains only ObjectCreation / Debris / ApplyRandomForce nuggets — no FireWeapon,
   Attack or DeliverPayload nuggets, which need the secondary.
6. FireWeaponWhenDeadBehavior: StartsActive = Yes with no TriggeredBy /
   ConflictsWith. Since the default is inactive, an upgrade-gated one can never be folded in.
7. AI: either the object has no AIUpdate, or the added markAsDead() / dead-state gate is
   acceptable — and there is no other InstantDeathBehavior or SlowDeathBehavior on the
   object contending for that flag.
8. Ordering: the merged modules are not interleaved with other die modules that observe
   object state between them, and none of the survivors care about OBJECT_STATUS_DESTROYED
   being set earlier than before.
9. Build: not RETAIL_COMPATIBLE_CRC, if bit-exact retail replay/CRC parity matters —
   points (e), (h) and (i) all diverge there.

Safe common cases

• The plain FXListDie (single DeathFX, defaults) + DestroyDie pair on a simple unit.
• CreateObjectDie (plain debris/object OCL) + FXListDie + DestroyDie.

Both collapse to a single InstantDeathBehavior with one entry per list.

Cases that cannot be consolidated

• The sneak-attack tunnel (TransferPreviousHealth / TransferSelection).
• Anything whose death OCL fires a weapon or delivers a payload at the killer.
• Upgrade-gated death explosions.
• Anything with OrientToObject = No.

The reverse direction

An InstantDeathBehavior with multi-entry lists cannot be decomposed back into the four
modules at all, because the random one-of selection has no equivalent there.

[Stubbjax]

I lost the track here and do not understand. So the original issue was that the differences between the various death effect modules was not well understood and it is unclear to me if they are now 100% understood.

The original issue is that I mistakenly combined any combination of DestroyDie, FXListDie, CreateObjectDie and FireWeaponWhenDeadBehavior into InstantDeathBehavior. This was because I combined them if at least two existed, rather than requiring DestroyDie + at least one other. This was an error in logic rather than understanding.

InstantDeathBehavior expands to DestroyDie + FXListDie + CreateObjectDie + FireWeaponWhenDeadBehavior. However, it inherits DestroyDie by default, while the other modules are assigned as fields. This means that if any of these modules were combined without a DestroyDie, the DestroyDie behaviour would still be inherited via the added InstantDeathBehavior module.

These two objects behave the same:

Object Example_A1
  Behavior = DestroyDie
  Behavior = FXListDie
  Behavior = CreateObjectDie
  Behavior = FireWeaponWhenDeadBehavior
End

Object Example_A2
  Behavior = InstantDeathBehavior
End

These two objects do not behave the same:

Object Example_B1
  Behavior = FXListDie
  Behavior = CreateObjectDie
  Behavior = FireWeaponWhenDeadBehavior
End

Object Example_B2
  Behavior = InstantDeathBehavior
End

The consolidated Example_B2 now has DestroyDie by extension. This is a change in behaviour, and now means the object will be deleted when health is reduced to 0, which is not the case for Example_B1. This is what happened to the 46 affected objects restored in the first commit.

The second commit address the fact that I missed applying the RequiredStatus field to the list of conflicting field names, which caused the SlowDeathBehavior on the fake buildings to not run due to being preceded and aborted by the InstantDeathBehavior.

How confident are we that the current change with a partly reverted consolidations restores 100% original functionality? Is the confidence 100%, if yes, why, and if not, why not play it safe and revert its entirety?

I am confident this will restore original behaviour. However, I would agree that reverting the entire consolidation change for now on the release-1 branch would be safest.

[xezon]

I am confident this will restore original behaviour. However, I would agree that reverting the entire consolidation change for now on the release-1 branch would be safest.

Ok. Please review the LLM generated info if there are any additional caveats for us to consider, and then make your decision.