Skip to content

bugfix: Allow shared network container exits - #3154

Closed
arazmj wants to merge 1 commit into
TheSuperHackers:mainfrom
arazmj:bugfix/shared-network-container-exit
Closed

bugfix: Allow shared network container exits#3154
arazmj wants to merge 1 commit into
TheSuperHackers:mainfrom
arazmj:bugfix/shared-network-container-exit

Conversation

@arazmj

@arazmj arazmj commented Aug 15, 2026

Copy link
Copy Markdown

Problem

The exit-command validation requires the requested exit object to be the passenger's exact containedBy object. That is correct for ordinary transports, but tunnel and cave passengers belong to a shared network and may legitimately exit through another endpoint.

As a result, a unit entering one GLA tunnel can remain stuck when the player selects a different tunnel as the exit.

Change

  • Accept another endpoint only when the source and requested exit are controlled by the same player.
  • Require both endpoints to be tunnel containers or both to expose the cave interface.
  • Keep rejecting unrelated containers, mixed network types, and enemy endpoints.
  • Apply the behavior to both Generals and Zero Hour, including Zero Hour's instant-exit path.
  • Keep the existing retail-compatible CRC guard unchanged.

Validation

  • Built both Generals and Zero Hour with the change.
  • Installed matched macOS and deterministic Windows clients.
  • Confirmed units can enter one GLA tunnel and exit another.
  • Completed a multiplayer game without a synchronization error.

Permit same-player tunnel and cave endpoints to release units stored by another endpoint while preserving strict rejection for unrelated containers.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Bugfix: allow same-player tunnel/cave exits via shared network endpoints

🐞 Bug fix 🕐 20-40 Minutes

Grey Divider

AI Description

• Allow exit commands to target a different tunnel/cave endpoint within the same player network.
• Keep rejecting mismatched containers, mixed network types, and enemy-controlled exit targets.
• Apply the same validation to Generals and Zero Hour (including Zero Hour instant-exit).
Diagram

graph TD
  A["Exit command"] --> B["AIUpdateInterface"] --> C{ "ContainedBy matches?" }
  C -->|"Yes"| D["Proceed with exit"]
  C -->|"No"| E{ "Shared network allowed?" } --> F["Same controlling player"] --> G["Network type matches"] --> D
  E -->|"No"| H["Reject command"]
  G -->|"Tunnel"| I["ContainModule: isTunnelContain"]
  G -->|"Cave"| J["BehaviorModule: CaveInterface"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Centralize shared-network exit logic in a common helper/shared module
  • ➕ Avoids duplicated logic between Generals and GeneralsMD variants
  • ➕ Reduces risk of future drift (e.g., one game path updated and the other missed)
  • ➕ Makes future extension to other shared networks easier
  • ➖ May require refactoring/include plumbing across build targets
  • ➖ Could be out of scope for a focused bugfix PR
2. Move the policy into ContainModuleInterface (e.g., CanExitVia(container))
  • ➕ Encapsulates exit-network rules alongside contain/tunnel semantics
  • ➕ Avoids scanning behavior modules in AIUpdate (clearer ownership of the rule)
  • ➕ Potentially reusable by other exit-related code paths
  • ➖ Bigger API surface change; higher risk for legacy code
  • ➖ May touch more modules and require additional tests/validation

Recommendation: The PR’s approach (tight, fail-closed exception in AIUpdate guarded by existing CRC compatibility rules) is appropriate for a bugfix with minimal blast radius. If this rule is expected to evolve or be reused, consider a follow-up refactor to centralize the shared-network exit predicate to prevent Generals vs Zero Hour logic drift.

Files changed (2) +93 / -9

Bug fix (2) +93 / -9
AIUpdate.cppAllow same-player tunnel/cave endpoints for exit validation +43/-3

Allow same-player tunnel/cave endpoints for exit validation

• Adds shared-network exit checks so a unit can exit via a different tunnel/cave endpoint owned by the same player. Detects tunnel networks via ContainModuleInterface::isTunnelContain() and cave networks by scanning behavior modules for a CaveInterface, while preserving the existing retail-compatible CRC guard behavior.

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

AIUpdate.cppApply shared-network exit validation to Zero Hour (including instant-exit) +50/-6

Apply shared-network exit validation to Zero Hour (including instant-exit)

• Mirrors the Generals fix in the Zero Hour codepath and applies it to both privateExit and privateExitInstantly. Ensures cross-endpoint exits are only allowed for same-player tunnel networks or same-player cave-interface containers, rejecting all other cases under the non-retail CRC gate.

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

@Caball009

Caball009 commented Aug 15, 2026

Copy link
Copy Markdown

This seems a bit premature as there's already a (work-in-progress) PR here: #3136

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Cave index not checked 🐞 Bug ≡ Correctness
Description
isSharedNetworkExitContainer allows exiting via any same-player cave endpoint solely based on the
presence of a CaveInterface, but caves can be partitioned into separate networks via
CaveIndex/TunnelTracker. This can permit exits between unrelated cave systems controlled by the same
player, bypassing intended cave-network isolation.
Code

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp[R107-110]

+	if (fromContain->isTunnelContain() && toContain->isTunnelContain())
+		return TRUE;
+
+	return isCaveContainer(fromContainer) && isCaveContainer(toContainer);
Evidence
The PR’s cave path only checks for CaveInterface presence, but cave systems are explicitly
partitioned by CaveIndex into separate TunnelTracker instances; CaveContain redirects containment to
the tracker for its index. Therefore, two cave containers can be controlled by the same player yet
belong to different trackers, and the current predicate would incorrectly allow cross-network exits.

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp[92-111]
Generals/Code/GameEngine/Include/GameLogic/Module/CaveContain.h[42-63]
Core/GameEngine/Source/GameLogic/System/CaveSystem.cpp[89-129]
Generals/Code/GameEngine/Source/GameLogic/Object/Contain/CaveContain.cpp[67-71]
Generals/Code/GameEngine/Source/GameLogic/Object/Contain/CaveContain.cpp[239-273]

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

## Issue description
`isSharedNetworkExitContainer` currently treats *any* two same-player cave containers as being in the same shared network. Caves can be grouped into multiple independent networks using `CaveIndex` (each index maps to a different `TunnelTracker`). Without verifying that both endpoints are in the same tracker/network, a unit contained in cave-network A can be ordered to exit from cave-network B, which violates the cave system's intended separation.

## Issue Context
- Cave networks are keyed by `CaveIndex` and backed by `TunnelTracker` instances; separate indices represent separate networks.
- `CaveContain` stores/queries passengers through the `TunnelTracker` for its `m_caveIndex`.

## Fix Focus Areas
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp[77-111]
- Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp[77-111]

## Suggested fix
Update the cave-acceptance condition in `isSharedNetworkExitContainer` to also require that both containers are backed by the same shared contained-items list (i.e., the same tracker), for example:

- Fetch `fromContain->getContainedItemsList()` and `toContain->getContainedItemsList()`.
- Require both non-null and pointer-equal when allowing the cave path:
 - `return isCaveContainer(fromContainer) && isCaveContainer(toContainer) && fromList && fromList == toList;`

(Optionally, you can apply the same `getContainedItemsList()` pointer-equality check to tunnel endpoints as an extra safety net, while still keeping the existing `isTunnelContain()` checks.)

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


Grey Divider

Tip of the day
💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment on lines +107 to +110
if (fromContain->isTunnelContain() && toContain->isTunnelContain())
return TRUE;

return isCaveContainer(fromContainer) && isCaveContainer(toContainer);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Cave index not checked 🐞 Bug ≡ Correctness

isSharedNetworkExitContainer allows exiting via any same-player cave endpoint solely based on the
presence of a CaveInterface, but caves can be partitioned into separate networks via
CaveIndex/TunnelTracker. This can permit exits between unrelated cave systems controlled by the same
player, bypassing intended cave-network isolation.
Agent Prompt
## Issue description
`isSharedNetworkExitContainer` currently treats *any* two same-player cave containers as being in the same shared network. Caves can be grouped into multiple independent networks using `CaveIndex` (each index maps to a different `TunnelTracker`). Without verifying that both endpoints are in the same tracker/network, a unit contained in cave-network A can be ordered to exit from cave-network B, which violates the cave system's intended separation.

## Issue Context
- Cave networks are keyed by `CaveIndex` and backed by `TunnelTracker` instances; separate indices represent separate networks.
- `CaveContain` stores/queries passengers through the `TunnelTracker` for its `m_caveIndex`.

## Fix Focus Areas
- GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp[77-111]
- Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate.cpp[77-111]

## Suggested fix
Update the cave-acceptance condition in `isSharedNetworkExitContainer` to also require that both containers are backed by the same shared contained-items list (i.e., the same tracker), for example:

- Fetch `fromContain->getContainedItemsList()` and `toContain->getContainedItemsList()`.
- Require both non-null and pointer-equal when allowing the cave path:
  - `return isCaveContainer(fromContainer) && isCaveContainer(toContainer) && fromList && fromList == toList;`

(Optionally, you can apply the same `getContainedItemsList()` pointer-equality check to tunnel endpoints as an extra safety net, while still keeping the existing `isTunnelContain()` checks.)

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

@arazmj

arazmj commented Aug 15, 2026

Copy link
Copy Markdown
Author

Additional real-world validation: completed another multiplayer game with two macOS GeneralsX clients and one deterministic Windows client. Map loading, shared tunnel exits, sound, and synchronization all remained working with no reported problems.

@Caball009

Copy link
Copy Markdown

@Caball009 Caball009 closed this Aug 15, 2026
@arazmj

arazmj commented Aug 16, 2026

Copy link
Copy Markdown
Author

Thanks for catching the CaveIndex issue. Although this PR remains closed as a duplicate of #3136, I incorporated the correction into GeneralsX PR #254: cross-endpoint exits now require both contain modules to return the same non-null contained-items-list pointer, which maps to the same TunnelTracker and keeps separate cave networks isolated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants