From 386076af0f2786b5f46ece4cdb03fd75e6f91783 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 11 Aug 2026 03:05:27 +0200 Subject: [PATCH] =?UTF-8?q?fix(ui):=20OP3=20review=20fixes=20=E2=80=94=20b?= =?UTF-8?q?yte-verified=20Magic=20chat=20lines,=20Gameplay/OptionPage=20sh?= =?UTF-8?q?ape,=20mid-air=20tri-state,=20shared=20geometry?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidated fix round for the two OP3 dual-lens reviews (docs/research/2026-08-11-op3-review-{mechanism,blast}.md), both APPROVE-WITH-FIXES. MUST-FIX: - The six "Use Mouse Turning Settings" chat lines were typed RetailLogTextType.ClientLocal (0x1A); retail types them 0x07 (Magic). BYTE-VERIFIED against the PDB-paired binary at all six gmConfigUI::SetMouseTurningDefaults call sites (0x0049E972/E9E2/EA52/ EAA4/EAF6/EB48): every site pushes `6a 07` (type=7) immediately before the text-pointer push and the AddTextToScroll call. Added a dedicated OptionsRuntimeBindings.DisplayMouseTurningMacroLine seam routed at Magic (scrolling chat transcript, light blue, timestamped) instead of the 4-slot SpewBox ClientLocal uses; the mid-air refusal and UA/RA keep ClientLocal (both independently confirmed correct). - Filed AD-77: the client-wide floating-only gmPanelUI host divergence (retail also exposes a docked 0x21000017 host) the plan §5 delegated to this review, scoped to every main panel, not just Options. SHOULD-FIX: - gmGameplayOptionsUI is not an OptionPage in retail (acclient.h:55857, UIElement_Field). OptionsPanelController now constructs the Gameplay slot's OptionPage with AfterApply deliberately null, so entering/ leaving that tab never publishes SaveCharacterOptionsRuntimeCmd. Corrected OptionPageModel's doc comment and rewrote the two tests that pinned the wrong (Gameplay-flushes) shape. - Added the OptionPage.OnOptionChanged seam (PlayerOptionPage:: OnOptionChanged @0x004F27D0) — fires as the last step of Apply/ Reset/Defaults, plus once per live LED edit via a new IOptionRow.AttachPageNotify hook (BoolOptionRow wires it into SetCurrentValue only, matching retail's Apply(1)-only HandleDialogAndNotices path). OP4-6 will bind Apply/Reset enable state to this. - Exit to Character Selection's mid-air refusal is now tri-state (Func IsGrounded): retail's UseTime only reaches the airborne test inside `else if (smartbox->player)`, so outside player mode (or with no live controller) the button is a SILENT no-op, not a refusal. Fixed the inverted comment at both call sites. - Options panel geometry now matches its nine gmPanelUI siblings sharing RetailPanelUiController's one main-panel rectangle (ResizeX=false, bottom-edge-only resize, no invented Min/MaxWidth/ Height) instead of being the only all-four-edge/horizontal-resize outlier whose width silently reverted whenever a sibling was shown. - Added the three missing test pins: Options/Character mutual exclusion through a REAL RetailPanelUiController registration, RetailDialogFactory.MakeConfirmation's omitted-queueKey overload sharing DefaultQueueKey, and UiTabPanel.ActivePageChanged never firing on a dormant (non-activated) host. - TS-74's What/Where now names the five store-only CameraTurning preferences explicitly instead of only mentioning them in Risk. - Test script gains the toolbar-button ghosted->enabled+highlight check, UseMouseTurning-survives-relogin and the five prefs-survive- relaunch steps, a UA/RA legibility eye-item, and the corrected bottom-edge-only geometry description for step 5. One-liners fixed in files already touched: symmetric close-button resolve-failure logging in OptionsPanelController.Bind (blast NOTE 8). Full Release suite: 12,947 passed / 4 skipped / 0 failed (baseline 12,935/4/0 post-OP7 — 12 net new tests; the two OptionPageModelTests "wrong-shape" tests were renamed/rewritten in place, not removed). Co-Authored-By: Claude Fable 5 --- .../retail-divergence-register.md | 5 +- .../2026-08-11-campaign-op-test-script.md | 66 ++++++- .../InteractionRetainedUiComposition.cs | 31 +++- src/AcDream.App/UI/Layout/OptionPageModel.cs | 74 ++++++-- .../UI/Layout/OptionsPanelController.cs | 20 ++- src/AcDream.App/UI/RetailUiRuntime.cs | 96 ++++++++--- .../OP2ReworkBlastRadiusConformanceTests.cs | 23 +++ .../UI/Layout/OptionPageModelTests.cs | 161 ++++++++++++++++-- .../UI/Layout/OptionsPanelControllerTests.cs | 41 ++++- .../UI/Layout/RetailDialogFactoryTests.cs | 29 ++++ .../UI/Layout/RetailPanelUiControllerTests.cs | 31 ++++ 11 files changed, 505 insertions(+), 72 deletions(-) diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 2b928397..c8a34cee 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -62,7 +62,7 @@ accepted-divergence entries (#96, #49, #50). --- -## 2. Adaptation (AD) — 56 active rows (AD-76/AD-75/AD-74 filed 2026-08-11 at Campaign OP slice OP3 — the Options panel's Exit to Character Selection "behaves as Exit Game" adaptation (D6), the Urgent Assistance/Report Abuse dead-URL interface-text short-circuit (D5), and In-Game Help Files' asset-missing inert button (D5); AD-73 filed 2026-08-11 at the Campaign OP OP2 rework — `UiTabPanel`'s dormant-until-`ActivateTabBehavior()` activation model, replacing retail's unconditional per-instance tab-table wiring, so the four already-shipped Type-8 hosts keep their existing controller-owned switching without a double-driver race; AD-72 filed 2026-08-08 at the Slice 5.3 review corrections — `VendorPricing`'s double-precision narrowing versus retail's x87 extended precision, same class as AD-33; AD-65 RETIRED and AD-69 FILED 2026-08-07 at Campaign S S4 — the away-arm now snaps per retail @0x00509c50, while AD-66's byte-confirmed sibling landing is WITHHELD pending #341's measurement-anomaly apparatus, and AD-69 records the seam-frame dist gap the same pass discovered; AD-56 RESTORED 2026-08-07 — the a8a7d64b revert had collaterally DELETED it, the inverse of the AD-55 zombie it also created; its plumb-fall-freeze condition is live again since TS-4’s real retirement at Slice 2B; AD-55 RE-RETIRED 2026-08-07 — its 2026-07-30 retirement at 252e8068 was collaterally resurrected by the a8a7d64b revert of the unrelated TS-4 commit; the code kept the cos(10°) fix throughout; AD-68 filed 2026-08-07 at the #338 closure — the async-residency placeholder mover shape (0.4/0.4 steps + capsule) has no retail counterpart because retail loads synchronously; AD-67 filed 2026-08-07 at the #32 closeout — the narrowed `SetContactPlane` keeps its per-write `ContactPlaneCellId`, which retail writes only at `init_contact_plane`; AD-49 filed 2026-08-06 at the #334 fix — the BSP part-array flood runs its outdoor cell rectangle at seed time rather than only from retail’s residency-gated walk, keeping both registration floods on one residency rule; AD-64 filed 2026-08-05 at the C5b architecture review's D1 fix — AD-60's W2 wire-cell REACHABILITY decision is expressed once per host because the two hosts run parallel non-shared inbound routes; the committed VALUE is single-sourced at `RuntimeEntityObjectLifetime.CommitWireCellRebucket`, and unification is filed as #324; AD-60 CORRECTED the same day — its surviving-channel enumeration presented "the local force path, the missile arm" as exhaustive when the entire no-window host belonged in it; AD-1 RETIRED 2026-08-05, C5a deletion sweep — the legacy outdoor demote/restore lift this row described was `PhysicsEngine.Resolve`'s own body, deleted with zero production callers; AD-42 DELETED 2026-08-04, C4 route 3 — its last surviving citation, the headless portal-arrival resync's two-call Resolve/ResolvePlacement split, was retired by the canonical `RuntimeAcceptedPositionDriveController` portal arm; AD-2 amended same route with the deferred-place timing adaptation, the T8 tolerated-overwrite note, and the leash-anchor nuance; AD-63 filed 2026-08-04, cancelled-park presentation rollback — the rollback restores every presentation registration the park's Withdraw removed EXCEPT the player's selection, which is user intent rather than a projection; AD-62 filed 2026-08-03, C4 route 2 round 2 — a deferred ForcePosition retired without committing is not re-applied and its ack is not sent; AD-61 filed 2026-08-02, C3c review round 1 — the #270 settle compression now covers the local player; AD-59/AD-60 filed 2026-08-02, continuation-executor slice) +## 2. Adaptation (AD) — 57 active rows (AD-77 filed 2026-08-11 at the Campaign OP OP3 review-fix round — the client-wide floating-only `gmPanelUI` host divergence (retail also exposes a docked `0x21000017` host) the plan's §5 delegated to the OP3 dual review, scoped to every main panel not just Options; AD-76/AD-75/AD-74 filed 2026-08-11 at Campaign OP slice OP3 — the Options panel's Exit to Character Selection "behaves as Exit Game" adaptation (D6), the Urgent Assistance/Report Abuse dead-URL interface-text short-circuit (D5), and In-Game Help Files' asset-missing inert button (D5); AD-73 filed 2026-08-11 at the Campaign OP OP2 rework — `UiTabPanel`'s dormant-until-`ActivateTabBehavior()` activation model, replacing retail's unconditional per-instance tab-table wiring, so the four already-shipped Type-8 hosts keep their existing controller-owned switching without a double-driver race; AD-72 filed 2026-08-08 at the Slice 5.3 review corrections — `VendorPricing`'s double-precision narrowing versus retail's x87 extended precision, same class as AD-33; AD-65 RETIRED and AD-69 FILED 2026-08-07 at Campaign S S4 — the away-arm now snaps per retail @0x00509c50, while AD-66's byte-confirmed sibling landing is WITHHELD pending #341's measurement-anomaly apparatus, and AD-69 records the seam-frame dist gap the same pass discovered; AD-56 RESTORED 2026-08-07 — the a8a7d64b revert had collaterally DELETED it, the inverse of the AD-55 zombie it also created; its plumb-fall-freeze condition is live again since TS-4’s real retirement at Slice 2B; AD-55 RE-RETIRED 2026-08-07 — its 2026-07-30 retirement at 252e8068 was collaterally resurrected by the a8a7d64b revert of the unrelated TS-4 commit; the code kept the cos(10°) fix throughout; AD-68 filed 2026-08-07 at the #338 closure — the async-residency placeholder mover shape (0.4/0.4 steps + capsule) has no retail counterpart because retail loads synchronously; AD-67 filed 2026-08-07 at the #32 closeout — the narrowed `SetContactPlane` keeps its per-write `ContactPlaneCellId`, which retail writes only at `init_contact_plane`; AD-49 filed 2026-08-06 at the #334 fix — the BSP part-array flood runs its outdoor cell rectangle at seed time rather than only from retail’s residency-gated walk, keeping both registration floods on one residency rule; AD-64 filed 2026-08-05 at the C5b architecture review's D1 fix — AD-60's W2 wire-cell REACHABILITY decision is expressed once per host because the two hosts run parallel non-shared inbound routes; the committed VALUE is single-sourced at `RuntimeEntityObjectLifetime.CommitWireCellRebucket`, and unification is filed as #324; AD-60 CORRECTED the same day — its surviving-channel enumeration presented "the local force path, the missile arm" as exhaustive when the entire no-window host belonged in it; AD-1 RETIRED 2026-08-05, C5a deletion sweep — the legacy outdoor demote/restore lift this row described was `PhysicsEngine.Resolve`'s own body, deleted with zero production callers; AD-42 DELETED 2026-08-04, C4 route 3 — its last surviving citation, the headless portal-arrival resync's two-call Resolve/ResolvePlacement split, was retired by the canonical `RuntimeAcceptedPositionDriveController` portal arm; AD-2 amended same route with the deferred-place timing adaptation, the T8 tolerated-overwrite note, and the leash-anchor nuance; AD-63 filed 2026-08-04, cancelled-park presentation rollback — the rollback restores every presentation registration the park's Withdraw removed EXCEPT the player's selection, which is user intent rather than a projection; AD-62 filed 2026-08-03, C4 route 2 round 2 — a deferred ForcePosition retired without committing is not re-applied and its ack is not sent; AD-61 filed 2026-08-02, C3c review round 1 — the #270 settle compression now covers the local player; AD-59/AD-60 filed 2026-08-02, continuation-executor slice) Recent retirements: AD-3/AD-4 retired 2026-07-31 by exact active/per-candidate visible-cell availability, full-catalog containment-root validation, and the @@ -171,6 +171,7 @@ readiness/requeue adaptation. See | AD-74 | **Filed 2026-08-11 at Campaign OP slice OP3 (D6).** The Options panel's "Exit to Character Selection" button (element `0x10000203`) behaves exactly like "Exit Game" (element `0x10000617`) after its own confirmation dialog + mid-air refusal, instead of retail's real behavior — logging the character off and returning to a pre-world character-select screen while keeping the login connection alive. | `src/AcDream.App/UI/RetailUiRuntime.cs` (`RequestExitToCharacterSelection`) | acdream has no pre-world character-select UI and `WorldSession` has no path back to `InCharacterSelect` from in-world — `Dispose()` tears down the entire session/socket (research doc `2026-08-10-keyboard-config-and-gameplay-tab.md` §2.4). Retail's own confirmation dialog (`ID_Client_EndCharacterSessionConfirm`) and mid-air refusal (`ClientTextRefusals.CantLogOffMidAir`) DO port exactly — only the post-confirmation destination differs. | A user clicking "Exit to Character Selection" expecting to pick a different character instead exits the client entirely, same as Exit Game. | `CM_UI::SendNotice_EndCharacterSession`; `gmGamePlayUI::RecvNotice_EndCharacterSession @0x004EBEA0`; `gmGamePlayUI::UseTime @0x004EA3A0` | | AD-75 | **Filed 2026-08-11 at Campaign OP slice OP3 (D5).** Urgent Assistance (`0x10000206`) and Report Abuse (`0x10000207`) never call `ShellExecuteA` against `http://support.turbine.com/ics/support/ticketnewwizard.asp?style=classic` — the endpoint is dead in 2026. Each button instead ALWAYS emits its own byte-verified retail failure body (the `ShellExecuteA`-failure `MessageBoxA` text, `(Error code %d)` dropped since no real Win32 error ever occurs, the URL kept verbatim) through the interface-text seam (`RetailLogTextType.ClientLocal`) instead of a native `MessageBoxA` popup. | `src/AcDream.Core/Chat/OptionsPanelText.cs` (`UrgentAssistanceUnavailable`/`ReportAbuseUnavailable`); `src/AcDream.App/UI/Layout/OptionsPanelController.cs` (button wiring) | The URL genuinely does not resolve to a live Turbine support endpoint; attempting `ShellExecuteA` would open a browser to a dead page rather than usefully fail. The retained failure TEXT is retail's own (byte-verified), just always shown instead of conditionally on a real launch failure, and routed to acdream's existing interface-text channel rather than a modal OS dialog (retail's own EoR-era mechanism has no acdream analogue for a one-off native `MessageBoxA`). | If Turbine ever revives the endpoint, both buttons would still short-circuit instead of opening it — a silent staleness, not a crash. | `gmGameplayOptionsUI::ListenToElementMessage @0x0049E110`; `ShellExecuteA` call sites `0x0049E154`/`0x0049E1F0`; research doc `2026-08-10-keyboard-config-and-gameplay-tab.md` §4.1/§4.2 | | AD-76 | **Filed 2026-08-11 at Campaign OP slice OP3 (D5).** In-Game Help Files (`0x10000205`) is authored and clickable but has no handler — clicking it does nothing visible. | `src/AcDream.App/UI/Layout/OptionsPanelController.cs` (button wiring — no callback bound) | Retail's own `KeyStone::OpenHelp` loads a third-party embedded help viewer (`plugins\ACHelpPlugin.dll` via `keystone.dll`) that acdream does not have and cannot port (no DAT-resident help content, no source). Retail ITSELF fails silently with the plugin absent (`KeyStone::m_fnAC2HelpPluginExecute` unresolved) — mirroring that as an inert button is the faithful behavior for "the asset is missing", not an invented stub screen. | A user clicking In-Game Help Files gets no feedback at all, same as retail with the plugin missing — indistinguishable from a dead button unless they already expect the asset-missing case. | `KeyStone::OpenHelp @0x00557010`; `KeyStone::Init @0x00556CF0` (the unresolved plugin function pointer); research doc `2026-08-10-keyboard-config-and-gameplay-tab.md` §4.5 | +| AD-77 | **Filed 2026-08-11 at the Campaign OP OP3 review-fix round (dual-review S4/MUST-FIX 2 — the plan's §5 "out of scope" list explicitly delegated this ruling to the OP3 review).** Retail exposes TWO `gmPanelUI` host variants for the same panel stack — a floating host (`0x2100006E`, `gmFloatyPanelUI`) and a docked host (`0x21000017`) — so a retail user can dock the Options panel (and every other `gmPanelUI` sibling) into a fixed screen position instead of leaving it freely floating. acdream mounts every main panel through `RetailWindowFrame.Mount` + `RetailPanelUiController.RegisterMainPanel` against the floating host ONLY; no code path resolves or mounts `0x21000017` at all. | `src/AcDream.App/UI/RetailUiRuntime.cs` (every `Mount*`/`RegisterMainPanel` call site for a `gmPanelUI` sibling — Character/Inventory/Spellbook/Effects/the four indicator-detail panels/Options); `src/AcDream.App/UI/Layout/RetailWindowFrame.cs` | This predates OP3 — every `gmPanelUI` sibling has shipped floating-only since its own slice landed; OP3 did not introduce the gap, it just added a tenth panel to an already-floating-only cohort. The plan explicitly scoped filing the row to "whichever slice's review deems it a divergence" rather than blocking any one panel's slice on building a docked-host variant no prior panel has either. | A user who expects to dock the Options panel (or any other main panel) the way retail allows cannot — every `gmPanelUI` sibling is floating-only in acdream, client-wide, not an Options-specific gap. | research doc `2026-08-10-options-panel-structure.md` §10.1 (docked/floating host pair); `docs/plans/2026-08-10-options-panel-campaign.md` §5 | --- @@ -404,7 +405,7 @@ AP-94..AP-112 for the confirmed retail-UI completion gaps. | TS-68 | **Filed 2026-08-09 (Campaign CH slice CH4); corrected 2026-08-09 at the CH4 REJECT-review, Blocker 1.** `@allegiance`/`@all` and `@house`/`@hou` are real retail management-command dispatchers with 12 and 15 subcommands respectively (registry doc §2.5/§2.5b). acdream ports only the subset with simple parameterless/single-field wire shapes (allegiance `info`/`hometown`/`ho`; house `recall`/`re`/`mansion_recall`/`alleg_recall`/`ma`/`abandon`). For `@house`, every other subcommand (open, close, storage, remove, boot, boot_all, remove_all, guest, available, hooks, on, off) still falls through to ACE server-passthrough (which replies "Unknown command") — unchanged from the original filing. **The original filing was WRONG for `@allegiance`/`@all`: retail's own `DoAllegiance` never reaches DoChannelCommand/server-passthrough for an unrecognized subcommand** — it prints "Please see @help Allegiance for more information on how to use this command." locally (`label_57da4b`, 0x0057DA4B) and stays entirely client-side. **Corrected again 2026-08-09 at the CH4 re-review, SHOULD-FIX 3.** Retail does NOT refuse boot/ban/officer/title/motd/name/lock/house/chat/broadcast — `DoAllegiance`'s dispatcher table EXECUTES each one locally through its own handler (e.g. `DoAllegianceBoot @ 0x0057D646` is the dispatcher's call site into `ClientCommunicationSystem::DoAllegianceBoot`; `DoAllegianceBan`/`DoAllegianceOfficer`/`DoAllegianceOfficerTitle`/`DoMotd`/`DoAllegianceName`/`DoAllegianceLock`/`DoAllegianceHouse` are its siblings in the same table). acdream has none of those nine handlers ported (tracked by issue #360) and instead shows the SAME unrecognized-subcommand refusal ("Please see @help Allegiance...", `label_57da4b`, 0x0057DA4B) for every one of them, pending the #360 port. What matches retail here is the OWNERSHIP RULE — the verb never reaches `DoChannelCommand`/server-passthrough for `@allegiance`/`@all` regardless of subcommand — NOT the subcommand's actual behavior, which retail executes and acdream does not yet. This still closes the real bug the original filing named (the unmatched subcommand text broadcast to the Allegiance chat channel, 0x02000000). The standalone `@motd` verb (reached directly, not via `@allegiance motd`) remains a separate, still-open gap. `RetailClientCommandCatalog.TryMatchHouse`/`TryMatchAllegiance` (`src/AcDream.UI.Abstractions/Panels/Chat/RetailClientCommandCatalog.cs`) | Retail would execute these locally (with its own usage/confirmation/refusal text). House's unported subcommands still reach ACE, which does not implement them as chat commands either — no functional loss on a real server, but a user typing e.g. `@house open` gets ACE's generic "Unknown command" instead of retail's real behavior. Allegiance's unported subcommands correctly stay local (never reach ACE) but show a generic refusal instead of retail's real per-subcommand execution — a user typing e.g. `@allegiance boot Name` gets "Please see @help Allegiance..." instead of retail's real boot confirmation/effect, until #360 ports the nine `DoAllegiance*`/`DoMotd`/`DoAllegianceHouse` handlers. | `ClientCommunicationSystem::DoAllegiance @ 0x0057D5A0`; `DoHouse @ 0x00580860`; ACE `GameActionType` opcodes for each subcommand (all exist server-side) | | TS-69 | **Filed 2026-08-09 (Campaign CH slice CH4).** `@day`, `@log`, and `@render` are registered retail verbs acdream recognizes only in the `/help ` lookup table, not as executable client commands. `@day` needs a sky/time-of-day override hook the renderer doesn't expose; `@log` needs a safely-lifecycled chat-to-file writer (deferred to avoid an unaudited file-handle leak across reconnects); `@render` has no acdream equivalent to retail's `SmartBox::HandleRenderOption` render-option surface. All three fall through to server passthrough. `RetailCommandHelpTable` (`src/AcDream.UI.Abstractions/Panels/Chat/RetailCommandHelpTable.cs`) | A user typing `@day`/`@log`/`@render` gets ACE's "Unknown command" instead of retail's local toggle/file-copy/render-option behavior — cosmetic/QoL only, no gameplay impact | `ClientCommunicationSystem::DoDay @ 0x005706F0`; `DoSetOutput @ 0x0057E4F0`; `DoRenderOption @ 0x0057E120` | | TS-67 | **Ambient contributions are computed in-plane.** Retail's `CLandBlock::add_ambient_sounds` @ `0x530310` positions each contributing land cell at its own SW terrain VERTEX, including that vertex's height, and `Ambient::CalcWeight` deliberately includes Z in its distance (where `CalcDir` deliberately excludes it — the two differ on purpose). acdream's gatherer supplies Z = 0 for the offset, so a cell's weight ignores the height difference between the listener and the terrain under that cell. | `src/AcDream.Core/Audio/AmbientSoundGatherer.cs` (`ContributeLandblock`) | Sampling the height needs the landblock's height table threaded into the walk alongside the terrain words; the walk already runs only on a 24 m crossing so the cost is not the obstacle, the extra plumbing at slice end was. The error is bounded by terrain relief inside 120 m and affects the crossfade weight only, never the direction. | On steep ground an ambient reads slightly louder than retail, because the true 3-D distance is longer than the planar one. | `CLandBlock::add_ambient_sounds @ 0x530310`; `Ambient::CalcWeight @ 0x550DD0` | -| TS-74 | **Filed 2026-08-11 at Campaign OP slice OP3.** acdream has no persistent "turn to face camera" mouse-turning MODE — `MouseLookState` only implements retail's MMB-hold `CameraInstantMouseLook`. The Options panel's "Use Mouse Turning Settings" button still sends the `PlayerOption.UseMouseTurning` bit (`SetSingleCharacterOption 0x0005`) and persists the five client-local Camera/Input preferences exactly as retail does — but flipping the bit ON has NO observable effect on acdream's camera today, because the mode it is supposed to enable was never built. | `src/AcDream.UI.Abstractions/Input/MouseLookState.cs` (the only mouse-look mode present); `src/AcDream.App/UI/Layout/MouseTurningSettingsMacro.cs` (sends the bit regardless) | Building the persistent mouse-turning camera mode is a camera/physics-scope feature, out of the Options-panel campaign's scope; the STORE-and-SEND half is honest and complete (matches every other stored-but-unconsumed option class in this register), so the bit round-trips correctly for any future consumer or a retail client reading the same character. | A user who clicks "Use Mouse Turning Settings" expecting the camera to start turning with mouse movement sees no camera change — only the (unwired) preferences persisting and the wire bit flipping. | `PlayerModule::UseMouseTurning @0x005D3380`; `CharacterOptions2.UseMouseTurning 0x00400000`; `claude-memory/project_camera_visibility_coupling.md` | +| TS-74 | **Filed 2026-08-11 at Campaign OP slice OP3; What/Where extended 2026-08-11 at the OP3 review-fix round (mechanism review S5).** acdream has no persistent "turn to face camera" mouse-turning MODE — `MouseLookState` only implements retail's MMB-hold `CameraInstantMouseLook`. The Options panel's "Use Mouse Turning Settings" button still sends the `PlayerOption.UseMouseTurning` bit (`SetSingleCharacterOption 0x0005`) and persists the five client-local `CameraTurningSettings` preferences exactly as retail does — but flipping the bit ON has NO observable effect on acdream's camera today, because the mode it is supposed to enable was never built. **All five persisted preferences are STORE-ONLY with no consumer, not just the camera mode itself:** `Camera_Stiffness`, `Camera_AdjustmentSpeed`, `Camera_AlignToSlope`, `Input_MouseLookSensitivity`, and `Input_InvertMouseLookYAxis` (research doc `2026-08-10-options-panel-structure.md` §4; OP6 is their contracted home) land in `settings.json`'s `cameraTurning` section and are read back only by the macro itself — acdream's ACTUALLY-live mouse sensitivity lives entirely separately, in `CameraPointerInputController`'s `_chase`/`_flySensitivity`/`_orbitSensitivity` fields (F8/F9-adjustable), so the macro's chat lines quote a `Default`-seeded "from" value (e.g. `0.550000`) that describes no live client state on a fresh profile. | `src/AcDream.UI.Abstractions/Input/MouseLookState.cs` (the only mouse-look mode present); `src/AcDream.App/UI/Layout/MouseTurningSettingsMacro.cs` (sends the bit regardless); `src/AcDream.UI.Abstractions/Panels/Settings/CameraTurningSettings.cs` (the five store-only keys); `src/AcDream.App/Input/CameraPointerInputController.cs` (the SEPARATE, actually-live sensitivity fields) | Building the persistent mouse-turning camera mode is a camera/physics-scope feature, out of the Options-panel campaign's scope; the STORE-and-SEND half is honest and complete (matches every other stored-but-unconsumed option class in this register), so the bit round-trips correctly for any future consumer or a retail client reading the same character. | A user who clicks "Use Mouse Turning Settings" expecting the camera to start turning with mouse movement sees no camera change — only the (unwired) preferences persisting and the wire bit flipping. Beyond that: a user who separately tunes acdream's live F8/F9 mouse sensitivity, then clicks this button, sees a chat line quoting an UNRELATED stored value, not their live sensitivity — two stores for one concept, user-visible once OP6 surfaces `cameraTurning` in the Config tab UI. | `PlayerModule::UseMouseTurning @0x005D3380`; `CharacterOptions2.UseMouseTurning 0x00400000`; `claude-memory/project_camera_visibility_coupling.md` | --- 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 a4c1fb59..78baa518 100644 --- a/docs/research/2026-08-11-campaign-op-test-script.md +++ b/docs/research/2026-08-11-campaign-op-test-script.md @@ -28,13 +28,24 @@ for this slice, not a bug. `gmPanelUI`-family panel was already open (Character Info, Skills, Vitae, Inventory, etc.), opening Options closes it first — this is retail's real "one active panel" behavior (`gmPanelUI`'s shared - geometry/exclusive-child model), not new to this panel. + geometry/exclusive-child model), not new to this panel. **Note the + button's STATE change, not just its behavior:** before this slice this + button was greyed out/disabled (panel id 10 was not in the catalog) and + clicking it did nothing. It should now be fully enabled, AND it should + highlight (same visual as the Inventory/Character/Magic buttons) while + the Options panel is open, un-highlighting when it closes. 4. **Click the panel's own close (X) button**, top-right of the window chrome. The panel closes — same action as F11. -5. **Drag the window by its border/chrome, resize it from any edge or - corner.** Behaves like the CH6 chat floaties: draggable, resizable on - all four edges, remembers its geometry across a close/reopen within - the same session. +5. **Drag the window by its border/chrome; resize it from the BOTTOM + edge only.** Options shares its geometry policy with every other + `gmPanelUI` sibling (Character/Inventory/Spellbook/the indicator-detail + panels): draggable, resizable from the bottom edge only (NOT the top, + left, or right edges, and NOT a corner — the panel does not resize + horizontally at all), and it remembers its HEIGHT across a close/reopen + within the same session, same as every sibling. This is a correction + from OP3-as-landed, which briefly gave Options all-four-edge/horizontal + resize that no sibling has and that silently reverted to 300px wide the + next time a sibling panel was shown. ### Tab switching @@ -79,14 +90,29 @@ for this slice, not a bug. — retail's own byte-verified failure text, always shown (the URL is dead in 2026, so acdream never attempts to open a browser — register row AD-75). **No native browser window opens, no OS dialog box.** -14. **Report Abuse.** Click it. Same shape as Urgent Assistance, with - its own retail text ("...to submit an abuse report..."). + **Legibility check (blast review SHOULD-FIX 4):** this is a + THREE-LINE, ~110-character-URL body landing in the SpewBox overlay, + which auto-expires after 5 seconds. Read the whole body BEFORE it + disappears — if you cannot finish reading it (especially the URL) in + that window, report it as a legibility problem; the honest fix would + be routing it to the scrolling chat transcript (which stays and + scrolls back) instead of shortening the timeout. +14. **Report Abuse.** Click it. Same shape as Urgent Assistance + (including the same legibility check), with its own retail text + ("...to submit an abuse report..."). 15. **Use Mouse Turning Settings.** Click it. Expect one retail chat line per changed value among: Camera Stiffness, Camera Adjustment, Mouse Sensitivity, Align To Slope, Invert Mouselook Axes, Turn to Face Camera — a FRESH character (never touched these settings) sees all six lines; clicking the button a second time in the same session sees zero lines (everything already at the macro's target). + **These six lines are byte-verified `RetailLogTextType.Magic` (0x07), + NOT the client-local refusal type** — expect LIGHT BLUE text in the + SCROLLING CHAT TRANSCRIPT (not the transient SpewBox overlay), with + a timestamp prefix, same as any other Magic-colored system line. If + you see bright red text in a transient 5-second overlay instead (the + ClientLocal/SpewBox look used by the mid-air refusal and UA/RA + above), that is the pre-fix-round bug — report it. **Known, registered gap (TS-74): acdream has no persistent "turn to face camera" mouse-turning MODE yet, so this button changes stored preferences and sends the wire bit, but you will not see the @@ -94,12 +120,34 @@ for this slice, not a bug. does not exist yet. Verify the CHAT LINES and that the button is clickable/does not crash; do not expect a camera-behavior change. +### Cross-session persistence (mechanism review S6) + +16. **Relog the SAME character** (log off through Exit Game, or + disconnect/reconnect, then log back in) after step 15 has already + shown all six lines once. Open the Options panel and click "Use + Mouse Turning Settings" again. Expect **zero** chat lines this time — + `UseMouseTurning` is an auto-save id (`SetSingleCharacterOption + 0x0005`), so the bit reached ACE on the first click and the fresh + post-relog `PlayerDescription` echoes it back as already-on. Seeing + the "Turn to Face Camera was changed..." line AGAIN after a clean + relog means the bit did not actually persist server-side — report it. +17. **Fully relaunch the client** (close the window, start a new + process) after step 15. Reopen the Options panel — no button click + needed. The five CLIENT-LOCAL preferences (Camera Stiffness, + Adjustment Speed, Mouse Sensitivity, Align To Slope, Invert Mouselook + Axes) persisted to `settings.json`'s `cameraTurning` section should + still read at the macro's target values from the PREVIOUS session — + click "Use Mouse Turning Settings" once more and confirm you again + see zero lines (proving the five client-local prefs, not just the + one server bit, survived the relaunch). + ### What to report - Any of the above NOT matching (wrong text, wrong tab default, window not draggable/resizable, close button not working, F11/toolbar not - opening the panel, opening Options not closing a sibling panel). -- Any exception/crash on any of the 15 steps. + opening the panel, opening Options not closing a sibling panel, toolbar + button not highlighting while the panel is open). +- Any exception/crash on any of the 17 steps. - Whether the confirmation dialog's exact wording looks retail-correct (it resolves from the DAT string table at runtime; a fallback English string only appears if that resolution fails, which would itself be diff --git a/src/AcDream.App/Composition/InteractionRetainedUiComposition.cs b/src/AcDream.App/Composition/InteractionRetainedUiComposition.cs index 17ffecd3..53e77251 100644 --- a/src/AcDream.App/Composition/InteractionRetainedUiComposition.cs +++ b/src/AcDream.App/Composition/InteractionRetainedUiComposition.cs @@ -800,13 +800,20 @@ internal sealed class RetailInteractionRetainedUiCompositionFactory d.Communication.AddText(text, RetailLogTextType.ClientLocal)), Options: new OptionsRuntimeBindings( CommandBus: () => late.Session.Commands, - // Same playerOnGround shape SessionPlayerComposition - // already computes for the diagnostic dumper (line - // ~322): "on the ground" is meaningless outside player - // mode, so the mid-air refusal never fires while flying/ - // spectating. - IsGrounded: () => d.PlayerMode.IsPlayerMode - && d.PlayerController.Controller is { IsAirborne: false }, + // Tri-state per gmGamePlayUI::UseTime @0x004EA3A0's exact + // branch structure (mechanism review S3 / blast review + // SHOULD-FIX 1, 2026-08-11 fix round): retail's airborne + // test only runs INSIDE `else if (smartbox->player)` — + // with no player object (not in player mode, or no live + // controller yet) neither the refusal nor the logoff + // itself ever fires, i.e. a SILENT no-op, not a refusal. + // The original two-way bool collapsed that third case + // into "not grounded", which fired the mid-air message + // outside player mode — the opposite of retail and the + // opposite of what the old comment here claimed. + IsGrounded: () => !d.PlayerMode.IsPlayerMode + ? (bool?)null + : d.PlayerController.Controller is { IsAirborne: false }, IsUseMouseTurningEnabled: () => CharacterOptionTable.TryGet( CharacterOptionId.UseMouseTurning, @@ -814,6 +821,16 @@ internal sealed class RetailInteractionRetainedUiCompositionFactory && (d.Character.Options.Options2 & entry.Mask) != 0u, DisplaySystemMessage: text => d.Communication.AddText(text, RetailLogTextType.ClientLocal), + // OP3 review-fix round (2026-08-11), MUST-FIX M1: BYTE- + // VERIFIED against the PDB-paired binary — all six + // gmConfigUI::SetMouseTurningDefaults chat-line sites + // push `6a 07` (type=7=Magic) immediately before the + // text-pointer push and the AddTextToScroll call + // (0x0049E972/E9E2/EA52/EAA4/EAF6/EB48). Magic routes to + // the scrolling chat transcript, not the 4-slot SpewBox + // ClientLocal uses. + DisplayMouseTurningMacroLine: text => + d.Communication.AddText(text, RetailLogTextType.Magic), LoadCameraTurning: d.Settings.LoadCameraTurning, SaveCameraTurning: d.Settings.SaveCameraTurning), StackSplitQuantity: d.StackSplitQuantity, diff --git a/src/AcDream.App/UI/Layout/OptionPageModel.cs b/src/AcDream.App/UI/Layout/OptionPageModel.cs index 01c28d2d..9c72c03a 100644 --- a/src/AcDream.App/UI/Layout/OptionPageModel.cs +++ b/src/AcDream.App/UI/Layout/OptionPageModel.cs @@ -31,6 +31,21 @@ public interface IOptionRow /// RestoreDefaultValue: m_current = m_default, /// then applies the default live. void RestoreDefaultValue(); + + /// + /// Wires this row's owning-page notify hook — retail's + /// UIOption::m_pOCH (option-change-handler) pointer, invoked by + /// UIOption::HandleDialogAndNotices @0x004EFB90's + /// m_pOCH->OnOptionChanged(this) call after a LIVE user edit + /// (a widget's own SetCurrentValue/Apply(1) path ONLY — + /// RestoreSavedValue/RestoreDefaultValue use retail's + /// Apply(0), which skips this per-row notify because the OWNING + /// VERB (/) + /// already calls once itself, + /// at its own tail). Called once by ; + /// mechanism review S2, 2026-08-11 fix round. + /// + void AttachPageNotify(Action notify); } /// @@ -44,6 +59,7 @@ public interface IOptionRow public sealed class BoolOptionRow : IOptionRow { private readonly Action? _apply; + private Action? _notifyPageOptionChanged; private bool _current; private bool _saved; private bool _default; @@ -77,14 +93,19 @@ public sealed class BoolOptionRow : IOptionRow /// /// Retail SetCurrentValue @0x00486970 — the LED-click entry point. /// Writes m_current and applies it live immediately; does NOT - /// touch (Apply is the only verb that commits). + /// touch (Apply is the only verb that commits). Also + /// notifies the owning page () — retail's + /// Apply(1)-only HandleDialogAndNotices path. /// public void SetCurrentValue(bool value) { _current = value; _apply?.Invoke(value); + _notifyPageOptionChanged?.Invoke(); } + public void AttachPageNotify(Action notify) => _notifyPageOptionChanged = notify; + public void SaveCurrentValue() => _saved = _current; public void RestoreSavedValue() @@ -134,13 +155,20 @@ public sealed class BoolOptionRow : IOptionRow /// /// /// -/// An empty page (zero registered rows — the Gameplay tab's own model, which -/// has no UIOption rows at all per research doc §6) makes every verb a -/// no-op and permanently false; -/// still fires on / (retail's -/// SaveCurrentValues flushes the blob regardless of whether THIS -/// page's own rows changed anything — the module's dirty flag is global, not -/// per-page). +/// An empty page (zero registered rows) makes every verb a no-op and +/// permanently false; when IS +/// wired, it still fires on / even +/// with zero rows (retail's SaveCurrentValues flushes the blob +/// regardless of whether THIS page's own rows changed anything — the +/// module's dirty flag is global, not per-page). This is a property of +/// the generic empty-page shape, not a description of the Gameplay tab. +/// gmGameplayOptionsUI (acclient.h:55857) derives from +/// UIElement_Field, not OptionPage/PlayerOptionPage at +/// all, so retail never calls SaveCurrentValues for it in the first +/// place — deliberately +/// constructs the Gameplay slot's instance with +/// left null so entering/leaving that tab +/// never flushes (mechanism review S1, 2026-08-11 fix round). /// /// public sealed class OptionPage @@ -154,17 +182,33 @@ public sealed class OptionPage /// ) — the seam a controller wires to the batched /// SaveOptions/blob-flush command. Never invoked by /// or (retail's Reset/Defaults - /// call Apply(0) per-row and re-run OnOptionChanged(0) + /// call Apply(0) per-row and reach /// directly — they never reach PlayerOptionPage::SaveCurrentValues, /// so they never flush). /// public Action? AfterApply { get; set; } + /// + /// Retail OptionPage::OnOptionChanged(0) — the ONLY thing that + /// enable-gates Apply/Reset (PlayerOptionPage::OnOptionChanged + /// @0x004F27D0: disabled when is false, enabled + /// otherwise; Defaults is NEVER gated — retail's override never fetches + /// its child id at all). Retail runs this as the LAST statement of all + /// three verbs (0x004F2C95 Apply, 0x004F2CE5 Defaults, + /// 0x004F2D4A Reset); a live user edit + /// ('s + /// SetCurrentValue-only path) also reaches it directly via + /// UIOption::HandleDialogAndNotices @0x004EFB90. OP4-6 bind + /// buttons to this seam; mechanism review S2, 2026-08-11 fix round. + /// + public Action? OnOptionChanged { get; set; } + /// Registers one row. Retail's OptionPage::RegisterOption /// @0x004F2E90, called from each page's InitOptions. public void Register(IOptionRow row) { ArgumentNullException.ThrowIfNull(row); + row.AttachPageNotify(() => OnOptionChanged?.Invoke()); _rows.Add(row); } @@ -173,17 +217,19 @@ public sealed class OptionPage public bool Changed => _rows.Any(static row => row.Changed); /// OptionPage::SaveCurrentValues @0x004F2C60 — Apply: - /// commits every row's baseline unconditionally, then flushes via - /// . + /// commits every row's baseline unconditionally, flushes via + /// , then re-evaluates . public void Apply() { foreach (IOptionRow row in _rows) row.SaveCurrentValue(); AfterApply?.Invoke(); + OnOptionChanged?.Invoke(); } /// OptionPage::RestoreSavedValues @0x004F2D00 — Reset: - /// reverts only the rows that are currently . + /// reverts only the rows that are currently , + /// then re-evaluates . /// Snapshotted before iterating so a row's own revert (which flips /// back to false) cannot skip a later /// row. @@ -191,15 +237,17 @@ public sealed class OptionPage { foreach (IOptionRow row in _rows.Where(static row => row.Changed).ToArray()) row.RestoreSavedValue(); + OnOptionChanged?.Invoke(); } /// OptionPage::RestoreDefaultValues @0x004F2CB0 — /// Defaults: restores every row unconditionally, live, without - /// committing. + /// committing, then re-evaluates . public void Defaults() { foreach (IOptionRow row in _rows) row.RestoreDefaultValue(); + OnOptionChanged?.Invoke(); } /// PlayerOptionPage::OnVisibilityChanged(true) — the page diff --git a/src/AcDream.App/UI/Layout/OptionsPanelController.cs b/src/AcDream.App/UI/Layout/OptionsPanelController.cs index 3c6fed31..62effb6e 100644 --- a/src/AcDream.App/UI/Layout/OptionsPanelController.cs +++ b/src/AcDream.App/UI/Layout/OptionsPanelController.cs @@ -122,7 +122,21 @@ public sealed class OptionsPanelController : IRetainedPanelController private OptionsPanelController(UiTabPanel tabPanel, Action? afterApply) { _tabPanel = tabPanel; - foreach (uint pageId in new[] { GameplayPageId, CharacterPageId, ChatPageId, ConfigPageId }) + + // Mechanism review S1 (2026-08-11 fix round): gmGameplayOptionsUI + // (acclient.h:55857) derives from UIElement_Field, NOT + // OptionPage/PlayerOptionPage at all — unlike the other three tabs + // (gmCharacterSettingsUI/gmChatOptionsUI/gmConfigUI, all + // : PlayerOptionPage). Retail never calls SaveCurrentValues for the + // Gameplay page, so it never flushes the batched blob on show/hide. + // The model still needs an (empty) page instance so + // OnActivePageChanged's TryGetValue lookup and OnHidden/OnShown + // (Reset/Apply over zero rows, both harmless no-ops) keep working + // uniformly across all four tabs — only AfterApply is deliberately + // left null here, so entering/leaving Gameplay never publishes + // SaveCharacterOptionsRuntimeCmd. + _pages.Add(GameplayPageId, new OptionPage { AfterApply = null }); + foreach (uint pageId in new[] { CharacterPageId, ChatPageId, ConfigPageId }) { var page = new OptionPage { AfterApply = afterApply }; _pages.Add(pageId, page); @@ -159,6 +173,10 @@ public sealed class OptionsPanelController : IRetainedPanelController if (layout.FindElement(CloseButtonId) is UiButton close) close.OnClick = callbacks.Toggle; + else + Console.WriteLine( + $"[D.2b] OptionsPanelController: close button 0x{CloseButtonId:X8} " + + "not found in the built layout — its handler was not wired."); BindButton(layout, ExitToCharacterSelectionId, callbacks.RequestExitToCharacterSelection); // ConfigureKeyboardId: INERT this slice — authored, clickable, no diff --git a/src/AcDream.App/UI/RetailUiRuntime.cs b/src/AcDream.App/UI/RetailUiRuntime.cs index 806e98c2..817ef468 100644 --- a/src/AcDream.App/UI/RetailUiRuntime.cs +++ b/src/AcDream.App/UI/RetailUiRuntime.cs @@ -156,10 +156,14 @@ public sealed record CharacterRuntimeBindings(CharacterSheetProvider Provider); /// already uses, since the live session's bus is not yet constructed at the /// point this binding record is built (composition order). /// Retail's mid-air logout-refusal check -/// (transient_state & 1) — mirrors the SAME -/// d.PlayerMode.IsPlayerMode && d.PlayerController.Controller is -/// { IsAirborne: false } pattern already used for -/// InteractionRetainedUiComposition's own playerOnGround read. +/// (transient_state & 1), tri-state per +/// gmGamePlayUI::UseTime @0x004EA3A0's exact branch structure: +/// else if (smartbox->player) { if (!grounded) refuse; else proceed; } +/// — the airborne test is reached ONLY when a player object exists at all. +/// null means "no player" (acdream: !d.PlayerMode.IsPlayerMode || +/// d.PlayerController.Controller is null), which retail's own structure +/// reaches neither branch for — no refusal, no proceed (mechanism review S3 +/// / blast review SHOULD-FIX 1, 2026-08-11). /// Live read of /// PlayerOption.UseMouseTurning (CharacterOptions2 bit /// 0x00400000) from the canonical RuntimeCharacterOptionsState — @@ -168,12 +172,28 @@ public sealed record CharacterRuntimeBindings(CharacterSheetProvider Provider); /// (RetailLogTextType.ClientLocal, SpewBox-only) — the SAME /// text => d.Communication.AddText(text, RetailLogTextType.ClientLocal) /// delegate shape / -/// already use. +/// already use. Used for the mid-air +/// refusal (byte-verified ClientLocal 0x1A) and the Urgent +/// Assistance/Report Abuse short-circuit (AD-75) — NOT the mouse-turning +/// macro's own six lines, which use +/// instead. +/// The mouse-turning macro's own +/// six retail chat lines — routed at RetailLogTextType.Magic (0x07), +/// BYTE-VERIFIED at all six ClientSystem::AddTextToScroll call sites +/// inside gmConfigUI::SetMouseTurningDefaults (raw bytes: 6a 07 +/// immediately before the text-pointer push and the call, at every one of +/// 0x0049E972/0x0049E9E2/0x0049EA52/0x0049EAA4/ +/// 0x0049EAF6/0x0049EB48, byte-verified against the PDB-paired +/// binary 2026-08-11 — the OP3 review-fix round). Magic routes through +/// RuntimeCommunicationState.AddText to the scrolling chat transcript +/// (light blue, timestamped, logged), NOT the 4-slot SpewBox +/// 's ClientLocal uses. public sealed record OptionsRuntimeBindings( Func CommandBus, - Func IsGrounded, + Func IsGrounded, Func IsUseMouseTurningEnabled, Action DisplaySystemMessage, + Action DisplayMouseTurningMacroLine, Func LoadCameraTurning, Action SaveCameraTurning); @@ -1855,11 +1875,20 @@ public sealed class RetailUiRuntime : IDisposable /// Selection button (element 0x10000203) — D6's "behaves as Exit /// Game" adaptation (register row, same commit), PLUS retail's /// confirmation dialog and mid-air refusal, which DO port exactly. - /// gmGamePlayUI::UseTime @0x004EA3A0's drain: on confirmation - /// accept, refuse with - /// via the interface-text seam while airborne - /// (transient_state & 1); otherwise proceed exactly like Exit - /// Game. + /// gmGamePlayUI::UseTime @0x004EA3A0's drain, its EXACT three-way + /// branch (review-fix round, 2026-08-11 — the original port collapsed + /// this to a two-way `if/else` that fired the refusal outside player + /// mode too): + /// + /// if (m_bLogoutConfirmed) { + /// if (m_shouldQuitOnLogout) { QueueUIMode(0x10000009); } + /// else if (smartbox->player) { + /// if (!grounded) refuse "Cannot log off while in mid-air."; + /// else LogOffCharacter(); + /// } + /// // no player at all: neither branch runs — SILENT no-op. + /// } + /// /// private void RequestExitToCharacterSelection() { @@ -1867,13 +1896,20 @@ public sealed class RetailUiRuntime : IDisposable { if (!accepted) return; - if (_bindings.Options.IsGrounded()) + switch (_bindings.Options.IsGrounded()) { - _bindings.Indicators.EndCharacterSession(); - } - else - { - _bindings.Options.DisplaySystemMessage(ClientTextRefusals.CantLogOffMidAir); + case true: + _bindings.Indicators.EndCharacterSession(); + break; + case false: + _bindings.Options.DisplaySystemMessage(ClientTextRefusals.CantLogOffMidAir); + break; + case null: + // Retail's `else if (smartbox->player)` gate: outside + // player mode (or with no live controller) there is no + // player object for UseTime to test at all, so neither + // the airborne refusal nor the logoff itself ever runs. + break; } }); } @@ -1883,7 +1919,9 @@ public sealed class RetailUiRuntime : IDisposable /// button (element 0x100005CC) — the pure /// orchestrated against live /// bindings: load current values, compute the macro, persist + apply - /// live, send the wire bit if it changed, emit every retail chat line, + /// live, send the wire bit if it changed, emit every retail chat line + /// (byte-verified Magic 0x07, review-fix round 2026-08-11 — see + /// ), /// then commit the (currently empty) Config page exactly like retail's /// own trailing SaveCurrentValues() /// (gmConfigUI::SetMouseTurningDefaults @0x0049EB57). @@ -1905,7 +1943,7 @@ public sealed class RetailUiRuntime : IDisposable } foreach (string line in result.ChatLines) - _bindings.Options.DisplaySystemMessage(line); + _bindings.Options.DisplayMouseTurningMacroLine(line); OptionsPanelController?.ConfigPage.Apply(); } @@ -1967,17 +2005,27 @@ public sealed class RetailUiRuntime : IDisposable Left = 150f, Top = 80f, Visible = false, - ResizeX = true, + // Blast review SHOULD-FIX 2 (2026-08-11): match every OTHER + // gmPanelUI sibling sharing RetailPanelUiController's ONE + // main-panel geometry rectangle (Character/Inventory/ + // Spellbook/Effects/the four indicator-detail panels) — + // ResizeX=false, bottom-edge-only resize, no invented + // Min/MaxWidth/Height. Options previously authored the ONLY + // divergent policy in that shared group (ResizeX=true, + // all-four-edge resize, literal 300x200 minimums); since + // every sibling forces `width = frame.Width` on the shared + // rect (RetailWindowManager.ResizeTo), a user-widened + // Options window silently reverted to 300 the next time ANY + // sibling was shown — this makes the whole cohort consistent + // again instead of one outlier with its own rules. + ResizeX = false, ResizeY = true, - ResizableEdges = ResizeEdges.Left | ResizeEdges.Right - | ResizeEdges.Top | ResizeEdges.Bottom, + ResizableEdges = ResizeEdges.Bottom, ConstrainDragToParent = true, ConstrainResizeToParent = true, ContentAnchors = AnchorEdges.Left | AnchorEdges.Top | AnchorEdges.Right | AnchorEdges.Bottom, ContentClickThrough = false, - MinWidth = 300f, - MinHeight = 200f, DrawChromeCenter = !AuthorsFullPanelCenter(rootInfo), Controller = controller, }); diff --git a/tests/AcDream.App.Tests/UI/Layout/OP2ReworkBlastRadiusConformanceTests.cs b/tests/AcDream.App.Tests/UI/Layout/OP2ReworkBlastRadiusConformanceTests.cs index c35e88ec..35ee0b73 100644 --- a/tests/AcDream.App.Tests/UI/Layout/OP2ReworkBlastRadiusConformanceTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/OP2ReworkBlastRadiusConformanceTests.cs @@ -69,6 +69,29 @@ public class OP2ReworkBlastRadiusConformanceTests Assert.True(host.ClickThrough); } + /// + /// Blast review SHOULD-FIX 3 / NOTE 2 (2026-08-11 fix round): AD-73's + /// dormancy contract is that a Type-8 host with an authored tab table + /// performs NO switching until a controller calls + /// — but nothing had pinned + /// ITSELF against a dormant + /// host (as opposed to BehaviorActive/ActivePageElementId, + /// which the test above already covers). SwitchTo is public and + /// its own doc invites a future direct call, so this guards the CURRENT + /// dormant-by-default contract, not SwitchTo itself. + /// + [Fact] + public void Vendor_TabHost_DormantHost_NeverRaisesActivePageChanged() + { + var layout = FixtureLoader.LoadVendor(); + var host = Assert.IsType(layout.FindElement(0x100000B8u)); + int raised = 0; + host.ActivePageChanged += (_, _) => raised++; + + Assert.False(host.BehaviorActive); + Assert.Equal(0, raised); + } + // ── Type 8 — panel roots with their own controller-owned switching ────── /// diff --git a/tests/AcDream.App.Tests/UI/Layout/OptionPageModelTests.cs b/tests/AcDream.App.Tests/UI/Layout/OptionPageModelTests.cs index 00740259..af96c065 100644 --- a/tests/AcDream.App.Tests/UI/Layout/OptionPageModelTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/OptionPageModelTests.cs @@ -4,16 +4,28 @@ namespace AcDream.App.Tests.UI.Layout; /// /// Pure logic tests for / — -/// no DAT, no widgets, no runtime. Campaign OP slice OP3 ships the model -/// against an EMPTY page (the Gameplay tab has no options at all — research -/// doc 2026-08-10-options-panel-structure.md §6); this file also -/// exercises a synthetic 2-option page to prove Apply/Reset/Defaults' -/// per-row semantics before OP4-6 wire real DAT-backed rows into the same -/// model. +/// no DAT, no widgets, no runtime. Exercises a SYNTHETIC empty +/// PlayerOptionPage-shaped page (zero registered rows — a shape +/// Character/Chat/Config's pages briefly hold before OP4-6 register rows into +/// them) to pin the generic model-level "empty page" property, and a +/// synthetic 2-option page to prove Apply/Reset/Defaults' per-row semantics +/// before OP4-6 wire real DAT-backed rows into the same model. +/// +/// +/// NEITHER shape models the Gameplay tab. gmGameplayOptionsUI +/// (acclient.h:55857) derives from UIElement_Field, not +/// OptionPage/PlayerOptionPage at all — research doc +/// 2026-08-10-options-panel-structure.md §6, mechanism review S1 +/// (2026-08-11 fix round). pins +/// the Gameplay-specific consequence (its instance +/// is constructed with deliberately left +/// null, so it never flushes). +/// /// public sealed class OptionPageModelTests { - // ── Empty page (the Gameplay tab's own shape) ─────────────────────────── + // ── Empty page (a generic zero-row PlayerOptionPage-shaped page — NOT + // modeling Gameplay, which is not an OptionPage at all) ───────────── [Fact] public void EmptyPage_ChangedIsAlwaysFalse() @@ -45,11 +57,15 @@ public sealed class OptionPageModelTests } [Fact] - public void EmptyPage_Apply_StillInvokesAfterApply() + public void EmptyPlayerOptionPageShapedPage_WithAfterApplyWired_Apply_StillFlushes() { // Retail's PlayerOptionPage::SaveCurrentValues flushes the batched // module regardless of whether THIS page's own rows changed - // anything — the module's dirty flag is global, not per-page. + // anything — the module's dirty flag is global, not per-page. This + // is a property of a page that IS AfterApply-wired (Character/Chat/ + // Config, briefly, before OP4-6 register their rows) — NOT of the + // Gameplay tab, whose own OptionPage instance is constructed with + // AfterApply left null (see OptionsPanelControllerTests). var page = new OptionPage(); int flushCount = 0; page.AfterApply = () => flushCount++; @@ -60,7 +76,7 @@ public sealed class OptionPageModelTests } [Fact] - public void EmptyPage_OnShown_InvokesAfterApply() + public void EmptyPlayerOptionPageShapedPage_WithAfterApplyWired_OnShown_StillFlushes() { var page = new OptionPage(); int flushCount = 0; @@ -71,6 +87,19 @@ public sealed class OptionPageModelTests Assert.Equal(1, flushCount); } + [Fact] + public void EmptyPage_WithNoAfterApplyWired_Apply_NeverFlushes() + { + // The Gameplay tab's own shape: AfterApply is null by construction + // (OptionsPanelController), so Apply (fired by the initial default- + // tab activation and every later tab entry) never publishes a flush. + var page = new OptionPage { AfterApply = null }; + + Exception? thrown = Record.Exception(page.Apply); + + Assert.Null(thrown); + } + [Fact] public void EmptyPage_ResetAndDefaults_DoNotInvokeAfterApply() { @@ -231,4 +260,116 @@ public sealed class OptionPageModelTests Assert.False(page.Changed); Assert.False(a.Saved != a.Current); } + + // ── OnOptionChanged seam (mechanism review S2, 2026-08-11 fix round) ──── + // PlayerOptionPage::OnOptionChanged @0x004F27D0: the sole Apply/Reset + // enable-gate, run as the LAST statement of all three verbs + // (0x004F2C95/0x004F2CE5/0x004F2D4A) plus once per live LED click via + // UIOption::HandleDialogAndNotices @0x004EFB90. + + [Fact] + public void OnOptionChanged_FiresAsLastStepOf_Apply() + { + var (page, a, _) = MakeTwoOptionPage(); + a.SetCurrentValue(false); + var order = new List(); + page.AfterApply = () => order.Add("afterApply"); + page.OnOptionChanged = () => order.Add("onOptionChanged"); + + page.Apply(); + + Assert.Equal(["afterApply", "onOptionChanged"], order); + } + + [Fact] + public void OnOptionChanged_FiresOnReset_EvenWithNoAfterApply() + { + var (page, a, _) = MakeTwoOptionPage(); + a.SetCurrentValue(false); + int notifyCount = 0; + page.OnOptionChanged = () => notifyCount++; + + page.Reset(); + + Assert.Equal(1, notifyCount); + } + + [Fact] + public void OnOptionChanged_FiresOnDefaults() + { + var (page, _, _) = MakeTwoOptionPage(); + int notifyCount = 0; + page.OnOptionChanged = () => notifyCount++; + + page.Defaults(); + + Assert.Equal(1, notifyCount); + } + + [Fact] + public void OnOptionChanged_FiresOnEmptyPage_ForEveryVerb() + { + // Defaults is NEVER gated by retail's own OnOptionChanged override + // (it never fetches the Defaults child id at all), but the page- + // level notify still fires from EVERY verb regardless of row count + // — the model doesn't special-case "which button retail happens to + // gate" here, only the seam itself. + var page = new OptionPage(); + int notifyCount = 0; + page.OnOptionChanged = () => notifyCount++; + + page.Apply(); + page.Reset(); + page.Defaults(); + + Assert.Equal(3, notifyCount); + } + + [Fact] + public void BoolOptionRow_SetCurrentValue_NotifiesAttachedPage() + { + // Retail's Apply(1)-only HandleDialogAndNotices path: a live LED + // click reaches the page's OnOptionChanged directly, distinct from + // (and in addition to) the per-row apply callback. + var row = new BoolOptionRow(initial: false, defaultValue: false); + int notifyCount = 0; + row.AttachPageNotify(() => notifyCount++); + + row.SetCurrentValue(true); + + Assert.Equal(1, notifyCount); + } + + [Fact] + public void BoolOptionRow_RestoreSavedValueAndRestoreDefaultValue_DoNotNotifyAttachedPage() + { + // Apply(0) paths — retail's Reset/Defaults call these directly and + // notify the page ONCE themselves at their own verb tail; a per-row + // notify here would double-fire (or fire once per reverted row + // instead of once per verb call). + var row = new BoolOptionRow(initial: false, defaultValue: true); + int notifyCount = 0; + row.AttachPageNotify(() => notifyCount++); + row.SetCurrentValue(true); + notifyCount = 0; // drop the SetCurrentValue notify above + + row.RestoreSavedValue(); + row.RestoreDefaultValue(); + + Assert.Equal(0, notifyCount); + } + + [Fact] + public void Register_AttachesPageNotify_SoSubsequentLiveEditsNotifyThePage() + { + var page = new OptionPage(); + var row = new BoolOptionRow(initial: false, defaultValue: false); + int notifyCount = 0; + page.OnOptionChanged = () => notifyCount++; + + page.Register(row); + row.SetCurrentValue(true); + + Assert.Equal(1, notifyCount); + } } diff --git a/tests/AcDream.App.Tests/UI/Layout/OptionsPanelControllerTests.cs b/tests/AcDream.App.Tests/UI/Layout/OptionsPanelControllerTests.cs index 78499254..02ca82c8 100644 --- a/tests/AcDream.App.Tests/UI/Layout/OptionsPanelControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/OptionsPanelControllerTests.cs @@ -60,8 +60,14 @@ public sealed class OptionsPanelControllerTests } [Fact] - public void ActivateTabs_SelectsGameplayAsDefault_AndAppliesItsPage() + public void ActivateTabs_SelectsGameplayAsDefault_ButNeverFlushesIt() { + // Mechanism review S1 (2026-08-11 fix round): gmGameplayOptionsUI is + // NOT an OptionPage in retail (acclient.h:55857) — its own OptionPage + // instance is constructed with AfterApply deliberately left null + // (OptionsPanelController's constructor), regardless of what the + // controller's OWN AfterApply callback is, so the initial default- + // tab activation (OnShown -> Apply) never publishes a flush. ImportedLayout layout = FixtureLoader.LoadOptionsPanelHost(); var calls = new List(); int gameplayFlushCount = 0; @@ -73,12 +79,11 @@ public sealed class OptionsPanelControllerTests Assert.True(controller.TabPanel.BehaviorActive); Assert.Equal(0x10000212u, controller.TabPanel.ActivePageElementId); // Gameplay slot - // OnShown() fired for the initial default tab -> Apply() -> AfterApply. - Assert.Equal(1, gameplayFlushCount); + Assert.Equal(0, gameplayFlushCount); } [Fact] - public void TabSwitch_RevertsLeavingPage_AndAppliesEnteringPage() + public void TabSwitch_RevertsLeavingGameplayPage_AndAppliesEnteringCharacterPage() { ImportedLayout layout = FixtureLoader.LoadOptionsPanelHost(); var calls = new List(); @@ -86,12 +91,13 @@ public sealed class OptionsPanelControllerTests OptionsPanelController controller = OptionsPanelController.Bind( layout, MakeCallbacks(calls) with { AfterApply = () => flushes.Add("flush") })!; controller.ActivateTabs(); - flushes.Clear(); // drop the initial-activation flush + flushes.Clear(); // drop the initial-activation flush (Gameplay never flushes anyway) controller.TabPanel.SwitchTo(0x10000211u); // Character page slot Assert.Equal(0x10000211u, controller.TabPanel.ActivePageElementId); - // Both OnHidden (Gameplay, Reset — no flush) and OnShown (Character, Apply — flush). + // OnHidden (Gameplay, Reset — no flush regardless) and OnShown + // (Character, a REAL AfterApply-wired page — Apply flushes). Assert.Equal(["flush"], flushes); } @@ -116,12 +122,17 @@ public sealed class OptionsPanelControllerTests [Fact] public void WholeWindowShow_AppliesCurrentlyActivePage() { + // Switches off the Gameplay default onto Character FIRST — Gameplay + // never flushes (S1), so testing "OnShown applies the currently + // active page" needs a REAL AfterApply-wired page active, exactly + // like a returning user re-opening the window on a non-default tab. ImportedLayout layout = FixtureLoader.LoadOptionsPanelHost(); var calls = new List(); int flushCount = 0; OptionsPanelController controller = OptionsPanelController.Bind( layout, MakeCallbacks(calls) with { AfterApply = () => flushCount++ })!; controller.ActivateTabs(); + controller.TabPanel.SwitchTo(0x10000211u); // Character page slot flushCount = 0; controller.OnShown(); @@ -129,6 +140,24 @@ public sealed class OptionsPanelControllerTests Assert.Equal(1, flushCount); } + [Fact] + public void GameplayPage_OnShownAndOnHidden_NeverFlush_EvenWhenControllerAfterApplyIsWired() + { + // Direct pin of S1's fix: cycling Gameplay's own OnShown/OnHidden + // (the page-model hooks OnActivePageChanged drives) never publishes + // a flush, independent of TabSwitch/ActivateTabs framing. + ImportedLayout layout = FixtureLoader.LoadOptionsPanelHost(); + var calls = new List(); + int flushCount = 0; + OptionsPanelController controller = OptionsPanelController.Bind( + layout, MakeCallbacks(calls) with { AfterApply = () => flushCount++ })!; + + controller.GameplayPage.OnShown(); + controller.GameplayPage.OnHidden(); + + Assert.Equal(0, flushCount); + } + // ── Close button ───────────────────────────────────────────────────────── [Fact] diff --git a/tests/AcDream.App.Tests/UI/Layout/RetailDialogFactoryTests.cs b/tests/AcDream.App.Tests/UI/Layout/RetailDialogFactoryTests.cs index 16f2ce4d..7084b093 100644 --- a/tests/AcDream.App.Tests/UI/Layout/RetailDialogFactoryTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/RetailDialogFactoryTests.cs @@ -81,6 +81,35 @@ public sealed class RetailDialogFactoryTests Assert.False(factory.IsOpen); } + /// + /// Blast review SHOULD-FIX 3 / NOTE 3 (2026-08-11 fix round): the Options + /// panel's Exit-to-Character-Selection confirmation goes through + /// RetailUiRuntime.ShowConfirmation -> MakeConfirmation(message, + /// callback) with NO explicit queueKey argument. Nothing pinned + /// that the omitted-key overload actually shares + /// with every other + /// confirmation in the codebase, rather than silently defaulting to a + /// DIFFERENT key that would let two confirmations sit active + /// simultaneously instead of queuing. + /// + [Fact] + public void MakeConfirmation_OmittedQueueKey_SharesDefaultQueueKey() + { + var root = new UiRoot { Width = 800f, Height = 600f }; + var layouts = new List(); + var factory = CreateFactory(root, layouts); + + factory.MakeConfirmation("uses the omitted-queueKey overload"); + factory.MakeConfirmation("explicit DefaultQueueKey", queueKey: RetailDialogFactory.DefaultQueueKey); + + // If the omitted-queueKey call had used a DIFFERENT key, both would + // be ACTIVE simultaneously (distinct queue groups run independently + // — QueueGroupsAndNonQueuedDialogsCanBeActiveTogether below) instead + // of the second queuing FIFO behind the first. + Assert.Equal(1, factory.ActiveCount); + Assert.Equal(1, factory.PendingCount); + } + [Fact] public void QueueGroupsAndNonQueuedDialogsCanBeActiveTogether() { diff --git a/tests/AcDream.App.Tests/UI/Layout/RetailPanelUiControllerTests.cs b/tests/AcDream.App.Tests/UI/Layout/RetailPanelUiControllerTests.cs index 1ea87958..e5fbbf95 100644 --- a/tests/AcDream.App.Tests/UI/Layout/RetailPanelUiControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/RetailPanelUiControllerTests.cs @@ -117,6 +117,37 @@ public sealed class RetailPanelUiControllerTests Assert.Equal((310f, 540f), (spellbook.Width, spellbook.Height)); } + /// + /// Blast review SHOULD-FIX 3 (2026-08-11 fix round): OP3's headline claim + /// is that registering the Options panel through + /// gives it + /// retail's "one active gmPanelUI child" mutual exclusion against every + /// OTHER already-shipped sibling "for free" — nothing pinned that against + /// a REAL sibling (as opposed to the synthetic ids the other tests in + /// this file use). Uses the actual + /// (10) and (11) ids. + /// + [Fact] + public void Options_AndCharacter_ShareExclusiveMainPanelLifecycle() + { + var root = new UiRoot { Width = 1280f, Height = 720f }; + RetailWindowHandle character = Mount(root, WindowNames.Character, 540f, 18f); + RetailWindowHandle options = Mount(root, WindowNames.Options, 150f, 80f); + using var controller = Create(root); + controller.RegisterMainPanel(RetailPanelCatalog.Character, WindowNames.Character, character); + controller.RegisterMainPanel(RetailPanelCatalog.Options, WindowNames.Options, options); + + controller.SetPanelVisibility(RetailPanelCatalog.Character, visible: true); + Assert.True(character.IsVisible); + Assert.False(options.IsVisible); + Assert.Equal(RetailPanelCatalog.Character, controller.ActivePanelId); + + controller.SetPanelVisibility(RetailPanelCatalog.Options, visible: true); + Assert.False(character.IsVisible); + Assert.True(options.IsVisible); + Assert.Equal(RetailPanelCatalog.Options, controller.ActivePanelId); + } + [Fact] public void IndicatorDetailPanel_SharesPlacementAndRestoresPreviousPanel() {