bugfix(contain): Ensure container exists when checking if a specific rider is free to exit

[Stubbjax]

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

This change adds extra safety by null-checking the container before accessing it in TransportContain::isSpecificRiderFreeToExit. It is unexpected that a TransportContain can query the free-to-exit status of a non-rider, which may be indicative of a deeper issue elsewhere.

[greptile-apps]

Greptile Summary

This PR adds a defensive null-check for specificObject->getContainedBy() before calling isDisabledByType(DISABLED_HELD) in TransportContain::isSpecificRiderFreeToExit, applied identically to both the vanilla Generals (Generals/) and Zero Hour (GeneralsMD/) codebases. The same DEBUG_ASSERTCRASH that was already present continues to fire in debug builds to surface the unexpected state, while the new && guard prevents a null-dereference crash in release builds where the assert is compiled out.

• The fix is logically correct: when getContainedBy() is null the "held" check is skipped and execution falls through to the remaining terrain/locomotor checks, which is the appropriate default behaviour for an object that has no recorded container.
• File headers are intact: Generals/ correctly carries "Command & Conquer Generals(tm)" and GeneralsMD/ carries "Command & Conquer Generals Zero Hour(tm)", both with Copyright 2025 Electronic Arts Inc. as required for original EA source files.
• The bug-fix comment date (02/03/2026) is within the current year and does not violate the date-comment policy.
• Code style is consistent with the surrounding function (condition on one line, return FALSE; on the next).

Confidence Score: 5/5

• Safe to merge — the change is a minimal, well-scoped defensive null-check with no logic regressions and correct parity between both game variants.
• Both changed sites are identical in structure and intent. The fix correctly preserves the debug-mode assert while preventing a release-build null-dereference. No P0 or P1 issues were found; all custom rules are satisfied.
• No files require special attention.

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Source/GameLogic/Object/Contain/TransportContain.cpp	Adds null-check guard for getContainedBy() before the DISABLED_HELD query in isSpecificRiderFreeToExit; change is correct, minimal, and consistent with existing style
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/TransportContain.cpp	Mirrors the identical null-check fix from the Generals/ variant for the Zero Hour expansion; correct and consistent

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([isSpecificRiderFreeToExit]) --> B{specificObject == nullptr?}
    B -- Yes --> C([return TRUE])
    B -- No --> D{AI says not free to exit?}
    D -- Yes --> E([return FALSE])
    D -- No --> F{RETAIL_COMPATIBLE_CRC?}
    F -- No check --> G([skip held check])
    F -- Not retail --> H{getContainedBy != null\nAND container is DISABLED_HELD?}
    H -- Yes --> I([return FALSE])
    H -- No --> J{Using airborne locomotor?}
    G --> J
    J -- Yes --> K([return TRUE])
    J -- No --> L{No AI update interface?}
    L -- Yes --> M([return FALSE])
    L -- No --> N{No locomotor?}
    N -- Yes --> O([return FALSE])
    N -- No --> P{Invalid movement terrain?}
    P -- Yes --> Q([return FALSE])
    P -- No --> R([return TRUE])

    style H fill:#90EE90,stroke:#006400
    style I fill:#FFB6C1,stroke:#8B0000

Loading

_{Reviews (1): Last reviewed commit: "fix: Ensure container exists when checki..." | Re-trigger Greptile}

[Caball009]

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

[Stubbjax]

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

Why?

[Caball009]

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]

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]

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]

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]

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]

Can we stop the evacuation sequence when the container dies?