bugfix: Allow shared network container exits - #3154
Conversation
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>
PR Summary by QodoBugfix: allow same-player tunnel/cave exits via shared network endpoints
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
|
This seems a bit premature as there's already a (work-in-progress) PR here: #3136 |
Code Review by Qodo
1. Cave index not checked
|
| if (fromContain->isTunnelContain() && toContain->isTunnelContain()) | ||
| return TRUE; | ||
|
|
||
| return isCaveContainer(fromContainer) && isCaveContainer(toContainer); |
There was a problem hiding this comment.
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
|
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. |
|
Thanks for catching the |
Problem
The exit-command validation requires the requested exit object to be the passenger's exact
containedByobject. 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
Validation