bugfix: Occupants killed by their containers now kill their occupants by Stubbjax · Pull Request #2239 · TheSuperHackers/GeneralsGameCode

Stubbjax · 2026-02-02T14:06:38Z

Closes #184

This change fixes an issue where occupants killed via the destruction of their container do not kill their occupants, and so on. For example, if a Chinook containing a Troop Crawler dies, the Red Guards inside the Troop Crawler survive and fall to the ground, which looks silly and can leave them floating in the air.

Before

A Troop Crawler dies inside a Chinook, but its occupants survive and fall to the ground or float in the air

YES_SURVIVORS.mp4

After

A Troop Crawler dies inside a Chinook, and kills its own occupants as a result

NO_SURVIVORS.mp4

greptile-apps · 2026-02-02T14:09:31Z

Greptile Summary

This PR fixes a bug where nested occupants (e.g., Red Guards inside a Troop Crawler inside a Chinook) survive when their parent container dies, causing them to incorrectly fall to the ground or float in the air.

The fix implements two key changes:

Added percentDamage parameter to processDamageToContained() to enable proper damage propagation through nested containers
Reordered death logic in non-retail builds to call killRidersWhoAreNotFreeToExit() before processDamageToContained(), ensuring nested containers properly kill their occupants
Added check to prevent riders from exiting when their parent container is held/disabled

The implementation correctly addresses the issue and maintains compatibility with both Generals and Zero Hour variants. However, there are missing null pointer checks in TransportContain.cpp that should be added for defensive programming.

Confidence Score: 4/5

This PR is safe to merge with minor defensive programming improvements needed
The core logic is sound and correctly fixes the nested container bug. The missing null pointer checks in TransportContain.cpp could potentially cause crashes in edge cases, but are unlikely given the calling context. Adding defensive checks would improve robustness.
Pay attention to TransportContain.cpp files in both Generals and GeneralsMD directories for the missing null pointer checks

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp	Reordered death logic to kill non-free riders before damage processing in non-retail builds; uses `percentDamage` parameter for nested container damage propagation
Generals/Code/GameEngine/Source/GameLogic/Object/Contain/TransportContain.cpp	Added check for parent container being held, but missing null pointer check on `getContainedBy()`
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp	Reordered death logic to kill non-free riders before damage processing in non-retail builds; optimized repeated float comparison with cached `killContained` boolean
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/TransportContain.cpp	Added check for parent container being held and improved death type handling, but missing null pointer check on `getContainedBy()`

_{Last reviewed commit: d4588e0}

greptile-apps

_{6 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

Generals/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp

greptile-apps

_{2 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

Generals/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp

greptile-apps · 2026-02-02T14:31:04Z

Additional Comments (1)

Generals/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp
Should use percentDamage parameter instead of data->m_damagePercentageToUnits to match the GeneralsMD implementation and ensure nested containers receive the correct damage value.

		Real damage = object->getBodyModule()->getMaxHealth() * percentDamage;

Prompt To Fix With AI

This is a comment left during a code review.
Path: Generals/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp
Line: 1307:1307

Comment:
Should use `percentDamage` parameter instead of `data->m_damagePercentageToUnits` to match the GeneralsMD implementation and ensure nested containers receive the correct damage value.

```suggestion
		Real damage = object->getBodyModule()->getMaxHealth() * percentDamage;
```

How can I resolve this? If you propose a fix, please make it concise.

xezon · 2026-02-02T22:25:09Z

Does this also happen with Troopcrawler in helix?

Stubbjax · 2026-02-03T02:01:13Z

Does this also happen with Troopcrawler in helix?

A Troop Crawler takes up eight slots and thus cannot fit inside a Helix. But we can demonstrate the same behaviour with a Helix by loading an Ambulance or Technical.

AMBO_LIX.mp4

xezon

This change will need to be Merged With Rebase, because the unify commit needs to be kept separate. The commit titles need to be fit to standard.

xezon · 2026-02-26T17:54:05Z

Generals/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp

@@ -1364,8 +1359,16 @@ void OpenContain::processDamageToContained()

 		DEBUG_ASSERTCRASH( object, ("Contain list must not contain null element") );

+		// TheSuperHackers @bugfix Stubbjax 02/02/2026 If the parent container kills its occupants
+		// on death, then those occupants also kill their occupants, and so on.
+		if (killContained)


Is this condition correct? What if the damage dealt to occupants is just 0.5? Shouldn't that also transfer the damage?

Why? If a Humvee could be stored inside an Overlord, and the Overlord was destroyed, would it make sense to apply that 0.5 damage to the Humvee's passengers in addition to the Humvee itself? There is no precedent for this.

Yes if the health of the contained transport is 0.4 and the damage is 0.5, then we want to kill the passengers of the transport as well right? Or will they be killed anyway? If they are, then it begs the question why is the damage applied in that event but not when applying 1.0 damage?

Upon further investigation, it turns out that the DamagePercentToUnits defined in the Chinook / Helix TransportContain modules is a red herring. Setting either of these fields to a value < 100% still results in the contained units dying, which is due to a call to killRidersWhoAreNotFreeToExit (though this causes the infantry corpses to drop out like vehicles do due to the lack of the burned death type via this logic path).

Only the direct occupants of airborne containers are considered not FREE_TO_EXIT (see ChinookAIUpdate::getAiFreeToExit), which prevents the killRidersWhoAreNotFreeToExit logic from triggering for successive occupants of contained transports such as those inside contained Troop Crawlers. It would seem killRidersWhoAreNotFreeToExit was never called in retail anyway due to the DamagePercentToUnits applying first, which is always set to 100% for airborne containers (where the FREE_TO_EXIT status applies).

I've solved this by returning false if the parent container has the held status in the isSpecificRiderFreeToExit function. I've also swapped the order of killRidersWhoAreNotFreeToExit and processDamageToContained, which avoids calling redundant logic in cases where riders are not FREE_TO_EXIT and DamagePercentToUnits < 100%.

Generals/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp

greptile-apps · 2026-03-02T15:54:05Z

Generals/Code/GameEngine/Source/GameLogic/Object/Contain/TransportContain.cpp

+#if !RETAIL_COMPATIBLE_CRC
+	// TheSuperHackers @bugfix Stubbjax 02/03/2026 If our parent container is held, then we
+	// are not free to exit.
+	if (specificObject->getContainedBy()->isDisabledByType(DISABLED_HELD))


Missing null check before dereferencing getContainedBy(). While objects in the contain list should have a valid container, defensive programming suggests checking for nullptr to prevent potential crashes.

Suggested change

if (specificObject->getContainedBy()->isDisabledByType(DISABLED_HELD))

if (specificObject->getContainedBy() && specificObject->getContainedBy()->isDisabledByType(DISABLED_HELD))

Prompt To Fix With AI

This is a comment left during a code review. Path: Generals/Code/GameEngine/Source/GameLogic/Object/Contain/TransportContain.cpp Line: 462 Comment: Missing null check before dereferencing `getContainedBy()`. While objects in the contain list should have a valid container, defensive programming suggests checking for nullptr to prevent potential crashes. ```suggestion if (specificObject->getContainedBy() && specificObject->getContainedBy()->isDisabledByType(DISABLED_HELD)) ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-03-02T15:54:07Z

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/TransportContain.cpp

+#if !RETAIL_COMPATIBLE_CRC
+	// TheSuperHackers @bugfix Stubbjax 02/03/2026 If our parent container is held, then we
+	// are not free to exit.
+	if (specificObject->getContainedBy()->isDisabledByType(DISABLED_HELD))


Missing null check before dereferencing getContainedBy(). While objects in the contain list should have a valid container, defensive programming suggests checking for nullptr to prevent potential crashes.

Suggested change

if (specificObject->getContainedBy()->isDisabledByType(DISABLED_HELD))

if (specificObject->getContainedBy() && specificObject->getContainedBy()->isDisabledByType(DISABLED_HELD))

Prompt To Fix With AI

This is a comment left during a code review. Path: GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/TransportContain.cpp Line: 573 Comment: Missing null check before dereferencing `getContainedBy()`. While objects in the contain list should have a valid container, defensive programming suggests checking for nullptr to prevent potential crashes. ```suggestion if (specificObject->getContainedBy() && specificObject->getContainedBy()->isDisabledByType(DISABLED_HELD)) ``` How can I resolve this? If you propose a fix, please make it concise.

bugfix: Occupants killed by their containers now kill their occupants

df4f197

Stubbjax self-assigned this Feb 2, 2026

Stubbjax added Bug Something is not working right, typically is user facing Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour NoRetail This fix or change is not applicable with Retail game compatibility labels Feb 2, 2026

greptile-apps bot reviewed Feb 2, 2026

View reviewed changes

Generals/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp Outdated Show resolved Hide resolved

greptile-apps bot reviewed Feb 2, 2026

View reviewed changes

Generals/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp Outdated Show resolved Hide resolved

Generals/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp Outdated Show resolved Hide resolved

Stubbjax force-pushed the apply-occupant-damage-recursively branch from 61f80fa to b1aaa96 Compare February 2, 2026 14:32

unify: Align Generals implementation with Zero Hour

05dc39e

Stubbjax force-pushed the apply-occupant-damage-recursively branch from b1aaa96 to 05dc39e Compare February 2, 2026 14:35

xezon reviewed Feb 26, 2026

View reviewed changes

Stubbjax added 4 commits March 2, 2026 12:14

tweak: Adjust solution implementation

52772fa

perf: Optimise order of operations

c4491ec

docs: Add comments

93adcf7

fix: Object::kill does not take arguments in Generals

d4588e0

greptile-apps bot reviewed Mar 2, 2026

View reviewed changes

	if (specificObject->getContainedBy()->isDisabledByType(DISABLED_HELD))
	if (specificObject->getContainedBy() && specificObject->getContainedBy()->isDisabledByType(DISABLED_HELD))

Conversation

Stubbjax commented Feb 2, 2026

Before

After

Uh oh!

greptile-apps bot commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Uh oh!

greptile-apps bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot commented Feb 2, 2026

Uh oh!

xezon commented Feb 2, 2026

Uh oh!

Stubbjax commented Feb 3, 2026

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

xezon Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Stubbjax Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

xezon Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Stubbjax Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

greptile-apps bot commented Feb 2, 2026 •

edited

Loading

greptile-apps bot left a comment •

edited

Loading