bugfix(aiupdate): Let Tunnel Network passengers exit from any tunnel

[Okladnoj]

Follow-up to #3089 — problem: evacuating a Tunnel Network does not work (build two
tunnels, load units through the first one, press Evacuate on the second one — nobody
comes out).

A Tunnel Network keeps a single contents list shared by every tunnel, while a
passenger's getContainedBy() points at the tunnel it entered, so #3089 rejects
everyone addressed to a different tunnel. Now we ask the addressed container whether it
holds the unit, instead of comparing container objects.

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

PR Summary by Qodo

Fix Tunnel Network evacuation by validating containment via shared contents list

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Fix exit command validation to support Tunnel Networks with shared contents lists.
• Reject exit requests only when the addressed tunnel does not actually contain the unit.
• Apply the same fix to both Generals and GeneralsMD engine variants.

Diagram

graph TD
  A["Exit/Evacuate command"] --> B["AIUpdateInterface::privateExit"] --> C["Read container's ContainModule"] --> D[("Contained items list")] --> E{"Contains unit?"} --> F["Allow exit processing"]
  E --> G["Reject/return"]
  subgraph Legend
    direction LR
    _p["Process"] ~~~ _d{"Decision"} ~~~ _ds[("Data")]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Fix containment ownership pointer for Tunnel Networks

• ➕ Preserves the original identity-based check (us->getContainedBy() == addressed container).
• ➕ Could reduce future surprises where shared-list containers behave differently.
• ➖ Higher risk: changes core containment semantics and potentially many callers.
• ➖ Harder to guarantee correctness if multiple tunnel objects intentionally share state.

2. Add a container API like contains(Object*)

• ➕ Centralizes containment validation behind an interface method (no list peeking at call sites).
• ➕ Allows specialized containers (Tunnel Network) to override containment logic cleanly.
• ➖ Requires broader refactor across exit paths and other containment checks.
• ➖ More code churn than this targeted fix.

Recommendation: The PR’s approach is the best low-risk fix for the regression described in #3089: validate exit commands by verifying the addressed container actually contains the unit (via the shared contents list). Consider a follow-up refactor to introduce a dedicated contains() query on the containment module to avoid repeating list-based checks at call sites.

Files changed (2) +12 / -6

Bug fix (2) +12 / -6

AIUpdate.cppValidate exit against addressed container contents (Tunnel Network-safe) +4/-2

Validate exit against addressed container contents (Tunnel Network-safe)

• Updates non-retail exit-command validation to check whether the addressed container’s contained-items list includes the unit. This prevents false rejection when Tunnel Network tunnels share a single contents list across multiple tunnel objects.

Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp

AIUpdate.cppMirror Tunnel Network exit validation fix for MD (normal + instant exit) +8/-4

Mirror Tunnel Network exit validation fix for MD (normal + instant exit)

• Applies the same containment-by-list validation to both privateExit and privateExitInstantly in the MD variant. Ensures evacuate/exit works regardless of which tunnel instance is targeted.

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp

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

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

1. O(n²) tunnel evacuate 🐞 Bug ➹ Performance

Description

In !RETAIL_COMPATIBLE_CRC builds, privateExit/privateExitInstantly now do a linear std::find over
ContainedItemsList for every exit command. When OpenContain::orderAllPassengersToExit iterates that
same list and issues one exit per passenger (TunnelContain uses a shared tunnel-system list), this
adds an extra scan per passenger and can turn a single evacuate into O(n²) list traversal.

Code

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp[R3836-3839]

+		const ContainModuleInterface* contain = objectToExit->getContain();
+		const ContainedItemsList* items = contain != nullptr ? contain->getContainedItemsList() : nullptr;

-		if (us->getContainedBy() != objectToExit)
+		if (items == nullptr || std::find(items->begin(), items->end(), us) == items->end())

Evidence

The PR adds a std::find membership check in AIUpdate. OpenContain evacuation loops over
getContainedItemsList() and calls aiExit/aiExitInstantly once per rider, so the new code adds
one extra linear scan per passenger. TunnelContain redirects getContainedItemsList() to a shared
tunnel-system list, making repeated scans over the same list likely during tunnel evacuation.

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp[3832-3841]
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp[3873-3882]
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp[1428-1450]
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/TunnelContain.cpp[328-336]
Generals/Code/GameEngine/Source/GameLogic/Object/Contain/OpenContain.cpp[1330-1344]
Generals/Code/GameEngine/Source/GameLogic/Object/Contain/TunnelContain.cpp[251-259]

Agent prompt

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

### Issue description
`AIUpdateInterface::privateExit` / `privateExitInstantly` validate the addressed container by scanning its `ContainedItemsList` with `std::find`. During `OpenContain::orderAllPassengersToExit`, this validation is invoked once per passenger while iterating the same list, producing avoidable repeated list scans.

### Issue Context
This path is used by Tunnel Networks because `TunnelContain::getContainedItemsList()` returns the shared `TunnelTracker` list.

### Fix Focus Areas
- Prefer O(1) fast-paths before `std::find`:
 - If `us->getContainedBy() == objectToExit`, accept without scanning.
 - If `us->getContainedBy()` has a `ContainModuleInterface` and its `getContainedItemsList()` pointer equals the addressed container’s `items` pointer (shared tunnel list), accept without scanning.
 - Only fall back to `std::find` when the above checks fail.

- file/path[start_line-end_line]
 - Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp[3677-3685]
 - GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp[3832-3840]
 - GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp[3873-3881]

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

---

2. Null list blocks exit 🐞 Bug ☼ Reliability

Description

The new validation returns early when getContainedItemsList() is nullptr, even if the unit is
legitimately contained by the addressed container (previously it only compared us->getContainedBy()
vs objectToExit). This is a behavioral change and can silently drop exit commands in cases where a
contain module returns a null list (e.g., TunnelContain when its tunnel system is unavailable).

Code

Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp[R3682-3685]

+		const ContainedItemsList* items = contain != nullptr ? contain->getContainedItemsList() : nullptr;

-		if (us->getContainedBy() != objectToExit)
+		if (items == nullptr || std::find(items->begin(), items->end(), us) == items->end())
			return;

Evidence

AIUpdate now returns when items == nullptr. TunnelContain’s getContainedItemsList() explicitly
returns nullptr when the owning player or tunnel system is missing, so this new behavior can cause
exit commands to be rejected solely due to list unavailability.

Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp[3679-3686]
Generals/Code/GameEngine/Source/GameLogic/Object/Contain/TunnelContain.cpp[251-259]
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/TunnelContain.cpp[328-336]

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 updated exit-command validation treats `items == nullptr` as “not contained” and returns. Previously, a passenger could still exit when `us->getContainedBy() == objectToExit` regardless of list access.

### Issue Context
At least one container implementation can return `nullptr` from `getContainedItemsList()` when its backing structure is unavailable (TunnelContain returns `nullptr` if `owningPlayer` or `owningPlayer->getTunnelSystem()` is null).

### Fix Focus Areas
- Preserve prior behavior as a safe fallback:
 - If `items == nullptr`, fall back to `if (us->getContainedBy() != objectToExit) return;` instead of unconditional return.
 - (Optional, complements perf fix) Accept when the addressed container shares the same `ContainedItemsList*` pointer as `us->getContainedBy()`.

- file/path[start_line-end_line]
 - Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp[3679-3685]
 - GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp[3834-3840]
 - GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp[3875-3881]

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

---

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 ↗

[Caball009]

Good find. I overlooked this.

[bobtista]

LGTM
The only nit I can see is some if () returns without braces, which is just formatting preference.

[xezon]

A few comments and questions

[Okladnoj]

@xezon
Date restored, double null test replaced with an early return, comment reworded.

On isTunnelContain(). That won't work — CaveContain returns FALSE from it but hands out the same shared list. The fix currently covers caves as well, and the gate would leave them broken.

There is no extra iteration anyway: the outer containedBy != objectToExit check already filters out ordinary containers.

On unbinding the container pointer earlier. That changes game logic and would show up in the CRC, so better as a separate PR.

[xezon]

Can we perhaps add bool isTunnelContain() { return true; } to CaveContain or will that break anything?

[Okladnoj]

Can we perhaps add bool isTunnelContain() { return true; } to CaveContain or will that break anything?

Probably will — isTunnelContain() currently doubles as a type test before a static_cast.

TunnelTracker::updateFullHealTime(), both trees:

if (!contain->isTunnelContain())
    continue;
const TunnelContain* tunnelContain = static_cast<const TunnelContain*>(contain);

Caves do reach that loop — they register through onTunnelCreated() (CaveContain.cpp:256, :272). With TRUE they would be cast to TunnelContain and getFullTimeForHeal() would read the wrong module data. The path sits behind PRESERVE_TUNNEL_HEAL_STACKING, but that flag is meant to go to 0.

Second one is AcademyStats.cpp:413: caves would count as a Tunnel Network. Not a crash, but there is no guard, so it would affect RETAIL too.

[xezon]

Ok. How about we add a new virtual function to contain modules bool isSharedContainer(), and make that true for TunnelContain and CaveContain. And then we can test the containment if the container is shared. Basically the idea here is to avoid unnecessary contain list inspection when not needed.

[Okladnoj]

Ok. How about we add a new virtual function to contain modules bool isSharedContainer(), and make that true for TunnelContain and CaveContain. And then we can test the containment if the container is shared. Basically the idea here is to avoid unnecessary contain list inspection when not needed.

Done. TRUE for TunnelContain and CaveContain, FALSE by default in OpenContain — no other module needed touching. Thanks for the architectural hint :)

Ran a ~95,700 frame replay, all good.

Implementation change since review