bugfix(tunnel): Restore retail compatibility after changes to TunnelTracker::onTunnelDestroyed() and TunnelContain::onCapture()

[Caball009]

• Closes Generals replay incompatibility #2496
• Follow-up for fix(tunnel): Fix undefined behavior in TunnelTracker::onTunnelDestroyed() and incomplete asset transfer in Team::setControllingPlayer() #1958

This PR makes 3 changes (one commit for each):

1. • Restore retail compatibility: adding TunnelContain::onCapture to Generals is not retail compatible so this needs to be behind the RETAIL_COMPATIBLE_CRC macro.
2. • Restore retail compatibility: the early return in TunnelTracker::onTunnelDestroyed is not retail compatible so this had to be changed.
   • Fix a potential crash. A side effect of the early return is that the members of the contain list (m_containList) hold on to a pointer to their container. This can lead to use-after-free bugs and crashes. The early return is removed.
3. • Add non-retail code to TunnelTracker::onTunnelDestroyed that only relies on m_tunnelIDs and doesn't use m_tunnelCount. It's also removed from TunnelTracker::xfer.

The m_tunnelCount integer underflow cannot be fixed with retail compatibility enabled, but the code should be well-defined and safe now.

See commits for cleaner diffs; third commit partially changes code from second commit.

---

Object *validTunnel = TheGameLogic->findObjectByID( m_tunnelIDs.front() );

I'm not entirely familiar with the STLPort implementation of std::list<T>, but accessing the front element of an empty std::list appears to result reliably in T{} for trivial types; INVALID_ID in this context. As a result, all contained objects have their container pointer updated to nullptr, neatly avoiding the use-after-free bugs and resulting crash.

---

Crash reproduction with the current main branch (VC6 / VS22, RETAIL_COMPATIBLE_CRC == 1), using Zero Hour GLA campaign mission 3:

Video and call stack

gen_zh_md_gla03_campaign_tunnel_crash.mp4

generalszh.exe!AIUpdateInterface::getNextMoodTarget(bool calledByAI, bool calledDuringIdle) Line 4557
generalszh.exe!AIIdleState::update() Line 1443
generalszh.exe!StateMachine::updateStateMachine() Line 439
generalszh.exe!AIStateMachine::updateStateMachine() Line 884
generalszh.exe!AIUpdateInterface::update() Line 1015
generalszh.exe!GameLogic::update() Line 3873
generalszh.exe!SubsystemInterface::UPDATE() Line 131
generalszh.exe!GameEngine::update() Line 921
generalszh.exe!Win32GameEngine::update() Line 91
generalszh.exe!GameEngine::execute() Line 983
generalszh.exe!GameMain() Line 55
generalszh.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow) Line 929

TODO:

• Replicate last commit to Generals.
• Update PR description to include the changes after the second commit.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Restore retail-compatible tunnel lifecycle and passenger cleanup

🐞 Bug fix ✨ Enhancement ⚙️ Configuration changes 🕐 20-40 Minutes

AI Description

• Restores retail-compatible tunnel lifecycle behavior in Generals and Zero Hour.
• Clears stale passenger container pointers when no replacement tunnel remains.
• Derives modern tunnel state from IDs while preserving retail counters and saves.

Diagram

graph TD
  Contain["Tunnel lifecycle"] --> Tracker["TunnelTracker"] --> Mode{"Retail build?"}
  Mode -->|Yes| Retail["Retail counter"] --> Destroy["Destroy handling"] --> Passengers["Passenger links"]
  Mode -->|No| Modern["Tunnel ID list"] --> Destroy
  Tracker --> Save["Save transfer"]
  Modern --> Save

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Always derive state from tunnel IDs

• ➕ Eliminates duplicated counter state and underflow risk.
• ➕ Provides one consistent destruction path for all builds.
• ➖ Breaks retail CRC compatibility.
• ➖ Changes legacy save and runtime behavior.

2. Retain early return with targeted cleanup

• ➕ Minimizes changes to the existing destruction routine.
• ➕ Could prevent stale passenger pointers after invalid removals.
• ➖ Still diverges from retail control flow.
• ➖ Leaves duplicated counter and ID-list state vulnerable to desynchronization.

Recommendation: Keep the PR's split implementation: preserve legacy counter behavior only where retail compatibility requires it, while making the ID list authoritative in modern builds. This limits compatibility risk and removes redundant state where possible; reviewers should closely validate both macro configurations and legacy save loading.

Files changed (8) +116 / -27

Bug fix (5) +106 / -25

TunnelContain.hConditionally declare the tunnel capture override +2/-0

Conditionally declare the tunnel capture override

• Excludes 'TunnelContain::onCapture' from retail-compatible Generals builds to restore the original virtual interface and CRC behavior.

Generals/Code/GameEngine/Include/GameLogic/Module/TunnelContain.h

TunnelTracker.cppPreserve destruction cleanup without unsafe empty-list access +13/-13

Preserve destruction cleanup without unsafe empty-list access

• Replaces the missing-tunnel early return with assertions and retail-compatible removal behavior. It safely clears passenger container pointers when the tunnel count and ID list diverge and no replacement tunnel exists.

Generals/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp

TunnelContain.cppUse last-tunnel query and guard capture handling +3/-1

Use last-tunnel query and guard capture handling

• Uses 'isLastTunnel()' when selling a tunnel and conditionally compiles the capture implementation outside retail-compatible Generals builds.

Generals/Code/GameEngine/Source/GameLogic/Object/Contain/TunnelContain.cpp

TunnelTracker.cppSplit retail and modern tunnel destruction paths +85/-10

Split retail and modern tunnel destruction paths

• Preserves legacy counter-based destruction in retail builds while modern builds use the ID list as authoritative state. It also adds safe passenger reassignment, registration diagnostics, and versioned transfer support for counter-free saves.

GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp

TunnelContain.cppAdopt last-tunnel query and conditional capture logic +3/-1

Adopt last-tunnel query and conditional capture logic

• Switches selling behavior to 'isLastTunnel()' and conditionally compiles the capture implementation according to retail compatibility requirements.

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/TunnelContain.cpp

Refactor (2) +8 / -2

TunnelTracker.hReplace raw tunnel-count access with last-tunnel query +1/-1

Replace raw tunnel-count access with last-tunnel query

• Replaces the exposed counter getter with an intent-specific 'isLastTunnel()' method used during tunnel sales.

Generals/Code/GameEngine/Include/Common/TunnelTracker.h

TunnelTracker.hMake tunnel tracking state build-mode dependent +7/-1

Make tunnel tracking state build-mode dependent

• Keeps the legacy tunnel counter only for retail-compatible builds. Modern builds determine last-tunnel status directly from the tunnel ID list.

GeneralsMD/Code/GameEngine/Include/Common/TunnelTracker.h

Other (1) +2 / -0

TunnelContain.hGuard the tunnel capture override for compatibility +2/-0

Guard the tunnel capture override for compatibility

• Applies the retail-compatibility condition to the 'onCapture' declaration while retaining it for applicable Zero Hour builds.

GeneralsMD/Code/GameEngine/Include/GameLogic/Module/TunnelContain.h

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

1. Tunnel creation assertion inverted 🐞 Bug ≡ Correctness

Description

onTunnelCreated asserts that a new tunnel ID is already tracked, so every normal first-time
registration triggers DEBUG_CRASHING builds while duplicate registrations pass. The ID is only
appended after this check, and the normal creation callback performs no earlier insertion.

Code

GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[R229-230]

+	DEBUG_ASSERTCRASH(std::find(m_tunnelIDs.begin(), m_tunnelIDs.end(), newTunnel->getID()) != m_tunnelIDs.end(),
+		("TunnelTracker::onTunnelCreated - New tunnel was already added; this shouldn't happen"));

Evidence

The assertion tests find(...) != end() before push_back adds the ID, while
TunnelContain::onObjectCreated directly calls this registration method for newly created tunnels.
DEBUG_ASSERTCRASH invokes DEBUG_CRASH when its condition is false.

GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[227-235]
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/TunnelContain.cpp[460-477]
Core/GameEngine/Include/Common/Debug.h[186-198]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`TunnelTracker::onTunnelCreated` uses an inverted assertion: it requires the new tunnel ID to already exist, causing normal registrations to trigger a debug crash while allowing duplicates.
## Issue Context
The tunnel ID is appended only after the assertion. The normal `TunnelContain::onObjectCreated` path calls this method without inserting the ID first.
## Fix Focus Areas
- GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[229-230]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Untracked destruction corrupts count 🐞 Bug ≡ Correctness

Description

onTunnelDestroyed now decrements m_tunnelCount even when remove found no matching tunnel or
the count is already zero, so a wrong-tracker notification can underflow the count or make the
tracker cave in while valid tunnels remain. This is reachable in retail Generals after
Team::setControllingPlayer: capture callbacks are skipped, but later tunnel deletion resolves the
new owner's tracker and reports an ID that tracker never registered.

Code

Generals/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[247]

+	m_tunnelCount--;

Evidence

onTunnelCreated is the operation that increments the count and appends the ID, but the changed
destroy path diagnoses a failed removal and nevertheless decrements the unsigned count. In retail
mode, team reassignment only updates partition state rather than invoking capture callbacks; the
tunnel remains marked registered, and its deletion looks up the current owner's tracker, providing a
concrete path to failed removal. DEBUG_CRASH is compiled to a no-op when debug crashing is
disabled, so it does not prevent the release-state corruption.

Generals/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[224-268]
Generals/Code/GameEngine/Source/Common/RTS/Team.cpp[1391-1418]
Generals/Code/GameEngine/Source/GameLogic/Object/Contain/TunnelContain.cpp[359-374]
Core/GameEngine/Include/Common/Debug.h[175-206]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`TunnelTracker::onTunnelDestroyed` unconditionally decrements the unsigned tunnel count after detecting that the tunnel was not registered. This can underflow or corrupt a different owner's tracker; retain the containment-pointer cleanup needed by this PR without applying registration bookkeeping when removal failed.
## Issue Context
The retail `Team::setControllingPlayer` path skips `Object::onCapture`, while `TunnelContain::onDelete` obtains the tracker from the object's current owner. Therefore an untracked destruction is a reachable state, not merely a defensive assertion case. Keep the Generals and GeneralsMD implementations behaviorally aligned.
## Fix Focus Areas
- Generals/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[234-268]
- GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[235-269]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Missing crash format argument 🐞 Bug ☼ Reliability

Description

The failed-removal assertion passes a %s format specifier to DebugCrash without supplying the
tunnel name argument. When this diagnostic executes, the variadic formatter reads an absent
argument, potentially crashing or emitting corrupt diagnostic output instead of reporting the
tracker failure.

Code

GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[R251-252]

+	DEBUG_ASSERTCRASH(oldSize != m_tunnelIDs.size(),
+		("TunnelTracker::onTunnelDestroyed - Attempting to remove object '%s' that has never been tracked as a tunnel"));

Evidence

The assertion's message contains %s but no second argument, whereas the equivalent non-retail
diagnostic supplies deadTunnel->getName().str(). The debug macro forwards the tuple directly to
variadic DebugCrash, so no argument is added automatically.

GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[247-252]
GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[284-289]
Core/GameEngine/Include/Common/Debug.h[175-198]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The failed tunnel-removal assertion contains a `%s` placeholder but does not pass the corresponding tunnel name, resulting in invalid variadic formatting when the assertion fires.
## Issue Context
`DEBUG_ASSERTCRASH` forwards its message tuple directly to the variadic `DebugCrash` function. The non-retail branch immediately below demonstrates the intended call with `deadTunnel->getName().str()`.
## Fix Focus Areas
- GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[251-252]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Tickets: 🎫 Generals replay incompatibility

Tip of the day

💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 4faa5ea ⚖️ Balanced

Results up to commit 4faa5ea

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

1. Tunnel creation assertion inverted 🐞 Bug ≡ Correctness ⭐ New

Description

onTunnelCreated asserts that a new tunnel ID is already tracked, so every normal first-time
registration triggers DEBUG_CRASHING builds while duplicate registrations pass. The ID is only
appended after this check, and the normal creation callback performs no earlier insertion.

Code

GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[R229-230]

+	DEBUG_ASSERTCRASH(std::find(m_tunnelIDs.begin(), m_tunnelIDs.end(), newTunnel->getID()) != m_tunnelIDs.end(),
+		("TunnelTracker::onTunnelCreated - New tunnel was already added; this shouldn't happen"));

Evidence

The assertion tests find(...) != end() before push_back adds the ID, while
TunnelContain::onObjectCreated directly calls this registration method for newly created tunnels.
DEBUG_ASSERTCRASH invokes DEBUG_CRASH when its condition is false.

GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[227-235]
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/TunnelContain.cpp[460-477]
Core/GameEngine/Include/Common/Debug.h[186-198]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`TunnelTracker::onTunnelCreated` uses an inverted assertion: it requires the new tunnel ID to already exist, causing normal registrations to trigger a debug crash while allowing duplicates.

## Issue Context
The tunnel ID is appended only after the assertion. The normal `TunnelContain::onObjectCreated` path calls this method without inserting the ID first.

## Fix Focus Areas
- GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[229-230]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Untracked destruction corrupts count 🐞 Bug ≡ Correctness

Description

onTunnelDestroyed now decrements m_tunnelCount even when remove found no matching tunnel or
the count is already zero, so a wrong-tracker notification can underflow the count or make the
tracker cave in while valid tunnels remain. This is reachable in retail Generals after
Team::setControllingPlayer: capture callbacks are skipped, but later tunnel deletion resolves the
new owner's tracker and reports an ID that tracker never registered.

Code

Generals/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[247]

+	m_tunnelCount--;

Evidence

onTunnelCreated is the operation that increments the count and appends the ID, but the changed
destroy path diagnoses a failed removal and nevertheless decrements the unsigned count. In retail
mode, team reassignment only updates partition state rather than invoking capture callbacks; the
tunnel remains marked registered, and its deletion looks up the current owner's tracker, providing a
concrete path to failed removal. DEBUG_CRASH is compiled to a no-op when debug crashing is
disabled, so it does not prevent the release-state corruption.

Generals/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[224-268]
Generals/Code/GameEngine/Source/Common/RTS/Team.cpp[1391-1418]
Generals/Code/GameEngine/Source/GameLogic/Object/Contain/TunnelContain.cpp[359-374]
Core/GameEngine/Include/Common/Debug.h[175-206]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`TunnelTracker::onTunnelDestroyed` unconditionally decrements the unsigned tunnel count after detecting that the tunnel was not registered. This can underflow or corrupt a different owner's tracker; retain the containment-pointer cleanup needed by this PR without applying registration bookkeeping when removal failed.
## Issue Context
The retail `Team::setControllingPlayer` path skips `Object::onCapture`, while `TunnelContain::onDelete` obtains the tracker from the object's current owner. Therefore an untracked destruction is a reachable state, not merely a defensive assertion case. Keep the Generals and GeneralsMD implementations behaviorally aligned.
## Fix Focus Areas
- Generals/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[234-268]
- GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[235-269]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Missing crash format argument 🐞 Bug ☼ Reliability ⭐ New

Description

The failed-removal assertion passes a %s format specifier to DebugCrash without supplying the
tunnel name argument. When this diagnostic executes, the variadic formatter reads an absent
argument, potentially crashing or emitting corrupt diagnostic output instead of reporting the
tracker failure.

Code

GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[R251-252]

+	DEBUG_ASSERTCRASH(oldSize != m_tunnelIDs.size(),
+		("TunnelTracker::onTunnelDestroyed - Attempting to remove object '%s' that has never been tracked as a tunnel"));

Evidence

The assertion's message contains %s but no second argument, whereas the equivalent non-retail
diagnostic supplies deadTunnel->getName().str(). The debug macro forwards the tuple directly to
variadic DebugCrash, so no argument is added automatically.

GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[247-252]
GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[284-289]
Core/GameEngine/Include/Common/Debug.h[175-198]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The failed tunnel-removal assertion contains a `%s` placeholder but does not pass the corresponding tunnel name, resulting in invalid variadic formatting when the assertion fires.

## Issue Context
`DEBUG_ASSERTCRASH` forwards its message tuple directly to the variadic `DebugCrash` function. The non-retail branch immediately below demonstrates the intended call with `deadTunnel->getName().str()`.

## Fix Focus Areas
- GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[251-252]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Tickets: 🎫 Generals replay incompatibility

Results up to commit 342ce12

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

1. Untracked destruction corrupts count 🐞 Bug ≡ Correctness

Description

onTunnelDestroyed now decrements m_tunnelCount even when remove found no matching tunnel or
the count is already zero, so a wrong-tracker notification can underflow the count or make the
tracker cave in while valid tunnels remain. This is reachable in retail Generals after
Team::setControllingPlayer: capture callbacks are skipped, but later tunnel deletion resolves the
new owner's tracker and reports an ID that tracker never registered.

Code

Generals/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[247]

+	m_tunnelCount--;

Evidence

onTunnelCreated is the operation that increments the count and appends the ID, but the changed
destroy path diagnoses a failed removal and nevertheless decrements the unsigned count. In retail
mode, team reassignment only updates partition state rather than invoking capture callbacks; the
tunnel remains marked registered, and its deletion looks up the current owner's tracker, providing a
concrete path to failed removal. DEBUG_CRASH is compiled to a no-op when debug crashing is
disabled, so it does not prevent the release-state corruption.

Generals/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[224-268]
Generals/Code/GameEngine/Source/Common/RTS/Team.cpp[1391-1418]
Generals/Code/GameEngine/Source/GameLogic/Object/Contain/TunnelContain.cpp[359-374]
Core/GameEngine/Include/Common/Debug.h[175-206]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`TunnelTracker::onTunnelDestroyed` unconditionally decrements the unsigned tunnel count after detecting that the tunnel was not registered. This can underflow or corrupt a different owner's tracker; retain the containment-pointer cleanup needed by this PR without applying registration bookkeeping when removal failed.
## Issue Context
The retail `Team::setControllingPlayer` path skips `Object::onCapture`, while `TunnelContain::onDelete` obtains the tracker from the object's current owner. Therefore an untracked destruction is a reachable state, not merely a defensive assertion case. Keep the Generals and GeneralsMD implementations behaviorally aligned.
## Fix Focus Areas
- Generals/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[234-268]
- GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[235-269]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Tickets: 🎫 Generals replay incompatibility

Results up to commit 1d42639

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

1. Untracked destruction corrupts count 🐞 Bug ≡ Correctness

Description

onTunnelDestroyed now decrements m_tunnelCount even when remove found no matching tunnel or
the count is already zero, so a wrong-tracker notification can underflow the count or make the
tracker cave in while valid tunnels remain. This is reachable in retail Generals after
Team::setControllingPlayer: capture callbacks are skipped, but later tunnel deletion resolves the
new owner's tracker and reports an ID that tracker never registered.

Code

Generals/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[247]

+	m_tunnelCount--;

Evidence

onTunnelCreated is the operation that increments the count and appends the ID, but the changed
destroy path diagnoses a failed removal and nevertheless decrements the unsigned count. In retail
mode, team reassignment only updates partition state rather than invoking capture callbacks; the
tunnel remains marked registered, and its deletion looks up the current owner's tracker, providing a
concrete path to failed removal. DEBUG_CRASH is compiled to a no-op when debug crashing is
disabled, so it does not prevent the release-state corruption.

Generals/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[224-268]
Generals/Code/GameEngine/Source/Common/RTS/Team.cpp[1391-1418]
Generals/Code/GameEngine/Source/GameLogic/Object/Contain/TunnelContain.cpp[359-374]
Core/GameEngine/Include/Common/Debug.h[175-206]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`TunnelTracker::onTunnelDestroyed` unconditionally decrements the unsigned tunnel count after detecting that the tunnel was not registered. This can underflow or corrupt a different owner's tracker; retain the containment-pointer cleanup needed by this PR without applying registration bookkeeping when removal failed.

## Issue Context
The retail `Team::setControllingPlayer` path skips `Object::onCapture`, while `TunnelContain::onDelete` obtains the tracker from the object's current owner. Therefore an untracked destruction is a reachable state, not merely a defensive assertion case. Keep the Generals and GeneralsMD implementations behaviorally aligned.

## Fix Focus Areas
- Generals/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[234-268]
- GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[235-269]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Tickets: 🎫 Generals replay incompatibility

[greptile-apps]

Greptile Summary

The PR restores retail-compatible tunnel behavior while making tunnel destruction cleanup well-defined in both game variants.

• Gates the added capture override where retail compatibility requires the original virtual layout.
• Separates retail-compatible tunnel counting from non-retail list-based bookkeeping.
• Safely handles an empty tunnel-ID list while updating contained-object links.
• Updates tunnel-tracker serialization for the removed non-retail count field.
• Corrects the previously reported registration, destruction-validation, and list-iterator issues.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp	Splits retail and non-retail tunnel lifecycle bookkeeping, guards empty-list access, and versions serialization consistently.
GeneralsMD/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp	Mirrors the tunnel lifecycle and serialization changes while correcting the three previously reported assertion and iterator defects.
Generals/Code/GameEngine/Source/GameLogic/Object/Contain/TunnelContain.cpp	Conditionally excludes the capture override from the retail-compatible Generals configuration.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/TunnelContain.cpp	Applies the matching capture-override compatibility guard to Zero Hour.
Generals/Code/GameEngine/Include/Common/TunnelTracker.h	Retains the explicit tunnel counter only for retail-compatible builds and derives the non-retail count from the ID list.
GeneralsMD/Code/GameEngine/Include/Common/TunnelTracker.h	Mirrors the conditional tunnel-count representation for Zero Hour.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Tunnel destroyed] --> B{Retail-compatible CRC?}
  B -- Yes --> C[Remove matching tunnel ID]
  C --> D[Decrement retail tunnel count]
  D --> E{Count is zero?}
  E -- Yes --> F[Destroy contained units and clear list]
  E -- No --> G[Choose remaining tunnel or nullptr]
  G --> H[Redirect occupants linked to dead tunnel]
  B -- No --> I[Find and erase matching list entry]
  I --> J{Tunnel ID list empty?}
  J -- Yes --> F
  J -- No --> K[Redirect occupants to remaining tunnel]

Loading

_{Reviews (16): Last reviewed commit: "Added complete retail incompatible versi..." | Re-trigger Greptile}

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit 342ce12

[OmarAglan]

@codex

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: a34726cff5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit 4faa5ea

[qodo-free-for-open-source-projects]

Code review by qodo was updated up to the latest commit 4faa5ea

[xezon]

In CRC we trust

[Caball009]

Code replicated to Generals (manually), PR description updated, and verified that the mismatching Generals replays are working again. Ready to be merged.