feat(client): Add GridHotkeys and NonGridHotkeys options - #3215
feat(client): Add GridHotkeys and NonGridHotkeys options#3215triatomic wants to merge 4 commits into
Conversation
…osition Adds an Options.ini client preference assigning command bar hotkeys by slot position rather than by the letter the string file marked with an ampersand, so the keys stay in the same place whatever is being built: GridHotkeys = Yes ; off by default GridHotkeyLayout = QWERTYUIOASDFGHJKL ; key per slot, reading order GridHotkeyColumns = 9 ; bar width for the mapping The engine numbers command slots down each column rather than across each row, so the layout string is mapped through the column count. Slots past the end of the layout, and buttons outside the command bar, fall back to the string file letter. With grid hotkeys on, a visible command bar button that claims a key wins over a meta event bound to the same letter - but only for an unmodified (or Shift-batched) press, so Ctrl and Alt combos keep their meta events. Available in both Zero Hour and Generals: the consuming code lives in the shared ControlBar, HotKey and MetaEvent translators.
Keys listed in Options.ini opt out of the grid entirely, so a mod can leave S, G and friends on their usual bindings while everything else grids: NonGridHotkeys = SG ; letters only, case insensitive, separators optional An excluded slot falls back to its string file letter, unless that letter collides with one the grid layout is still using - addHotKey keeps whichever button registered first and silently drops the other, so the excluded slot is left without a key rather than stealing one at random. An excluded key also keeps its meta event unconditionally, which is the point of listing it. Available in both Zero Hour and Generals, like GridHotkeys.
PR Summary by QodoAdd configurable grid and non-grid command-bar hotkeys
AI Description
Diagram
High-Level Assessment
Files changed (11)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| // Rows come from the layout's own length, not MAX_COMMANDS_PER_SET, which is the | ||
| // internal cap of 32 rather than however many slots the mod actually shows. | ||
| const Int rows = ( layout.getLength() + columns - 1 ) / columns; |
There was a problem hiding this comment.
3. Short layouts lose keys 🐞 Bug ≡ Correctness
The remapping derives the row count from layout.length / columns, so a valid layout shorter than GridHotkeyColumns produces one row and maps every slot after slot zero beyond the string. For example, layout QWER with the default nine columns assigns only Q, contradicting the stated behavior that only slots past the layout end should be keyless.
Agent Prompt
## Issue description
Short custom layouts are treated as changing the bar's row count, causing covered prefix slots to map out of bounds instead of receiving their configured keys.
## Issue Context
The layout length controls how many reading-order slots have keys; it should not redefine the command bar's physical row count. Preserve the documented `columns = 0` slot-order behavior.
## Fix Focus Areas
- Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp[2483-2502]
- Core/GameEngine/Source/Common/OptionPreferences.cpp[234-262]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Fixed in afd0926: a non-alphanumeric layout character is now a placeholder - the slot is covered by the grid but keyless - so a short layout keeps its row shape by padding (e.g. QWERT....). The option's documentation now states the whole-rows contract, since the row count is derived from the layout's length.
|
| Filename | Overview |
|---|---|
| Core/GameEngine/Source/Common/OptionPreferences.cpp | Parses grid-hotkey enablement, layout, column count, and case-insensitive exclusion options with documented defaults. |
| Core/GameEngine/Source/GameClient/GUI/ControlBar/ControlBar.cpp | Maps command-bar slots to positional keys and applies string-key fallback and collision handling for excluded slots. |
| Core/GameEngine/Source/GameClient/MessageStream/HotKey.cpp | Allows Shift-only command hotkeys and exposes whether a visible command button currently claims a key. |
| Core/GameEngine/Source/GameClient/MessageStream/MetaEvent.cpp | Routes eligible claimed keys to command-bar handling while preserving Ctrl, Alt, and excluded-key meta bindings. |
| Generals/Code/GameEngine/Source/Common/GlobalData.cpp | Initializes and loads the new client options for Generals. |
| GeneralsMD/Code/GameEngine/Source/Common/GlobalData.cpp | Initializes and loads the same client options for Zero Hour. |
Sequence Diagram
sequenceDiagram
participant Input as Keyboard input
participant Meta as MetaEventTranslator
participant Hotkey as HotKeyTranslator
participant Bar as Command bar
Input->>Meta: Key press
Meta->>Meta: Check modifiers, exclusions, and claimed key
alt Eligible grid button claims key
Meta-->>Hotkey: Keep message for hotkey handling
Hotkey->>Bar: Send GBM_SELECTED
else Meta event retains key
Meta->>Meta: Execute mapped meta event
end
Reviews (2): Last reviewed commit: "fix(client): Support placeholder layout ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // only a button the player can actually press counts as claiming the key | ||
| if( BitIsSet( win->winGetStatus(), WIN_STATUS_HIDDEN ) ) | ||
| return FALSE; | ||
|
|
||
| return TRUE; |
There was a problem hiding this comment.
When an unmodified meta-event key overlaps a visible but disabled grid button, isHotKeyClaimed suppresses the meta event even though executeHotKey rejects the button, causing the key to perform no action beyond the disabled-click sound.
| // only a button the player can actually press counts as claiming the key | |
| if( BitIsSet( win->winGetStatus(), WIN_STATUS_HIDDEN ) ) | |
| return FALSE; | |
| return TRUE; | |
| // only a button the player can actually press counts as claiming the key | |
| if( BitIsSet( win->winGetStatus(), WIN_STATUS_HIDDEN ) || | |
| !BitIsSet( win->winGetStatus(), WIN_STATUS_ENABLED ) ) | |
| return FALSE; | |
| return TRUE; |
Knowledge Base Used: Game client runtime
Prompt To Fix With AI
This is a comment left during a code review.
Path: Core/GameEngine/Source/GameClient/MessageStream/HotKey.cpp
Line: 175-179
Comment:
**Disabled buttons claim keys**
When an unmodified meta-event key overlaps a visible but disabled grid button, `isHotKeyClaimed` suppresses the meta event even though `executeHotKey` rejects the button, causing the key to perform no action beyond the disabled-click sound.
```suggestion
// only a button the player can actually press counts as claiming the key
if( BitIsSet( win->winGetStatus(), WIN_STATUS_HIDDEN ) ||
!BitIsSet( win->winGetStatus(), WIN_STATUS_ENABLED ) )
return FALSE;
return TRUE;
```
**Knowledge Base Used:** [Game client runtime](https://app.greptile.com/thesuperhackers/-/custom-context/knowledge-base/thesuperhackers/generalsgamecode/-/docs/game-client-runtime.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Deliberate, and now documented in code (afd0926): a visible but disabled button keeps claiming its key on purpose. executeHotKey gives the disabled-click feedback for it, which is the response the player expects when pressing the key of a greyed-out cameo they can see - having the press fall through to an unrelated meta event bound to the same letter would turn every "can't afford it yet" press into a misinput. The suggested change is intentionally not taken.
HotKeyTranslator bailed on any modifier, so holding Shift made command hotkeys do nothing at all. Shift+clicking a cameo queued five units, but shift plus the same button's hotkey queued none -- the keypress never reached the hotkey map. Shift now passes through. Ctrl and Alt still bail, since those carry their own bindings such as control groups, which command hotkeys must not shadow. The key lookup already asked for the unshifted character (getPrintableKey with state 0), so Shift+B still resolves to "b" and matches the lowercased hotkey map without further work.
Addresses the review findings: - A non alphanumeric character in GridHotkeyLayout is now a placeholder: the slot is covered by the grid but gets no key. A layout shorter than a full bar keeps its row shape by padding with dots - the row count is derived from the layout's length, so a partial row would otherwise shift every key after it. - isHotKeyClaimed deliberately treats a visible but disabled button as claiming its key, now stated in code: the player pressing a greyed out cameo's key gets the disabled click feedback, rather than the press falling through to an unrelated meta event bound to the same letter.
1 participant
Adds two opt-in client options giving the command bar fixed positional hotkeys. Both default off; behavior is retail unless set in Options.ini.
GridHotkeys (
= Yes) — keys each command bar button by its slot position rather than the letter the string file marked with an ampersand, so keys stay in the same place whatever is being built.GridHotkeyLayout(defaultQWERTYUIOASDFGHJKL) gives the key per slot in reading order;GridHotkeyColumns(default 9) maps the layout through the bar's column-major slot numbering. A visible button that claims a key wins over a meta event on the same letter — for unmodified (or Shift-batched) presses only, so Ctrl/Alt combos keep their meta events.NonGridHotkeys (e.g.
= SG) — keys listed here opt out of the grid; those slots fall back to their string-file letter (unless it collides with a letter the grid still uses, in which case the slot goes keyless rather than stealing at random), and the listed keys keep their normal meta events.Works in both Zero Hour and Generals — the consuming code lives in the shared ControlBar/HotKey/MetaEvent translators, so both titles get it with their own GlobalData plumbing.
These options ship in the Contra mod's engine fork and have been played there; this PR is the port onto current main, adapted to the MetaEventTranslator refactor from #2758. Code was written with LLM assistance and human-reviewed, adapted and playtested by the author.
Prepared for Squash and Merge.