bugfix(behavior): Allow multiple InstantDeathBehavior modules to be processed#2990
Conversation
|
| Filename | Overview |
|---|---|
| Generals/Code/GameEngine/Source/GameLogic/Object/Behavior/InstantDeathBehavior.cpp | Moves the non-retail AI dead-state gate after death side effects so multiple instant-death modules can run. |
| GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Behavior/InstantDeathBehavior.cpp | Mirrors the Generals instant-death ordering change for the Zero Hour source tree. |
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
Generals/Code/GameEngine/Source/GameLogic/Object/Behavior/InstantDeathBehavior.cpp:172
**Death Weapon Re-enters Alive Object**
When an `InstantDeathBehavior` death weapon immediately damages its own source, this call can re-enter death handling before the new late `markAsDead()` runs. The object still passes `isDieApplicable()` and the dead-state check has not fired yet, so the same FX, OCL, or weapon side effects can run again and produce duplicate spawns, effects, or damage in non-retail-compatible builds.
### Issue 2 of 2
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Behavior/InstantDeathBehavior.cpp:172
**Death Weapon Re-enters Alive Object**
When an `InstantDeathBehavior` death weapon immediately damages its own source, this call can re-enter death handling before the new late `markAsDead()` runs. The object still passes `isDieApplicable()` and the dead-state check has not fired yet, so the same FX, OCL, or weapon side effects can run again and produce duplicate spawns, effects, or damage in non-retail-compatible builds.
Reviews (1): Last reviewed commit: "bugfix: Allow multiple InstantDeathBehav..." | Re-trigger Greptile
| @@ -132,6 +133,7 @@ void InstantDeathBehavior::onDie( const DamageInfo *damageInfo ) | |||
| return; | |||
| ai->markAsDead(); | |||
There was a problem hiding this comment.
SlowDeathBehavior also has it at the top. Is this a problem?
The same concerns should apply to the existing individual modules which lack this check though, so I don't think it's valid.
The |
Did you test it? Have a unit that fires a weapon on death that can damage itself and see if the module enters an infinite loop of damaging itself. What units will this change noticably affect? |
Yes. I tested this with a self-damaging Nuke Battlemaster and its if( m_currentHealth <= 0 && m_prevHealth > 0 )
obj->onDie( damageInfo );I imagine this would be a much greater issue if it wasn't already handled higher up and every individual module had to guard against it. The
None other than all SCUD Storm variants after GeneralsGamePatch2#118. Every other case with multiple Side note: |
This change allows multiple
InstantDeathBehaviormodules to be processed when an object dies.The
InstantDeathBehaviormodule is essentially a composite of theFXListDie,CreateObjectDieandFireWeaponWhenDeadBehaviormodules - all of which can be processed multiple times when an object dies. It is counterintuitive that combining such modules intoInstantDeathBehaviorcomes with the caveat that multiple instances can no longer be processed.All cases in the retail data have inverted
DeathTypesto ensure exclusivity. For example:However, this does not work when
DeathTypesare not exclusive, in which case only the first applicable module will trigger. For example,ModuleTag_13will never trigger with the following setup: