Skip to content

bugfix(contain): Occupants killed by their containers now kill their occupants #2239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
xezon merged 7 commits into from
Mar 18, 2026
+45 −11
Merged

bugfix(contain): Occupants killed by their containers now kill their occupants #2239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
891735b
bugfix: Occupants killed by their containers now kill their occupants
Stubbjax Feb 2, 2026
d1943d7
unify: Align Generals implementation with Zero Hour
Stubbjax Feb 2, 2026
9eaf868
tweak: Adjust solution implementation
Stubbjax Mar 1, 2026
1f155e5
perf: Optimise order of operations
Stubbjax Mar 2, 2026
d0581da
docs: Add comments
Stubbjax Mar 2, 2026
ecf8ac1
fix: Object::kill does not take arguments in Generals
Stubbjax Mar 2, 2026
e069268
tweak: Add assertions
Stubbjax Mar 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Generals/Code/GameEngine/Include/GameLogic/Module/ContainModule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ class ContainModuleInterface
// Player Occupancy.
virtual PlayerMaskType getPlayerWhoEntered() const = 0;

virtual void processDamageToContained() = 0; ///< Do our % damage to units now.
virtual void processDamageToContained(Real percentDamage) = 0; ///< Do our % damage to units now.

virtual void enableLoadSounds( Bool enable ) = 0;

Expand Down
2 changes: 1 addition & 1 deletion Generals/Code/GameEngine/Include/GameLogic/Module/OpenContain.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ class OpenContain : public UpdateModule,
// returns true iff there are objects currently waiting to enter.
virtual Bool hasObjectsWantingToEnterOrExit() const override;

virtual void processDamageToContained() override; ///< Do our % damage to units now.
virtual void processDamageToContained(Real percentDamage) override; ///< Do our % damage to units now.

virtual void enableLoadSounds( Bool enable ) override { m_loadSoundsEnabled = enable; }

Expand Down
20 changes: 13 additions & 7 deletions Generals/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,14 +760,20 @@ void OpenContain::onDie( const DamageInfo * damageInfo )
if (!getOpenContainModuleData()->m_dieMuxData.isDieApplicable(getObject(), damageInfo))
return;

#if !RETAIL_COMPATIBLE_CRC
killRidersWhoAreNotFreeToExit();
#endif

//Check to see if we are going to inflict damage on contained units.
if( getOpenContainModuleData()->m_damagePercentageToUnits > 0 )
{
//Cycle through the units and apply damage to them!
processDamageToContained();
processDamageToContained(getOpenContainModuleData()->m_damagePercentageToUnits);
}

#if RETAIL_COMPATIBLE_CRC
killRidersWhoAreNotFreeToExit();
#endif

// Leaving this commented out to show it can't work. We are about to die, so they will have zero
// chance to hit an exitState::Update. At least we would clean them up in onDelete.
Expand Down Expand Up @@ -1291,9 +1297,9 @@ void OpenContain::orderAllPassengersToExit( CommandSourceType commandSource )
}

//-------------------------------------------------------------------------------------------------
void OpenContain::processDamageToContained()
void OpenContain::processDamageToContained(Real percentDamage)
{
const OpenContainModuleData* data = getOpenContainModuleData();
const bool killContained = percentDamage == 1.0f;

#if RETAIL_COMPATIBLE_CRC

Expand All @@ -1308,7 +1314,7 @@ void OpenContain::processDamageToContained()
Object *object = *it++;

//Calculate the damage to be inflicted on each unit.
Real damage = object->getBodyModule()->getMaxHealth() * data->m_damagePercentageToUnits;
Real damage = object->getBodyModule()->getMaxHealth() * percentDamage;

DamageInfo damageInfo;
damageInfo.in.m_damageType = DAMAGE_UNRESISTABLE;
Expand All @@ -1317,7 +1323,7 @@ void OpenContain::processDamageToContained()
damageInfo.in.m_amount = damage;
object->attemptDamage( &damageInfo );

if( !object->isEffectivelyDead() && data->m_damagePercentageToUnits == 1.0f )
if( !object->isEffectivelyDead() && killContained )
object->kill(); // in case we are carrying flame proof troops we have been asked to kill

// TheSuperHackers @info Calls to Object::attemptDamage and Object::kill will not remove
Expand Down Expand Up @@ -1365,7 +1371,7 @@ void OpenContain::processDamageToContained()
DEBUG_ASSERTCRASH( object, ("Contain list must not contain null element") );

// Calculate the damage to be inflicted on each unit.
Real damage = object->getBodyModule()->getMaxHealth() * data->m_damagePercentageToUnits;
Real damage = object->getBodyModule()->getMaxHealth() * percentDamage;

DamageInfo damageInfo;
damageInfo.in.m_damageType = DAMAGE_UNRESISTABLE;
Expand All @@ -1374,7 +1380,7 @@ void OpenContain::processDamageToContained()
damageInfo.in.m_amount = damage;
object->attemptDamage( &damageInfo );

if( !object->isEffectivelyDead() && data->m_damagePercentageToUnits == 1.0f )
if( !object->isEffectivelyDead() && killContained )
object->kill(); // in case we are carrying flame proof troops we have been asked to kill

if ( object->isEffectivelyDead() )
Expand Down
8 changes: 8 additions & 0 deletions Generals/Code/GameEngine/Source/GameLogic/Object/Contain/TransportContain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,14 @@ Bool TransportContain::isSpecificRiderFreeToExit(Object* specificObject)
if (ai && ai->getAiFreeToExit(specificObject) != FREE_TO_EXIT)
return FALSE;

#if !RETAIL_COMPATIBLE_CRC
// TheSuperHackers @bugfix Stubbjax 02/03/2026 If our parent container is held, then we
// are not free to exit.
DEBUG_ASSERTCRASH(specificObject->getContainedBy(), ("rider must be contained"));
if (specificObject->getContainedBy()->isDisabledByType(DISABLED_HELD))
Copy link

@greptile-apps greptile-apps bot Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

return FALSE;
#endif

// I can always kick people out if I am in the air, I know what I'm doing
if (me->isUsingAirborneLocomotor())
return TRUE;
Expand Down
11 changes: 9 additions & 2 deletions GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -880,14 +880,20 @@ void OpenContain::onDie( const DamageInfo * damageInfo )
if (!getOpenContainModuleData()->m_dieMuxData.isDieApplicable(getObject(), damageInfo))
return;

#if !RETAIL_COMPATIBLE_CRC
killRidersWhoAreNotFreeToExit();
#endif

//Check to see if we are going to inflict damage on contained units.
if( getDamagePercentageToUnits() > 0 )
{
//Cycle through the units and apply damage to them!
processDamageToContained(getDamagePercentageToUnits());
}

#if RETAIL_COMPATIBLE_CRC
killRidersWhoAreNotFreeToExit();
#endif

// Leaving this commented out to show it can't work. We are about to die, so they will have zero
// chance to hit an exitState::Update. At least we would clean them up in onDelete.
Expand Down Expand Up @@ -1462,6 +1468,7 @@ void OpenContain::orderAllPassengersToHackInternet( CommandSourceType commandSou
void OpenContain::processDamageToContained(Real percentDamage)
{
const OpenContainModuleData *data = getOpenContainModuleData();
const bool killContained = percentDamage == 1.0f;

#if RETAIL_COMPATIBLE_CRC

Expand All @@ -1485,7 +1492,7 @@ void OpenContain::processDamageToContained(Real percentDamage)
damageInfo.in.m_amount = damage;
object->attemptDamage( &damageInfo );

if( !object->isEffectivelyDead() && percentDamage == 1.0f )
if( !object->isEffectivelyDead() && killContained )
object->kill(); // in case we are carrying flame proof troops we have been asked to kill

// TheSuperHackers @info Calls to Object::attemptDamage and Object::kill will not remove
Expand Down Expand Up @@ -1542,7 +1549,7 @@ void OpenContain::processDamageToContained(Real percentDamage)
damageInfo.in.m_amount = damage;
object->attemptDamage( &damageInfo );

if( !object->isEffectivelyDead() && percentDamage == 1.0f )
if( !object->isEffectivelyDead() && killContained )
object->kill(); // in case we are carrying flame proof troops we have been asked to kill

if ( object->isEffectivelyDead() )
Expand Down
13 changes: 13 additions & 0 deletions GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/TransportContain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,12 @@ void TransportContain::killRidersWhoAreNotFreeToExit()
if (d->m_destroyRidersWhoAreNotFreeToExit)
TheGameLogic->destroyObject(obj);
else
#if RETAIL_COMPATIBLE_CRC
obj->kill();
#else
// TheSuperHackers @info Burned death prevents infantry corpses dropping out of the container.
obj->kill(DAMAGE_UNRESISTABLE, d->m_isBurnedDeathToUnits ? DEATH_BURNED : DEATH_NORMAL);
#endif
}
}
}
Expand All @@ -562,6 +567,14 @@ Bool TransportContain::isSpecificRiderFreeToExit(Object* specificObject)
if (ai && ai->getAiFreeToExit(specificObject) != FREE_TO_EXIT)
return FALSE;

#if !RETAIL_COMPATIBLE_CRC
// TheSuperHackers @bugfix Stubbjax 02/03/2026 If our parent container is held, then we
// are not free to exit.
DEBUG_ASSERTCRASH(specificObject->getContainedBy(), ("rider must be contained"));
if (specificObject->getContainedBy()->isDisabledByType(DISABLED_HELD))
return FALSE;
#endif

// I can always kick people out if I am in the air, I know what I'm doing
if (me->isUsingAirborneLocomotor())
return TRUE;
Expand Down
Loading