From 0dcb53db066f5a287b1651be618f464e0dcfb9b8 Mon Sep 17 00:00:00 2001 From: Caball009 <82909616+Caball009@users.noreply.github.com> Date: Fri, 20 Mar 2026 16:26:46 +0100 Subject: [PATCH 1/8] Added 'processDamageToContainedInternal' for refactor. --- .../GameEngine/Include/GameLogic/Module/OpenContain.h | 3 +++ .../Source/GameLogic/Object/Contain/OpenContain.cpp | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/GeneralsMD/Code/GameEngine/Include/GameLogic/Module/OpenContain.h b/GeneralsMD/Code/GameEngine/Include/GameLogic/Module/OpenContain.h index 1b229d6c3b2..a731669bef9 100644 --- a/GeneralsMD/Code/GameEngine/Include/GameLogic/Module/OpenContain.h +++ b/GeneralsMD/Code/GameEngine/Include/GameLogic/Module/OpenContain.h @@ -219,6 +219,9 @@ class OpenContain : public UpdateModule, virtual Bool hasObjectsWantingToEnterOrExit() const override; virtual void processDamageToContained(Real percentDamage) override; ///< Do our % damage to units now. +#if RETAIL_COMPATIBLE_CRC + void processDamageToContainedInternal(const ContainedItemsList* items, Real percentDamage); +#endif virtual Bool isWeaponBonusPassedToPassengers() const override; virtual WeaponBonusConditionFlags getWeaponBonusPassedToPassengers() const override; diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp index f436ed26245..9ddbddcaafa 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp @@ -1462,7 +1462,17 @@ void OpenContain::orderAllPassengersToHackInternet( CommandSourceType commandSou } } +#if RETAIL_COMPATIBLE_CRC + +//------------------------------------------------------------------------------------------------- +void OpenContain::processDamageToContainedInternal(const ContainedItemsList* items, Real percentDamage) +{ + const OpenContainModuleData* data = getOpenContainModuleData(); + const bool killContained = percentDamage == 1.0f; + +} +#endif //------------------------------------------------------------------------------------------------- void OpenContain::processDamageToContained(Real percentDamage) From 6e5d403888f497d72dd80fd010ff82666dacd70a Mon Sep 17 00:00:00 2001 From: Caball009 <82909616+Caball009@users.noreply.github.com> Date: Fri, 20 Mar 2026 16:29:38 +0100 Subject: [PATCH 2/8] Moved loop to internal function. --- .../GameLogic/Object/Contain/OpenContain.cpp | 75 ++++++++++--------- 1 file changed, 38 insertions(+), 37 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp index 9ddbddcaafa..ac84e41fce3 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp @@ -1470,6 +1470,43 @@ void OpenContain::processDamageToContainedInternal(const ContainedItemsList* ite const OpenContainModuleData* data = getOpenContainModuleData(); const bool killContained = percentDamage == 1.0f; + ContainedItemsList::const_iterator it = items->begin(); + const size_t listSize = items->size(); + + while( it != items->end() ) + { + Object *object = *it++; + + //Calculate the damage to be inflicted on each unit. + Real damage = object->getBodyModule()->getMaxHealth() * percentDamage; + + DamageInfo damageInfo; + damageInfo.in.m_damageType = DAMAGE_UNRESISTABLE; + damageInfo.in.m_deathType = data->m_isBurnedDeathToUnits ? DEATH_BURNED : DEATH_NORMAL; + damageInfo.in.m_sourceID = getObject()->getID(); + damageInfo.in.m_amount = damage; + object->attemptDamage( &damageInfo ); + + 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 + // the occupant from the host container straight away. Instead it will be removed when the + // Object deletion is finalized in a Game Logic update. This will lead to strange behavior + // where the occupant will be removed after death with a delay. This behavior cannot be + // changed without breaking retail compatibility. + + // TheSuperHackers @bugfix xezon 05/06/2025 Stop iterating when the list was cleared. + // This scenario can happen if the killed occupant(s) apply deadly damage on death + // to the host container, which then attempts to remove all remaining occupants + // on the death of the host container. This is reproducible by destroying a + // GLA Battle Bus with at least 2 half damaged GLA Terrorists inside. + if (listSize != items->size()) + { + DEBUG_ASSERTCRASH( listSize == 0, ("List is expected empty") ); + break; + } + } } #endif @@ -1485,43 +1522,7 @@ void OpenContain::processDamageToContained(Real percentDamage) const ContainedItemsList* items = getContainedItemsList(); if( items ) { - ContainedItemsList::const_iterator it = items->begin(); - const size_t listSize = items->size(); - - while( it != items->end() ) - { - Object *object = *it++; - - //Calculate the damage to be inflicted on each unit. - Real damage = object->getBodyModule()->getMaxHealth() * percentDamage; - - DamageInfo damageInfo; - damageInfo.in.m_damageType = DAMAGE_UNRESISTABLE; - damageInfo.in.m_deathType = data->m_isBurnedDeathToUnits ? DEATH_BURNED : DEATH_NORMAL; - damageInfo.in.m_sourceID = getObject()->getID(); - damageInfo.in.m_amount = damage; - object->attemptDamage( &damageInfo ); - - 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 - // the occupant from the host container straight away. Instead it will be removed when the - // Object deletion is finalized in a Game Logic update. This will lead to strange behavior - // where the occupant will be removed after death with a delay. This behavior cannot be - // changed without breaking retail compatibility. - - // TheSuperHackers @bugfix xezon 05/06/2025 Stop iterating when the list was cleared. - // This scenario can happen if the killed occupant(s) apply deadly damage on death - // to the host container, which then attempts to remove all remaining occupants - // on the death of the host container. This is reproducible by destroying a - // GLA Battle Bus with at least 2 half damaged GLA Terrorists inside. - if (listSize != items->size()) - { - DEBUG_ASSERTCRASH( listSize == 0, ("List is expected empty") ); - break; - } - } + } #else From 38dbc74bd53de0e459825dacebf297d4a5cfb217 Mon Sep 17 00:00:00 2001 From: Caball009 <82909616+Caball009@users.noreply.github.com> Date: Fri, 20 Mar 2026 16:31:10 +0100 Subject: [PATCH 3/8] Added call to internal function & cleanup. --- .../Source/GameLogic/Object/Contain/OpenContain.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp index ac84e41fce3..d824d2e7e72 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp @@ -1514,16 +1514,9 @@ void OpenContain::processDamageToContainedInternal(const ContainedItemsList* ite //------------------------------------------------------------------------------------------------- void OpenContain::processDamageToContained(Real percentDamage) { - const OpenContainModuleData *data = getOpenContainModuleData(); - const bool killContained = percentDamage == 1.0f; - #if RETAIL_COMPATIBLE_CRC - const ContainedItemsList* items = getContainedItemsList(); - if( items ) - { - - } + processDamageToContainedInternal(&m_containList, percentDamage); #else @@ -1538,6 +1531,9 @@ void OpenContain::processDamageToContained(Real percentDamage) // on death of a unit to another unit in the host container. If this functionality // is desired, then this implementation needs to be revisited. + const OpenContainModuleData* data = getOpenContainModuleData(); + const bool killContained = percentDamage == 1.0f; + ContainedItemsList list; m_containList.swap(list); m_containListSize = 0; From 91ac7d03ab7e8ef0c8d086650bf92b83c5a2a47a Mon Sep 17 00:00:00 2001 From: Caball009 <82909616+Caball009@users.noreply.github.com> Date: Fri, 20 Mar 2026 16:34:05 +0100 Subject: [PATCH 4/8] Changed comments. --- .../Source/GameLogic/Object/Contain/OpenContain.cpp | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp index d824d2e7e72..6273fbdae4c 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp @@ -1477,7 +1477,7 @@ void OpenContain::processDamageToContainedInternal(const ContainedItemsList* ite { Object *object = *it++; - //Calculate the damage to be inflicted on each unit. + // Calculate the damage to be inflicted on each unit. Real damage = object->getBodyModule()->getMaxHealth() * percentDamage; DamageInfo damageInfo; @@ -1490,17 +1490,11 @@ void OpenContain::processDamageToContainedInternal(const ContainedItemsList* ite 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 - // the occupant from the host container straight away. Instead it will be removed when the + // TheSuperHackers @info Calls to Object::attemptDamage and Object::kill may not remove + // the occupant from the host container straight away. Instead it would be removed when the // Object deletion is finalized in a Game Logic update. This will lead to strange behavior // where the occupant will be removed after death with a delay. This behavior cannot be // changed without breaking retail compatibility. - - // TheSuperHackers @bugfix xezon 05/06/2025 Stop iterating when the list was cleared. - // This scenario can happen if the killed occupant(s) apply deadly damage on death - // to the host container, which then attempts to remove all remaining occupants - // on the death of the host container. This is reproducible by destroying a - // GLA Battle Bus with at least 2 half damaged GLA Terrorists inside. if (listSize != items->size()) { DEBUG_ASSERTCRASH( listSize == 0, ("List is expected empty") ); From c64c9a1b80062d95cc9963ad2bcced6849766937 Mon Sep 17 00:00:00 2001 From: Caball009 <82909616+Caball009@users.noreply.github.com> Date: Fri, 20 Mar 2026 16:36:49 +0100 Subject: [PATCH 5/8] Added list copy logic. --- .../Include/GameLogic/Module/OpenContain.h | 2 +- .../GameLogic/Object/Contain/OpenContain.cpp | 23 +++++++++---------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Include/GameLogic/Module/OpenContain.h b/GeneralsMD/Code/GameEngine/Include/GameLogic/Module/OpenContain.h index a731669bef9..94f54c349ff 100644 --- a/GeneralsMD/Code/GameEngine/Include/GameLogic/Module/OpenContain.h +++ b/GeneralsMD/Code/GameEngine/Include/GameLogic/Module/OpenContain.h @@ -220,7 +220,7 @@ class OpenContain : public UpdateModule, virtual void processDamageToContained(Real percentDamage) override; ///< Do our % damage to units now. #if RETAIL_COMPATIBLE_CRC - void processDamageToContainedInternal(const ContainedItemsList* items, Real percentDamage); + void processDamageToContainedInternal(Object* const* objects, size_t size, Real percentDamage); #endif virtual Bool isWeaponBonusPassedToPassengers() const override; diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp index 6273fbdae4c..e534215c699 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp @@ -1465,17 +1465,14 @@ void OpenContain::orderAllPassengersToHackInternet( CommandSourceType commandSou #if RETAIL_COMPATIBLE_CRC //------------------------------------------------------------------------------------------------- -void OpenContain::processDamageToContainedInternal(const ContainedItemsList* items, Real percentDamage) +void OpenContain::processDamageToContainedInternal(Object* const* objects, size_t size, Real percentDamage) { const OpenContainModuleData* data = getOpenContainModuleData(); const bool killContained = percentDamage == 1.0f; - ContainedItemsList::const_iterator it = items->begin(); - const size_t listSize = items->size(); - - while( it != items->end() ) + for (size_t i = 0; i < size; ++i) { - Object *object = *it++; + Object* object = objects[i]; // Calculate the damage to be inflicted on each unit. Real damage = object->getBodyModule()->getMaxHealth() * percentDamage; @@ -1495,11 +1492,6 @@ void OpenContain::processDamageToContainedInternal(const ContainedItemsList* ite // Object deletion is finalized in a Game Logic update. This will lead to strange behavior // where the occupant will be removed after death with a delay. This behavior cannot be // changed without breaking retail compatibility. - if (listSize != items->size()) - { - DEBUG_ASSERTCRASH( listSize == 0, ("List is expected empty") ); - break; - } } } @@ -1510,7 +1502,14 @@ void OpenContain::processDamageToContained(Real percentDamage) { #if RETAIL_COMPATIBLE_CRC - processDamageToContainedInternal(&m_containList, percentDamage); + DEBUG_ASSERTCRASH(m_containListSize == m_containList.size(), ("contain list size doesn't match size of container")); + + // TheSuperHackers @bugfix Caball009 11/03/2026 Use a temporary copy of the contain list to iterate over, + // because Object::attemptDamage in OpenContain::processDamageToContainedInternal may remove some or all elements from the list. + + const std::vector containCopy(m_containList.begin(), m_containList.end()); + + processDamageToContainedInternal(&containCopy[0], containCopy.size(), percentDamage); #else From 144e6b4ff15def08f7f8db49285a8279cab44f8f Mon Sep 17 00:00:00 2001 From: Caball009 <82909616+Caball009@users.noreply.github.com> Date: Fri, 20 Mar 2026 16:37:57 +0100 Subject: [PATCH 6/8] Added optimization for small list size. --- .../GameLogic/Object/Contain/OpenContain.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp index e534215c699..d91cacf043e 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp @@ -1507,9 +1507,20 @@ void OpenContain::processDamageToContained(Real percentDamage) // TheSuperHackers @bugfix Caball009 11/03/2026 Use a temporary copy of the contain list to iterate over, // because Object::attemptDamage in OpenContain::processDamageToContainedInternal may remove some or all elements from the list. - const std::vector containCopy(m_containList.begin(), m_containList.end()); + constexpr const UnsignedInt smallContainerSize = 16; + if (m_containListSize < smallContainerSize) + { + Object* containCopy[smallContainerSize]; + std::copy(m_containList.begin(), m_containList.end(), containCopy); + + processDamageToContainedInternal(containCopy, m_containListSize, percentDamage); + } + else + { + const std::vector containCopy(m_containList.begin(), m_containList.end()); - processDamageToContainedInternal(&containCopy[0], containCopy.size(), percentDamage); + processDamageToContainedInternal(&containCopy[0], containCopy.size(), percentDamage); + } #else From 4d5d684be5afb356d31893a53ef0aaf06045a44a Mon Sep 17 00:00:00 2001 From: Caball009 <82909616+Caball009@users.noreply.github.com> Date: Fri, 20 Mar 2026 16:43:13 +0100 Subject: [PATCH 7/8] Hoisted 'isBurnedDeathToUnits' outside of the loop. --- .../Source/GameLogic/Object/Contain/OpenContain.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp index d91cacf043e..4570ed659d9 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp @@ -1467,7 +1467,7 @@ void OpenContain::orderAllPassengersToHackInternet( CommandSourceType commandSou //------------------------------------------------------------------------------------------------- void OpenContain::processDamageToContainedInternal(Object* const* objects, size_t size, Real percentDamage) { - const OpenContainModuleData* data = getOpenContainModuleData(); + const bool isBurnedDeathToUnits = getOpenContainModuleData()->m_isBurnedDeathToUnits; const bool killContained = percentDamage == 1.0f; for (size_t i = 0; i < size; ++i) @@ -1479,7 +1479,7 @@ void OpenContain::processDamageToContainedInternal(Object* const* objects, size_ DamageInfo damageInfo; damageInfo.in.m_damageType = DAMAGE_UNRESISTABLE; - damageInfo.in.m_deathType = data->m_isBurnedDeathToUnits ? DEATH_BURNED : DEATH_NORMAL; + damageInfo.in.m_deathType = isBurnedDeathToUnits ? DEATH_BURNED : DEATH_NORMAL; damageInfo.in.m_sourceID = getObject()->getID(); damageInfo.in.m_amount = damage; object->attemptDamage( &damageInfo ); @@ -1535,7 +1535,7 @@ void OpenContain::processDamageToContained(Real percentDamage) // on death of a unit to another unit in the host container. If this functionality // is desired, then this implementation needs to be revisited. - const OpenContainModuleData* data = getOpenContainModuleData(); + const bool isBurnedDeathToUnits = getOpenContainModuleData()->m_isBurnedDeathToUnits; const bool killContained = percentDamage == 1.0f; ContainedItemsList list; @@ -1555,7 +1555,7 @@ void OpenContain::processDamageToContained(Real percentDamage) DamageInfo damageInfo; damageInfo.in.m_damageType = DAMAGE_UNRESISTABLE; - damageInfo.in.m_deathType = data->m_isBurnedDeathToUnits ? DEATH_BURNED : DEATH_NORMAL; + damageInfo.in.m_deathType = isBurnedDeathToUnits ? DEATH_BURNED : DEATH_NORMAL; damageInfo.in.m_sourceID = getObject()->getID(); damageInfo.in.m_amount = damage; object->attemptDamage( &damageInfo ); From cebb0a5d5450742f18c5a984fb6b3de2fc9cb31b Mon Sep 17 00:00:00 2001 From: Caball009 <82909616+Caball009@users.noreply.github.com> Date: Fri, 20 Mar 2026 17:40:56 +0100 Subject: [PATCH 8/8] Changed comments. --- .../Source/GameLogic/Object/Contain/OpenContain.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp index 4570ed659d9..00c8d502bad 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp @@ -1505,7 +1505,7 @@ void OpenContain::processDamageToContained(Real percentDamage) DEBUG_ASSERTCRASH(m_containListSize == m_containList.size(), ("contain list size doesn't match size of container")); // TheSuperHackers @bugfix Caball009 11/03/2026 Use a temporary copy of the contain list to iterate over, - // because Object::attemptDamage in OpenContain::processDamageToContainedInternal may remove some or all elements from the list. + // because Object::attemptDamage may remove some or all elements from the list while iterating over it, which may be unsafe. constexpr const UnsignedInt smallContainerSize = 16; if (m_containListSize < smallContainerSize) @@ -1525,11 +1525,7 @@ void OpenContain::processDamageToContained(Real percentDamage) #else // TheSuperHackers @bugfix xezon 05/06/2025 Temporarily empty the m_containList - // to prevent a potential child call to catastrophically modify the m_containList. - // This scenario can happen if the killed occupant(s) apply deadly damage on death - // to the host container, which then attempts to remove all remaining occupants - // on the death of the host container. This is reproducible by destroying a - // GLA Battle Bus with at least 2 half damaged GLA Terrorists inside. + // because Object::attemptDamage may remove some or all elements from the list while iterating over it, which may be unsafe. // Caveat: While the m_containList is empty, it will not be possible to apply damage // on death of a unit to another unit in the host container. If this functionality