Move all key input handlers into Input Actions rather than raw key presses #50

Closed
opened 2026-08-30 10:47:13 +01:00 by liamjd · 2 comments
Owner

Every keyboard control except building rotation and ui_cancel is currently read as a
raw physical keycode. Move them all onto named InputMap actions, so bindings live in one
place, can be rebound later, and can be reasoned about when two systems want the same key.

Why now

#44 wanted E for counter-clockwise blueprint rotation. E is already taken by
camera_rig.gd:175 (rotate view counter-clockwise), and the conflict is not resolvable
while the camera reads keys the way it does — see "Polling bypasses event handling" below.
#44 shipped with R/T instead as a stopgap. E is reserved for counter-clockwise blueprint
rotation once this issue lands
, at which point R/T should be revisited.

What is currently raw

Where Keys Style
entities/camera/camera_rig.gd WASD, arrows, Shift (sprint), Q, E polled, Input.is_physical_key_pressed()
common/game.gd Escape (menu), backtick (debug mode) event, _unhandled_key_input
ui/debug_layer.gd _debug_commands dictionary keyed by keycode event, _unhandled_input
entities/buildings/placement_controller.gd R, T already actions
ui/menus/options_panel.gd — already ui_cancel

Guidance

Polling vs. events is a separate axis from raw-key vs. action

These are two independent decisions and it is worth not conflating them:

  • Continuous, held-key input — camera pan and turn — belongs in _process and stays
    polled. It becomes Input.is_action_pressed("camera_pan_left"), not an event handler.
  • Discrete, one-shot input — toggling the menu, debug commands, rotating a blueprint —
    belongs in _unhandled_input / _unhandled_key_input as
    event.is_action_pressed("..."), so it can be consumed with
    get_viewport().set_input_as_handled().

Migrating to actions does not mean migrating everything to events.

Polling bypasses event handling entirely

Input.is_physical_key_pressed() (and Input.is_action_pressed()) query OS-level key
state directly. They do not travel through the viewport's event pipeline, so
set_input_as_handled() cannot suppress them, and neither can node order or process
priority.

This is precisely why E cannot be shared today: bind it to blueprint rotation and the
press both rotates the blueprint and spins the camera, with no ordering trick available.

Once both sides read actions, the fix is a modal guard — the camera skips its rotate
actions while a blueprint is selected — rather than an attempt to consume the input.
Decide where that mode flag lives (GameState, or the camera asking the placement
controller) as part of this work.

Physical keycodes survive the migration

camera_rig.gd:18-20 says keys are read as physical keycodes "so the project needs no
[input] config and non-QWERTY layouts still work", implying actions would cost the layout
independence. They would not. InputMap events carry physical_keycode just as raw
events do — the actions added in #44 already use it. Migrating keeps the layout behaviour
and gains rebindability. That comment should be corrected as part of this issue, not
treated as a reason to hesitate.

Modifier matching is subset-based — the Shift trap

An action matches when the modifiers it declares are a subset of those actually held. An
action bound to plain R therefore also fires on Shift+R, Ctrl+R and Alt+R.

#44 originally bound CW to R and CCW to Shift+R. Verified in 4.7.2:

R with shift=false  ->  CW=true  CCW=false
R with shift=true   ->  CW=true  CCW=true     <-- both fire

Both handlers ran, +1 then -1, and CCW appeared to do nothing. Two ways out, if any
binding here ever wants a modifier:

  • test the more specific (modifier-bearing) action first and elif the plain one — it
    works, but it looks arbitrary and needs a comment saying why the order matters; or
  • avoid modifier variants of an existing bare binding altogether.

Prefer the second unless there is a reason not to. Shift is already the camera sprint
modifier, which is polled, so it is not a free modifier in any case.

Debug commands are keyed by keycode and display the key

ui/debug_layer.gd keys _debug_commands by physical_keycode and renders the binding
with OS.get_keycode_string(code). Migrating means the dictionary is keyed by action name
and the label comes from InputMap.action_get_events(name) instead, taking the first
InputEventKey and reading physical_keycode off it. This is the fiddliest part of the
change — it is the one place a binding is shown to the player, and it is what a rebinding
UI would later reuse.

Naming convention

Settle one and apply it throughout. The two actions added in #44 are
Rotate_building_CW / Rotate_building_CCW, which match neither Godot's built-in
ui_cancel style nor the project's snake_case GDScript. Suggest lowercase snake_case with
a system prefix — camera_pan_left, camera_rotate_cw, building_rotate_cw,
debug_toggle — and rename the #44 actions to match rather than leaving two conventions.

Scope

In: the three files in the table above, plus renaming the #44 actions.

Out: a rebinding UI, gamepad or mouse-button bindings, and input contexts/action sets.
Those become possible after this, but nothing here should be built speculatively for them.

Acceptance criteria

  • No Input.is_physical_key_pressed(), Input.is_key_pressed() or event.physical_keycode
    comparison remains outside ui/debug_layer.gd's label rendering.
  • Every binding is declared in project.godot's [input] section using
    physical_keycode, under one naming convention.
  • Camera pan, sprint and Q/E turn behave exactly as before — still polled, still
    screen-relative, still snapping to 90 degrees.
  • Escape still toggles the menu; backtick still toggles debug mode and still does nothing
    in a release build.
  • The debug panel still lists each command with its key.
  • E rotates the blueprint counter-clockwise while placing, and does not turn the camera at
    the same time; it still turns the camera when nothing is selected.
  • The stale note at camera_rig.gd:18-20 is corrected.
  • A UI panel from the Options menu listing all the key bindings
Every keyboard control except building rotation and `ui_cancel` is currently read as a raw physical keycode. Move them all onto named InputMap actions, so bindings live in one place, can be rebound later, and can be reasoned about when two systems want the same key. ## Why now #44 wanted **E** for counter-clockwise blueprint rotation. E is already taken by `camera_rig.gd:175` (rotate view counter-clockwise), and the conflict is *not* resolvable while the camera reads keys the way it does — see "Polling bypasses event handling" below. #44 shipped with R/T instead as a stopgap. **E is reserved for counter-clockwise blueprint rotation once this issue lands**, at which point R/T should be revisited. ## What is currently raw | Where | Keys | Style | |---|---|---| | `entities/camera/camera_rig.gd` | WASD, arrows, Shift (sprint), Q, E | polled, `Input.is_physical_key_pressed()` | | `common/game.gd` | Escape (menu), backtick (debug mode) | event, `_unhandled_key_input` | | `ui/debug_layer.gd` | `_debug_commands` dictionary keyed by keycode | event, `_unhandled_input` | | `entities/buildings/placement_controller.gd` | R, T | **already actions** | | `ui/menus/options_panel.gd` | — | **already `ui_cancel`** | ## Guidance ### Polling vs. events is a separate axis from raw-key vs. action These are two independent decisions and it is worth not conflating them: - **Continuous, held-key input** — camera pan and turn — belongs in `_process` and stays polled. It becomes `Input.is_action_pressed("camera_pan_left")`, *not* an event handler. - **Discrete, one-shot input** — toggling the menu, debug commands, rotating a blueprint — belongs in `_unhandled_input` / `_unhandled_key_input` as `event.is_action_pressed("...")`, so it can be consumed with `get_viewport().set_input_as_handled()`. Migrating to actions does not mean migrating everything to events. ### Polling bypasses event handling entirely `Input.is_physical_key_pressed()` (and `Input.is_action_pressed()`) query OS-level key state directly. They do **not** travel through the viewport's event pipeline, so `set_input_as_handled()` cannot suppress them, and neither can node order or process priority. This is precisely why E cannot be shared today: bind it to blueprint rotation and the press both rotates the blueprint *and* spins the camera, with no ordering trick available. Once both sides read actions, the fix is a **modal guard** — the camera skips its rotate actions while a blueprint is selected — rather than an attempt to consume the input. Decide where that mode flag lives (`GameState`, or the camera asking the placement controller) as part of this work. ### Physical keycodes survive the migration `camera_rig.gd:18-20` says keys are read as physical keycodes "so the project needs no `[input]` config and non-QWERTY layouts still work", implying actions would cost the layout independence. **They would not.** InputMap events carry `physical_keycode` just as raw events do — the actions added in #44 already use it. Migrating keeps the layout behaviour and gains rebindability. That comment should be corrected as part of this issue, not treated as a reason to hesitate. ### Modifier matching is subset-based — the Shift trap An action matches when the modifiers it *declares* are a subset of those actually held. An action bound to plain R therefore also fires on Shift+R, Ctrl+R and Alt+R. #44 originally bound CW to R and CCW to Shift+R. Verified in 4.7.2: ``` R with shift=false -> CW=true CCW=false R with shift=true -> CW=true CCW=true <-- both fire ``` Both handlers ran, `+1` then `-1`, and CCW appeared to do nothing. Two ways out, if any binding here ever wants a modifier: - test the **more specific** (modifier-bearing) action first and `elif` the plain one — it works, but it looks arbitrary and needs a comment saying why the order matters; or - avoid modifier variants of an existing bare binding altogether. Prefer the second unless there is a reason not to. Shift is already the camera sprint modifier, which is polled, so it is not a free modifier in any case. ### Debug commands are keyed by keycode and *display* the key `ui/debug_layer.gd` keys `_debug_commands` by `physical_keycode` and renders the binding with `OS.get_keycode_string(code)`. Migrating means the dictionary is keyed by action name and the label comes from `InputMap.action_get_events(name)` instead, taking the first `InputEventKey` and reading `physical_keycode` off it. This is the fiddliest part of the change — it is the one place a binding is shown to the player, and it is what a rebinding UI would later reuse. ### Naming convention Settle one and apply it throughout. The two actions added in #44 are `Rotate_building_CW` / `Rotate_building_CCW`, which match neither Godot's built-in `ui_cancel` style nor the project's snake_case GDScript. Suggest lowercase snake_case with a system prefix — `camera_pan_left`, `camera_rotate_cw`, `building_rotate_cw`, `debug_toggle` — and rename the #44 actions to match rather than leaving two conventions. ## Scope In: the three files in the table above, plus renaming the #44 actions. Out: a rebinding UI, gamepad or mouse-button bindings, and input contexts/action sets. Those become possible after this, but nothing here should be built speculatively for them. ## Acceptance criteria - No `Input.is_physical_key_pressed()`, `Input.is_key_pressed()` or `event.physical_keycode` comparison remains outside `ui/debug_layer.gd`'s label rendering. - Every binding is declared in `project.godot`'s `[input]` section using `physical_keycode`, under one naming convention. - Camera pan, sprint and Q/E turn behave exactly as before — still polled, still screen-relative, still snapping to 90 degrees. - Escape still toggles the menu; backtick still toggles debug mode and still does nothing in a release build. - The debug panel still lists each command with its key. - E rotates the blueprint counter-clockwise while placing, and does not turn the camera at the same time; it still turns the camera when nothing is selected. - The stale note at `camera_rig.gd:18-20` is corrected. - A UI panel from the Options menu listing all the key bindings
Author
Owner

The debug layer keybindings will be exempt from this change. They are direct key bindings inside debug_layer and will not be configurable by the player.

The debug layer keybindings will be exempt from this change. They are direct key bindings inside debug_layer and will not be configurable by the player.
Author
Owner

Although the debug R key clashes with the building placement R key, I don't really care at this stage. Changing the map seed while attempting to place a building is a crazy thing to do.

Although the debug R key clashes with the building placement R key, I don't really care at this stage. Changing the map seed while attempting to place a building is a crazy thing to do.
Sign in to join this conversation.
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
liamjd/UntitledColonyBuilder#50
No description provided.