diff --git a/docs/ISSUES.md b/docs/ISSUES.md index d1e4f885..59f826eb 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -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 diff --git a/src/AcDream.UI.Abstractions/Input/KeyBindings.cs b/src/AcDream.UI.Abstractions/Input/KeyBindings.cs index e75d4e68..0102dc29 100644 --- a/src/AcDream.UI.Abstractions/Input/KeyBindings.cs +++ b/src/AcDream.UI.Abstractions/Input/KeyBindings.cs @@ -358,6 +358,15 @@ public sealed class KeyBindings b.Add(new(new KeyChord(Key.F8, ModifierMask.Ctrl), InputAction.AcdreamSensitivityDown)); b.Add(new(new KeyChord(Key.F9, ModifierMask.Ctrl), InputAction.AcdreamSensitivityUp)); b.Add(new(new KeyChord(Key.F10, ModifierMask.Ctrl), InputAction.AcdreamCycleWeather)); + // #358: AcdreamToggleAudioMute was bound in AcdreamCurrentDefaults() + // (line ~123 above) but never carried over here when K.1c made + // RetailDefaults the GameWindow startup source — AcdreamCurrentDefaults + // is preserved only as a WASD-layout regression anchor and is not + // loaded in production, so the chord was registered in a table the + // live dispatcher never consults. Retail's own keymap has no Ctrl+M + // binding (grepped retail-default.keymap.txt clean), so this doesn't + // collide with anything retail-faithful. + b.Add(new(new KeyChord(Key.M, ModifierMask.Ctrl), InputAction.AcdreamToggleAudioMute)); // K-fix2 (2026-04-26): free-fly toggle keyboard shortcut. // Retail leaves Ctrl+Shift+F unbound (retail F = SelectionPickUp, diff --git a/tests/AcDream.UI.Abstractions.Tests/Input/MuteChordDispatchTests.cs b/tests/AcDream.UI.Abstractions.Tests/Input/MuteChordDispatchTests.cs new file mode 100644 index 00000000..a3c9b1d1 --- /dev/null +++ b/tests/AcDream.UI.Abstractions.Tests/Input/MuteChordDispatchTests.cs @@ -0,0 +1,100 @@ +using System.Collections.Generic; +using AcDream.UI.Abstractions.Input; +using Silk.NET.Input; + +namespace AcDream.UI.Abstractions.Tests.Input; + +/// +/// Issue #358 — Ctrl+M () +/// never fired from a live session's , despite +/// the downstream GameplayInputCommandController callback being +/// verified correct and the chord's modifier matching being verified +/// correct in isolation. +/// +/// +/// Root cause, found by the first test below failing against the FULL +/// production wiring shape (real , +/// not an ad-hoc test binding; the dispatcher's actual default scope stack — +/// production never calls PushScope/PopScope anywhere, grepped +/// clean across src/AcDream.App, so the stack is permanently +/// + , exactly +/// what the constructor pushes; and a synthetic Ctrl+M keydown): the Ctrl+M +/// binding existed ONLY in +/// (the pre-K.1c WASD-only preset, preserved solely as a regression anchor +/// for tests pinning that older layout — its own doc comment says outright +/// it is "NOT the GameWindow startup source after K.1c"). Production loads +/// bindings through , which falls back +/// to when no keybinds.json +/// exists on disk (the verified state on this machine) — and +/// never carried the Ctrl+M binding +/// over. This is the same class of defect as the a5a7eb4f jump fix — +/// two construction paths for the same table shape, only one of them wired +/// to production — except here it is two DEFAULT-BINDING-SET methods rather +/// than two controller instances, and the binding was simply added to the +/// wrong one. Fixed by adding the chord to +/// alongside the other +/// Acdream* debug/utility actions relocated there to avoid retail +/// conflicts (KeyBindings.cs, the "Acdream debug actions" block) — retail's +/// own keymap (docs/research/named-retail/retail-default.keymap.txt) +/// has no Ctrl+M binding at all, so this doesn't collide with anything +/// retail-faithful. +/// +/// +public sealed class MuteChordDispatchTests +{ + private static (InputDispatcher dispatcher, FakeKeyboardSource kb, FakeMouseSource mouse, List<(InputAction Action, ActivationType Activation)> fired) + BuildWithRetailDefaults() + { + var kb = new FakeKeyboardSource(); + var mouse = new FakeMouseSource(); // WantCaptureKeyboard defaults to false — no widget focused. + KeyBindings bindings = KeyBindings.RetailDefaults(); + InputDispatcher dispatcher = InputDispatcher.CreateDetached(kb, mouse, bindings); + dispatcher.Attach(); // dispatcher constructor already pushes [Always, Game] — the + // production default; nothing in src/AcDream.App ever pushes + // another scope on top of it. + var fired = new List<(InputAction, ActivationType)>(); + dispatcher.Fired += (a, t) => fired.Add((a, t)); + return (dispatcher, kb, mouse, fired); + } + + [Fact] + public void CtrlM_WithNoWidgetFocused_FiresAcdreamToggleAudioMute() + { + // The baseline production shape: KeyBindings.LoadOrDefault's fallback + // (RetailDefaults, matching the verified "no keybinds.json on disk" + // state), the dispatcher's real default [Always, Game] scope stack, + // nothing capturing the keyboard. This is the regression pin for + // #358's fix — before the fix, RetailDefaults carried no binding for + // AcdreamToggleAudioMute at all and this assertion failed with an + // EMPTY fired collection (not a wrong-action mismatch), proving the + // gap was a missing table entry, not a matching/scope/capture bug. + (_, FakeKeyboardSource kb, _, var fired) = BuildWithRetailDefaults(); + + kb.EmitKeyDown(Key.M, ModifierMask.Ctrl); + + Assert.Contains((InputAction.AcdreamToggleAudioMute, ActivationType.Press), fired); + } + + [Fact] + public void CtrlM_WhileAnyWidgetHoldsKeyboardFocus_IsSuppressed() + { + // Supporting evidence gathered during the #358 investigation, kept as + // a regression pin in its own right: InputDispatcher.OnKeyDown + // (src/AcDream.UI.Abstractions/Input/InputDispatcher.cs) returns + // BEFORE calling FindActive at all when _mouse.WantCaptureKeyboard is + // true. Production wires WantCaptureKeyboard to UiRoot.WantsKeyboard, + // which is "KeyboardFocus is not null" — ANY focused widget, not + // just a text-entry field specifically (the only AcceptsFocus=true + // widget type in the retained UI is UiField, so in practice this + // means any focused text field, e.g. chat entry or a Settings + // numeric box). This gate is real and total, but it was RULED OUT as + // #358's root cause: the baseline test above reproduced the bug with + // nothing focused at all. + (_, FakeKeyboardSource kb, FakeMouseSource mouse, var fired) = BuildWithRetailDefaults(); + mouse.WantCaptureKeyboard = true; + + kb.EmitKeyDown(Key.M, ModifierMask.Ctrl); + + Assert.DoesNotContain((InputAction.AcdreamToggleAudioMute, ActivationType.Press), fired); + } +}