diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 88bb6e61..805f4f6f 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -185,7 +185,7 @@ AP-94..AP-112 for the confirmed retail-UI completion gaps. |---|---|---|---|---|---| | AP-201 | **Filed 2026-08-11 at the OP5 review-fix round (S2), predates OP5 but was made user-visible by it.** `UiTemplateListBox`'s internal row viewport (`UiScrollablePanel.LayoutScrollableChildren`) culls a child WHOLE — `child.Visible = top >= -0.5f && top + child.Height <= Height + 0.5f` — rather than clipping the visible portion of a row that straddles the viewport edge, because the UI renderer has no scissor stack. Retail's own `UIElement_ListBox`/scroll-region rendering clips partially-visible rows at the pixel boundary, same as any native scroll view. Every row in this viewport was 8-36px until Campaign OP slice OP5 added five self-sized filter blocks (12x20=240px / 13x20=260px, AP-195) to the Chat tab's ~560px viewport; a 240-260px block straddling the viewport edge at a given scroll offset now disappears ENTIRELY (a visible "pop") instead of clipping, where the pre-OP5 8-36px rows made the same all-or-nothing cull read as ordinary row-granular scrolling. | `src/AcDream.App/UI/UiScrollablePanel.cs:69` (the cull predicate); consumed by `src/AcDream.App/UI/UiTemplateListBox.cs` (`Viewport`) — the Character/Chat/Config Options-panel tabs and any other controller-built row list sharing this viewport | A scissor stack does not exist anywhere in the retained-UI renderer yet (class's own doc comment, `UiScrollablePanel.cs:8-12`, predates this row); whole-row culling is a correct, cheap stand-in for every list whose rows are small relative to the viewport, which was true for every consumer before OP5. | A tall block (any future row taller than roughly the viewport's own height, not just OP5's filter blocks) can vanish completely for a range of scroll offsets instead of showing a partial view — the OP5 gate script's own step 2 documents the exact symptom so it is not mistaken for a self-sizing regression (`docs/research/2026-08-11-campaign-op-test-script.md`). Scrolling further always restores the block whole; no data or state is lost, only the presentation pops. | No scissor-stack retail oracle needed — this is a stand-in for ordinary native clip-rect rendering every GUI toolkit (including retail's own) provides; issue #371 tracks adding a real per-row clip rect to `UiScrollablePanel` | | AP-202 | **Filed 2026-08-11 at Campaign OP slice OP8 (D4).** Configure Keyboard persists every rebind to `keybinds.json` only. Retail's own storage is a `\Asheron's Call\.keymap` text file (`CInputManager_WIN32::SaveKeyMap @0x00686C20`, `PFileParser`), with Load-File/Save-As buttons for NAMED keymap profiles and a `keymap` key in `UserPreferences.ini` selecting which one loads at startup (research doc §5.7). D4 chose the existing, tested `keybinds.json` schema over building a second `PFileParser`-compatible text codec + named-profile management; this row's the Load File/Save As buttons on the Configure Keyboard screen (`0x10000027`/`0x10000029`) are wired but INERT. | `src/AcDream.App/UI/Layout/KeyboardConfigController.cs` (`WireScreenButtons`'s Load/Save-As no-op); `src/AcDream.UI.Abstractions/Input/KeyBindings.cs` (`SaveToFile`/`LoadOrDefault`) | `keybinds.json` already round-trips every retail action this screen can bind (identity table + the DAT-defaults conformance test), so the ONLY capability lost is exchanging `.keymap` files with a real retail client or another acdream install by named profile — a real feature gap, not a correctness gap. | A user who expects to export/import a named `.keymap` profile (e.g. to share a control layout with a retail-client friend) cannot; every rebind still works and persists locally. | `docs/research/2026-08-10-keyboard-config-and-gameplay-tab.md` §5.7-§5.8; `CInputManager_WIN32::SaveKeyMap @0x00686C20`; `gmKeyboardUI::SaveKeymap @0x004DCF90` | -| AP-203 | **Filed 2026-08-11 at Campaign OP slice OP8.** Of the DAT ActionMap's 306 user-bindable rows, `RetailActionIdentityTable` (`src/AcDream.UI.Abstractions/Input/RetailActionIdentityTable.cs`) resolves roughly half to a live acdream `InputAction`; the rest render, bind, conflict-check, and persist (via `RetailUnmappedKeyBindings`, a sibling `*-unmapped.json` file) exactly like any other row, but have no live gameplay consumer to dispatch through. The two largest classes: 82 of 87 Emote rows (only Cry/Laugh/Cheer/Wave/PointState dispatch an animation today — acdream has no general emote-animation player), and all 48 CharacterSettings hotkey rows (ctx `0x10000008` — these are hotkeys for the SAME `PlayerOption`/`CharacterOptions` preference bits OP1's `CharacterOptionTable` and OP4's Character-tab checkboxes already model; wiring "press this key, flip that same server-synced bit" is a real feature, a hotkey-to-option-toggle dispatcher, that does not exist anywhere in acdream yet). Smaller residuals: Spell Slot 10-12, Quickslot 10-13 (both hit a PRE-EXISTING `InputAction` enum gap this slice did not introduce), and roughly twenty UI-panel-toggle rows for panels acdream has no analog for (Vitae/Link Status/House/Map/Character Info/the two Magic panels/...). | `src/AcDream.UI.Abstractions/Input/RetailActionIdentityTable.cs` (class doc has the full accounting); `src/AcDream.App/UI/Layout/KeyboardConfigController.cs` (`CurrentForUnmapped`/`SetForUnmapped`) | Guessing a mapping for an ambiguous row risks silently misrouting a rebind to the wrong gameplay action (worse than an honest "not wired yet" — the identity table's own class doc states this directly); every mapping that WAS added was cross-verified two ways (label match + DAT-default-vs-`KeyBindings.RetailDefaults()` byte match, see `RetailActionIdentityRoundTripTests`). | A user rebinds e.g. an emote or a CharacterSettings hotkey on the Configure Keyboard screen and the binding persists but has no observable in-game effect — matches retail's OWN screen shape (the row exists, is bindable) while honestly lacking retail's gameplay behavior behind it. | `docs/research/2026-08-10-keyboard-config-and-gameplay-tab.md` §5.1-§5.3; live-DAT probe 2026-08-11 (306-row/six-ActionClass accounting, `RetailActionMapReaderTests`) | +| AP-203 | **Filed 2026-08-11 at Campaign OP slice OP8.** Of the DAT ActionMap's 306 user-bindable rows, `RetailActionIdentityTable` (`src/AcDream.UI.Abstractions/Input/RetailActionIdentityTable.cs`) resolves roughly half to a live acdream `InputAction`; the rest render, bind, conflict-check, and persist (via `RetailUnmappedKeyBindings`, a sibling `*-unmapped.json` file) exactly like any other row, but have no live gameplay consumer to dispatch through. The two largest classes: 82 of 87 Emote rows (only Cry/Laugh/Cheer/Wave/PointState dispatch an animation today — acdream has no general emote-animation player), and all 48 CharacterSettings hotkey rows (ctx `0x10000008` — these are hotkeys for the SAME `PlayerOption`/`CharacterOptions` preference bits OP1's `CharacterOptionTable` and OP4's Character-tab checkboxes already model; wiring "press this key, flip that same server-synced bit" is a real feature, a hotkey-to-option-toggle dispatcher, that does not exist anywhere in acdream yet). Smaller residuals: Spell Slot 10-12, Quickslot 10-13 (both hit a PRE-EXISTING `InputAction` enum gap this slice did not introduce), and roughly twenty UI-panel-toggle rows for panels acdream has no analog for (Vitae/Link Status/House/Map/Character Info/the two Magic panels/...). | `src/AcDream.UI.Abstractions/Input/RetailActionIdentityTable.cs` (class doc has the full accounting); `src/AcDream.App/UI/Layout/KeyboardConfigController.cs` (`CurrentForUnmapped`/`SetForUnmapped`) | Guessing a mapping for an ambiguous row risks silently misrouting a rebind to the wrong gameplay action (worse than an honest "not wired yet" — the identity table's own class doc states this directly); every mapping that WAS added was cross-verified two ways (label match + DAT-default-vs-`KeyBindings.RetailDefaults()` byte match, see `RetailActionIdentityRoundTripTests`). | A user rebinds e.g. an emote or a CharacterSettings hotkey on the Configure Keyboard screen and the binding persists but has no observable in-game effect — matches retail's OWN screen shape (the row exists, is bindable) while honestly lacking retail's gameplay behavior behind it. ADDENDUM (2026-08-11, OP8 re-review round 2): this row's scope EXPLICITLY includes the ten CameraAlternateControls (InputMap 0x6) rows the M2 de-alias narrowed to store-only — a case the generic wording understated because their SIBLING rows (InputMap 0x5, the same verbs) ARE live on the same screen: the 0x6 rows display their DAT-default arrow keys (display-only seeding), persist user edits, and drive nothing; only the 0x5 scheme reaches the InputDispatcher. Store-only rows are also EXCLUDED from the conflict universe (they cannot actually collide) — mapped cross-context sharing remains ISSUES #373. | `docs/research/2026-08-10-keyboard-config-and-gameplay-tab.md` §5.1-§5.3; live-DAT probe 2026-08-11 (306-row/six-ActionClass accounting, `RetailActionMapReaderTests`) | | ~~AP-204~~ | **RETIRED 2026-08-11 at the OP8 rework (M3, combined review).** Originally filed for two narrowings: (1) silent auto-reassign on a cross-row conflict instead of retail's modal `OpenOverwriteBindingDialog`, and (2) OK/Cancel wired as left-click instead of retail's right-click-release gesture. (1) is FIXED — `KeyboardConfigController.BeginSlotCapture` now opens a real confirm dialog through `RetailDialogFactory.MakeConfirmation` (the SAME seam `GameplayConfirmationController` uses) BEFORE reassigning, listing every conflicting row (N-way), and only applies on accept; decline leaves every row untouched. (2) is NOT fixed and does not warrant its own row: it is authored-input-only with zero observable difference to a user (retail's own right-click-release on just this pair of buttons carries no distinguishing visual cue either, and every other Campaign OP button already uses left-click) — noted as a code comment at the OK/Cancel wiring site instead of a register row, matching this register's convention of reserving rows for divergences that could produce an observable symptom. | `src/AcDream.App/UI/Layout/KeyboardConfigController.cs` (`FindConflicts`/`BeginSlotCapture`; `WireScreenButtons`'s OK/Cancel `OnClick` comment); `src/AcDream.App/UI/RetailUiRuntime.cs` (`MountKeyboardConfig`'s `ConfirmOverwrite` wiring) | — | — | `docs/research/2026-08-10-keyboard-config-and-gameplay-tab.md` §5.4 (`UIOption_ActionKeyMap::KeyHitHandler @0x00489570`, `OpenOverwriteBindingDialog @0x00488BF0`, `OpenCantOverwriteBindingDialog @0x00489300`) and §5.5 (OK/Cancel `idMessage 0x19` gesture) | | AP-194 | `CharacterOptionTable`'s `ClientDefault` column (what the Character tab's Defaults button restores) disagrees with the raw constructor default word for three ids: `ConfirmVolatileRareUse` (`0x2D`), `ShowHelm` (`0x2F`), and `ShowCloak` (`0x32`) are all ON in retail's constructor default `CharacterOptions2 = 0x00948700` (`PlayerModule::PlayerModule @0x005D51F0`, byte-verified literal write) but report default-OFF via `PlayerModule::GetDefaultOptionValue @0x005D2A30`, whose own per-option table stops at id `0x2A` and returns `false` for everything past it. This is retail's OWN behavior, reproduced deliberately — the Defaults button does not reproduce a fresh `PlayerModule`. **CONFIRMED 2026-08-11 at Campaign OP slice OP4**: `CharacterOptionsPageController` seeds every `BoolOptionRow`'s default directly from this column (`EveryRow_DefaultValue_MatchesCharacterOptionTableClientDefault`, `tests/AcDream.App.Tests/UI/Layout/CharacterOptionsPageControllerTests.cs`); the directive below was followed, not re-litigated. OP4 also independently traced retail's OWN mechanism for the Character tab specifically — `UIOption_Checkbox::SetPlayerOption @0x00486e80` (pseudo-C line 147375) sets `m_default` directly from `GetDefaultOptionValue`, confirming this column (not the separate `DBPropertyCollection`/`InqDefaultGameplayOptionProperty` mechanism that governs the Chat/Config tabs' `m_propName`-bound rows) is the correct and ONLY source for this tab. | `src/AcDream.Runtime/Gameplay/CharacterOptionTable.cs` (`ClientDefault` column; see the type's XML doc); `src/AcDream.App/UI/Layout/CharacterOptionsPageController.cs` | Byte-verified at both addresses (wire research §2.5 for the constructor literals, §8.2 for `GetDefaultOptionValue`'s own table and bounds check) — this is not a guess, it is retail's documented quirk. "Fixing" it to match the constructor default would make acdream's Defaults button MORE correct than retail's own, which is the opposite of this project's goal. | A future OP-campaign slice (OP4, the Character tab's Defaults button) must consult THIS column, not the constructor default word, or a future reader may "fix" this back and silently diverge from retail. | `PlayerModule::GetDefaultOptionValue @0x005D2A30`; `UIOption_Checkbox::SetPlayerOption @0x00486e80` (N-4 anchor-column correction, OP4 review-fix round 2026-08-11 — was mislabeled `PlayerModule::SetPlayerOption`, same address, wrong class); `PlayerModule::PlayerModule @0x005D51F0`; `docs/research/2026-08-10-set-character-options-wire.md` §8.2 | | AP-193 | Character option id `0x34` (`ListenToPKDeathMessages` / "Listen to PK death messages") is mapped to `CharacterOptions2` bit `0x02000000` and modeled as a batched (non-auto-save) option purely on ACE's own enum — the id does not exist in the 2013 EoR PDB (`PlayerOption` there terminates at `TotalNumberOfPlayerOptions_PlayerOption = 0x34`), so neither the mask nor its `IsAutoSaveOption`/`GetDefaultOptionValue` classification is byte-verifiable against our binary. | `src/AcDream.Runtime/Gameplay/CharacterOptionTable.cs` (`HearPkDeathMessages` row) | The user's retail memory (and ACE's own `CharacterOption` enum) both carry this option; shipping wire+store coverage for it is strictly better than omitting the row the Character tab's screenshots show, and ACE never actually reads the bit server-side (`PlayerFactory.cs:659-660` — "possibly was added to Defaults post PDB we have"), so a wrong id/mask/auto-save guess here has zero server-observable consequence either way. | If the final EoR client's real id/mask/auto-save classification ever surfaces (a later PDB, or a byte-level trace against a 2015+ binary), this row's values may be wrong and need correcting — until then treat them as ACE-sourced, not retail-verified. | ACE `PlayerFactory.cs:659-660`, `CharacterOptions2.cs` (`ListenToPKDeathMessages = 0x02000000`); `named-retail/acclient.h:4162-4218` (2013 `PlayerOption` terminates at `0x34`); `docs/research/2026-08-10-set-character-options-wire.md` §8.1 | diff --git a/docs/research/2026-08-11-campaign-op-test-script.md b/docs/research/2026-08-11-campaign-op-test-script.md index 984316e0..1164cb0e 100644 --- a/docs/research/2026-08-11-campaign-op-test-script.md +++ b/docs/research/2026-08-11-campaign-op-test-script.md @@ -855,6 +855,22 @@ Three live runs against local ACE (`127.0.0.1:9000`, `+Acdream`, ## OP8 — Configure Keyboard +**Known, tracked behaviors — do NOT file as defects (read before testing):** + +- **Shared combat keys prompt a false conflict (ISSUES #373).** Retail + legitimately shares Insert/Delete/End/PageUp/PageDown across the three + combat contexts (Melee/Missile/Magic) via the DAT's `ConflictingMaps` + table, which acdream does not read yet. Rebinding any of those keys — or + any chord one combat context already holds — will show a conflict + confirmation retail would not. DECLINE the prompt and move on; the + binding you declined stays untouched. Report only if declining CHANGES a + binding anyway. +- **Camera Alternate Controls rows are display/store-only (AP-203).** They + show retail's authored arrow-key defaults and accept edits, but do not + drive the camera — only the primary Camera scheme is live. Verify they + RENDER (arrow keys visible); do not expect camera behavior from them. + + The screen is retail's own separate full-screen window (`gmKeyboardUI`, LayoutDesc `0x21000009`), NOT a fifth tab of the Options panel. It opens from the Gameplay tab's Configure Keyboard button — **the OP3 INERT diff --git a/src/AcDream.App/UI/Layout/KeyboardConfigController.cs b/src/AcDream.App/UI/Layout/KeyboardConfigController.cs index c4ec8299..65c160d6 100644 --- a/src/AcDream.App/UI/Layout/KeyboardConfigController.cs +++ b/src/AcDream.App/UI/Layout/KeyboardConfigController.cs @@ -366,10 +366,19 @@ public sealed class KeyboardConfigController ? (liveBindings[0].Activation, liveBindings[0].Scope) : (ActivationType.Press, InputScope.Game); + IReadOnlyList defaults = DatDefaultsToChords(row.DefaultBindings); + IReadOnlyList storedUnmapped = mapped + ? Array.Empty() + : bindings.CurrentForUnmapped((row.InputMapId, row.ActionId)); + // OP8 re-review round 2 (SHOULD-FIX): an unmapped/store-only row with + // no persisted chords displays its DAT DEFAULTS — retail shows the + // authored bindings (the Camera Alternate rows' arrow keys) and a + // blank row misreads as "unbound". Display-only: nothing here feeds + // the InputDispatcher, and the store only gains the defaults if the + // user actually edits the row (the apply closure below). IReadOnlyList initial = mapped ? liveBindings.Select(b => b.Chord).ToArray() - : bindings.CurrentForUnmapped((row.InputMapId, row.ActionId)); - IReadOnlyList defaults = DatDefaultsToChords(row.DefaultBindings); + : storedUnmapped.Count > 0 ? storedUnmapped : defaults; var model = new ActionKeyMapOptionRow(initial, defaults, apply: value => { @@ -538,6 +547,15 @@ public sealed class KeyboardConfigController foreach (RowView other in _rows) { if (ReferenceEquals(other, exclude)) continue; + // OP8 re-review round 2 R1: store-only rows (MappedAction null — + // the Camera Alternate scheme, Emote/CharacterSettings hotkeys) + // never reach the InputDispatcher, so a chord they display cannot + // actually collide with anything; counting them made the ten + // arrow-key defaults trip a false N-way confirm on any arrow + // rebind. Retail-mapped cross-context sharing (ConflictingMaps — + // the Insert/Delete/End/PageUp/PageDown combat cluster) remains + // deferred as ISSUES #373; only INERT rows are excluded here. + if (other.MappedAction is null) continue; if (other.Model.Current.Contains(chord)) rows.Add(other); } diff --git a/tests/AcDream.App.Tests/UI/Layout/KeyboardConfigControllerTests.cs b/tests/AcDream.App.Tests/UI/Layout/KeyboardConfigControllerTests.cs index 680fe8ff..05369a20 100644 --- a/tests/AcDream.App.Tests/UI/Layout/KeyboardConfigControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/KeyboardConfigControllerTests.cs @@ -557,12 +557,19 @@ public sealed class KeyboardConfigControllerTests Assert.Equal(InputAction.CameraRotateLeft, ctx5.MappedAction); Assert.Null(ctx6.MappedAction); // unmapped — no live dual-binding infrastructure (M2) - // Rebinding ctx5's row must not touch ctx6's storage, and vice versa. + // Round-2 SHOULD-FIX: an unmapped row with no persisted chords now + // DISPLAYS its DAT defaults (retail shows the arrow keys; blank read + // as "unbound"). Display-only — storage stays untouched until the + // user edits THIS row. + Assert.NotEmpty(ctx6.Model.Current); + var ctx6InitialDisplay = ctx6.Model.Current.ToArray(); + + // Rebinding ctx5's row must not touch ctx6's display or storage. ctx5.KeyButtons[0].OnClick!.Invoke(); fake.Capture(ChordW); Assert.Contains(ChordW, ctx5.Model.Current); - Assert.Empty(ctx6.Model.Current); // ctx6 never seeded/written by ctx5's edit - Assert.Empty(fake.Unmapped); // ctx6 untouched — only ctx5's mapped write happened + Assert.Equal(ctx6InitialDisplay, ctx6.Model.Current); // unchanged by ctx5's edit + Assert.Empty(fake.Unmapped); // ctx6's STORE untouched — display seeding writes nothing ctx6.KeyButtons[0].OnClick!.Invoke(); fake.Capture(ChordA); @@ -584,4 +591,23 @@ public sealed class KeyboardConfigControllerTests Assert.Null(controller); } + + // ── OP8 re-review round 2: identity-map injectivity is load-bearing for + // BOTH M1 (per-row activation capture) and M2 (no dual rows editing one + // action) — pin it so a future mapping addition cannot silently alias. ── + + [Fact] + public void IdentityMap_IsInjective_NoTwoRowsShareOneInputAction() + { + var seen = new Dictionary(); + foreach (((uint mapId, uint actionId), InputAction action) in RetailActionIdentityTable.Map) + { + Assert.False( + seen.TryGetValue(action, out (uint MapId, uint ActionId) prior), + $"InputAction.{action} is mapped by BOTH (0x{prior.MapId:X}, 0x{prior.ActionId:X}) " + + $"and (0x{mapId:X}, 0x{actionId:X}) — aliasing reintroduces the M2 twin-row clobber."); + seen[action] = (mapId, actionId); + } + Assert.True(seen.Count > 100, $"sanity: only {seen.Count} mapped actions seen"); + } }