bugfix: Detonated GLA Demo Battle Buses are no longer instantly deleted without firing their death weapons

[Stubbjax]

This change fixes an issue where the GLA Demo Battle Bus would not trigger any death behaviours besides SlowDeathBehavior modules when initially killed via an unhandled death type. This resolves the issue where a SUICIDED death would instantly delete the Battle Bus without dealing any suicide damage if the Demolitions upgrade was researched.

Before

The Battle Bus deals no damage to surrounding enemies when suicided

BEFORE.mp4

After

The Battle Bus now deals damage to surrounding enemies when suicided

AFTER.mp4

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Ensure Battle Bus detonations trigger all death behaviors

🐞 Bug fix 🕐 20-40 Minutes

AI Description

• Distinguishes Battle Bus second-life transitions from normal slow-death selections.
• Routes normal selections through the full death pipeline, firing all configured behaviors.
• Stops second-life processing when damage side effects already killed the object.

Diagram

graph TD
  A["Lethal damage"] --> B["Undead body"] --> C["Active damage"] --> D{"Object dead?"}
  D -- Yes --> E["Stop processing"]
  D -- No --> F["Death selector"]
  F -- "Bus first death" --> G["Second life"]
  F -- "Real death" --> H["Die callbacks"]

Loading

High-Level Assessment

The polymorphic capability check is appropriate because the selected behavior knows whether it represents the Battle Bus transition or a real death. Re-entering ActiveBody's damage pipeline also preserves established health, scoring, and onDie dispatch semantics; duplicating those callbacks in UndeadBody would be riskier.

Files changed (3) +19 / -0

Bug fix (3) +19 / -0

BattleBusSlowDeathBehavior.hExpose Battle Bus death-transition state +1/-0

Expose Battle Bus death-transition state

• Overrides the new death classification method using the behavior's existing real-death state. This preserves the specialized first-death transition while allowing later deaths to use normal processing.

GeneralsMD/Code/GameEngine/Include/GameLogic/Module/BattleBusSlowDeathBehavior.h

SlowDeathBehavior.hAdd real-death classification to slow-death behaviors +2/-0

Add real-death classification to slow-death behaviors

• Extends the slow-death interface with a real-death query. Standard slow-death behaviors default to real deaths so UndeadBody can route them through complete death handling.

GeneralsMD/Code/GameEngine/Include/GameLogic/Module/SlowDeathBehavior.h

UndeadBody.cppRoute unhandled second-life deaths through full death processing +16/-0

Route unhandled second-life deaths through full death processing

• Stops second-life handling if the initial damage application already killed the object. When selection resolves to a regular slow-death behavior, it forces death through ActiveBody so every onDie module—including Demolitions death weapons—executes.

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Body/UndeadBody.cpp

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[greptile-apps]

Greptile Summary

This PR changes Battle Bus second-life handling so that second-life-specific slow-death behaviors are selected separately from real-death behaviors, while falling back to a genuine death when no second-life behavior applies.

• Adds an explicit second-life routing flag to damage information.
• Separates slow-death probability calculation from behavior execution.
• Defers applying second-life health and armor until an applicable behavior has been selected.
• Preserves real-death handling for unsupported death types.
• The previous probability-selection finding was manually resolved.

Confidence Score: 4/5

The behavioral fix appears sound, but the explicit repository alignment requirement must be satisfied before merging.

The previous probability-selection thread was manually resolved without explanation, and no new behavioral or security failure was established. The remaining issue is the unaligned m_enterSecondLife declaration, which violates an explicit repository formatting requirement.

Files Needing Attention: Core/GameEngine/Include/GameLogic/Damage.h

Important Files Changed

Filename	Overview
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Body/UndeadBody.cpp	Reorders lethal-damage handling to select an applicable second-life behavior before applying the second-life state, with real-death fallback.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Behavior/SlowDeathBehavior.cpp	Centralizes probability summation and filters slow-death behaviors according to whether damage is entering second life.
Core/GameEngine/Include/GameLogic/Damage.h	Adds the second-life routing flag, but its declaration violates the repository’s alignment convention.
GeneralsMD/Code/GameEngine/Include/GameLogic/Module/SlowDeathBehavior.h	Extends the slow-death interface with real-death classification and shared probability calculation.
GeneralsMD/Code/GameEngine/Include/GameLogic/Module/BattleBusSlowDeathBehavior.h	Exposes the Battle Bus behavior’s current real-death state for applicability filtering.
GeneralsMD/Code/GameEngine/Include/GameLogic/Module/UndeadBody.h	Splits second-life selection from application and reports whether selection succeeded.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Lethal first-life damage] --> B[Mark request as entering second life]
    B --> C[Find applicable second-life slow-death behaviors]
    C -->|At least one| D[Apply capped damage]
    D --> E[Install second-life health and armor]
    E --> F[Begin selected slow-death behavior]
    C -->|None| G[Clear second-life marker and force real death]
    G --> H[Run ordinary death behaviors]

Loading

Prompt To Fix All With AI

### Issue 1
Core/GameEngine/Include/GameLogic/Damage.h:290
**Unaligned Member Declaration**

The new `m_enterSecondLife` member does not align with the surrounding type, name, and comment columns. This violates the repository directive to preserve column alignment within a modified data-oriented block, so the requirement must be satisfied before merging.

```suggestion
	Bool                     m_enterSecondLife;
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (3): Last reviewed commit: "Improve battle bus second life impl (zer..." | Re-trigger Greptile}

[xezon]

I am confused by this implementation. The function says startSecondLife, so we now expect a second life with health restore, armor set change, ..., but here it will attempt the kill afterwards. Should this code perhaps be executed before this function is called? Aka shouldStartSecondLife is false when suicided?

The reason I am raising this is, the code reads confusing this way, regardless whether functionally it is working correctly in game.

[Stubbjax]

This implementation follows the existing setup where a death type handled by a different SlowDeathBehavior (not BattleBusSlowDeathBehavior) would immediately kill the unit. Instead of manually calling the SlowDeathBehavior modules' death logic, they are now intrinsically called through the damage → death path, and only if there's no SlowDeathBehavior to handle it.

Explicitly checking for SUICIDED or any such death types is not as clean or flexible because a mod might use a custom setup where the death types are handled differently. For example, someone might make a zombie unit that immediately dies without rising again if it dies via a HOLY death type.

Perhaps the naming is the source of the confusion. We already have attemptDamage - would attemptSecondLife be more suitable?

[xezon]

It is not just the function name. Look at all the things that happen before the suicide:

m_isSecondLife = TRUE;

setMaxHealth(data->m_secondLifeMaxHealth, FULLY_HEAL);

setArmorSetFlag(ARMORSET_SECOND_LIFE);

So it fully prepares for a second life, but then it dies. How does that make sense?

[Stubbjax]

Perhaps it does not make sense, but this is how the retail logic already works. Whether the original logic makes sense is secondary to this fix, which I intended to involve minimal refactoring for safety and simplicity. Would you prefer I move all of those lines below the death logic?

[xezon]

Yes I think the suicide kill should be before it decides to go to second life.

[xezon]

I would like to push a commit to this branch.

[xezon]

I have applied additional edits to this change in order to try bring it into a state that makes more logical sense. I have done only limited testing with RETAIL_COMPATIBLE_CRC=0. I did not test VC6 and RETAIL_COMPATIBLE_CRC=1. Edits were made by hand without AI usage.

[Stubbjax]

I have applied additional edits to this change in order to try bring it into a state that makes more logical sense. I have done only limited testing with RETAIL_COMPATIBLE_CRC=0. I did not test VC6 and RETAIL_COMPATIBLE_CRC=1. Edits were made by hand without AI usage.

I'm not a fan of adding data for a specific death implementation to the damage class, which should be generic / application-agnostic. We can already determine whether a second life will be entered from the modules themselves, so why do damage instances now need to pass this information?

This new approach also effectively couples and enforces the BattleBusSlowDeathBehavior as a requirement for the UndeadBody module to take effect. The original fix (and retail behaviour) allows the UndeadBody to act independently - the object will still enter its second life if no BattleBusSlowDeathBehavior module is defined or the death type is unhandled, whereas now it will just die. Is this desirable? This will break any mods that rely on this behaviour.

[xezon]

The DamageInfoInput field is necessary to properly tell the death module what death we want. If the death module cannot service Second Life, then it will not be picked as a candidate, and it will look for other candidates. Only the BattleBusSlowDeathBehavior can do second life. To me this looks logical.

If we need Second Life to be death module independent, then it needs a different strategy. What would be the use case for that?

Edit: Ok I see UndeadBody is supposed to drive Second Life, not the Slow Death module.

  Body = UndeadBody ModuleTag_02 ; Body module that treats the first death as a state change
    MaxHealth = 400.0
    InitialHealth = 400.0

    SubdualDamageCap = 750.0
    SubdualDamageHealRate = 500
    SubdualDamageHealAmount = 50.0

    SecondLifeMaxHealth = 650.0
  End

Then it needs a different approach indeed.