fix(input): #358 — RetailDefaults() never carried the Ctrl+M mute binding over from AcdreamCurrentDefaults()

Root cause, confirmed by a full-production-wiring repro test rather than
guessed: InputAction.AcdreamToggleAudioMute was bound to Ctrl+M only in
KeyBindings.AcdreamCurrentDefaults() -- the pre-K.1c WASD-only preset,
whose own doc comment says it is preserved solely as a regression anchor
and is explicitly NOT the GameWindow startup source after K.1c.
KeyBindings.RetailDefaults() -- what KeyBindings.LoadOrDefault actually
falls back to when no keybinds.json exists on disk (the verified state
on the affected machine) -- has its own "Acdream debug actions" block
(Ctrl+F1/F2/F3/F7/F8/F9/F10, Ctrl+Shift+F) but never carried the Ctrl+M
mute binding over into it. The live dispatcher therefore had no Ctrl+M
entry in its binding table at all -- not a modifier-matching bug, not a
scope bug, not a retained-UI-capture bug. Same class as the a5a7eb4f
jump fix (two construction paths, one wired to production), except here
it's two default-binding-set methods rather than two controller
instances, and the binding was simply added to the wrong one. This also
explains the prior "loaded 152 bindings both before and after" mystery:
the count correctly didn't change, because the earlier addition went
into a method nothing in production loads or counts.

MuteChordDispatchTests.CtrlM_WithNoWidgetFocused_FiresAcdreamToggleAudioMute
reproduces the full production shape (real RetailDefaults(), the
dispatcher's actual default [Always, Game] scope stack -- production
never calls PushScope/PopScope anywhere, grepped clean across
src/AcDream.App -- and a synthetic Ctrl+M keydown) and failed with an
EMPTY fired collection before this fix, which is what pinpointed
"missing table entry" over the other ranked hypotheses. A second test,
CtrlM_WhileAnyWidgetHoldsKeyboardFocus_IsSuppressed, pins a separate but
real mechanism found along the way (InputDispatcher.OnKeyDown returns
before FindActive when WantCaptureKeyboard is true, and production wires
that to "any focused widget", not just chat text entry) that was ruled
out as #358's cause since the baseline repro failed with nothing
focused.

Fix: KeyBindings.RetailDefaults() now also binds Ctrl+M to
AcdreamToggleAudioMute. Retail's own keymap has no Ctrl+M binding, so
this doesn't collide with anything retail-faithful. #358 closed in
ISSUES.md with the confirmed mechanism; connected verification (does
Ctrl+M actually mute in a live client) is still owed -- this session's
hard constraints excluded client launches.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-10 14:45:51 +02:00
parent cc58289967
commit dda76d9faf
3 changed files with 162 additions and 28 deletions

View file

@ -443,41 +443,66 @@ published-controller behaviour is unchanged.
## #358 — Ctrl+M mute keybind never fires from the dispatcher
**Status:** OPEN — filed 2026-08-08, deferred by user direction ("we can carry
that on for later"). The mute mechanism itself is implemented and untested
only for want of a working trigger.
**Status:** DONE — root cause found and fixed. Filed 2026-08-08; closed via
the CH6c-fix-adjacent session that wrote the full-production-wiring repro
test the deferred investigation asked for.
**Symptom:** pressing Ctrl+M in a live session does nothing. The downstream
chain is NOT the problem — evidence from the session log:
**Mechanism (confirmed, not inferred):** `InputAction.AcdreamToggleAudioMute`
was bound to Ctrl+M only in `KeyBindings.AcdreamCurrentDefaults()`
(`src/AcDream.UI.Abstractions/Input/KeyBindings.cs:123`) — the pre-K.1c
WASD-only preset, whose own doc comment says it is preserved solely "as a
regression anchor for tests that pin the older modifier-blind WASD layout"
and is explicitly "NOT the GameWindow startup source after K.1c."
`KeyBindings.RetailDefaults()` (the method `KeyBindings.LoadOrDefault`
actually falls back to when no `keybinds.json` exists on disk — the verified
state on the affected machine) never carried the Ctrl+M binding over: it has
its own "Acdream debug actions: relocated to Ctrl+F* to avoid retail
conflicts" block (Ctrl+F1/F2/F3/F7/F8/F9/F10, Ctrl+Shift+F) but
`AcdreamToggleAudioMute` was simply never added to it. The live dispatcher
therefore had no Ctrl+M entry in its binding table at all — not a modifier-
matching bug, not a scope bug, not a retained-UI-capture bug. Same class as
the `a5a7eb4f` jump fix (two construction paths, one wired to production),
except here it's two DEFAULT-BINDING-SET methods rather than two controller
instances, and the binding was added to the wrong one.
- `[input]` action logging was live (101 fired actions: movement, selection,
UseSelected) and recorded **zero** events for `AcdreamToggleAudioMute`
the chord never fired from `InputDispatcher`, so the
`GameplayInputCommandController` switch case and the engine's `Muted`
property (verified paths, commit `2cf94dbc`) were never reached.
- `KeyBindings.LoadOrDefault`'s merge-over-defaults was read and looks
correct (an action with defaults but absent from the saved file picks up
the default binding), so a stale `keybinds.json` SHOULD have gained the
new binding. Unverified whether it actually did: the client logged
"loaded 152 bindings" both before AND after the binding was added, which
is suspicious — either the pre-change default count was 151 (then 152 is
right) or the merge did not add it (then the count should have grown).
Nobody has printed the default count or dumped the merged set yet.
**How it was found:** `tests/AcDream.UI.Abstractions.Tests/Input/MuteChordDispatchTests.cs`
(`CtrlM_WithNoWidgetFocused_FiresAcdreamToggleAudioMute`) reproduces the full
production wiring shape — real `KeyBindings.RetailDefaults()` (not an ad-hoc
test binding), the dispatcher's actual default `[Always, Game]` scope stack
(production never calls `PushScope`/`PopScope` anywhere — grepped clean
across `src/AcDream.App`, so Chat/EditField/Dialog/etc. scopes are dead code
today), and a synthetic Ctrl+M keydown with nothing capturing the keyboard.
It failed with an EMPTY fired collection before the fix (not a wrong-action
mismatch), which is what pinpointed "missing table entry" over the other
hypotheses. This also resolves the prior "loaded 152 bindings both before and
after" mystery: the count correctly did NOT change across that prior session
because the earlier binding addition went into `AcdreamCurrentDefaults()`,
which nothing in production ever loads or counts.
**Hypotheses, in order:** (1) the merged set genuinely lacks the binding
(count question above — cheapest to check first: log `RetailDefaults().Count`
or grep the saved json after a save); (2) the dispatcher's modifier matching
does not fire a Ctrl+M chord — e.g. exact-vs-subset modifier semantics, or
the bare `M` binding (`SelectionNextFellow`, no modifiers) shadowing it; note
the log shows no `SelectionNextFellow` firing on the press either, which
leans against simple shadowing and toward the chord matching nothing at all;
(3) the retained UI consuming Ctrl-chords before the dispatcher.
**Ruled out, but real and now pinned as a separate regression test**
(`CtrlM_WhileAnyWidgetHoldsKeyboardFocus_IsSuppressed`):
`InputDispatcher.OnKeyDown` returns before calling `FindActive` at all when
`_mouse.WantCaptureKeyboard` is true, and production wires that to
`UiRoot.WantsKeyboard` = "`KeyboardFocus is not null`" — ANY focused widget
(in practice only `UiField` instances ever set `AcceptsFocus = true` in the
retained UI, so this means any focused text-entry field), not scoped to a
specific text box. This gate is total and pre-empts every chord, not just
Ctrl+M, but the baseline repro test proved the bug reproduced with nothing
focused at all, so this was not #358's cause.
**Fix:** `src/AcDream.UI.Abstractions/Input/KeyBindings.cs`
`RetailDefaults()` now also binds Ctrl+M → `AcdreamToggleAudioMute` in the
Acdream-debug-actions block. Retail's own keymap
(`docs/research/named-retail/retail-default.keymap.txt`) has no Ctrl+M
binding, so this doesn't collide with anything retail-faithful.
**The mechanism behind the key** (already landed, `2cf94dbc`): engine
`Muted` sets the AL listener gain 0/1 — unused since A2 moved mixing to the
CPU, so it silences already-playing voices instantly without touching the
retail mixing math or persisted volumes. Once the trigger works, no further
audio-side work is needed.
retail mixing math or persisted volumes. The trigger now works; no further
audio-side work is needed. Connected verification (does Ctrl+M actually mute
in a live client) is still owed the next time a client launch is available —
this session's hard constraints excluded client launches.
## #359 — 0x019E PlayerKilled line prints to participants — retail suppresses it