[Caball009]

• Fixes Tunnel occupants can cause crashes when attacking after tunnel transfer leaves units in invalid state #3339
• Closes Contained unit dereferences its destroyed container during combat after a surrender with asset transfer #3316 (retail compatibility and memory poisoning cannot be used together in this instance)
• Related PR bugfix(object): Remove stranded occupants from the TunnelTracker and fix the object lookup for GameLogic::destroyAllObjectsImmediate() #3308

The following is retail behavior and doesn't happen with retail compatibility disabled: when a Tunnel Network with units is transferred to another player, it leaves the units in an invalid state. They disappear from the tunnel's GUI and become inaccessible to the player. If that tunnel is sold or destroyed the units remain behind hidden and indestructible. The units hold on to a pointer to the tunnel object, which may get reused for something else. In this invalid state, some units can attack enemy units. This may crash the game.

These crashes are specific to retail Generals, AFAICT due to the missing TunnelContain::onCapture overload. This PR prevents two crashes because of this issue, and destroys all the units after the first crash would have happened.

---

Custom map and VS22 replays that reproduce the two crashes for the last commit: replays.zip.

Both replays can also be used to crash generalsv-weekly-2026-10-02 despite the mismatches.

---

TODO:

• Replicate to Zero Hour.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

• Last reviewed commit: 08fc877d782c4dda979a07599b9d027563e5adbd
• Latest commit, not reviewed: 3289e16

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• ✅ Review completed - (🔄 Check again to review again)

Walkthrough

In retail-compatible Generals builds, the attack state now handles a missing containment module by removing and destroying objects in the matching tunnel containment list, then returning STATE_FAILURE.

Changes

Tunnel occupant validation

Layer / File(s)	Summary
Tunnel containment cleanup Generals/Code/GameEngine/Include/GameLogic/GameLogic.h, Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp	GameLogic declares and implements friend_destroyFromTunnelContain under RTS_GENERALS && RETAIL_COMPATIBLE_CRC. The method searches player tunnel trackers for the stranded object, removes each object from the matching containment list, and queues each for destruction. The TunnelTracker.h and Water.h includes are also updated.
Attack-state containment guard Generals/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp	When the containment module is null in the specified Generals build configuration, AIAttackAimAtTargetState::onEnter calls friend_destroyFromTunnelContain and returns STATE_FAILURE.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: 🟡 Moderate · up to 08fc8

The crash fix also destroys every unit in the affected player's tunnel network, not just the broken unit. A player who keeps another tunnel can lose units without warning. The cleanup should be limited to the stranded unit before merging.

### Architecture Summary

Architecture risk: 🔵 Low · up to 69836

The change affects 1 system.

Changed systems: Generals

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — Generals (service) was modified; 6 changed files map to changed impact.

Before / after behavior

• observed — Modified behavior in Generals/Code/GameEngine/Include/Common/TunnelTracker.h: removeFromContain now returns Bool rather than void; its arguments and default for exposeStealthUnits are unchanged.
• observed — Modified behavior in Generals/Code/GameEngine/Include/GameLogic/Object.h: Added the Object::friend_removeFromTunnelContain() method declaration.
• observed — Modified behavior in Generals/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp: removeFromContain now returns Bool instead of void.
• observed — Modified behavior in Generals/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp: After removing a found object and updating the hero count when applicable, the method returns true; if the object is absent, it returns false.

Pre-merge checks: 4

Check name	Status	Explanation
Linked Issues check		The changes address the two crash paths in [#3339]. In the Generals retail-compatible build, AIAttackAimAtTargetState::onEnter detects a contained object with no containment module, removes it from …
Out of Scope Changes check		The changed declarations, tunnel-tracker cleanup, AI guards, and conditional includes support [#3339]. The changes are limited to preventing attacks by invalid transferred tunnel occupants and destroy…
Description check		The description directly explains the crash scenario, affected retail-compatible builds, implemented failover behavior, linked issues, and reproduction evidence.
Title check		The title clearly identifies the bug fix and the crash scenario involving units from a previously transferred Tunnel Network.

Full details: Linked Issues check

Explanation

The changes address the two crash paths in [#3339]. In the Generals retail-compatible build, AIAttackAimAtTargetState::onEnter detects a contained object with no containment module, removes it from tunnel tracking, destroys it, and exits before firing logic uses the invalid containment state. The related AI target-selection path performs the same cleanup. GameLogic::friend_destroyFromTunnelContain removes and destroys stranded objects from the affected tunnel tracker. The issue provides replay evidence but does not require an automated test implementation.

Full details: Out of Scope Changes check

Explanation

The changed declarations, tunnel-tracker cleanup, AI guards, and conditional includes support [#3339]. The changes are limited to preventing attacks by invalid transferred tunnel occupants and destroying the resulting stranded objects. No unrelated feature or data change is established.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Skyaero42]

Would be good to test against with a combo of TSH and retail players. The retail players should crash out and the TSH players should be able to continue. Would any additional failover help i.e. deleting the unit when the crash is detected?

[Caball009]

Would be good to test against with a combo of TSH and retail players. The retail players should crash out and the TSH players should be able to continue.

Seems to me that the replay I included already provides that information.

Would any additional failover help i.e. deleting the unit when the crash is detected?

I'll have a look at this.

[Caball009]

Those units now get destroyed when the crash gets prevented. I've added a second replay for the new behavior.

[Skyaero42]

Looks good to me

There'll be a few more changes after this.

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 719389f1-e4a3-42d4-9da8-1f2d47f4c048

📥 Commits

Reviewing files that changed from the base of the PR and between 6983639f1654342b5a8f061b9496ec840ad47269 and 08fc877d782c4dda979a07599b9d027563e5adbd.

📒 Files selected for processing (3)

• Generals/Code/GameEngine/Include/GameLogic/GameLogic.h
• Generals/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp
• Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

[greptile-apps]

Confidence Score: 4/5

[Medium impact] The change affects two combat-related paths in retail-compatible Generals and Zero Hour.

The repository's comment-date requirement must be satisfied before merging; no new blocking behavior defect was established.

Findings

1.  Comment contains a prohibited date ▶

Summary

Adds recovery for stranded tunnel passengers when retail-compatible Generals reaches either unsafe attack path.

• Attack paths clear stranded tunnel passengers when their container data is gone.

Previous-thread context: Caball009 explained that every passenger in the affected tracker is stranded. Greptile conceded the healthy-passenger concern, and the thread was resolved. Caball009 also states that retail compatibility and memory poisoning cannot be used together in this case; that limitation is acknowledged rather than reported again.

_{Reviews (2) · Last reviewed commit: "Replicated to Zero Hour." · Reviewed by Greptile}

[xezon]

Needs replicate.

[Caball009]

I had to refactor a bit of Generals and Zero Hour code to prepare for the code replication to Zero Hour.

#if RTS_GENERALS && RETAIL_COMPATIBLE_CRC
	if( containedBy && weapon ) 
#else
	if( containedBy && weapon && contain && contain->isEnclosingContainerFor( source ) )
#endif

Non-retail compatible Generals now uses the second condition, just like Zero Hour.

[xezon]

There is an open complaint from macroslop bot