Skip to content

feat(client): Shift click to queue or cancel five units at once - #3222

Open
triatomic wants to merge 2 commits into
TheSuperHackers:mainfrom
triatomic:qol/shift-click-queue-five
Open

feat(client): Shift click to queue or cancel five units at once#3222
triatomic wants to merge 2 commits into
TheSuperHackers:mainfrom
triatomic:qol/shift-click-queue-five

Conversation

@triatomic

Copy link
Copy Markdown

Shift-clicking a unit cameo queues five copies instead of one, and shift-clicking a build queue slot cancels five instead of one. No option needed — plain clicks behave exactly as retail; only the Shift modifier changes anything.

Queueing re-runs canMakeUnit before each additional unit rather than firing five messages blindly. That check covers money, queue space, parking places and per-player unit caps — all of which move as the batch is queued — so a shift-click on the last affordable unit queues what it can and stops quietly. Only the first unit reports a failure, so the player is not spammed with five identical "not enough money" messages.

Cancelling walks the queue forward from the clicked slot, taking the most recently queued copies first and leaving the item currently building alone for as long as possible. It only cancels entries of the same production type, so a shift-click cannot silently eat unrelated queued items.

Each unit still gets its own production id and its own MSG_QUEUE_UNIT_CREATE, so the logic side sees exactly what it would from five separate clicks. No new message type, and nothing about the batch is resolved client-side beyond reading the modifier key — no determinism risk in multiplayer or replays.

Works in both Zero Hour and Generals — everything lives in the shared command bar processing.

This ships in the Contra mod's engine fork and has been played there; this PR is the port onto current main. Code was written with LLM assistance and human-reviewed, adapted and playtested by the author.

Prepared for Squash and Merge.

Shift clicking a unit cameo queues five copies instead of one, and shift
clicking a build queue slot cancels five instead of one.

Queueing re-runs canMakeUnit before each additional unit rather than firing
five messages blindly. That check covers money, queue space, parking places and
per player unit caps, all of which move as the batch is queued, so a shift
click on the last affordable unit queues what it can and stops quietly. Only
the first unit reports a failure, so the player is not spammed with five
identical "not enough money" messages.

Cancelling walks the queue forwards from the clicked slot, which takes the most
recently queued copies first and leaves the item currently building alone for
as long as possible. It only cancels entries of the same production type, so a
shift click cannot silently eat unrelated queued items.

Each unit still gets its own production id and its own MSG_QUEUE_UNIT_CREATE,
so the logic side sees exactly what it would from five separate clicks. No new
message type, and nothing about the batch is resolved client side beyond
reading the modifier key.
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Add Shift-click batching for unit production queues

✨ Enhancement 🕐 10-20 Minutes

Grey Divider

AI Description

• Adds Shift-click batching for up to five unit queue and cancellation actions.
• Revalidates each queued unit against changing resources, capacity, and unit limits.
• Preserves existing per-unit messages and unchanged plain-click behavior.
Diagram

graph TD
  K["Keyboard Input"] --> C["Control Bar"] --> D{"Command Type"}
  D -->|Queue| V["Build Validation"] --> M["Message Stream"] --> G["Game Logic"]
  D -->|Cancel| Q["Queue Selection"] --> M
Loading
High-Level Assessment

The current approach is appropriate: it composes existing single-unit commands, preserves production IDs and replay/multiplayer behavior, and reuses authoritative eligibility checks. A dedicated batch message was considered but would expand protocol and game-logic scope without a clear benefit for a five-action UI shortcut.

Files changed (1) +44 / -6

Enhancement (1) +44 / -6
ControlBarCommandProcessing.cppBatch unit queue and cancellation actions on Shift-click +44/-6

Batch unit queue and cancellation actions on Shift-click

• Reads the Shift modifier to queue up to five units, rechecking production eligibility before every additional request and assigning each unit a unique production ID. Shift-cancelling emits cancellation messages for the clicked slot and up to four subsequent unit-production queue entries, while plain clicks retain existing behavior.

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

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

qodo-free-for-open-source-projects Bot commented Aug 26, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Batch cancels unrelated units 🐞 Bug ≡ Correctness
Description
The batch loop treats every later PRODUCTION_UNIT entry as the same type as the clicked unit, so a
mixed queue can cancel up to four unrelated unit types. Cancellation is by production ID only, and
no downstream logic restores the intended same-template filtering.
Code

Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarCommandProcessing.cpp[R524-525]

+					if( m_queueData[ j ].type != PRODUCTION_UNIT )
+						continue;
Evidence
Queue population records only the broad PRODUCTION_UNIT enum and production ID even though the
production entry exposes its specific object template. The new loop checks only that broad enum, and
the game-logic handler cancels the supplied ID without checking a template, proving that mixed unit
types are canceled.

Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarCommand.cpp[599-628]
Core/GameEngine/Include/GameClient/ControlBar.h[951-962]
Generals/Code/GameEngine/Source/GameLogic/Object/Update/ProductionUpdate.cpp[445-452]
Core/GameEngine/Source/GameLogic/System/GameLogicDispatch.cpp[1843-1868]

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

## Issue description
Shift-cancel currently selects all later unit-production entries rather than only copies of the clicked unit.
## Issue Context
`QueueEntry` stores only production kind and ID, while each `ProductionEntry` also exposes the produced object template. Preserve or resolve that template and compare it before emitting each additional cancellation.
## Fix Focus Areas
- Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarCommandProcessing.cpp[493-529]
- Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarCommand.cpp[599-628]
- Core/GameEngine/Include/GameClient/ControlBar.h[951-962]

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



Remediation recommended

2. Cancellation reverses recency priority ✓ Resolved 🐞 Bug ≡ Correctness
Description
The loop scans from i + 1 upward, but production entries are displayed oldest-to-newest, so it
cancels the earliest eligible queued entries rather than the most recently queued ones. This can
remove units close to production while leaving newer copies at the tail, contrary to the
batch-cancellation behavior described by the change.
Code

Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarCommandProcessing.cpp[520]

+				for( Int j = i + 1; j < MAX_BUILD_QUEUE_BUTTONS && cancelled < SHIFT_CLICK_BATCH_SIZE; ++j )
Evidence
The control bar assigns increasing window indexes while traversing firstProduction() to
nextProduction(), and production insertion explicitly appends entries at the end of the list.
Therefore increasing j from i + 1 visits progressively newer entries but cancels the oldest of
them first, not the newest.

Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarCommand.cpp[599-623]
Generals/Code/GameEngine/Source/GameLogic/Object/Update/ProductionUpdate.cpp[980-1002]
Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarCommandProcessing.cpp[513-529]

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 additional cancellation scan runs in ascending queue order and therefore chooses older entries before newer entries.
## Issue Context
The UI is populated by traversing the production linked list from its head, while new productions are appended at the tail. Select matching entries from the highest populated queue index downward, while respecting the clicked-slot boundary and batch limit.
## Fix Focus Areas
- Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarCommandProcessing.cpp[513-530]

ⓘ 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 start a comment with 'qodo' or '@qodo' to chat about any finding

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarCommandProcessing.cpp Outdated
@greptile-apps

greptile-apps Bot commented Aug 26, 2026

Copy link
Copy Markdown

Greptile Summary

Shift-clicking a unit cameo now queues up to five units after repeated eligibility checks, while Shift-clicking a queue slot emits cancellation messages for the clicked entry and up to four later unit entries.

  • Adds Shift-key handling and a five-unit batch size.
  • Rechecks unit-production eligibility before each additional queue request.
  • Walks the displayed queue from newest to oldest when issuing additional cancellation requests.

Confidence Score: 4/5

The PR is not yet safe to merge because Shift-cancel can still remove and refund unrelated unit templates from a mixed production queue.

The current loop deliberately selects all later unit-type queue entries without comparing their templates, and the logic handler cancels each supplied production ID unconditionally.

Files Needing Attention: Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarCommandProcessing.cpp

Important Files Changed

Filename Overview
Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarCommandProcessing.cpp Adds five-item Shift queue/cancel behavior, but the previously reported mixed-template cancellation remains in the current implementation.

Sequence Diagram

sequenceDiagram
    participant Player
    participant ControlBar
    participant MessageStream
    participant GameLogic
    Player->>ControlBar: Shift-click queue slot
    ControlBar->>MessageStream: Cancel clicked production ID
    loop Up to four later PRODUCTION_UNIT slots
        ControlBar->>MessageStream: Cancel slot production ID
    end
    MessageStream->>GameLogic: Deliver cancellation commands
    GameLogic->>GameLogic: Cancel and refund each matching production ID
Loading

Reviews (2): Last reviewed commit: "fix(client): Cancel the newest queued en..." | Re-trigger Greptile

Comment on lines +523 to +525
continue;
if( m_queueData[ j ].type != PRODUCTION_UNIT )
continue;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Cancellation conflates unit templates

When later queue slots contain different unit templates, this loop treats every PRODUCTION_UNIT as a copy of the clicked unit and cancels it by production ID, causing Shift-click to remove and refund up to four unrelated queued units.

Prompt To Fix With AI
This is a comment left during a code review.
Path: Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBarCommandProcessing.cpp
Line: 523-525

Comment:
**Cancellation conflates unit templates**

When later queue slots contain different unit templates, this loop treats every `PRODUCTION_UNIT` as a copy of the clicked unit and cancels it by production ID, causing Shift-click to remove and refund up to four unrelated queued units.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Deliberate, and now stated in the code (b2b70ca): the batch is positional, not per template - its point is to clear what was just queued, whatever it was, the same way five individual clicks on the tail would. The PRODUCTION_UNIT check exists to keep an upgrade entry from being swept up, not to filter templates. The suggested template comparison is intentionally not taken.

…batch

The extra cancels scanned upward from the clicked slot, and the queue is
displayed oldest to newest - so the batch took the oldest entries after
the click, removing units close to production while newer copies
survived at the tail. Walk from the tail towards the clicked slot
instead, so the most recently queued entries go first.

The batch deliberately stays unfiltered by template: its point is to
clear what was just queued, whatever it was. Only unit entries are
taken, so an upgrade in the queue is never swept up - now stated in the
comment.
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.

1 participant