bugfix(contain): Ensure container exists when checking if a specific rider is free to exit by Stubbjax · Pull Request #2513 · TheSuperHackers/GeneralsGameCode

Stubbjax · 2026-03-31T00:35:17Z

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

Occupants of a container now abort their AIExitState if their container dies. As a result, occupants will no longer attempt to reserve or exit doors on destroyed containers, which could potentially result in accessing invalid states of the original container object.

…e to exit

greptile-apps · 2026-03-31T00:37:43Z

Greptile Summary

This PR adds a dead-container guard to AIExitState::update() in both the Generals and Zero Hour codebases. When the container goal is effectively dead (riders already ejected), the state now immediately returns STATE_FAILURE instead of proceeding to isExitBusy() / exitObjectViaDoor(), preventing the AI from trying to exit via an already-destroyed transport. The fix is correctly scoped under #if !RETAIL_COMPATIBLE_CRC to avoid breaking network CRC parity with the original game.

Confidence Score: 5/5

Safe to merge — small, focused defensive guard with no logic regressions and correct CRC scoping

No P0 or P1 issues found. The fix is well-placed: it gates after the existing goalExitInterface null-check and before exitObjectViaDoor. Both Generals and Zero Hour copies are updated symmetrically.

No files require special attention

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp	Adds `isEffectivelyDead()` guard in `AIExitState::update()` under `!RETAIL_COMPATIBLE_CRC` to short-circuit exit logic when the container is already dead
GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp	Mirror of the Generals change — identical `isEffectivelyDead()` guard added to the Zero Hour copy of `AIExitState::update()`

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[AIExitState::update] --> B{goal != nullptr?}
    B -- No --> Z[STATE_FAILURE]
    B -- Yes --> C{goalAI->getAiFreeToExit == WAIT_TO_EXIT?}
    C -- Yes --> Y[STATE_CONTINUE]
    C -- No --> D{goalExitInterface == nullptr?}
    D -- Yes --> Z
    D -- No --> E{goal->isEffectivelyDead?\n#if !RETAIL_COMPATIBLE_CRC}
    E -- Yes --> Z
    E -- No --> F{goalExitInterface->isExitBusy?}
    F -- Yes --> Y
    F -- No --> G[reserveDoorForExit]
    G --> H{exitDoor == DOOR_NONE_AVAILABLE?}
    H -- Yes --> Z
    H -- No --> I[exitObjectViaDoor]
    I --> J{state changed?}
    J -- Yes --> Y
    J -- No --> K[STATE_SUCCESS]

_{Reviews (3): Last reviewed commit: "refactor: Convert fix into a standalone ..." | Re-trigger Greptile}

Caball009

If this can lead to a crash, the PR should include a crash reproduction.

Stubbjax · 2026-03-31T01:35:50Z

If this can lead to a crash, the PR should include a crash reproduction.

Why?

Caball009 · 2026-03-31T15:09:46Z

If this can lead to a crash, the PR should include a crash reproduction.

Why?

That strikes me as an odd question. The PR is labeled as a crash fix. The PR description and the surrounding code suggest that this shouldn't be a nullptr. Even if that weren't the case, 'for posterity' should be a sufficient answer when it comes to documenting crash fixes.

xezon · 2026-03-31T17:07:18Z

Yes it is concerning that this function is called on an object that is not a rider. The assert in this function makes clear that this should not happen. Can you check why it happens and if it can be fixed higher up? Even after this fix it would still be a bug, because the assert can be hit.

Stubbjax · 2026-03-31T22:53:56Z

If this can lead to a crash, the PR should include a crash reproduction.

Why?

That strikes me as an odd question. The PR is labeled as a crash fix. The PR description and the surrounding code suggest that this shouldn't be a nullptr. Even if that weren't the case, 'for posterity' should be a sufficient answer when it comes to documenting crash fixes.

I ask because I've only ever seen this to be a suggestion rather than a requirement.

A developer might not be in a position to spend hours replicating whatever potentially obscure or convoluted scenario leads to a crash. It could take days or even weeks to figure out. Do we let users keep crashing until that happens? When it comes to live software, plugging the hole is typically the priority; understanding how and why can come later.

Stubbjax · 2026-03-31T22:59:41Z

Can you check why it happens and if it can be fixed higher up?

Thankfully it was an easy 3-minute reproduction in this case: Just have a vehicle in the middle of evacuation when it dies. A full Assault Outpost is the best unit for the job. Simply evacuate it and then have it die before all troops have exited.

Issue reproduction was provided.

Caball009 · 2026-04-01T01:02:29Z

I ask because I've only ever seen this to be a suggestion rather than a requirement.

A developer might not be in a position to spend hours replicating whatever potentially obscure or convoluted scenario leads to a crash. It could take days or even weeks to figure out. Do we let users keep crashing until that happens? When it comes to live software, plugging the hole is typically the priority; understanding how and why can come later.

The PR description didn't say that this was a crash hotfix, and even if it did, TSH typically doesn't move so quickly that those sort of PRs are merged fast enough.

The GeneralsOnline devs put out their own hotfix an hour after the creation of this PR, so there's no longer a sense of urgency:
https://github.com/GeneralsOnlineDevelopmentTeam/GameClient/blob/df03a1203fcda3bc86d86d50e6f9923395cceeb0/GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/TransportContain.cpp#L570-L577

Thankfully it was an easy 3-minute reproduction in this case: Just have a vehicle in the middle of evacuation when it dies. A full Assault Outpost is the best unit for the job. Simply evacuate it and then have it die before all troops have exited.

Thank you, that was helpful. I have dismissed my review, but I do think we should check why this happens at all. FWIW I was able to reproduce this issue with an Attack Outpost but not a Troop Crawler. I created a replay for VC6, but it also works with VS22: crash_containedby.zip

Probably not surprising, if specificObject->getContainedBy() == nullptr, me->getContain()->getContainCount() == 0.

Here's the callstack if needed.

generalszh.exe!TransportContain::isSpecificRiderFreeToExit(Object * specificObject)
generalszh.exe!TransportContain::reserveDoorForExit(const ThingTemplate * objType, Object * specificObject)
generalszh.exe!AIExitState::update()
generalszh.exe!StateMachine::updateStateMachine()
generalszh.exe!AIStateMachine::updateStateMachine()
generalszh.exe!AIUpdateInterface::update()
generalszh.exe!GameLogic::update()
generalszh.exe!SubsystemInterface::UPDATE()
generalszh.exe!GameEngine::update()
generalszh.exe!Win32GameEngine::update()
generalszh.exe!GameEngine::execute()
generalszh.exe!GameMain()
generalszh.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow)

xezon · 2026-04-01T17:28:29Z

Can we stop the evacuation sequence when the container dies?

bobtista · 2026-04-10T19:16:59Z

 	// are not free to exit.
 	DEBUG_ASSERTCRASH(specificObject->getContainedBy(), ("rider must be contained"));
-	if (specificObject->getContainedBy()->isDisabledByType(DISABLED_HELD))
+	if (specificObject->getContainedBy() && specificObject->getContainedBy()->isDisabledByType(DISABLED_HELD))


Is it worth storing the result from getContainedBy() so we don't call it two or three times here?

xezon · 2026-04-18T18:56:56Z

Needs more investigation to prevent non-existing riders exiting.

Stubbjax · 2026-05-03T11:50:03Z

Needs more investigation to prevent non-existing riders exiting.

The occupants now abort the AIExitState if the containing object dies.

xezon · 2026-05-04T19:56:20Z

Needs more investigation to prevent non-existing riders exiting.

The occupants now abort the AIExitState if the containing object dies.

The pull desc needs updating.

xezon · 2026-05-04T19:59:21Z

 		if( goalExitInterface == nullptr )
+#else
+		// TheSuperHackers @bugfix Stubbjax 03/05/2026 Stop trying to exit if the container is dead, as we are already ejected.
+		if (goalExitInterface == nullptr || goal->isEffectivelyDead())


Better write like so:

#if !RETAIL_COMPATIBLE_CRC if (goal->isEffectivelyDead()) return STATE_FAILURE; #endif

xezon · 2026-05-04T20:05:38Z

 		if( goalExitInterface == nullptr )
+#else
+		// TheSuperHackers @bugfix Stubbjax 03/05/2026 Stop trying to exit if the container is dead, as we are already ejected.
+		if (goalExitInterface == nullptr || goal->isEffectivelyDead())


Now the next question is, why do the exited passengers still have an AI State Machine for exiting the container? I wonder if another piece of the puzzle here is to cancel the AIExitState before passengers are ejected from the dead host container. I do not know if that is possible. There are functions such as: AIStateMachine::clear, AIStateMachine::resetToDefaultState to play with.

Now the next question is, why do the exited passengers still have an AI State Machine for exiting the container? I wonder if another piece of the puzzle here is to cancel the AIExitState before passengers are ejected from the dead host container. I do not know if that is possible. There are functions such as: AIStateMachine::clear, AIStateMachine::resetToDefaultState to play with.

I don't believe that is currently possible or even desirable; the container should generally not be aware of the internal AI states of its contents. An object could maybe notify its state machine of its destruction, but a destroyed container should not be telling its contents what state to take or how to behave - I'd expect that to be a decision made by the states themselves.

Yes that can be solved by adding a OnContainerDestroyed to passenger objects and then call that on container destruction.

fix: Ensure container exists when checking if a specific rider is fre…

2bb9b2c

…e to exit

Stubbjax self-assigned this Mar 31, 2026

Caball009 previously requested changes Mar 31, 2026

View reviewed changes

xezon changed the title ~~fix: Ensure container exists when checking if a specific rider is free to exit~~ bugfix(contain): Ensure container exists when checking if a specific rider is free to exit Mar 31, 2026

bobtista reviewed Apr 10, 2026

View reviewed changes

bugfix: Abort AIExitState if the containing object dies

206aaec

xezon reviewed May 4, 2026

View reviewed changes

refactor: Convert fix into a standalone block

90b207d

Conversation

Stubbjax commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps Bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

Caball009 left a comment

Choose a reason for hiding this comment

Uh oh!

Stubbjax commented Mar 31, 2026

Uh oh!

Caball009 commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xezon commented Mar 31, 2026

Uh oh!

Stubbjax commented Mar 31, 2026

Uh oh!

Stubbjax commented Mar 31, 2026

Uh oh!

Caball009 commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xezon commented Apr 1, 2026

Uh oh!

bobtista Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

xezon commented Apr 18, 2026

Uh oh!

Stubbjax commented May 3, 2026

Uh oh!

xezon commented May 4, 2026

Uh oh!

xezon May 4, 2026

Choose a reason for hiding this comment

Uh oh!

xezon May 4, 2026

Choose a reason for hiding this comment

Uh oh!

Stubbjax May 5, 2026

Choose a reason for hiding this comment

Uh oh!

xezon May 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Stubbjax commented Mar 31, 2026 •

edited

Loading

greptile-apps Bot commented Mar 31, 2026 •

edited

Loading

Caball009 commented Mar 31, 2026 •

edited

Loading

Caball009 commented Apr 1, 2026 •

edited

Loading