Skip to content

fix: make press-only widget actions reachable from mouse-button binds#8168

Open
Arcadiyyyyyyyy wants to merge 1 commit into
beyond-all-reason:masterfrom
Arcadiyyyyyyyy:action-mouse-bind-audit
Open

fix: make press-only widget actions reachable from mouse-button binds#8168
Arcadiyyyyyyyy wants to merge 1 commit into
beyond-all-reason:masterfrom
Arcadiyyyyyyyy:action-mouse-bind-audit

Conversation

@Arcadiyyyyyyyy

Copy link
Copy Markdown
Contributor

Work done

Follow-up to #7952, which fixed the chain action and flipped its
registration from "p" to "pt" so it could be triggered from mouse-button
binds. Reviewer feedback on that PR asked for a separate PR to audit the
other cases where the same "p""pt" change is appropriate. This is that
audit.

Background. Widget actions are registered with actionHandler:AddAction(self, name, fn, data, types),
where types selects the dispatch sets the action lives in:

  • "p" (key-press) → keyPressActions, reached by keyboard binds via actionHandler:KeyAction.
  • "t" (text) → textActions, reached by mouse-button binds and the console via actionHandler:TextAction.

An action registered "p"-only is invisible to the text path, so binding it to
a mouse button silently does nothing. On the text path the handler is invoked
exactly like a press (isRepeat = false, release = nil), so making a one-shot
press action "pt" is safe and simply extends it to mouse binds and the console.

Change. Converted 30 one-shot press actions across 14 widgets from "p"
to "pt":

Widget Actions
camera_flip cameraflip
cmd_blueprint blueprint_create, blueprint_next, blueprint_prev, blueprint_delete, buildfacing, buildspacing
cmd_commandq_manager command_skip_current, command_cancel_last
cmd_comselect selectcomm
cmd_onoff onoff
cmd_factoryqmanager factory_preset
gui_attackrange_gl4 cursor_range_toggle, attack_range_inc, attack_range_dec
gui_teamstats teamstatus_close
minimap_rotation_manager minimap_rotate
gui_gridmenu gridmenu_category, gridmenu_next_page, gridmenu_cycle_builder
gui_pregame_build stop, buildfacing, buildspacing, buildmenu_pregame_deselect
unit_auto_group add_to_autogroup, remove_from_autogroup, remove_one_unit_from_group, load_autogroup_preset
unit_key_select select
unit_stateprefs stateprefs_clearunit

Deliberately left as "p" (hold-to-activate). Every action that registers a
"p" press paired with an "r" release — the modifier is set true on press and
false on release — is not converted. A mouse/text bind cannot fire the
release, so adding "t" would leave the modifier stuck on. These are:
buildsplit, commandinsert, factory_preset_show, reclaim_highlight,
unit_stats, selectloop/selectloop_deselect/selectloop_add,
selectbox/selectbox_<modifier>, stateprefs_record, stateprefs_clear.
(Note stateprefs_clearunit is converted — it is one-shot and has no paired
release, unlike its siblings.)

No handler logic changed; only the types string on the AddAction calls.

Addresses Issue(s)

Test steps

1. Widget load / no errors (all conversions)

  • Start a skirmish and let all default widgets load.
  • Confirm no Lua errors in infolog.txt. All converted widgets that are
    enabled by default (CameraFlip, TeamStats, Attack Range GL4, Commander
    Selector, Onoff, Minimap Rotation Manager, Grid menu, Auto Group, Blueprint,
    Command Queue Manager, Pregame Queue, State Prefs V2) loaded cleanly with no
    error following their load line. A bad types string would throw during
    widget:Initialize() — none did. (cmd_factoryqmanager and unit_key_select
    are disabled by default and were not loaded; their change is inert until
    enabled.)

2. Mouse-button bind now works (the fix)

  • /bind mouse5 cameraflip in the console, then press mouse5 → camera
    flips 180°. On master the identical bind does nothing. This before/after
    difference confirms the fix.
  • Spot-checked additional converted actions from separate widgets via mouse
    bind (e.g. /bind mouse4 attack_range_inc cycles the range display;
    /bind mouse5 minimap_rotate steps minimap rotation) — all now respond to the
    mouse button.

3. Keyboard binds still work (regression)

  • The same actions bound to a keyboard key behave exactly as before the
    change — no double-dispatch, no change in keyboard behaviour.

4. Hold-to-activate actions untouched (regression)

  • Verified an excluded action (buildsplit, kept "p") still activates on
    keyboard hold and releases cleanly — no stuck modifier. Confirmed none of the
    paired press/release actions were converted.

AI / LLM usage statement:

The audit and code changes were produced with Claude (Opus 4.8); the human
contribution was identifying the follow-up scope, reviewing each candidate, and
validating the fixes in-game.

Audit follow-up to beyond-all-reason#7952. Mouse-button binds dispatch through the text
path (actionHandler:TextAction -> textActions), so actions registered
press-only ("p") are silently dropped when bound to a mouse button.

Convert 30 one-shot press actions across 14 widgets from "p" to "pt" so
they are reachable from both keyboard and mouse-button binds. Hold-to-
activate actions that pair a "p" press with an "r" release (e.g.
buildsplit, selectbox, selectloop, stateprefs_record/clear) are left as
"p" on purpose: a text/mouse bind cannot fire the release, which would
leave their modifier stuck.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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