tweak(Controlbar): Allow observer to view contained units if not following player

[Mr-Sheerlock]

This PR is an enhancement/suggestion for observer Controlbar functionality. It allows an observer to view exact units contained by a building/unit.

I added the m_observerLookAtPlayer null check because otherwise when following a player and the player selects a containing building/unit the UI doesn't support a way to stop following the player.

Verification:

• Checked civilian buildings, technicals/battle busses, china bunkers, GLA multiple tunnels and palace in ZH and Generals.
• Verified Evacuations Commands don't execute when pressing on any unit.

Potential issues:

• Might conflict if there are plans to escalate defeated players to observers.
• Unit slots are clickable which is incoherent UI-wise.
• Some buildings like barracks show slots despite not being garrison-able (probably because units can enter them anyway)

and also a limitation I just thought about: would be lovely if we can select a garrisoned unit and view its own garrisoned units too.

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

PR Summary by Qodo

Observer ControlBar: show container inventory when not following a player

✨ Enhancement 🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Show Structure Inventory UI for observers when selecting units/buildings with contain slots.
• Gate inventory view to only when not currently following a player (null look-at).
• Fall back to Observer List context when inventory view is not applicable.

Diagram

graph TD
  A["Observer selection"] --> B["ControlBar::update() (observer)"] --> C{"Selected object has\ncontain capacity?"}
  C -->|"yes"| D{"Following player?"}
  D -->|"no"| E["Switch/Update: CB_CONTEXT_STRUCTURE_INVENTORY"] --> F["populate/update inventory buttons"]
  D -->|"yes"| G["Switch: CB_CONTEXT_OBSERVER_LIST"] --> H["populateObserverList()"]
  C -->|"no"| G

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Add a dedicated read-only observer inventory context

• ➕ Avoids reusing an interactive context that includes exit/evacuate semantics
• ➕ Can clearly disable clicks/commands and tailor visuals for observers
• ➖ More UI/context code and wiring to maintain
• ➖ Risk of duplicating existing inventory rendering logic

2. Allow inventory view while following player (with explicit “stop following” affordance)

• ➕ Observer can keep following a player while inspecting containers
• ➕ More consistent access to container info
• ➖ Requires new UX/state transitions to avoid getting stuck
• ➖ Higher risk of regressions in observer follow behavior

3. Keep current reuse, but hard-disable inventory interactions for observers

• ➕ Minimal changes while improving UI coherence
• ➕ Reduces risk of accidental command execution from clickable slots
• ➖ Still couples observer behavior to a context designed for player control
• ➖ May require additional command-processing guards

Recommendation: Current approach (reusing CB_CONTEXT_STRUCTURE_INVENTORY) is a pragmatic, low-diff way to expose contained-unit visibility to observers, and the m_observerLookAtPlayer null gate prevents follow-state lock-in. Consider a follow-up to explicitly disable structure-inventory button interactions for observers (or provide a read-only observer inventory context) to address the remaining UI incoherence noted in the PR description.

Files changed (1) +16 / -0

Enhancement (1) +16 / -0

ControlBar.cppRoute observer UI to Structure Inventory when selectable has contain slots +16/-0

Route observer UI to Structure Inventory when selectable has contain slots

• Extends the observer-only branch in ControlBar::update() to detect selected objects with contain capacity and switch to CB_CONTEXT_STRUCTURE_INVENTORY. Adds a guard to only show this inventory view when m_observerLookAtPlayer is null (not following a player), otherwise restoring CB_CONTEXT_OBSERVER_LIST.

Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp

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

Code Review by Qodo

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

1. Inventory slots overflow 🐞 Bug ☼ Reliability

Description

ControlBar::update() now switches observers into CB_CONTEXT_STRUCTURE_INVENTORY for any selected
object with getContainMax() > 0, but the structure inventory UI only supports 10 occupant slots. If
a container ever has >10 contained objects (e.g., tunnel networks when MaxTunnelCapacity is
configured above 10), populateStructureInventory() will call populateButtonProc() past the supported
slot count, tripping the MAX_STRUCTURE_INVENTORY_BUTTONS assert and/or overwriting non-inventory
buttons.

Code

Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp[R1483-1486]

+		ContainModuleInterface* observerContain = obj ? obj->getContain() : nullptr;
+		Bool showObserverInventory = (observerContain != nullptr && observerContain->getContainMax() > 0);
+
+		if (showObserverInventory && m_observerLookAtPlayer == nullptr)

Evidence

The new observer routing uses only getContainMax() > 0 to enter structure inventory, but the
structure inventory implementation asserts that it can only populate 10 occupant buttons and still
iterates over all contained objects. Tunnel networks can be configured to have capacities above 10
via GlobalData, making this path reachable for observers.

Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp[1451-1499]
Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarStructureInventory.cpp[63-91]
Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarStructureInventory.cpp[179-196]
Generals/Code/GameEngine/Source/Common/RTS/TunnelTracker.cpp[107-112]
Generals/Code/GameEngine/Source/Common/GlobalData.cpp[395-400]

Agent prompt

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

### Issue description
Observer mode now routes any selectable container (ContainMax > 0) into `CB_CONTEXT_STRUCTURE_INVENTORY`. The structure inventory UI is hard-limited to `MAX_STRUCTURE_INVENTORY_BUTTONS` (10). If `iterateContained()` yields more than 10 occupants, `populateButtonProc()` hits its `DEBUG_ASSERTCRASH` (and in non-assert builds can start repurposing the Stop/Evacuate buttons and potentially go beyond UI expectations).

### Issue Context
- `ControlBar::update()` (observer branch) uses only `getContainMax() > 0` as the gate.
- `populateStructureInventory()` iterates *all* contained objects and calls `populateButtonProc()`.
- `populateButtonProc()` asserts `buttonIndex < MAX_STRUCTURE_INVENTORY_BUTTONS`.
- Tunnel network capacity is configurable via `GlobalData::m_maxTunnelCapacity` (INI: `MaxTunnelCapacity`), so it can exceed 10.

### Fix Focus Areas
- Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp[1483-1496]
- Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarStructureInventory.cpp[63-90]
- Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarStructureInventory.cpp[179-196]

### What to change
Implement *one* of these safe guards (preferably both A and B):
1. **A (UI-level hardening):** In `populateButtonProc()`, if `buttonIndex >= MAX_STRUCTURE_INVENTORY_BUTTONS`, return early (do not write into `m_containData` / do not enable controls). This prevents asserts/crashes and prevents Stop/Evacuate slots from being repurposed.
2. **B (observer routing guard):** In observer `update()`, only route to `CB_CONTEXT_STRUCTURE_INVENTORY` when `observerContain->getContainCount() <= MAX_STRUCTURE_INVENTORY_BUTTONS` (or clamp display to 10 with a clear rule). If count exceeds, fall back to `CB_CONTEXT_OBSERVER_LIST` or add paging/scrolling support.

Include an explicit comment explaining the 10-slot UI limitation so future changes to tunnel capacity don’t reintroduce the problem.

ⓘ 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 type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

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

1. Inventory slots overflow 🐞 Bug ☼ Reliability

ControlBar::update() now switches observers into CB_CONTEXT_STRUCTURE_INVENTORY for any selected
object with getContainMax() > 0, but the structure inventory UI only supports 10 occupant slots. If
a container ever has >10 contained objects (e.g., tunnel networks when MaxTunnelCapacity is
configured above 10), populateStructureInventory() will call populateButtonProc() past the supported
slot count, tripping the MAX_STRUCTURE_INVENTORY_BUTTONS assert and/or overwriting non-inventory
buttons.

Agent Prompt

### Issue description
Observer mode now routes any selectable container (ContainMax > 0) into `CB_CONTEXT_STRUCTURE_INVENTORY`. The structure inventory UI is hard-limited to `MAX_STRUCTURE_INVENTORY_BUTTONS` (10). If `iterateContained()` yields more than 10 occupants, `populateButtonProc()` hits its `DEBUG_ASSERTCRASH` (and in non-assert builds can start repurposing the Stop/Evacuate buttons and potentially go beyond UI expectations).

### Issue Context
- `ControlBar::update()` (observer branch) uses only `getContainMax() > 0` as the gate.
- `populateStructureInventory()` iterates *all* contained objects and calls `populateButtonProc()`.
- `populateButtonProc()` asserts `buttonIndex < MAX_STRUCTURE_INVENTORY_BUTTONS`.
- Tunnel network capacity is configurable via `GlobalData::m_maxTunnelCapacity` (INI: `MaxTunnelCapacity`), so it can exceed 10.

### Fix Focus Areas
- Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp[1483-1496]
- Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarStructureInventory.cpp[63-90]
- Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarStructureInventory.cpp[179-196]

### What to change
Implement *one* of these safe guards (preferably both A and B):
1. **A (UI-level hardening):** In `populateButtonProc()`, if `buttonIndex >= MAX_STRUCTURE_INVENTORY_BUTTONS`, return early (do not write into `m_containData` / do not enable controls). This prevents asserts/crashes and prevents Stop/Evacuate slots from being repurposed.
2. **B (observer routing guard):** In observer `update()`, only route to `CB_CONTEXT_STRUCTURE_INVENTORY` when `observerContain->getContainCount() <= MAX_STRUCTURE_INVENTORY_BUTTONS` (or clamp display to 10 with a clear rule). If count exceeds, fall back to `CB_CONTEXT_OBSERVER_LIST` or add paging/scrolling support.

Include an explicit comment explaining the 10-slot UI limitation so future changes to tunnel capacity don’t reintroduce the problem.

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