From bc43fb1d1d5cf26076c3727e935e71f56dfba8a4 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 11 Aug 2026 05:30:26 +0200 Subject: [PATCH] =?UTF-8?q?fix(ui,runtime):=20OP4=20review=20fixes=20?= =?UTF-8?q?=E2=80=94=20live=20re-seed,=20enable-gating,=20Combat=20panel?= =?UTF-8?q?=20re-point,=20universal=20timestamps?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both OP4 reviews converged on one headline bug (Character-tab rows never re-read live server truth after their pre-login constructor-word seed) plus overlapping MUST-FIXes. All ten converged/consolidated findings land here: MUST-FIX: - BoolOptionRow.SaveCurrentValue now re-reads its live binding (retail's GetValue()-into-SaveCurrentValue) on every OnShown — panel open, tab switch in, initial activation — instead of trusting the pre-login constructor word it was built with. Reset/tab-switch can now only restore values that were actually live at the last show. LockUI's host.Root.UiLocked one-shot mount seed now also converges on every PlayerDescription via the existing OnCharacterOptionsChanged hook. - Apply/Reset are wired to OptionPage.OnOptionChanged in production (Ghosted when nothing changed, Normal when dirty, run once at bind so both start disabled per retail's PostInit); Defaults stays ungated. - The Combat panel's three LEDs (Repeat Attacks/Auto Target/Keep in View) now read/write the same RuntimeCharacterOptionsState seam the Character tab uses instead of a disconnected client-local GameplaySettings copy — closes the "two writable copies" divergence. The three now-orphaned GameplaySettings fields and RuntimeSettingsController's mirror properties/SetCombatGameplay are deleted outright; the headless host's hardcoded AutoRepeatAttack/AutoTarget now read the live option bit. - RuntimeSettingsController.SetUiLocked's convergence guard now compares against the last value actually applied to the runtime target instead of the persisted GameplaySettings.LockUI snapshot, which could already match a server-derived request without ever having been pushed. SHOULD-FIX: - DisplayTimeStamps now prefixes every chat producer (ChatLog.Append is the one seam all of them funnel through), not just AddText's own callers — heard speech, emotes, Turbine channels, and combat text were previously missed. The prefix format escapes its colons and forces InvariantCulture instead of the culture-dependent TimeSeparator placeholder. - sky.frag now honors uFogParams.w (fog mode) like the mesh/terrain shaders, so Disable Distance Fog stops the sky dome's horizon band from blending toward fog color too. - Corrected the "byte-verified" overclaim on the timestamp format string doc comment (BN-sourced, wire doc U6) and the AP-194 anchor-column class-name typo; the RunAsDefaultMovement doc comments now cite retail's actual acclient.h enumerator name. - Added: DispatcherMovementInputSource's option x modifier truth table (incl. || AutoRunActive with the option off), the per-page Apply/Reset enable-gate tests, a real checkbox.OnClick/ToggleBehavior-driven click test, and hash-pins for the six header string keys. - Gate script step 8 corrected for the logout-flush false-failure (closing the panel before relogging is load-bearing); a new step documents the enable-gate sequence and the Combat-panel/Character-tab cross-check. Register: AP-196 (the Group-C default-source change + GameplaySettings retirement) and AP-197 (the ignored per-character timestamp format override) filed in this commit. Full Release suite: 13,044 passed / 4 skipped / 0 failed (was 13,008/4/0; net +36 tests from new coverage and legitimate assertion updates from the GameplaySettings retirement). Co-Authored-By: Claude Fable 5 --- .../retail-divergence-register.md | 6 +- .../2026-08-11-campaign-op-test-script.md | 42 ++++- .../Combat/LiveCombatAttackOperations.cs | 45 +++-- .../InteractionRetainedUiComposition.cs | 4 +- .../Input/DispatcherMovementInputSource.cs | 4 +- .../Net/LiveSessionRuntimeFactory.cs | 19 ++- src/AcDream.App/Rendering/GameWindow.cs | 6 +- src/AcDream.App/Rendering/Shaders/sky.frag | 11 +- .../Shaders/spv/shaders.manifest.json | 2 +- .../Rendering/Shaders/spv/sky.frag.spv | Bin 3220 -> 3364 bytes .../Settings/RuntimeSettingsController.cs | 57 ++----- .../Layout/CharacterOptionsPageController.cs | 9 +- .../UI/Layout/CombatUiController.cs | 55 +++--- src/AcDream.App/UI/Layout/OptionPageModel.cs | 51 +++++- .../UI/Layout/OptionsPanelController.cs | 39 ++++- src/AcDream.App/UI/RetailUiRuntime.cs | 18 +- src/AcDream.Core/Chat/ChatLog.cs | 48 ++++++ .../Hosting/HeadlessGameplayOperations.cs | 14 +- .../Gameplay/RuntimeCommunicationState.cs | 45 +++-- .../RuntimeLocalPlayerMovementState.cs | 7 +- .../Panels/Settings/GameplaySettings.cs | 16 +- .../Panels/Settings/SettingsPanel.cs | 20 +-- .../Panels/Settings/SettingsStore.cs | 6 - ...haracterOptionCombatSettingsSourceTests.cs | 12 +- .../Combat/CombatCameraTargetSourceTests.cs | 15 +- .../DispatcherMovementInputSourceTests.cs | 52 ++++++ .../RuntimeSettingsControllerTests.cs | 61 ++++--- .../CharacterOptionsPageControllerTests.cs | 160 ++++++++++++++++++ .../UI/Layout/CombatUiControllerTests.cs | 41 +++-- .../UI/Layout/OptionsPanelControllerTests.cs | 107 ++++++++++++ tests/AcDream.Core.Tests/Chat/ChatLogTests.cs | 81 +++++++++ .../RuntimeCommunicationStateTests.cs | 50 +++++- .../Panels/Settings/GameplaySettingsTests.cs | 8 +- .../Panels/Settings/SettingsPanelTests.cs | 13 +- .../Panels/Settings/SettingsStoreTests.cs | 3 +- .../Panels/Settings/SettingsVMTests.cs | 16 +- 36 files changed, 923 insertions(+), 220 deletions(-) diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index b65e7efe..2e54d388 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -175,7 +175,7 @@ readiness/requeue adaptation. See --- -## 3. Documented approximation (AP) — 136 active rows (AP-195 filed 2026-08-11 at the Campaign OP OP2 re-review closure — `UIOption_CheckboxBitfield64::Refresh`'s ALL-set LED media swap and the `ResizeTo`/`CalculatePaperSize` self-sizing tail are not ported, and the block's row stacking is a second, divergent implementation of the ListBox mechanism, all due at OP5; AP-194 filed 2026-08-10 at Campaign OP slice OP1 — the GetDefaultOptionValue vs constructor-default disagreement for ConfirmVolatileRareUse/ShowHelm/ShowCloak (see the row below); AP-193 filed 2026-08-10 at Campaign OP slice OP1 — the 0x34 HearPKDeathMessages id/mask mapping is ACE-sourced (see the row below); AP-192 filed 2026-08-10 at the Campaign CH round-5 polish (S2) — authored outline `0x21`/`0x22` now reaches every text-bearing widget, but only at the element's effective-default state; per-STATE outline switching (dialog/character/combat buttons author `0x21` in state `0x3` only) is not ported; AP-191 filed 2026-08-10 at Campaign CH round 4 items 1+2 — the chat transcript's missing tag-colour (`0x1D`, green) and tag-font (`0x1C`) are deferred, needing a per-run tag concept `UiText.Line` does not have yet; AP-184 RETIRED 2026-08-10 at Campaign CH round 4 — the three PARTIAL `/help` group topics (channels/chatting/commands) are now COMPLETE verbatim listings, `ClientCommunicationSystem::HelpStupidChannelHack @0x0056f290` fully decoded (the "vftable slot" operands are the same pooled/mislabeled-data artifact as AP-186's own precedent, not real vtable dispatch — reading the function's own disassembly for the `push imm32` preceding each constructor call resolves them), closing ISSUES.md #364 (full retirement note later in this same list, at its own "AP-184 RETIRED 2026-08-10 at Campaign CH round 4, closing ISSUES.md #364 — filed 2026-08-09..." entry); AP-113 RETIRED 2026-08-10 at the consolidated-review round, SHOULD-FIX 3/1 byproduct — DoLifestone's own bad-args refusal text is now byte-recovered, see its retirement note below; AP-183 and AP-186 RETIRED 2026-08-10 by issue #363's interface-text seam — see their retirement notes below; AP-190 filed 2026-08-10 at Campaign CH slice CH6c — window opacity now fades every RetailWindowManager window on retail's focus-driven Default/Active mechanism, not just ChatInterface-derived ones, and ships gmMainChatUI's 1.0/1.0 default as the ONE shared default across every registered window (fixed from the original 0.5/1.0 base-ChatInterface value, per the row's own REWORDED (2)) instead of applying it only to ChatInterface-derived windows, retiring AP-40 (the prior "opacity is fixed at 0.75, no focus transition" row) in the same commit; AP-189 filed 2026-08-10 at the CH6a/b REJECT-review rework, SHOULD-FIX 5 — acdream's ONE shared 500-entry/200-line-display-tail chat log gives every window a shallower EFFECTIVE per-window scrollback depth than retail's own per-window 10,000-line log, though the accumulate-while-closed and independent-per-window-scroll BEHAVIORS are both correctly reproduced; AP-188 filed 2026-08-10 at Campaign CH slice CH6b — a floating chat window's chat entry always sends on the Say channel because the floaty LayoutDesc authors no talk-focus menu and acdream does not (yet) share the main window's currently-selected channel across all five chat-window instances; AP-187 filed 2026-08-10 at Campaign CH slice CH6b — the four floating chat windows' text-type filters persist in local `settings.json` only (`ChatSettings.ChatWindow1..4Filter`), with no analog to retail's server-side `0x1000008C` GameplayOptions blob, so a character's floaty filter customization does not travel between acdream installs or round-trip to/from a retail client sharing the same character; AP-186 RETIRED 2026-08-10, issue #363's interface-text seam — `ChatVM` now carries an `OnInterfaceText` hook (`Action?`) the App-layer composition wires to `RuntimeCommunicationState.AddText(text, RetailLogTextType.ClientLocal)`, exactly fix shape (a) this row's own filing proposed; `ChatCommandRouter`'s two local-presentation fallbacks (`RetailCommandHelpTable.UnknownCommand` and the degenerate-prefix "Unknown command: {verb}." refusal) now call `ShowInterfaceText` and reach the SpewBox, with a null-fallback into the chat log (still tagged `ClientLocal`) for hosts that never wire the hook (headless has no `ChatVM` at all). Closes ISSUES.md #367; AP-185 filed 2026-08-10 at Campaign CH slice CH6a — the chat window's UiLocked border-art cosmetic swap is unported, see the row for detail; AP-184 RETIRED 2026-08-10 at Campaign CH round 4, closing ISSUES.md #364 — filed 2026-08-09 at Campaign CH user-gate round 2, item 3, recording that three of the seven retail `/help` group-topic listings (channels/chatting/commands) remained PARTIAL because their detail text is built in full or in part by `ClientCommunicationSystem::HelpStupidChannelHack @0x0056f290`, which the filing believed "not decodable with confidence from a static string sweep" because Binary Ninja renders its three internal string operands as dereferences of unrelated vtable slots (`&ClientCommunicationSystem::\`vftable'.RecvNotice_StartBarberNotice` etc.). That belief was WRONG — the same pooled/mislabeled-data artifact this register already documented elsewhere (AP-113's retirement note) applies here too: reading the function's own disassembly for the `push imm32` immediately preceding each `PStringBase::PStringBase` constructor call (rather than trusting BN's line-grouped rendering, which hides the true instruction order) resolves all three operands directly — `"@"` + a one-character tag sliced from a shared wide literal `U"fvpca"`/`U"mh,."` (a wide string read through a narrow `char*` truncates at the first zero high byte, the "hack" retail's own function name calls out) + `" - Sends a broadcast to your "` + `ChannelSystem::GetChannelName`'s own literal switch-table result + `".\n"`. `ChannelsGroupDetail` (entirely 6 such calls), `ChattingGroupDetail` (6 more, plus a `HelpReply@0x00577A50` Summary-branch quirk that unconditionally emits reply+pr+mr together — read directly, not assumed), and `CommandsGroupDetail` (`HelpAllGroup`, a straight-line concatenation of every other group's Detail branch plus a handful of its own short one-liners, including a CONFIRMED retail saveui/loadui duplicate) are now COMPLETE verbatim listings, matching the four (death/status/text/allegiances) the original filing already had. See `RetailCommandHelpTable`'s class remarks and `RetailCommandHelpTableTests` for the full per-line address citations. Round 2 item 2 also deletes `PortalWaitNoticeController` (the dedicated centered-overlay presentation the user reported was the wrong retail surface) and reroutes the portal-space wait-cue notice through the same `AddText`/SpewBox chokepoint every other on-screen interface-text site uses — AP-178's open SpewBox position/extent/font/colour questions now cover this notice too, since its separate controller and consts are gone; no new row was needed for the surface mismatch itself, since it was never separately registered (`PortalWaitNoticeController`'s own doc comment asserted "not a chat message" as an accepted design, not a flagged divergence). AP-150 RETIRED 2026-08-09 at Campaign CH user-gate round 1, item D (#329) — `PortalTunnelPresentation.TickRotation` now emits `"In Portal Space - Please Wait..."` unconditionally on every rotation-segment expiry, exactly matching `gmSmartBoxUI::UseTime`'s `else`-arm at 0x004D6FCD, instead of gating on `_waitCueVisible`, which only ever went true after the invented 5-second `RuntimeWorldTransitState.RetailWaitCueDelay` hold; `RetailWaitCueDelay`/`ObserveWait`/`SetWaitCue` remain as `LocalPlayerTeleportController`'s own hold-delay telemetry (`RuntimePortalSnapshot.WaitCueShown`) but no longer gate the on-screen cue, so they are not a residual of this row — closes issue #329; AP-183 RETIRED 2026-08-10, issue #363 — every named site now routes through the `ChatVM.ShowInterfaceText`/`OnInterfaceText` seam (see AP-186's retirement note) at its correct retail type: `DoStupidChannelHack` ("You must specify the text you wish to say!", newly wired — the six legacy channel verbs previously fell through `ChatInputParser.Parse`'s pure `return null` with no message at all), `DoChannelList`/`On`/`Off` ("Please specify the channel name.", reclassified), `DoAllegiance` ("Please see @help Allegiance...", reclassified), `DoHouseAvailableList` (reclassified AND corrected to retail's own "Please see @help hslist for more information on how to use this command" string, replacing the acdream-synthesized "Usage: /hslist " fallback — verified `acclient_2013_pseudo_c.txt:381481`/`1029383`), and `DoReply` ("Someone must @tell you first!", newly wired for the message-but-no-last-teller branch only — bare `/r` with no message at all is a separate retail branch, deliberately still unported). `DoSpeaker`/`DoEndurance`/`DoTitle` are untouched, confirmed still correct at `0x00`. The generic bad-args fallback (`ChatCommandRouter.Submit`'s catalog dispatch) now resolves `WeenieErrorMessages.Resolve(0x026u, null)` ("That is not a valid command.", the exact port of `DoCommand @0x0057E46D`'s `HandleFailureEvent(0x26)`) instead of synthesizing a `"Usage: {Usage}"` line — cross-checked against five decompiled handlers (`DoDie` plus the four above), all `0x1A`, confirming the uniform routing decision; AP-182 filed 2026-08-09 at Campaign CH slice CH4, corrected at the CH4 REJECT-review (nit 11) — `@title` is wired to a pure no-op (the value is neither stored nor consumed anywhere) and also omits `DoTitle`'s three local failure messages; recount at the CH3 Opus review corrected a pre-existing off-by-one; AP-181 filed 2026-08-09, Campaign CH slice CH3 — the local chat spam throttle (`IsMessageSpam`) has no acdream port. AP-178 NARROWED 2026-08-09 at the CH2 REJECT-review rework NIT 3, wording corrected at the CH2 re-review nits pass (`docs/plans/2026-08-09-chat-parity-campaign.md`, nits 1/2/6) — the original `dats.Portal` pass used an id source that was not Portal's own (`dats.Portal.GetAllIdsOfType()` is empty for this type), so it established nothing about Portal either way; extending a correctly-paired sweep to `dats.Local` FOUND the SpewBox element there; extent (`450×72`) and `MaxConcurrentItems` (`4`, not the code-default `1`) are now AUTHORED, leaving absolute screen position, colour, AND vertical content flow (now TOP-aligned, acdream's own invention pending measurement) open. AP-180 filed 2026-08-09 at the CH2 REJECT-review rework — `RuntimeCommunicationState.AddText`'s `windowId` parameter is accepted but not consumed, so retail's dual-destination echo (a `0x1A` message with a non-zero `windowId` lands in both the SpewBox and its originating chat window) is unimplemented; latent today since every production caller passes `windowId = 0`. AP-177/AP-178/AP-179 filed 2026-08-09, Campaign CH slice CH2 (interface text / SpewBox) — AP-177 records the invented 5-second SpewBox line lifetime (retail's real timeout is keystone-owned and unmeasured); AP-178's original filing recorded the invented SpewBox screen position/extent/font/colour/MaxConcurrentItems after `SpewBoxLayoutDumpDiagnostic`'s Portal-only sweep found zero elements of class 0x10000016 — see the NARROWED note above for the corrected finding; AP-179 is the OnCombatLine half of the RETIRED AP-176 split out to its own row. AP-176 RETIRED the same day — the WeenieErrorMessages full 344-row `HandleFailureEvent` port (`WeenieErrorMessages.Resolve`) replaces the single-stand-in-`LogTextType` approximation that row recorded for `ChatLog.OnWeenieError`. AP-175 filed 2026-08-09, Campaign CH slice CH1 — PopUpString renders as a chat-log line instead of retail's modal dialog; AP-39 updated the same day — chat coloring is now retail's exact 34-value `LogTextType` table, not a synthetic per-`ChatKind` approximation of it. AP-173 and AP-174 filed 2026-08-08, Campaign A slice A2 — AP-173 expresses retail's ±15 dB DirectSound pan as an OpenAL azimuth by inverting the constant-power pan law, since AL exposes no per-channel gain for a mono source; AP-174 records acdream's extra master volume knob on top of retail's three, folded into retail's single master multiply so the −50 dB cutoff and dB quantisation move with it. AP-172 and AP-171 filed 2026-08-08, #354 spell-bar drag-reorder fix — the favorite-bar reorder gesture defers its own list rebuild for the drag's duration so `UiRoot`'s drag-cancel safety net cannot destroy the in-flight cell, compensating the drop-time target index for the resulting stale sibling numbering; final positions and the wire pair are retail-exact, only the mid-drag visual reflow timing differs. AP-170 filed 2026-08-08, grand-gate finding G3 — an out-of-range vendor Use now arms on arrival instead of sending immediately, because the user's local ACE server polls for the player to actually reach use range before opening the shop panel and a too-early Use is silently lost; AP-169 filed 2026-08-08, grand-gate finding G2 — the vendor toolbar split-slider resolver falls back to the packed shop-supply-count field when the item's own `PublicWeenieDesc._stackSize` is absent, because the user's local ACE server never populates the latter for a browse-list item; AP-167/AP-168 filed 2026-08-09 at the Opus review of `92ea3977` (findings F1/F6) — Buy All's container-vs-item slot classification approximates retail's bitfield/capacity test with `ItemType.Container` [AP-168], and SellSingleItem's non-empty-container refusal branch is not ported [AP-167]; AP-164 RETIRED the same review (finding F4) — BF_RETAINED is now checked end to end; AP-162 NARROWED the same review (finding F1) — Buy All's four client-side pre-send guards are now ported, leaving only the single-item TryBuy path without one; AP-161 gains a REVIEW CORRECTIONS paragraph the same review (findings F1-F13) summarizing the rest as bug fixes to already-claimed behavior, not new divergences. AP-164/AP-165/AP-166 filed 2026-08-09 at Slice 6b/6c (staging+sell arc) — InqAcceptability's non-sellable bitfield is unmodeled [AP-164], the Buy-side stackable-removal-amount test substitutes DescStackSize for retail's _maxStackSize [AP-165], and the Buying/Selling tabs' own purse/count text plus the cross-panel pending-sell inventory highlight are unwired [AP-166]; AP-161 NARROWED the same day — the row's last vendor-specific residual (Buying/Selling tabs render but carry no data binding) CLOSES now that both tabs are fully wired (staging, drag-to-sell, InqAcceptability gating, Sell 0x0060, the X-close confirmation), leaving only the two long-standing PRE-EXISTING residuals (dropdown arrow-cap glyph, alt-currency m_last_sale simplification) plus the three new AP-164/165/166 residuals just filed; AP-162 EXTENDED the same day — the same no-client-pre-check omission now also covers the batched "Buy All" path (TryBuyAll), not just the single-item TryBuy. AP-162/AP-163 filed 2026-08-09 at Slice 6.3 (buy arc) — no client-side Buy affordability/capacity pre-check [AP-162] and the shop-item guid-collision skip-not-clobber policy [AP-163]; AP-161 NARROWED the same day — the private-selection and unwired-examine residuals CLOSE at Slice 6.1/6.2, leaving only the dropdown arrow-cap glyph and the alt-currency `m_last_sale` simplification, plus a confirmed-absent-from-retail note on double-click-to-buy. AP-161 REWRITTEN 2026-08-09 at the Slice 5.4 review (findings F1-F8) — the popup-never-rendered, wrong-quantity-price, no-auto-select, dropped-icon-layer, stale-category-on-vendor-switch, and unguarded-Apply-fanout bugs the review found are fixed (`VendorUiController.cs`, `VendorState.cs`, `GameEventWiring.cs`, `RetailUiRuntime.cs`); the row now records only the four consciously-deferred residuals it still owns (private per-panel selection vs. retail's global `ACCWeenieObject::selectedID`, the unwired shop-item examine route, the dropdown button-face arrow-cap glyph, and the alt-currency held-amount's `m_last_sale`-free simplification). AP-110's "retail-correct per-unit prices" phrasing is corrected the same day to "quantity-correct pricing" — the OLD phrase mischaracterized what retail even shows (a `GetObjectSplitSize`-quantity price, not literally one unit) independent of whether the code was buggy. AP-161 filed 2026-08-09 at Slice 5.4 (vendor browse panel) — the authored "Buying"/"Selling" tabs render and switch pages but carry no data binding, per contract decision 8's required successor to AP-110's narrowing; AP-110 NARROWED the same day — "vendor" is retired from its absent-panels list now that the "Items" browse tab is user-reachable. AP-160 filed 2026-08-07 at Slice 5.3 — the client-local vendor-panel distance watcher closes on plain 3D center distance instead of retail/ACE's cylinder-gap distance, because Runtime has no per-entity collision radius/height source outside the App-layer's Setup-cylinder resolver. AP-158 RETIRED 2026-08-06 by the #333 fix, closing #337 — the `maxReach` distance pre-filter is DELETED rather than re-centred, because retail has none: `CObjCell::find_obj_collisions` @0x0052b750 walks the cell's shadow list and calls `CPhysicsObj::FindObjCollisions` unconditionally. The row's predicted symptom was observed live at Neftet before it was fixed — a tall prop AP-156 had just placed correctly still not blocking, plus jumps sinking into the mesh and corpses falling through. Perf measured, not assumed: at the live-maximum 38 in-cell candidates 10.61 µs → 16.68 µs per resolve. AP-159 filed 2026-08-06 at the #334 fix — the INDOOR half of AP-156’s traversal residual is all that remains of it; the outdoor half is CLOSED by the `find_bbox_cell_list` port, and AP-156’s RISK COLUMN IS CORRECTED at the same commit: it recorded the residual as “extra broadphase candidates, never a missed one”, which generalised the indoor direction to the whole row and is exactly why #334 — a MISSED one, and a user-observed loss of collision on landblock-spanning formations — sat inside it unnoticed. AP-158 filed 2026-08-06 at the AP-156 fix review — the shadow broadphase's `maxReach` distance pre-filter is acdream's own invention with NO retail counterpart, and it measures from the part origin, so it can discard a genuine contact for exactly the off-centre parts AP-156 just placed correctly; issue #333. AP-156 CORRECTED at the same review: its population was understated — 172 is AP-152's DISPATCH population, not AP-156's CONTAINMENT population. AP-155 NARROWED and AP-156/AP-157 filed 2026-08-06 at the AP-152 retail-conformance review. AP-155 bundled two divergences with different code paths, populations and gates under one id; its flood half is now AP-156, **with its direction corrected**. AP-155(b) recorded the BSP flood approximation as OVER-inclusive and used that direction as the reason the residual was safe to defer; measured over the installed DAT it was UNDER-inclusive for 428 of the 530 BSP-bearing Setups (the AP-156 fix review corrected the originally-recorded '170 of 172'), because `BuildFloodSpheres` carried each physics-BSP part's root bounding-sphere RADIUS while discarding that sphere's own ORIGIN and centring it on the part origin. That is the #98/#168 class, and for 43 Setups the post-AP-152 flood was strictly smaller than the pre-AP-152 one. AP-156 records the correction and the fix — `ShadowShape.BoundsCenter`, filled from the same resolver that supplies the radius, plus the retirement of the 10-sphere clamp on a branch where retail has none — and keeps open only the sphere-vs-portal TRAVERSAL approximation. AP-157 is the previously unregistered third-branch substitution: retail floods from one `CPartArray::GetSortingSphere` where acdream floods from every Sphere shape, and acdream's cylinder flood ignores `CylHeight`. AP-152 RETIRED 2026-08-06, one day after it was filed: `ShadowShapeBuilder.FromSetup` now dispatches BSP-first instead of unioning, and `ShadowObjectRegistry.BuildFloodSpheres` now applies `calc_cross_cells`' own BSP → cylsphere → sorting-sphere order. Four statements in the row were false and are corrected in its retirement text — most importantly its predicted symptom, "catching on a doorway sill", which could not have been occurring: `Transition.BspOnlyDispatch` had already made the extra primitive inert at collision-query time since 2026-05-25. The live half was CELL MEMBERSHIP, the #98/#168 symptom class, which had no such guard. AP-153/AP-154/AP-155 filed at that retirement — retail's dispatch flag is cached once at part-array construction where acdream's gate is live [AP-153]; acdream's query-time guard takes a CLIENT-DERIVED flag off the WIRE and never derives it, an undeclared dependency on ACE reading the same DAT bit [AP-154]; and the static publication paths emit a Setup Sphere as a height-capped Cylinder while `BuildFloodSpheres` approximates retail's bounding BOX with bounding SPHERES [AP-155, whose flood-priority half is closed by the same commit]. AP-152 filed 2026-08-06 at the AP-22 retirement — the LIVE collision path emits Setup primitives and per-part physics-BSP shapes additively where retail's `CPhysicsObj::FindObjCollisions` dispatches exclusively; 172 of 5,935 installed Setups are affected, including BSP doors, so it needs its own visual gate and was deliberately not folded into the AP-22 commit; the count is unchanged because AP-22 retired in the same commit. AP-22 RETIRED 2026-08-06 — retail synthesizes no shape for a shapeless object (`CPhysicsObj::FindObjCollisions` 0x0050f050 exits at `0x0050f22f je 0x50f31b` returning the seeded OK_TS, and `CPartArray::GetRadius`/`GetHeight` are absent from its whole call set), so the invented `setup.Radius` cylinder was deleted rather than re-derived; the row's site list named one file that never contained the fallback and omitted the two that did, one of them the headless-only copy, and its "rare decorative props" risk described an unreachable branch — 0 of 5,935 installed Setups can satisfy the guard. AP-150/AP-151 filed 2026-08-06 at the #280 dual review — the wait cue's five-second arming is acdream's own and not retail's trigger [AP-150], and the reveal gate is materially stricter than retail's DAT-residency prefetch predicate on the mesh-build/GPU-upload axis [AP-151], the opposite asymmetry from AP-149; AP-149 filed 2026-08-05 at the #280 portal-prefetch fix — the reveal gate's outer ring accepts terrain-only publication where retail requires LandBlockInfo and every building EnvCell; the fix closes the reveal-window/visible-window ratio, not this residual; AP-148 filed 2026-08-05 at the C5b closeout — acdream's local-player Gate A requires the wire TELEPORT_TS to be EQUAL where retail requires only that it not be OLDER, verified by disassembly against the PDB-paired binary after two review rounds read the Binary Ninja tautology and missed it; AP-147 filed 2026-08-05 at the C5b architecture review, finding D3 — the accepted-Position delta stream's cardinality change and its torn intermediate; AP-138 amended at the same review — C5b staled its route-2 first-submit `CurrentCellId` measurement; AP-131 RETIRED 2026-08-05, C5b, closing #275 — the steady-state merge's `installPlacementFrame: true, clearParent: true` literals no longer exist; `InboundPhysicsStateController.TryApplyPosition` now computes both flags PRE-MERGE from `(disposition, hasAnimations(old))`, which is exactly `RuntimeAuthoritativePositionRouteClassifier.ClassifyAcceptedPosition`'s own `ApplyPlacementFrameBeforeRouting`/`UnparentBeforeRouting` rows (false/false on the Gate A force row, `!HasAnimations`/true on every accepted non-force route). Retail decides both writes BEFORE `MoveOrTeleport` is consulted — Gate A @0x0045400C returns @0x0045409D ahead of `unset_parent` @0x00454129 and the `HasAnims` `SetPlacementFrame` gate @0x00454137 — so the flags need no route, no player distance and no signature change. The row's predicted symptoms are gone: an animated entity's ordinary Position no longer installs a placement frame retail skips, and a ForcePosition no longer unparents. Evidence: `InboundPhysicsStateControllerTests` — `ApplyOnAnimatedEntity_NeverInstallsTheWirePlacementFrame`, `ApplyOnNonAnimatedEntity_InstallsTheWirePlacementFrame`, `ForcePositionOnParentedLocalPlayer_RetainsTheParentAttachment`, and the 12-row `MergedPrePlacementFieldsMatchTheClassifiedRouteFlags` matrix which uses the production classifier as its oracle rather than re-encoding the table; all four sabotage-verified in both directions. The row's "the legacy caller is deleted at the production cutover" framing was overtaken: the caller was CORRECTED, not deleted, and remains the only production Position wire caller; AP-145 RETIRED 2026-08-05, C5a commit 1, closing #318 — `TryPublishPlace` now publishes the local player's Place through `LocalPlayerShadowSynchronizer.SyncPose`, the same publisher ordinary per-tick movement uses, instead of a direct `LocalPlayerShadowState.Set` that never touched `PhysicsEngine.ShadowObjects`; AP-1 RETIRED 2026-08-05, C5a deletion sweep — `PhysicsEngine.Resolve`/`ResolvePlacement`/`HasCellSurface` deleted outright, zero production callers, so "production zero-delta routes remain on the legacy resolver" is now structurally false; AP-146 filed 2026-08-05, #319 fix — the local player's canonical cell is written only at login/inbound-Position/teleport, not per ordinary-movement tick as retail's SetPositionInternal does; #319's fix makes a player-parented child inherit exactly this coarseness, stale-but-equal to the parent, not a new staleness class; follow-up filed as issue #320; AP-144 filed 2026-08-05, C4 route 3 round 3 (R7) — the portal-arrival movement-event send reuses `UsePositionFromServer` (`autonomy_level != 2`) where retail's actual gate, `SendMovementEvent`, is `autonomy_level != 0`; the two agree everywhere except level 1, which no production caller can reach today; AP-142/AP-143 filed 2026-08-04, C4 route 7 — the parented-child single-field cell model (id/pointer collapse, zero-not-stale removal propagation, same-cell tick-loop subsumption) and the headless parent-realize drive's skipped holding-location validation; AP-141 filed 2026-08-04, C4 route 5, NARROWED 2026-08-04 at the round-2 delta review — the far-branch StopInterpolating clause was wrong for the adopted-body case (it is now ported there) and the row's language now distinguishes "never armed" from "never re-anchored"; CORRECTED 2026-08-04 at the round-3 delta review — the risk column's "would drag the body toward a stale anchor" claim was itself wrong (the leash anchor is write-only; `ConstraintManager::adjust_offset` only brakes, never pulls) and is retracted; every half remains test-gated only, since ACE never sends a missile UpdatePosition; AP-140 filed AND RETIRED 2026-08-04 — filed at the Bug B Opus review because the two accepted-Position routing gates read the client `Airborne` flag, i.e. walkability, where retail's free-flight predicate is CONTACT, and Bug B had just turned "in contact, not on walkable ground" from unreachable into ordinary; retired the same day by pointing both gates at `PhysicsBody.InContact`, retail's literal `transient_state & 1` test at `InterpolationManager::adjust_offset` @0x00555D52 (bit 0 = `CONTACT_TS`, acclient.h:3690), while leaving `Airborne` and all five of its `!Body.OnWalkable` writers untouched — the narrow shape the row itself pinned. A remote sliding on a steep face now interpolates as retail does instead of snapping at UpdatePosition cadence; AP-139 filed 2026-08-04, Bug B remote steep-contact slide — the interpolation-queue clear on the landing edge, carried over from the deleted hand-rolled remote landing block; AP-81 narrowed the same day by that fix, which retired its whole GRAVITY half; AP-87 annotated the same day — its predicted symptom was observed live and then fixed at the source, with the row's own thresholds and conditions deliberately unchanged; AP-138 filed 2026-08-04, C4 route 4b-2 dual Opus review, parts (1) and (2) rewritten the same day at the DELTA review — the far snap's refusable-placement residual: store_position only on the outcomes that never reached the engine, the two quiescence parks made restorable at the source, with the rollback gated on the cell it actually restores into, rather than refused by a pre-flight that structurally cannot see them, and the leash not armed through a superseded incarnation; AP-137 filed 2026-08-04, C4 route 4b-2 and rewritten the same day at that review, `teleport_hook`'s call list completed at the delta review — the acdream-only null/rejected/cell-less leftover arm, what the deleted duplicated 96 m/4 m constant pairs actually computed, and the vacuous headless satisfaction; AP-136 filed 2026-08-04, C4 route 4b-1 review, NARROWED 2026-08-04 at the C4 route 4b-2 delta review and AMENDED 2026-08-04 by the cancelled-park presentation rollback (the row's "restored visible" claim covered only the CANONICAL half; the presentation half was never rolled back, which left a parked-then-cancelled remote that stops moving invisible in the world AND absent from the radar for the rest of the session — a defect, now fixed by the `WithdrawalRestored` receipt, with the selection residual filed as AD-63) — a cancelled lost-cell park re-shows the entity where retail keeps it hidden until cell load, and the rollback's scope now covers the two placement-side quiescence parks whenever the cell it restores into is not itself quiescing — round 4 (2026-08-04) applies that same test a second time at RESTORE time, because a retained park's rollback lands a packet later; AP-135 filed 2026-08-03, C4 route 4a — the airborne no-op's retained acdream bookkeeping; the stated total was 2 rows stale before that filing and is now a literal count of this section; AP-130/AP-131/AP-132 filed 2026-08-02, continuation-executor slice; AP-5 retired 2026-07-31 at Campaign P Slice 2A — every successful `step_down` now performs retail's final `PLACEMENT_INSERT`; AP-3/AP-4 retired 2026-07-31 at Campaign P Slice 1B — `transitional_insert` and `edge_slide` now preserve retail's valid-contact early return and Branch-1-first order; AP-127 retired 2026-07-31 by #268 — the complete augmentation chain is shared by character UI and Runtime movement; AP-30 retired 2026-07-30 by the movement parity audit — retail Frame::is_equal genuinely uses the 0.0002 epsilon [byte-confirmed], so the row recorded a NON-divergence; acdream already matches; AP-129 narrowed 2026-07-30 at the P4 Opus review fix — `CanMoveInto`/`RestrictionDB::IsAllowedIn` are now ported and fed end-to-end (CreateObject HouseOwner/HouseRestrictions/Monarch tail fields + live `House_UpdateRestrictions 0x0248`, resolved through `PhysicsEngine.Objects`), retiring the original "CanMoveInto entirely unmodeled, unconditional fail-closed" gap the row described — the review was triggered by `RestrictionObjPrevalenceInspectionTests` showing 103,766 of 729,888 installed EnvCells (the whole housing estate) carry a baked `RestrictionObj`, so the unconditional fail-closed default would have locked every house for every player including its own owner; AP-10 retired 2026-07-30 at Campaign P Slice P4 — restored retail's 0.1 m dry-corner water sink-in, full suite green proving the sticky-bit no-regression argument; AP-71 retired same slice — `check_entry_restrictions` ported at the head of the indoor `FindEnvCollisions` branch, `CellPhysics.RestrictionObj` wired from the DAT-baked `EnvCell` field in both the dev and production caching paths; AP-128 filed 2026-07-30 at the P3 Opus review — PK-timer clock basis; AP-25 retired 2026-07-30 at Campaign P Slice P1 — the vitae/enchantment-aware run/jump skill chain; AP-7 retired 2026-07-30 at Campaign P Slice P2 — `calc_friction`'s threshold ported to retail's confirmed 0.25f; its still-open cos(10°)-vs-0.99999536f Sledding constant question moved to AD-55) +## 3. Documented approximation (AP) — 138 active rows (AP-197 filed 2026-08-11 at the OP4 review-fix round (SF-1/S4) — Display Timestamps hardcodes retail's constructor-default format string instead of the per-character GenericQualitiesData key-1 override the parser reads and discards; AP-196 filed 2026-08-11 at the OP4 review-fix round (MUST-FIX 3 / blast M2) — the Group-C re-point's observable-default changes (ViewCombatTarget true→false) and the GameplaySettings store retirement (AutoTarget/AutoRepeatAttack/ViewCombatTarget deleted, the other five kept as write-behind mirrors); AP-195 filed 2026-08-11 at the Campaign OP OP2 re-review closure — `UIOption_CheckboxBitfield64::Refresh`'s ALL-set LED media swap and the `ResizeTo`/`CalculatePaperSize` self-sizing tail are not ported, and the block's row stacking is a second, divergent implementation of the ListBox mechanism, all due at OP5; AP-194 filed 2026-08-10 at Campaign OP slice OP1 — the GetDefaultOptionValue vs constructor-default disagreement for ConfirmVolatileRareUse/ShowHelm/ShowCloak (see the row below); AP-193 filed 2026-08-10 at Campaign OP slice OP1 — the 0x34 HearPKDeathMessages id/mask mapping is ACE-sourced (see the row below); AP-192 filed 2026-08-10 at the Campaign CH round-5 polish (S2) — authored outline `0x21`/`0x22` now reaches every text-bearing widget, but only at the element's effective-default state; per-STATE outline switching (dialog/character/combat buttons author `0x21` in state `0x3` only) is not ported; AP-191 filed 2026-08-10 at Campaign CH round 4 items 1+2 — the chat transcript's missing tag-colour (`0x1D`, green) and tag-font (`0x1C`) are deferred, needing a per-run tag concept `UiText.Line` does not have yet; AP-184 RETIRED 2026-08-10 at Campaign CH round 4 — the three PARTIAL `/help` group topics (channels/chatting/commands) are now COMPLETE verbatim listings, `ClientCommunicationSystem::HelpStupidChannelHack @0x0056f290` fully decoded (the "vftable slot" operands are the same pooled/mislabeled-data artifact as AP-186's own precedent, not real vtable dispatch — reading the function's own disassembly for the `push imm32` preceding each constructor call resolves them), closing ISSUES.md #364 (full retirement note later in this same list, at its own "AP-184 RETIRED 2026-08-10 at Campaign CH round 4, closing ISSUES.md #364 — filed 2026-08-09..." entry); AP-113 RETIRED 2026-08-10 at the consolidated-review round, SHOULD-FIX 3/1 byproduct — DoLifestone's own bad-args refusal text is now byte-recovered, see its retirement note below; AP-183 and AP-186 RETIRED 2026-08-10 by issue #363's interface-text seam — see their retirement notes below; AP-190 filed 2026-08-10 at Campaign CH slice CH6c — window opacity now fades every RetailWindowManager window on retail's focus-driven Default/Active mechanism, not just ChatInterface-derived ones, and ships gmMainChatUI's 1.0/1.0 default as the ONE shared default across every registered window (fixed from the original 0.5/1.0 base-ChatInterface value, per the row's own REWORDED (2)) instead of applying it only to ChatInterface-derived windows, retiring AP-40 (the prior "opacity is fixed at 0.75, no focus transition" row) in the same commit; AP-189 filed 2026-08-10 at the CH6a/b REJECT-review rework, SHOULD-FIX 5 — acdream's ONE shared 500-entry/200-line-display-tail chat log gives every window a shallower EFFECTIVE per-window scrollback depth than retail's own per-window 10,000-line log, though the accumulate-while-closed and independent-per-window-scroll BEHAVIORS are both correctly reproduced; AP-188 filed 2026-08-10 at Campaign CH slice CH6b — a floating chat window's chat entry always sends on the Say channel because the floaty LayoutDesc authors no talk-focus menu and acdream does not (yet) share the main window's currently-selected channel across all five chat-window instances; AP-187 filed 2026-08-10 at Campaign CH slice CH6b — the four floating chat windows' text-type filters persist in local `settings.json` only (`ChatSettings.ChatWindow1..4Filter`), with no analog to retail's server-side `0x1000008C` GameplayOptions blob, so a character's floaty filter customization does not travel between acdream installs or round-trip to/from a retail client sharing the same character; AP-186 RETIRED 2026-08-10, issue #363's interface-text seam — `ChatVM` now carries an `OnInterfaceText` hook (`Action?`) the App-layer composition wires to `RuntimeCommunicationState.AddText(text, RetailLogTextType.ClientLocal)`, exactly fix shape (a) this row's own filing proposed; `ChatCommandRouter`'s two local-presentation fallbacks (`RetailCommandHelpTable.UnknownCommand` and the degenerate-prefix "Unknown command: {verb}." refusal) now call `ShowInterfaceText` and reach the SpewBox, with a null-fallback into the chat log (still tagged `ClientLocal`) for hosts that never wire the hook (headless has no `ChatVM` at all). Closes ISSUES.md #367; AP-185 filed 2026-08-10 at Campaign CH slice CH6a — the chat window's UiLocked border-art cosmetic swap is unported, see the row for detail; AP-184 RETIRED 2026-08-10 at Campaign CH round 4, closing ISSUES.md #364 — filed 2026-08-09 at Campaign CH user-gate round 2, item 3, recording that three of the seven retail `/help` group-topic listings (channels/chatting/commands) remained PARTIAL because their detail text is built in full or in part by `ClientCommunicationSystem::HelpStupidChannelHack @0x0056f290`, which the filing believed "not decodable with confidence from a static string sweep" because Binary Ninja renders its three internal string operands as dereferences of unrelated vtable slots (`&ClientCommunicationSystem::\`vftable'.RecvNotice_StartBarberNotice` etc.). That belief was WRONG — the same pooled/mislabeled-data artifact this register already documented elsewhere (AP-113's retirement note) applies here too: reading the function's own disassembly for the `push imm32` immediately preceding each `PStringBase::PStringBase` constructor call (rather than trusting BN's line-grouped rendering, which hides the true instruction order) resolves all three operands directly — `"@"` + a one-character tag sliced from a shared wide literal `U"fvpca"`/`U"mh,."` (a wide string read through a narrow `char*` truncates at the first zero high byte, the "hack" retail's own function name calls out) + `" - Sends a broadcast to your "` + `ChannelSystem::GetChannelName`'s own literal switch-table result + `".\n"`. `ChannelsGroupDetail` (entirely 6 such calls), `ChattingGroupDetail` (6 more, plus a `HelpReply@0x00577A50` Summary-branch quirk that unconditionally emits reply+pr+mr together — read directly, not assumed), and `CommandsGroupDetail` (`HelpAllGroup`, a straight-line concatenation of every other group's Detail branch plus a handful of its own short one-liners, including a CONFIRMED retail saveui/loadui duplicate) are now COMPLETE verbatim listings, matching the four (death/status/text/allegiances) the original filing already had. See `RetailCommandHelpTable`'s class remarks and `RetailCommandHelpTableTests` for the full per-line address citations. Round 2 item 2 also deletes `PortalWaitNoticeController` (the dedicated centered-overlay presentation the user reported was the wrong retail surface) and reroutes the portal-space wait-cue notice through the same `AddText`/SpewBox chokepoint every other on-screen interface-text site uses — AP-178's open SpewBox position/extent/font/colour questions now cover this notice too, since its separate controller and consts are gone; no new row was needed for the surface mismatch itself, since it was never separately registered (`PortalWaitNoticeController`'s own doc comment asserted "not a chat message" as an accepted design, not a flagged divergence). AP-150 RETIRED 2026-08-09 at Campaign CH user-gate round 1, item D (#329) — `PortalTunnelPresentation.TickRotation` now emits `"In Portal Space - Please Wait..."` unconditionally on every rotation-segment expiry, exactly matching `gmSmartBoxUI::UseTime`'s `else`-arm at 0x004D6FCD, instead of gating on `_waitCueVisible`, which only ever went true after the invented 5-second `RuntimeWorldTransitState.RetailWaitCueDelay` hold; `RetailWaitCueDelay`/`ObserveWait`/`SetWaitCue` remain as `LocalPlayerTeleportController`'s own hold-delay telemetry (`RuntimePortalSnapshot.WaitCueShown`) but no longer gate the on-screen cue, so they are not a residual of this row — closes issue #329; AP-183 RETIRED 2026-08-10, issue #363 — every named site now routes through the `ChatVM.ShowInterfaceText`/`OnInterfaceText` seam (see AP-186's retirement note) at its correct retail type: `DoStupidChannelHack` ("You must specify the text you wish to say!", newly wired — the six legacy channel verbs previously fell through `ChatInputParser.Parse`'s pure `return null` with no message at all), `DoChannelList`/`On`/`Off` ("Please specify the channel name.", reclassified), `DoAllegiance` ("Please see @help Allegiance...", reclassified), `DoHouseAvailableList` (reclassified AND corrected to retail's own "Please see @help hslist for more information on how to use this command" string, replacing the acdream-synthesized "Usage: /hslist " fallback — verified `acclient_2013_pseudo_c.txt:381481`/`1029383`), and `DoReply` ("Someone must @tell you first!", newly wired for the message-but-no-last-teller branch only — bare `/r` with no message at all is a separate retail branch, deliberately still unported). `DoSpeaker`/`DoEndurance`/`DoTitle` are untouched, confirmed still correct at `0x00`. The generic bad-args fallback (`ChatCommandRouter.Submit`'s catalog dispatch) now resolves `WeenieErrorMessages.Resolve(0x026u, null)` ("That is not a valid command.", the exact port of `DoCommand @0x0057E46D`'s `HandleFailureEvent(0x26)`) instead of synthesizing a `"Usage: {Usage}"` line — cross-checked against five decompiled handlers (`DoDie` plus the four above), all `0x1A`, confirming the uniform routing decision; AP-182 filed 2026-08-09 at Campaign CH slice CH4, corrected at the CH4 REJECT-review (nit 11) — `@title` is wired to a pure no-op (the value is neither stored nor consumed anywhere) and also omits `DoTitle`'s three local failure messages; recount at the CH3 Opus review corrected a pre-existing off-by-one; AP-181 filed 2026-08-09, Campaign CH slice CH3 — the local chat spam throttle (`IsMessageSpam`) has no acdream port. AP-178 NARROWED 2026-08-09 at the CH2 REJECT-review rework NIT 3, wording corrected at the CH2 re-review nits pass (`docs/plans/2026-08-09-chat-parity-campaign.md`, nits 1/2/6) — the original `dats.Portal` pass used an id source that was not Portal's own (`dats.Portal.GetAllIdsOfType()` is empty for this type), so it established nothing about Portal either way; extending a correctly-paired sweep to `dats.Local` FOUND the SpewBox element there; extent (`450×72`) and `MaxConcurrentItems` (`4`, not the code-default `1`) are now AUTHORED, leaving absolute screen position, colour, AND vertical content flow (now TOP-aligned, acdream's own invention pending measurement) open. AP-180 filed 2026-08-09 at the CH2 REJECT-review rework — `RuntimeCommunicationState.AddText`'s `windowId` parameter is accepted but not consumed, so retail's dual-destination echo (a `0x1A` message with a non-zero `windowId` lands in both the SpewBox and its originating chat window) is unimplemented; latent today since every production caller passes `windowId = 0`. AP-177/AP-178/AP-179 filed 2026-08-09, Campaign CH slice CH2 (interface text / SpewBox) — AP-177 records the invented 5-second SpewBox line lifetime (retail's real timeout is keystone-owned and unmeasured); AP-178's original filing recorded the invented SpewBox screen position/extent/font/colour/MaxConcurrentItems after `SpewBoxLayoutDumpDiagnostic`'s Portal-only sweep found zero elements of class 0x10000016 — see the NARROWED note above for the corrected finding; AP-179 is the OnCombatLine half of the RETIRED AP-176 split out to its own row. AP-176 RETIRED the same day — the WeenieErrorMessages full 344-row `HandleFailureEvent` port (`WeenieErrorMessages.Resolve`) replaces the single-stand-in-`LogTextType` approximation that row recorded for `ChatLog.OnWeenieError`. AP-175 filed 2026-08-09, Campaign CH slice CH1 — PopUpString renders as a chat-log line instead of retail's modal dialog; AP-39 updated the same day — chat coloring is now retail's exact 34-value `LogTextType` table, not a synthetic per-`ChatKind` approximation of it. AP-173 and AP-174 filed 2026-08-08, Campaign A slice A2 — AP-173 expresses retail's ±15 dB DirectSound pan as an OpenAL azimuth by inverting the constant-power pan law, since AL exposes no per-channel gain for a mono source; AP-174 records acdream's extra master volume knob on top of retail's three, folded into retail's single master multiply so the −50 dB cutoff and dB quantisation move with it. AP-172 and AP-171 filed 2026-08-08, #354 spell-bar drag-reorder fix — the favorite-bar reorder gesture defers its own list rebuild for the drag's duration so `UiRoot`'s drag-cancel safety net cannot destroy the in-flight cell, compensating the drop-time target index for the resulting stale sibling numbering; final positions and the wire pair are retail-exact, only the mid-drag visual reflow timing differs. AP-170 filed 2026-08-08, grand-gate finding G3 — an out-of-range vendor Use now arms on arrival instead of sending immediately, because the user's local ACE server polls for the player to actually reach use range before opening the shop panel and a too-early Use is silently lost; AP-169 filed 2026-08-08, grand-gate finding G2 — the vendor toolbar split-slider resolver falls back to the packed shop-supply-count field when the item's own `PublicWeenieDesc._stackSize` is absent, because the user's local ACE server never populates the latter for a browse-list item; AP-167/AP-168 filed 2026-08-09 at the Opus review of `92ea3977` (findings F1/F6) — Buy All's container-vs-item slot classification approximates retail's bitfield/capacity test with `ItemType.Container` [AP-168], and SellSingleItem's non-empty-container refusal branch is not ported [AP-167]; AP-164 RETIRED the same review (finding F4) — BF_RETAINED is now checked end to end; AP-162 NARROWED the same review (finding F1) — Buy All's four client-side pre-send guards are now ported, leaving only the single-item TryBuy path without one; AP-161 gains a REVIEW CORRECTIONS paragraph the same review (findings F1-F13) summarizing the rest as bug fixes to already-claimed behavior, not new divergences. AP-164/AP-165/AP-166 filed 2026-08-09 at Slice 6b/6c (staging+sell arc) — InqAcceptability's non-sellable bitfield is unmodeled [AP-164], the Buy-side stackable-removal-amount test substitutes DescStackSize for retail's _maxStackSize [AP-165], and the Buying/Selling tabs' own purse/count text plus the cross-panel pending-sell inventory highlight are unwired [AP-166]; AP-161 NARROWED the same day — the row's last vendor-specific residual (Buying/Selling tabs render but carry no data binding) CLOSES now that both tabs are fully wired (staging, drag-to-sell, InqAcceptability gating, Sell 0x0060, the X-close confirmation), leaving only the two long-standing PRE-EXISTING residuals (dropdown arrow-cap glyph, alt-currency m_last_sale simplification) plus the three new AP-164/165/166 residuals just filed; AP-162 EXTENDED the same day — the same no-client-pre-check omission now also covers the batched "Buy All" path (TryBuyAll), not just the single-item TryBuy. AP-162/AP-163 filed 2026-08-09 at Slice 6.3 (buy arc) — no client-side Buy affordability/capacity pre-check [AP-162] and the shop-item guid-collision skip-not-clobber policy [AP-163]; AP-161 NARROWED the same day — the private-selection and unwired-examine residuals CLOSE at Slice 6.1/6.2, leaving only the dropdown arrow-cap glyph and the alt-currency `m_last_sale` simplification, plus a confirmed-absent-from-retail note on double-click-to-buy. AP-161 REWRITTEN 2026-08-09 at the Slice 5.4 review (findings F1-F8) — the popup-never-rendered, wrong-quantity-price, no-auto-select, dropped-icon-layer, stale-category-on-vendor-switch, and unguarded-Apply-fanout bugs the review found are fixed (`VendorUiController.cs`, `VendorState.cs`, `GameEventWiring.cs`, `RetailUiRuntime.cs`); the row now records only the four consciously-deferred residuals it still owns (private per-panel selection vs. retail's global `ACCWeenieObject::selectedID`, the unwired shop-item examine route, the dropdown button-face arrow-cap glyph, and the alt-currency held-amount's `m_last_sale`-free simplification). AP-110's "retail-correct per-unit prices" phrasing is corrected the same day to "quantity-correct pricing" — the OLD phrase mischaracterized what retail even shows (a `GetObjectSplitSize`-quantity price, not literally one unit) independent of whether the code was buggy. AP-161 filed 2026-08-09 at Slice 5.4 (vendor browse panel) — the authored "Buying"/"Selling" tabs render and switch pages but carry no data binding, per contract decision 8's required successor to AP-110's narrowing; AP-110 NARROWED the same day — "vendor" is retired from its absent-panels list now that the "Items" browse tab is user-reachable. AP-160 filed 2026-08-07 at Slice 5.3 — the client-local vendor-panel distance watcher closes on plain 3D center distance instead of retail/ACE's cylinder-gap distance, because Runtime has no per-entity collision radius/height source outside the App-layer's Setup-cylinder resolver. AP-158 RETIRED 2026-08-06 by the #333 fix, closing #337 — the `maxReach` distance pre-filter is DELETED rather than re-centred, because retail has none: `CObjCell::find_obj_collisions` @0x0052b750 walks the cell's shadow list and calls `CPhysicsObj::FindObjCollisions` unconditionally. The row's predicted symptom was observed live at Neftet before it was fixed — a tall prop AP-156 had just placed correctly still not blocking, plus jumps sinking into the mesh and corpses falling through. Perf measured, not assumed: at the live-maximum 38 in-cell candidates 10.61 µs → 16.68 µs per resolve. AP-159 filed 2026-08-06 at the #334 fix — the INDOOR half of AP-156’s traversal residual is all that remains of it; the outdoor half is CLOSED by the `find_bbox_cell_list` port, and AP-156’s RISK COLUMN IS CORRECTED at the same commit: it recorded the residual as “extra broadphase candidates, never a missed one”, which generalised the indoor direction to the whole row and is exactly why #334 — a MISSED one, and a user-observed loss of collision on landblock-spanning formations — sat inside it unnoticed. AP-158 filed 2026-08-06 at the AP-156 fix review — the shadow broadphase's `maxReach` distance pre-filter is acdream's own invention with NO retail counterpart, and it measures from the part origin, so it can discard a genuine contact for exactly the off-centre parts AP-156 just placed correctly; issue #333. AP-156 CORRECTED at the same review: its population was understated — 172 is AP-152's DISPATCH population, not AP-156's CONTAINMENT population. AP-155 NARROWED and AP-156/AP-157 filed 2026-08-06 at the AP-152 retail-conformance review. AP-155 bundled two divergences with different code paths, populations and gates under one id; its flood half is now AP-156, **with its direction corrected**. AP-155(b) recorded the BSP flood approximation as OVER-inclusive and used that direction as the reason the residual was safe to defer; measured over the installed DAT it was UNDER-inclusive for 428 of the 530 BSP-bearing Setups (the AP-156 fix review corrected the originally-recorded '170 of 172'), because `BuildFloodSpheres` carried each physics-BSP part's root bounding-sphere RADIUS while discarding that sphere's own ORIGIN and centring it on the part origin. That is the #98/#168 class, and for 43 Setups the post-AP-152 flood was strictly smaller than the pre-AP-152 one. AP-156 records the correction and the fix — `ShadowShape.BoundsCenter`, filled from the same resolver that supplies the radius, plus the retirement of the 10-sphere clamp on a branch where retail has none — and keeps open only the sphere-vs-portal TRAVERSAL approximation. AP-157 is the previously unregistered third-branch substitution: retail floods from one `CPartArray::GetSortingSphere` where acdream floods from every Sphere shape, and acdream's cylinder flood ignores `CylHeight`. AP-152 RETIRED 2026-08-06, one day after it was filed: `ShadowShapeBuilder.FromSetup` now dispatches BSP-first instead of unioning, and `ShadowObjectRegistry.BuildFloodSpheres` now applies `calc_cross_cells`' own BSP → cylsphere → sorting-sphere order. Four statements in the row were false and are corrected in its retirement text — most importantly its predicted symptom, "catching on a doorway sill", which could not have been occurring: `Transition.BspOnlyDispatch` had already made the extra primitive inert at collision-query time since 2026-05-25. The live half was CELL MEMBERSHIP, the #98/#168 symptom class, which had no such guard. AP-153/AP-154/AP-155 filed at that retirement — retail's dispatch flag is cached once at part-array construction where acdream's gate is live [AP-153]; acdream's query-time guard takes a CLIENT-DERIVED flag off the WIRE and never derives it, an undeclared dependency on ACE reading the same DAT bit [AP-154]; and the static publication paths emit a Setup Sphere as a height-capped Cylinder while `BuildFloodSpheres` approximates retail's bounding BOX with bounding SPHERES [AP-155, whose flood-priority half is closed by the same commit]. AP-152 filed 2026-08-06 at the AP-22 retirement — the LIVE collision path emits Setup primitives and per-part physics-BSP shapes additively where retail's `CPhysicsObj::FindObjCollisions` dispatches exclusively; 172 of 5,935 installed Setups are affected, including BSP doors, so it needs its own visual gate and was deliberately not folded into the AP-22 commit; the count is unchanged because AP-22 retired in the same commit. AP-22 RETIRED 2026-08-06 — retail synthesizes no shape for a shapeless object (`CPhysicsObj::FindObjCollisions` 0x0050f050 exits at `0x0050f22f je 0x50f31b` returning the seeded OK_TS, and `CPartArray::GetRadius`/`GetHeight` are absent from its whole call set), so the invented `setup.Radius` cylinder was deleted rather than re-derived; the row's site list named one file that never contained the fallback and omitted the two that did, one of them the headless-only copy, and its "rare decorative props" risk described an unreachable branch — 0 of 5,935 installed Setups can satisfy the guard. AP-150/AP-151 filed 2026-08-06 at the #280 dual review — the wait cue's five-second arming is acdream's own and not retail's trigger [AP-150], and the reveal gate is materially stricter than retail's DAT-residency prefetch predicate on the mesh-build/GPU-upload axis [AP-151], the opposite asymmetry from AP-149; AP-149 filed 2026-08-05 at the #280 portal-prefetch fix — the reveal gate's outer ring accepts terrain-only publication where retail requires LandBlockInfo and every building EnvCell; the fix closes the reveal-window/visible-window ratio, not this residual; AP-148 filed 2026-08-05 at the C5b closeout — acdream's local-player Gate A requires the wire TELEPORT_TS to be EQUAL where retail requires only that it not be OLDER, verified by disassembly against the PDB-paired binary after two review rounds read the Binary Ninja tautology and missed it; AP-147 filed 2026-08-05 at the C5b architecture review, finding D3 — the accepted-Position delta stream's cardinality change and its torn intermediate; AP-138 amended at the same review — C5b staled its route-2 first-submit `CurrentCellId` measurement; AP-131 RETIRED 2026-08-05, C5b, closing #275 — the steady-state merge's `installPlacementFrame: true, clearParent: true` literals no longer exist; `InboundPhysicsStateController.TryApplyPosition` now computes both flags PRE-MERGE from `(disposition, hasAnimations(old))`, which is exactly `RuntimeAuthoritativePositionRouteClassifier.ClassifyAcceptedPosition`'s own `ApplyPlacementFrameBeforeRouting`/`UnparentBeforeRouting` rows (false/false on the Gate A force row, `!HasAnimations`/true on every accepted non-force route). Retail decides both writes BEFORE `MoveOrTeleport` is consulted — Gate A @0x0045400C returns @0x0045409D ahead of `unset_parent` @0x00454129 and the `HasAnims` `SetPlacementFrame` gate @0x00454137 — so the flags need no route, no player distance and no signature change. The row's predicted symptoms are gone: an animated entity's ordinary Position no longer installs a placement frame retail skips, and a ForcePosition no longer unparents. Evidence: `InboundPhysicsStateControllerTests` — `ApplyOnAnimatedEntity_NeverInstallsTheWirePlacementFrame`, `ApplyOnNonAnimatedEntity_InstallsTheWirePlacementFrame`, `ForcePositionOnParentedLocalPlayer_RetainsTheParentAttachment`, and the 12-row `MergedPrePlacementFieldsMatchTheClassifiedRouteFlags` matrix which uses the production classifier as its oracle rather than re-encoding the table; all four sabotage-verified in both directions. The row's "the legacy caller is deleted at the production cutover" framing was overtaken: the caller was CORRECTED, not deleted, and remains the only production Position wire caller; AP-145 RETIRED 2026-08-05, C5a commit 1, closing #318 — `TryPublishPlace` now publishes the local player's Place through `LocalPlayerShadowSynchronizer.SyncPose`, the same publisher ordinary per-tick movement uses, instead of a direct `LocalPlayerShadowState.Set` that never touched `PhysicsEngine.ShadowObjects`; AP-1 RETIRED 2026-08-05, C5a deletion sweep — `PhysicsEngine.Resolve`/`ResolvePlacement`/`HasCellSurface` deleted outright, zero production callers, so "production zero-delta routes remain on the legacy resolver" is now structurally false; AP-146 filed 2026-08-05, #319 fix — the local player's canonical cell is written only at login/inbound-Position/teleport, not per ordinary-movement tick as retail's SetPositionInternal does; #319's fix makes a player-parented child inherit exactly this coarseness, stale-but-equal to the parent, not a new staleness class; follow-up filed as issue #320; AP-144 filed 2026-08-05, C4 route 3 round 3 (R7) — the portal-arrival movement-event send reuses `UsePositionFromServer` (`autonomy_level != 2`) where retail's actual gate, `SendMovementEvent`, is `autonomy_level != 0`; the two agree everywhere except level 1, which no production caller can reach today; AP-142/AP-143 filed 2026-08-04, C4 route 7 — the parented-child single-field cell model (id/pointer collapse, zero-not-stale removal propagation, same-cell tick-loop subsumption) and the headless parent-realize drive's skipped holding-location validation; AP-141 filed 2026-08-04, C4 route 5, NARROWED 2026-08-04 at the round-2 delta review — the far-branch StopInterpolating clause was wrong for the adopted-body case (it is now ported there) and the row's language now distinguishes "never armed" from "never re-anchored"; CORRECTED 2026-08-04 at the round-3 delta review — the risk column's "would drag the body toward a stale anchor" claim was itself wrong (the leash anchor is write-only; `ConstraintManager::adjust_offset` only brakes, never pulls) and is retracted; every half remains test-gated only, since ACE never sends a missile UpdatePosition; AP-140 filed AND RETIRED 2026-08-04 — filed at the Bug B Opus review because the two accepted-Position routing gates read the client `Airborne` flag, i.e. walkability, where retail's free-flight predicate is CONTACT, and Bug B had just turned "in contact, not on walkable ground" from unreachable into ordinary; retired the same day by pointing both gates at `PhysicsBody.InContact`, retail's literal `transient_state & 1` test at `InterpolationManager::adjust_offset` @0x00555D52 (bit 0 = `CONTACT_TS`, acclient.h:3690), while leaving `Airborne` and all five of its `!Body.OnWalkable` writers untouched — the narrow shape the row itself pinned. A remote sliding on a steep face now interpolates as retail does instead of snapping at UpdatePosition cadence; AP-139 filed 2026-08-04, Bug B remote steep-contact slide — the interpolation-queue clear on the landing edge, carried over from the deleted hand-rolled remote landing block; AP-81 narrowed the same day by that fix, which retired its whole GRAVITY half; AP-87 annotated the same day — its predicted symptom was observed live and then fixed at the source, with the row's own thresholds and conditions deliberately unchanged; AP-138 filed 2026-08-04, C4 route 4b-2 dual Opus review, parts (1) and (2) rewritten the same day at the DELTA review — the far snap's refusable-placement residual: store_position only on the outcomes that never reached the engine, the two quiescence parks made restorable at the source, with the rollback gated on the cell it actually restores into, rather than refused by a pre-flight that structurally cannot see them, and the leash not armed through a superseded incarnation; AP-137 filed 2026-08-04, C4 route 4b-2 and rewritten the same day at that review, `teleport_hook`'s call list completed at the delta review — the acdream-only null/rejected/cell-less leftover arm, what the deleted duplicated 96 m/4 m constant pairs actually computed, and the vacuous headless satisfaction; AP-136 filed 2026-08-04, C4 route 4b-1 review, NARROWED 2026-08-04 at the C4 route 4b-2 delta review and AMENDED 2026-08-04 by the cancelled-park presentation rollback (the row's "restored visible" claim covered only the CANONICAL half; the presentation half was never rolled back, which left a parked-then-cancelled remote that stops moving invisible in the world AND absent from the radar for the rest of the session — a defect, now fixed by the `WithdrawalRestored` receipt, with the selection residual filed as AD-63) — a cancelled lost-cell park re-shows the entity where retail keeps it hidden until cell load, and the rollback's scope now covers the two placement-side quiescence parks whenever the cell it restores into is not itself quiescing — round 4 (2026-08-04) applies that same test a second time at RESTORE time, because a retained park's rollback lands a packet later; AP-135 filed 2026-08-03, C4 route 4a — the airborne no-op's retained acdream bookkeeping; the stated total was 2 rows stale before that filing and is now a literal count of this section; AP-130/AP-131/AP-132 filed 2026-08-02, continuation-executor slice; AP-5 retired 2026-07-31 at Campaign P Slice 2A — every successful `step_down` now performs retail's final `PLACEMENT_INSERT`; AP-3/AP-4 retired 2026-07-31 at Campaign P Slice 1B — `transitional_insert` and `edge_slide` now preserve retail's valid-contact early return and Branch-1-first order; AP-127 retired 2026-07-31 by #268 — the complete augmentation chain is shared by character UI and Runtime movement; AP-30 retired 2026-07-30 by the movement parity audit — retail Frame::is_equal genuinely uses the 0.0002 epsilon [byte-confirmed], so the row recorded a NON-divergence; acdream already matches; AP-129 narrowed 2026-07-30 at the P4 Opus review fix — `CanMoveInto`/`RestrictionDB::IsAllowedIn` are now ported and fed end-to-end (CreateObject HouseOwner/HouseRestrictions/Monarch tail fields + live `House_UpdateRestrictions 0x0248`, resolved through `PhysicsEngine.Objects`), retiring the original "CanMoveInto entirely unmodeled, unconditional fail-closed" gap the row described — the review was triggered by `RestrictionObjPrevalenceInspectionTests` showing 103,766 of 729,888 installed EnvCells (the whole housing estate) carry a baked `RestrictionObj`, so the unconditional fail-closed default would have locked every house for every player including its own owner; AP-10 retired 2026-07-30 at Campaign P Slice P4 — restored retail's 0.1 m dry-corner water sink-in, full suite green proving the sticky-bit no-regression argument; AP-71 retired same slice — `check_entry_restrictions` ported at the head of the indoor `FindEnvCollisions` branch, `CellPhysics.RestrictionObj` wired from the DAT-baked `EnvCell` field in both the dev and production caching paths; AP-128 filed 2026-07-30 at the P3 Opus review — PK-timer clock basis; AP-25 retired 2026-07-30 at Campaign P Slice P1 — the vitae/enchantment-aware run/jump skill chain; AP-7 retired 2026-07-30 at Campaign P Slice P2 — `calc_friction`'s threshold ported to retail's confirmed 0.25f; its still-open cos(10°)-vs-0.99999536f Sledding constant question moved to AD-55) Wave-0 UI ledger repair (2026-07-10) retired stale AP-38, resolved the AP-84 collision, restored overwritten paperdoll rows as AP-92/AP-93, and registered @@ -183,9 +183,11 @@ AP-94..AP-112 for the confirmed retail-UI completion gaps. | # | Divergence | Where (file:line) | Why it is safe / justified | Risk if assumption breaks | Retail oracle | |---|---|---|---|---|---| -| 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`; `PlayerModule::SetPlayerOption @0x00486e80`; `PlayerModule::PlayerModule @0x005D51F0`; `docs/research/2026-08-10-set-character-options-wire.md` §8.2 | +| 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-195 | **Filed 2026-08-11 (Campaign OP OP2 re-review closure; supersedes the ported half of retired TS-72).** `UIOption_CheckboxBitfield64` ports HALF of retail's `Refresh @0x004859C0`: the ANY-set predicate driving each row's checkbox bool is exact, but the same retail pass also computes the ALL-set predicate to swap each row's LED media between the two surfaces authored on template consumer `0x10000520` (`P0x10000082 = 0x06004D17` checked-art / `P0x10000083 = 0x06004D19` unchecked-art) — acdream reads neither property anywhere. Retail's `CreateChildren` tail also self-sizes the block (`ResizeTo(GetWidth(), CalculatePaperSize(0, -1))`) because `UIOption_CheckboxBitfield64` IS a `UIElement_ListBox` (its `PostInit` tail-calls `UIElement_Scrollable::PostInit`); acdream's block instead keeps its authored 272×100 extent while stacking rows itself — a second, divergent implementation of the row-stacking mechanism beside `UiTemplateListBox` (which stacks into a lazy `UiScrollablePanel`). The row-index attribute stamp (`SetAttribute_Int(cb, 0x10000084, i)` @`0x00485E3E`) is deliberately replaced by a typed mask closure — equivalent click routing without the attribute round-trip; NOT part of this row's gap. `src/AcDream.App/UI/UiCheckboxBitfield64.cs` (`IsSet`/`AddChild`) | The checkbox bool — the half every wire-visible behavior flows through — is exact and conformance-tested (multi-bit discriminating test); the LED media swap and self-sizing are presentation-only, invisible until the Chat tab actually mounts a bitfield block, and porting them properly wants the ListBox unification (reuse `UiTemplateListBox`'s stacking or record why not) rather than a third stacking path bolted on in a closure commit | The Chat tab's five filter blocks (the only authored bitfield consumers, 12/13 rows each) would render rows whose LED art never switches between checked/unchecked surfaces and whose block height stays the authored 272×100 instead of growing to fit 13 rows (~260 px + chrome) — visibly wrong the moment OP5 mounts them; both gaps MUST close (or this row be consciously re-scoped) in OP5 before its connected gate | `UIOption_CheckboxBitfield64::Refresh @0x004859C0`; `CreateChildren @0x00485DF0`; `ListenToElementMessage @0x00485AE0` (`BitUtils::SetBitsOnOrOff`); `docs/research/2026-08-11-op2-rereview-mechanism.md` §2.1–§2.3 | | 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 | +| AP-196 | **Filed 2026-08-11 at the OP4 review-fix round (MUST-FIX 3 / blast M2).** OP4's Group-C re-point (`AutoTarget`/`AutoRepeatAttack`/`ViewCombatTarget`/`VividTargetingIndicator`/`CoordinatesOnRadar`/`LockUI`/`AcceptLootPermits`/`ToggleRun`) moved these options' EFFECTIVE default source from the client-local `GameplaySettings.Default` record (or a persisted `settings.json`) to the retail constructor word, read live through `RuntimeCharacterOptionsState`/`CharacterOptionTable.ClientDefault` — a fresh install's observable default for `ViewCombatTarget` FLIPS from `true` (the old `GameplaySettings.Default`) to `false` (retail's real `CharacterOptions1` default — bit `0x80` is clear in `0x50C4A54A`). This fix round additionally DELETED the three now-fully-orphaned `GameplaySettings` fields (`AutoTarget`/`AutoRepeatAttack`/`ViewCombatTarget`) and their `RuntimeSettingsController` mirror properties/`SetCombatGameplay` method outright — the Combat panel's own three LEDs (`CombatUiController`) now read/write the SAME `RuntimeCharacterOptionsState` seam the Character tab's rows use, closing the "two writable copies" divergence the blast review found. The other five re-pointed options remain present in `GameplaySettings` as WRITE-BEHIND persistence/draft mirrors (`settings.json` still records the last-known value for restart continuity and Settings-panel draft consistency) — but the AUTHORITATIVE read for gameplay behavior is always the live server bit. | `src/AcDream.UI.Abstractions/Panels/Settings/GameplaySettings.cs`; `src/AcDream.App/Settings/RuntimeSettingsController.cs`; `src/AcDream.App/UI/Layout/CombatUiController.cs`; `src/AcDream.App/Combat/LiveCombatAttackOperations.cs` (`CharacterOptionCombatSettingsSource`) | Server-authoritative reads matching CH3's established precedent are strictly more correct than a client-local snapshot that can silently diverge from the character's real server state; the three deleted fields had zero remaining production consumers once the Combat panel's LEDs were re-pointed, so deleting rather than deprecating avoids a permanently-dead second store. | A fresh install (or a character whose `settings.json` predates this change) shows `ViewCombatTarget` OFF by default where it previously showed ON — a deliberate correctness fix, not a regression, if a future report cites "combat camera doesn't track by default." Any future code that still expects `GameplaySettings` to carry `AutoTarget`/`AutoRepeatAttack`/`ViewCombatTarget` fails to compile — a forcing function, not a silent gap. | `CharacterOptions1.Default = 0x50C4A54A` (`PlayerModule::PlayerModule @0x005D51F0`); `CharacterOptionTable.cs`'s `ClientDefault` column; `docs/research/2026-08-11-op4-review-blast.md` §S3 | +| AP-197 | **Filed 2026-08-11 at the OP4 review-fix round (SF-1/S4).** "Display Timestamps" hardcodes retail's `PlayerModule` constructor-default format string `"%#H:%M:%S "` rather than reading the PER-CHARACTER override `GenericQualitiesData::InqString(m_pPlayerOptionsData, 1, &m_TimeStampFormat)` carries when the wire's `GenericQualitiesData` string-key `1` is populated — acdream's `PlayerDescription` parser reads and discards that field (wire research doc: "timestamp string (`0x80`) \| read, discarded \| ❌ \| never sent"). | `src/AcDream.Core/Chat/ChatLog.cs` (`FormatTimestampPrefix`); parser site cited at `docs/research/2026-08-10-set-character-options-wire.md:647` | The 2013 client's own constructor default is the only format any fresh/default character would ever show — retail ships no options-panel control that authors a custom one — so hardcoding the one value every real player sees is a safe, honest approximation until a consumer needs the per-character override. | A character whose account somehow carries a non-default persisted timestamp format (a modded/legacy server, or a hypothetical later retail patch exposing a UI for it) sees acdream render the DEFAULT format instead of their stored one — cosmetic only (still a valid H:MM:SS-shaped timestamp), never a wire or data-loss risk. | `PlayerModule::PlayerModule @0x005D51F0` (ctor default literal); `GenericQualitiesData::InqString` call site (wire doc §3.3); `docs/research/2026-08-10-set-character-options-wire.md` U6 | | AP-172 | **Filed 2026-08-08 (#354 fix — spell-bar drag reorder).** Retail removes a lifted favorite from `PlayerModule` (+ UI list + wire) the instant a drag starts and the remaining shortcuts visibly slide left to close the gap for the rest of the gesture (`RecvNotice_ItemListBeginDrag` → `RemoveSpellFromMenu`, live). acdream's controller performs the same PlayerModule/wire removal at drag-begin but DEFERS the whole favorite-list's visual rebuild until the drag concludes (drop or off-bar release) — the lifted cell's icon stays visible in its old slot and siblings do not slide until release, instead of reflowing continuously through the gesture. `DropFavorite` compensates by porting retail's own `AddFavorite`-side index adjustment (decrement the target index by one when the lifted item's original index was before it) against the now-intentionally-stale sibling numbering, so the FINAL landed position is byte-identical to retail's in every case exercised (`DragFavoriteOntoAnotherSlot_ThroughTheRealPointerPipeline_ReordersAndSyncsWire`). **NARROWED + CORRECTED 2026-08-08 (drop-ring change).** Correction: this row originally claimed empty-tail-slot drops were "already-live-count-relative and are untouched" — false. The #354 `-1` adjustment sat inside `DropFavorite`, which the empty-cell path also calls, so its live-count-clamped (post-lift-numbered) index was double-corrected: lifting a non-last favorite onto the empty tail landed it second-to-last instead of last. `FavoriteDropIndex` is now THE one landing computation and applies retail's rule exactly — the `-1` is gated on the lifted spell's pre-lift-numbered removal site (retail's `RemoveSpellFromMenu`-return-gated decrement @0x004C7157), which for a live-numbered empty-tail target is retail's `RemoveSpellFromMenu == -1` no-adjustment case (test `SpellFavoriteDrag_DroppedOnTheEmptyTail_AppendsAtTheEnd` fails against the double-correcting code). Narrowing: the mid-drag presentation now includes retail's authored drag-over Accept ring — `SpellCastSubMenu::OnItemListDragOver` @0x004C5990 setting the per-cell authored DragAccept child (element 0x1000045A, `UIElement_UIItem::PostInit` @0x004E1870) to `ItemSlot_DragOver_Accept` (UIStateId 0x10000040 → authored art 0x060011F9) on the hovered cell while a spell drag is live, cleared on leave/drop (`UiCatalogSlot.DragOverAcceptance` → `UiItemSlot.DrawDragAcceptOverlay`), with the ring and the drop sharing `FavoriteDropIndex` so the ring cannot promise a different landing. | `src/AcDream.App/UI/Layout/SpellcastingUiController.cs` (`BeginFavoriteDrag`, `EndFavoriteDrag`, `DropFavorite`, `Tick` — the `_favoriteDragActive` gate) | `UiRoot`'s subtree-removal safety net (`ClearSubtreeOwnership`, `UiRoot.cs:240-247`) cancels any in-flight drag whose source widget is destroyed, and `Rebuild()` tears down and recreates every cell in the list (`UiItemList.Flush` → `RemoveChild` per cell) rather than incrementally diffing. Left unguarded, the press-time removal's `SpellbookChanged` event would let the very next per-frame `Tick()` (production drives this unconditionally via `RetailUiRuntime.Tick`) destroy the cell driving the gesture and silently cancel the reorder before the user could complete the drop — this was the reported bug. Deferring the rebuild for the gesture's duration is the minimal fix that does not touch the shared `UiRoot` drag machinery every other panel (toolbar/inventory/vendor/paperdoll) also depends on. | A future rewrite that makes `Rebuild()` an incremental per-cell diff (add/remove/reflow one cell) instead of flush-and-recreate-all would make this deferral unnecessary and should retire this row along with it — until then, a player watching their OWN spell bar mid-drag sees the vacated slot's icon linger and siblings snap into place only on release, rather than reflowing live as retail does — and one ring consequence of that frozen bar: when dragging rightward past the source, the Accept ring's SCREEN slot sits one cell right of where the icon finally lands (retail's live-reflowed bar makes them coincide); the ring is on the correct CELL in both — the spell lands immediately before that cell's spell, retail's exact insert-before semantic. No effect on final position, the wire pair sent, or any other panel; cross-window spellbook→favorite drops are live-count-relative and untouched (the empty-tail claim this sentence used to carry was corrected 2026-08-08 — see the Divergence column). | `gmSpellcastingUI::RecvNotice_ItemListBeginDrag` @0x004C7360 (`SpellCastSubMenu::RemoveSpellFromMenu`, immediate live-list removal at lift); `SpellCastSubMenu::AddFavorite` @0x004C7060 (`RemoveSpellFromMenu`'s return value gating the `-1`-if-lifted-before-target `m_numSpells` adjustment before `ItemList_InsertSpellShortcut`); `PlayerModule::AddSpellFavorite` @0x005D43E0 (`InsertPos`); `PlayerModule::RemoveSpellFavorite` @0x005D4910 | | AP-160 | **Filed 2026-08-07, Slice 5.3 (vendor browse lifecycle). CORRECTED AND EXTENDED 2026-08-07 at the Slice 5.3 review corrections (fixes 4/5).** **Correction (fix 4):** this row's own Retail-oracle citation originally grouped `WorldObject_Use.cs:50,57` under the SAME citation as `Vendor.CheckClose`/`GetCylinderDistance`, which read as if the `wo.UseRadius ?? 0.6f` fallback lived inside the close watcher. It does not: `WorldObject_Use.cs:50,57` is `WorldObject.IsWithinUseRadiusOf`, the APPROACH check ("how close you need to be to open the shop") — a wholly different method from `Vendor.CheckClose`, which reads `UseRadius` directly with no fallback of its own (`UseRadius` is `float?`; a nullable comparison against a null right operand is always `false`, so `CheckClose` never closes at all on an unauthored radius). `EnforceRange`'s own code comment carried the same mis-attribution and, worse, actually APPLIED that mis-borrowed 0.6f as its fallback; it now passes the raw authored `UseRadius` with no fallback of any kind (0 when absent/unauthored, matching retail's own memset-zero `PublicWeenieDesc::_useRadius` default — a plain `float` field, `acclient.h:37181`, no sentinel). Retail's own behavior for a radius-0 handler is exactly this: close on the very first nonzero-distance check. **Extension (fix 5):** the watcher reads the SERVER-ECHOED ACCEPTED position snapshot (`RuntimeEntityRecord.Snapshot.Position`), sampled once per advanced frame at the post-network-command-phase, not retail's continuous live-pose push (retail's own client simulates and renders every entity's pose every frame; `CPlayerSystem`'s range handler reads that live pose, never a periodically-echoed one). Between accepted-position updates the watcher's distance measurement is therefore up to one update-interval stale. The one BLIND WINDOW this staleness could open into a wrong in/out-of-range verdict — an in-session portal/teleport, where the player's and vendor's position snapshots can briefly sit in DIFFERENT landblock coordinate frames mid-transit — is closed unconditionally by this same review's fix 1b (`RuntimeWorldTransitState.HasPendingTeleportStart`/`IsTeleportActive` short-circuit the whole distance computation before it runs, closing the session instead of measuring across the transit), so the staleness itself never reaches that particular failure mode; it remains recorded here as a standing precision gap for the window fix 1b does NOT cover (ordinary out-of-transit movement between the same-generation position updates a slow network tick can leave briefly stale). **Original text:** The client-local vendor-panel distance watcher closes on PLAIN 3D center-to-center distance instead of retail/ACE's CYLINDER-GAP distance (both objects' own collision radius and height subtracted from the center distance before comparing to `UseRadius`). Retail: `gmVendorUI::OpenVendor` registers `CPlayerSystem::RegisterObjectRangeHandler` keyed to the vendor's own `PublicWeenieDesc._useRadius`; ACE's server-side belt-and-suspenders `Vendor.CheckClose` closes on `GetCylinderDistance(lastPlayer) > UseRadius`, i.e. `Position::cylinder_distance`/`Physics.Common.Position.CylinderDistance` with each side's real `GetRadius()`/`GetHeight()`. **NARROWED 2026-08-08 (vendor-verify gate): the watcher now measures retail's cylinder-gap via the ResolveObjectTableHost radii — the plain-center shortcut was self-closing sessions inside the walk-to-use acceptance band (opened at 4.29 m center vs authored radius 3, closed same frame). Residuals: heights pass 0, unresolvable hosts degrade to center distance (close-early only).** | `src/AcDream.Runtime/Gameplay/RuntimeVendorRangeQuery.cs` (`EnforceRange`) | `AcDream.Runtime` does not resolve a live per-entity collision radius/height for an arbitrary NPC outside the App-layer's Setup-cylinder resolver (`WorldSelectionQuery`'s `_setupCylinder`, App-only — out of Runtime's reach per the Core-structure rules, and `PhysicsBody`/`RuntimeEntityRecord` carry no radius/height field). Plain center distance is a well-defined, non-degenerate substitute (using `ObjectRangeMath.ObjectsInRange`'s existing `useRadii: false` branch rather than inventing a new metric) for a CLIENT-LOCAL UI convenience that never touches the wire or any authoritative state — closing the panel is not gated by, nor gates, anything server-visible. Reading the accepted-position snapshot rather than a continuously-integrated live pose is the same "Runtime has no live render-side pose, only the last accepted wire snapshot" constraint every other Runtime-side distance query in this codebase already accepts. | The panel can close up to (player radius + vendor radius) sooner than exact retail — typically well under a meter for a two-legged NPC — so a player standing exactly at the boundary of a large-radius vendor's `UseRadius` may see the panel close slightly earlier than retail would. No effect on any transaction, wire message, or authoritative state (Slice 6's buy/sell owns those). Retiring the cylinder-gap half requires a Runtime-owned per-entity collision radius/height source, which does not exist today; retiring the staleness half requires a continuously-updated live-pose source Runtime does not keep either. | `CPlayerSystem::RegisterObjectRangeHandler` pc:203677/0x004C4C34; `gmVendorUI::OnObjectRangeExit` pc:199486/0x004C02F0; ACE `Vendor.CheckClose`/`GetCylinderDistance` (`references/ACE/Source/ACE.Server/WorldObjects/Vendor.cs:322-367`) — a SEPARATE method, `WorldObject.IsWithinUseRadiusOf` (`WorldObject_Use.cs:44-52`), owns the unrelated `?? 0.6f` approach-check fallback; `acclient.h:37181` (`float _useRadius`, plain memset-zero field, no sentinel); `docs/research/2026-08-08-slice5-vendor-browse-research.md` §A.3/§B.1/§B.2 | | AP-141 | **Filed 2026-08-04, C4 route 5 (projectile authoritative placement); NARROWED 2026-08-04 at the round-2 delta review (B1/B2) — the far-branch clause was factually wrong for the adopted-body case and is corrected below.** Three related projectile-only shapes, all pinned by design (D-P4) rather than ported: (a) the near-`Interpolate` disposition is a NO-OP for a live missile, where retail would lazily build interpolation machinery (`InterpolateTo` @0x005163AF) for it; (b) the post-operation `ConstrainTo` @0x00454272 (`MakePositionManager` @0x00510523 then `PositionManager::ConstrainTo`) is never ARMED for a projectile — retail's single arming site has no kind test, so retail WOULD build a `PositionManager` on demand and arm a missile's leash on any nonzero `MoveOrTeleport` return; acdream never arms it on any disposition, including the adopted-body case (whose PRE-EXISTING leash the teleport/far branches now un-arm or clear queue state for, but never RE-anchor, per retail's post-operation `ConstrainTo`); (c) a null-classified or `Rejected*` accepted Position for a missile is swallowed (write nothing) rather than caught up through any remote-shaped policy. | `src/AcDream.Runtime/Session/RuntimeRemotePlacementDriveController.cs` (`ApplyAcceptedProjectilePosition`) | acdream deliberately does not construct an `EntityPhysicsHost`/`PositionManager`/`InterpolationManager` chain for a ballistic body — the route-5b split the C4 route 5 contract rejected. The context that makes this safe rather than merely convenient: ACE never sends `UpdatePosition` for a missile (`references/ACE/Source/ACE.Server/WorldObjects/WorldObject_Tick.cs:333-334`, `SendUpdatePosition()` commented out inside the `PhysicsState.Missile` branch at `:265`) — every half of this row is deterministic-test-gated only, never exercised against a real server. **The far branch's `StopInterpolating` skip is retail-faithful ONLY for a BARE missile** (no `RemoteMotion` — retail's own `position_manager != 0` guard @0x005163C9 skips it for a never-interpolated object, so acdream's skip is faithful by consequence there). For the ADOPTED-BODY case (`TryBind`'s shared-body branch: an ordinary remote whose Missile bit was set by a later State packet, still carrying its `RemoteMotion`), retail's guard IS satisfied and retail WOULD clear the queue — acdream now ports this (`route.StopInterpolating && record.RemoteMotion is RemoteMotion adopted → adopted.Interp.Clear()`), matching the teleport branch's equivalent `StopInterpolating` action inside `teleport_hook`. What remains divergent for the adopted case is the post-operation `ConstrainTo` re-anchor @0x00454272 — retail re-anchors an existing leash at the just-updated position on every nonzero return; acdream never arms/re-anchors it on any projectile disposition (clause (b)). | A future change that DOES give projectiles a `PositionManager` (or a headless/no-window remote-motion consumer that expects one) must re-decide this row rather than silently building the machinery ad hoc; until then, a live missile never shows an ARMED constraint leash and never catches up via the near/UnroutedCatchUp policy — both unreachable in play. An adopted-body missile's INHERITED leash (armed before it became a missile) is un-armed by the teleport hook, has its queue cleared by both teleport and far, but is never re-anchored at the new position by either — its brake accumulator (`ConstraintPosOffset`) is not reset to zero at each accepted Position the way retail's @0x00454272 re-anchor does. **Correction, round 3 (2026-08-04): the round-2 wording here — that a stale leash "would drag the body toward a stale anchor" — was wrong and is retracted.** `ConstraintManager.ConstraintPos` is write-only in both retail and the port (never read by `AdjustOffset`), and `ConstraintManager::adjust_offset` @0x00556180 only tapers or zeroes an already-composed per-tick offset while `InContact` — a leash brakes motion the interp/sticky chain already produced; it has no mechanism to move anything toward the anchor. The real residual is confined to one tick of un-reset brake accumulator, contact-gated, and it cannot move an airborne far-snapped missile at all (the clamp branch does not run while airborne). | `CPhysicsObj::MoveOrTeleport` 0x00516330 (`InterpolateTo` @0x005163AF, `IsMovingTo` @0x0050EB10 returning 0 without a `MovementManager`; far branch `StopInterpolating` @0x005163C9-@0x005163CB); `SmartBox::HandleReceivedPosition` 0x00453FD0 (`ConstrainTo` arming site @0x00454272); `CPhysicsObj::ConstrainTo` 0x00510520 (`MakePositionManager` @0x00510523); `ConstraintManager::adjust_offset` 0x00556180 (brake-only taper, write-only anchor); `WorldObject_Tick.cs:333-334`/`:265` (ACE never-sends evidence) | 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 7ba8db09..342b517d 100644 --- a/docs/research/2026-08-11-campaign-op-test-script.md +++ b/docs/research/2026-08-11-campaign-op-test-script.md @@ -219,11 +219,22 @@ TrySetOption` seam every other Options-panel consumer already uses. Relog and reopen the panel: the row should still read your new value (Apply flushed the `0x01A1` blob, ACE persisted it, the fresh `PlayerDescription` echoes it back). -8. **Toggle a batched row WITHOUT clicking Apply**, then relog. The row - should revert to its PRE-toggle value on reopen — an un-flushed - batched change never reached the wire (the 480 s auto-save timer is a - Runtime-level mechanism tested at OP1; do not wait 8 minutes for this - gate — just don't click Apply). +8. **Close the Options panel (F11) FIRST, then toggle a batched row + WITHOUT clicking Apply**, then relog. The row should revert to its + PRE-toggle value on reopen — an un-flushed batched change never + reached the wire (the 480 s auto-save timer is a Runtime-level + mechanism tested at OP1; do not wait 8 minutes for this gate — just + don't click Apply). **Closing the panel first is load-bearing, not + optional**: logout flushes the dirty blob (`CPlayerSystem:: + LogOffCharacter` calls `SaveToServer`), so "relog without Apply + reverts" only holds because reaching Exit Game requires switching to + the Gameplay tab first — retail's own `OnVisibilityChanged(false) -> + RestoreSavedValues` — which reverts the uncommitted edit BEFORE + logout's flush ever sees it. A user who force-quits the client (or any + route that skips hiding the Character page) will see the OPPOSITE: + the un-Applied toggle DOES persist, because logout still flushes + whatever is dirty at that moment. Both are retail-correct; this step + exercises the panel-close path specifically. ### Apply / Reset / Defaults semantics @@ -296,6 +307,27 @@ TrySetOption` seam every other Options-panel consumer already uses. 18. **Relog and confirm all six Group-C options above read their server-persisted value**, not a locally-cached default. +### Apply/Reset enable-gating (MUST-FIX 2, OP4 review-fix round) + +19. **On first open of the Character tab, Apply and Reset are greyed out** + (disabled — retail's `PostInit` runs `OnOptionChanged(0)` so the pair + starts disabled). **Click any LED and they light up** (enabled). + **Click Apply and they grey out again.** **Defaults is never greyed + out** — before a change, immediately after clicking it, or after + Apply — at any point in this sequence. + +### Combat panel — the SAME LEDs, a second surface (MUST-FIX 3 / blast M2) + +20. **Open the Combat window (its own toolbar button, not the Options + panel) and confirm its own Repeat Attacks / Auto Target / Keep in + View checkboxes match the Character tab's rows for the SAME three + options** (Automatically Repeat Attacks / Auto Target / Keep Combat + Targets in View) — both surfaces now read the identical server bit, + so toggling one and reopening/refreshing the other must show the + SAME state. Before this fix round the Combat panel read a separate, + disconnected client-local copy that could silently disagree with the + Character tab and never reached the wire for two of the three. + ### What to report - Any row with a missing label/tooltip (note which one). diff --git a/src/AcDream.App/Combat/LiveCombatAttackOperations.cs b/src/AcDream.App/Combat/LiveCombatAttackOperations.cs index d2afcd23..ccac0219 100644 --- a/src/AcDream.App/Combat/LiveCombatAttackOperations.cs +++ b/src/AcDream.App/Combat/LiveCombatAttackOperations.cs @@ -3,7 +3,6 @@ using AcDream.App.Net; using AcDream.Core.Combat; using AcDream.Core.Net.Messages; using AcDream.Runtime.Gameplay; -using AcDream.UI.Abstractions.Panels.Settings; namespace AcDream.App.Combat; @@ -20,30 +19,28 @@ internal interface ICombatGameplaySettingsSource bool ViewCombatTarget { get; } } -internal sealed class GameplaySettingsState : ICombatGameplaySettingsSource -{ - public GameplaySettings Value { get; set; } = GameplaySettings.Default; - public bool AutoTarget => Value.AutoTarget; - public bool AutoRepeatAttack => Value.AutoRepeatAttack; - public bool ViewCombatTarget => Value.ViewCombatTarget; -} - /// -/// D7 Group-C re-point (Campaign OP slice OP4, 2026-08-11): -/// AutoTarget/AutoRepeatAttack/ViewCombatTarget move -/// from the client-local GameplaySettings record (what -/// and the legacy -/// RuntimeSettingsController read) to the canonical -/// server-authoritative — the -/// CH3 precedent (server bit is authoritative; the Character-tab panel -/// row's LED click writes THROUGH RuntimeCharacterOptionsState. -/// TrySetOption before this source can ever observe the new value, so -/// no separate reseed/sync step is needed here). Also closes two -/// previously-unfiled divergences (character-options-map.md §0): -/// AutoRepeatAttack and (via ClientCommandController's -/// /consent re-point, same commit) AcceptCorpseLootingPermissions -/// were client-local and never reached the wire even though retail -/// auto-saves both (0x0005 immediately). +/// D7 Group-C re-point (Campaign OP slice OP4, 2026-08-11), widened at the +/// OP4 review-fix round (2026-08-11, MUST-FIX 3 / blast M2): +/// AutoTarget/AutoRepeatAttack/ViewCombatTarget read +/// exclusively from the canonical server-authoritative +/// — the CH3 precedent (server +/// bit is authoritative; the Character-tab panel row's LED click writes +/// THROUGH RuntimeCharacterOptionsState.TrySetOption before this +/// source can ever observe the new value, so no separate reseed/sync step +/// is needed here). This is now the ONLY +/// implementation — the client-local GameplaySettings record's own +/// three same-named fields, the legacy RuntimeSettingsController +/// mirror properties, and the dead GameplaySettingsState adapter +/// class were all deleted the same round (register row AP-196); the +/// Combat panel's own three LEDs (CombatUiController) were +/// re-pointed to this SAME seam, closing the "two writable copies" +/// divergence the fix round found. Also closes two previously-unfiled +/// divergences (character-options-map.md §0): AutoRepeatAttack and +/// (via ClientCommandController's /consent re-point, same +/// commit) AcceptCorpseLootingPermissions were client-local and +/// never reached the wire even though retail auto-saves both +/// (0x0005 immediately). /// internal sealed class CharacterOptionCombatSettingsSource : ICombatGameplaySettingsSource { diff --git a/src/AcDream.App/Composition/InteractionRetainedUiComposition.cs b/src/AcDream.App/Composition/InteractionRetainedUiComposition.cs index 3c8dd92c..aebeaa4d 100644 --- a/src/AcDream.App/Composition/InteractionRetainedUiComposition.cs +++ b/src/AcDream.App/Composition/InteractionRetainedUiComposition.cs @@ -643,9 +643,7 @@ internal sealed class RetailInteractionRetainedUiCompositionFactory d.Settings.SetUiLocked), Combat: new CombatRuntimeBindings( d.Actions.Combat, - combatAttack, - () => d.Settings.Gameplay, - d.Settings.SetCombatGameplay), + combatAttack), Magic: new MagicRuntimeBindings( d.Character.Spellbook, magic.Casting, diff --git a/src/AcDream.App/Input/DispatcherMovementInputSource.cs b/src/AcDream.App/Input/DispatcherMovementInputSource.cs index 2ef5eb2c..4e196129 100644 --- a/src/AcDream.App/Input/DispatcherMovementInputSource.cs +++ b/src/AcDream.App/Input/DispatcherMovementInputSource.cs @@ -76,7 +76,9 @@ internal sealed class DispatcherMovementInputSource : IMovementInputSource // to walking — impossible in retail. // // Campaign OP slice OP4 (2026-08-11): `!walking` was a hardcoded - // "run by default" assumption. PlayerOption RunAsDefaultMovement + // "run by default" assumption. Retail PlayerOption id 0xA + // (acclient.h's ToggleRun_PlayerOption — ACE calls the same bit + // RunAsDefaultMovement; N7, OP4 review-fix round 2026-08-11) // (retail default ON, matching the prior hardcoded behavior // exactly) now supplies the default; the walk-mode modifier // still temporarily INVERTS whichever default is active, same diff --git a/src/AcDream.App/Net/LiveSessionRuntimeFactory.cs b/src/AcDream.App/Net/LiveSessionRuntimeFactory.cs index 48cc1a73..af545f03 100644 --- a/src/AcDream.App/Net/LiveSessionRuntimeFactory.cs +++ b/src/AcDream.App/Net/LiveSessionRuntimeFactory.cs @@ -320,8 +320,25 @@ internal sealed class LiveSessionRuntimeFactory // still diverge from server truth (e.g. an older save, or a // character whose allegiance/society changed), and the server // is always authoritative. + // + // OP4 review-fix round (2026-08-11, MF-1/blast M1): re-seed + // LockUI's visual push the SAME way — the radar's polled + // `GetOptionBit` lambda self-corrects every frame, but + // `host.Root.UiLocked` (InteractionRetainedUiComposition.cs) + // is a ONE-SHOT assignment taken pre-login, at composition + // time, from RuntimeCharacterOptionsState's constructor- + // default word. Routing the real value through SetUiLocked + // here — the exact seam ToggleUiLock/SaveGameplay already + // use to push RuntimeSettingsTargets.ApplyUiLock — converges + // the retained window lock to server truth on every fresh + // PlayerDescription, matching the radar's own convergence + // instead of only updating on the next manual /lockui toggle. OnCharacterOptionsChanged: (_, options2) => - _interaction.Settings.SyncChatFromServerOptions(options2)); + { + _interaction.Settings.SyncChatFromServerOptions(options2); + _interaction.Settings.SetUiLocked( + _domain.Character.Options.GetOptionBit(CharacterOptionId.LockUI)); + }); } private LiveSessionCommandBindings CreateCommandBindings( diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 9d688d4a..1760e50c 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -669,8 +669,10 @@ public sealed class GameWindow : _movementInput = new AcDream.App.Input.DispatcherMovementInputSource( _playerControllerSlot, _inputCapture); - // Campaign OP slice OP4 (2026-08-11): PlayerOption - // RunAsDefaultMovement, polled — see RuntimeLocalPlayerMovementState. + // Campaign OP slice OP4 (2026-08-11): retail PlayerOption id 0xA + // (acclient.h's ToggleRun_PlayerOption — ACE's name for the same + // bit is RunAsDefaultMovement; N7, OP4 review-fix round + // 2026-08-11), polled — see RuntimeLocalPlayerMovementState. // RunAsDefaultMovementSource's doc comment. Same one-time, // whole-lifetime bind as DisableDistanceFogSource above. _playerControllerSlot.RunAsDefaultMovementSource = () => diff --git a/src/AcDream.App/Rendering/Shaders/sky.frag b/src/AcDream.App/Rendering/Shaders/sky.frag index 4eb4836a..bff4f922 100644 --- a/src/AcDream.App/Rendering/Shaders/sky.frag +++ b/src/AcDream.App/Rendering/Shaders/sky.frag @@ -57,7 +57,16 @@ void main() { vec3 rgb = sampled.rgb * vTint; - if (uApplyFog > 0.5) { + // SHOULD-FIX S2/mech NOTE N-3 (OP4 review-fix round, 2026-08-11): + // PlayerOption DisableDistanceFog forces FogMode.Off (uFogParams.w + // == 0) — mesh_modern.frag/terrain_modern.frag both gate their own + // fog blend on this same word (`if (mode == 0) return lit;`), but the + // sky dome's blend here read only uApplyFog (the CPU per-submesh + // "is this layer foggable at all" flag) and never uFogParams.w, so + // toggling the option stopped terrain/objects fading into fog while + // the dome's horizon band kept blending toward fog color. + int fogMode = int(uFogParams.w); + if (uApplyFog > 0.5 && fogMode != 0) { const float SKY_FOG_FLOOR = 0.2; float skyFogFactor = max(vFogFactor, SKY_FOG_FLOOR); rgb = mix(uFogColor.rgb, rgb, skyFogFactor); diff --git a/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json b/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json index 3751771e..586be187 100644 --- a/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json +++ b/src/AcDream.App/Rendering/Shaders/spv/shaders.manifest.json @@ -92,7 +92,7 @@ }, { "stage": "frag", - "sourceSha256": "8de9a5d8f819d1abf893f134cf8ed9fa7edf937a353a74d2befab7b05a3ff700", + "sourceSha256": "2ddf210d69b0c4a3c0870eecfb0ccba2097d93b365729739402bfe91e3b120d0", "compiled": true } ] diff --git a/src/AcDream.App/Rendering/Shaders/spv/sky.frag.spv b/src/AcDream.App/Rendering/Shaders/spv/sky.frag.spv index 011bc700cd2e0fc465ea9bde77ff3f95341c52e1..c8b5e56719a0a8b2beca1ab42940b670a779a3a5 100644 GIT binary patch literal 3364 zcmZ9N%Wu?G6vuBnLlIh_Xv;&O&P}nIN{S*+0D!?&P`mrbarCj{ypBXG1aPzykish*4f3Y zr_W#TU?bp~bO#s*2f!4V2OThozxHdv43hRu;0nEk+*|3jbZeGpu~l}h0kixK-Ma=? zaii$2urpt|cEK4#?s&n~GB;ar=BwRY!I^{HLc#TA?qtE4n|4nZ+%1`Vj(vuHY-j(P zrYY|zlQZ5jupizZwO=c^sQr4u#eDr(a4}y$7hKHO zF9jF#^;^NMLpGP+3vPYpZWdgB=Kf)~@4j8XgE@xYLbfOBzH8^uTGcyQXG^!i$~URn zx|O#eSw{nG#oqkYy?=XJm&{4s`(3N|9r&B0cDs-V*wq`{uz}qrcPQ)rzPPrbZ(=u| zv4;8Un`i)IsBg|X8#VP;zc2b|S3)~sp50pN)SAdXRX)a6=Sf%GSdPWdYBe+Fx+R}= zW7gB{=q_uqFT2>i+n#A3Z(ZK|)%}e%ckkD>30(TO*yZfI{$uRg%ImK_p1Bb$_afVy z65eZm_W@^WC%V52+RBY!?YoyN{gjQlpWXbmF{ZljS}uC>K%pD!Ac&kF%xBgy!hZ_i zkKZE@HLl*Hclw#fB)c*7HxG5^y3C`E?o355k0Gjd zoYL}&-}y>!w>@{3F9E-4=$Db5z3_h@dHqkLhwa~o$mVRG)~Vjf+yN%K7uh%E((WdV zzhBe*bjNU!O}Xn0r|zO`#(@lAK!xi2YtKp`k0&l59QVU#tq;bIDwDfU=%cf zE4Vw68=wa) znz=`iwe{>zzS3jtuD^-x*`q+tZ>Jpb7Lc9k0Xk&PPXK#xH}K5)HeYA?NnoAMxc>I^ zDIg#HJe4_lSM)Q!%`Fh!9A8hTl?*IS* literal 3220 zcmZ9N&2N=e5Qk6iy%eDZTKN{J_qIxmt)(bme$d_`c&pxoYB7i*8Vod11V0cpnrI2S z^{-K5bi*Hji90tgT)A@LhKNQJpXYtgl#`z1*E91wGjnFo4`EuYGuN`q1GSJvXOj6+quD#A~sOD;M6n>cM7! zdwLK|frDTc^uT9e1b^+bE5^9!7WjWI_GJ9E^o zTX5bZce3EDNN%Cv%u%jiaONPlSa5@xJ5zAxq1~$mcVFhtv-jwMIoCBe({AK%Xqx<< zLf*S7x2k@Qp}TtY#onr0qkTHS?utHzZXPcC6x_W&)qch({nqwPKe@Gj=3n|nZ&%>s zKDm2-bq-B9^STAXF8Y1D;G*9@7hLrF*Mf_F|5k9(?>`D|1G4x2S#TROcemh%GWRdL z-^BXsZ`G>ic^ut-_#K?@4lsXpXWj3jy|tZSca`5kE_xjEDyNTjZ4kcA%+0VH-?_C1 zU7*b=c2^l+F5(+B>>~b1!A1Ob!Ns?jfwO+&>FfPH_VFA)bnEv%b>Dc*=~le->kz|x zly@MRe;e$?-fySwox9V99=Ud*Z(;X7 z@7m5^d*24$p+1^*Hfk8E-l#s>mC#O@XI)DxZAEszeAnKs?(d-7WWH-VvNLV2JMvk5 zXDdB`?y?s9vX9-^_DuVD>+;4|_czwujjwG7xb*iM$=P@PC)u@?*I#`qb7TApJLA?G z2>-QvU@uppEb>>|RGsX=T;#jY{u!$ZaqH7IV)YM%LD| z4mgXywe1Jq;rVI0V}Bn7^1@{t>oo_@9|OLVuZW>O4fK(>7U%g8aORzJ{mk_+kpBUj zqwL2r=dAxl7oTEx{e!({PXal6s~mCWke&NsDlqOmFvmxLXI)?(&fW{a`E(}rx8@gt zeAL^|oV+XQjozJx^Nzr9?$ctnwSHr`Yx_6(M&9msxYa)01ll+!`dZUG71*ycz&w2$ zzK5vw6=cu8#>*J5fpb9qM&`~V8{0E;vnLmTbxZ?0t59XMy7u)CZy&prln&Y5y|bfA*uqPEYGt=$#e7szt< hBe<2!HP{#Fne%o7=&$_>Fqc{K@b|>C|A=r5{12nUr)~fM diff --git a/src/AcDream.App/Settings/RuntimeSettingsController.cs b/src/AcDream.App/Settings/RuntimeSettingsController.cs index 5c461727..d1c5dd0d 100644 --- a/src/AcDream.App/Settings/RuntimeSettingsController.cs +++ b/src/AcDream.App/Settings/RuntimeSettingsController.cs @@ -1,4 +1,3 @@ -using AcDream.App.Combat; using AcDream.Core.Net.Messages; using AcDream.UI.Abstractions.Input; using AcDream.UI.Abstractions.Panels.Settings; @@ -168,8 +167,7 @@ internal interface IRuntimeSettingsPreviewSource /// are never constructed or disposed here. /// internal sealed class RuntimeSettingsController : - IRuntimeSettingsPreviewSource, - ICombatGameplaySettingsSource + IRuntimeSettingsPreviewSource { private const string DefaultToonKey = "default"; @@ -183,6 +181,11 @@ internal sealed class RuntimeSettingsController : private bool _startupAudioApplied; private bool _startupApplied; private bool _uiLockConverged = true; + // MUST-FIX 4 (OP4 review-fix round, 2026-08-11, blast M3): the last + // `locked` value actually pushed to `_runtimeTargets.ApplyUiLock` — + // the guard `SetUiLocked` compares against, decoupled from whatever + // `Gameplay.LockUI`'s own persisted/draft snapshot currently holds. + private bool? _lastAppliedUiLocked; public RuntimeSettingsController( IRuntimeSettingsStorage storage, @@ -233,12 +236,6 @@ internal sealed class RuntimeSettingsController : public AudioSettings AudioPreview => _viewModel?.AudioDraft ?? Audio; - public bool AutoTarget => Gameplay.AutoTarget; - - public bool AutoRepeatAttack => Gameplay.AutoRepeatAttack; - - public bool ViewCombatTarget => Gameplay.ViewCombatTarget; - public void ApplyStartup(IRuntimeSettingsStartupTarget target) { ArgumentNullException.ThrowIfNull(target); @@ -352,12 +349,22 @@ internal sealed class RuntimeSettingsController : public void SetUiLocked(bool locked) { - if (Gameplay.LockUI == locked && _uiLockConverged) + // MUST-FIX 4 (OP4 review-fix round, 2026-08-11, blast M3): the + // guard used to compare `locked` against `Gameplay.LockUI` — valid + // only while `ToggleUiLock` computed `locked` AS `!Gameplay.LockUI` + // (pre-OP4). OP4 re-pointed `ToggleUiLock` to derive `locked` from + // the SERVER bit (`RuntimeCharacterOptionsState.GetOptionBit`) + // instead, a DIFFERENT store that can already equal the persisted + // `Gameplay.LockUI` without this method ever having pushed that + // value to `_runtimeTargets` — the guard must compare against what + // was ACTUALLY applied, not a value from an unrelated store. + if (_lastAppliedUiLocked == locked && _uiLockConverged) return; _uiLockConverged = false; Gameplay = Gameplay with { LockUI = locked }; _runtimeTargets?.ApplyUiLock(locked); + _lastAppliedUiLocked = locked; _viewModel?.SetGameplay( _viewModel.GameplayDraft with { LockUI = locked }); @@ -412,35 +419,6 @@ internal sealed class RuntimeSettingsController : } } - public void SetCombatGameplay(GameplaySettings gameplay) - { - Gameplay = gameplay ?? throw new ArgumentNullException(nameof(gameplay)); - if (_viewModel is not null) - { - _viewModel.SetGameplay(_viewModel.GameplayDraft with - { - AutoTarget = gameplay.AutoTarget, - AutoRepeatAttack = gameplay.AutoRepeatAttack, - ViewCombatTarget = gameplay.ViewCombatTarget, - }); - } - - try - { - _storage.SaveGameplay(gameplay); - _viewModel?.ApplyExternalGameplayChange(current => current with - { - AutoTarget = gameplay.AutoTarget, - AutoRepeatAttack = gameplay.AutoRepeatAttack, - ViewCombatTarget = gameplay.ViewCombatTarget, - }); - } - catch (Exception ex) - { - _log($"settings: combat option save failed: {ex.Message}"); - } - } - /// /// Campaign OP slice OP3: the five client-local preferences the "Use /// Mouse Turning Settings" Gameplay-tab macro reads/writes. Read @@ -545,6 +523,7 @@ internal sealed class RuntimeSettingsController : Gameplay = gameplay; _uiLockConverged = false; _runtimeTargets?.ApplyUiLock(gameplay.LockUI); + _lastAppliedUiLocked = gameplay.LockUI; _uiLockConverged = true; _log($"settings: gameplay saved to {_storage.Location}"); } diff --git a/src/AcDream.App/UI/Layout/CharacterOptionsPageController.cs b/src/AcDream.App/UI/Layout/CharacterOptionsPageController.cs index 06e8dbe4..098e76a0 100644 --- a/src/AcDream.App/UI/Layout/CharacterOptionsPageController.cs +++ b/src/AcDream.App/UI/Layout/CharacterOptionsPageController.cs @@ -368,6 +368,11 @@ public static class CharacterOptionsPageController bool initial = bindings.CurrentValue(spec.Id); checkbox.Selected = initial; + // MUST-FIX 1 (OP4 review-fix round, 2026-08-11): `read` re-seeds + // this row from the LIVE option word on every OnShown (panel + // open, tab switch in) — retail's SaveCurrentValue/GetValue. + // `refresh` pushes the re-read value onto the checkbox WITHOUT + // going through `apply` (which would send it back over the wire). var row_ = new BoolOptionRow( initial, entry.ClientDefault, @@ -375,7 +380,9 @@ public static class CharacterOptionsPageController { checkbox.Selected = value; bindings.SetOption(spec.Id, value); - }); + }, + read: () => bindings.CurrentValue(spec.Id), + refresh: value => checkbox.Selected = value); page.Register(row_); checkbox.OnClick = () => row_.SetCurrentValue(checkbox.Selected); diff --git a/src/AcDream.App/UI/Layout/CombatUiController.cs b/src/AcDream.App/UI/Layout/CombatUiController.cs index f2413e0d..d04c5579 100644 --- a/src/AcDream.App/UI/Layout/CombatUiController.cs +++ b/src/AcDream.App/UI/Layout/CombatUiController.cs @@ -1,6 +1,6 @@ using AcDream.Core.Combat; +using AcDream.Core.Net.Messages; using AcDream.Runtime.Gameplay; -using AcDream.UI.Abstractions.Panels.Settings; namespace AcDream.App.UI.Layout; @@ -15,9 +15,26 @@ namespace AcDream.App.UI.Layout; /// RecvNotice_DesiredAttackPowerChanged (0x004CC110), /// ListenToElementMessage (0x004CC430), and /// RecvNotice_SetCombatMode (0x004CC620). +/// +/// +/// MUST-FIX 3 (OP4 review-fix round, 2026-08-11, blast M2): the three +/// LEDs (Repeat Attacks / Auto Target / Keep in View) read and write +/// through the SAME RuntimeCharacterOptionsState server-bit seam +/// the Character tab's rows for these same retail PlayerOptions +/// already use — retired the client-local GameplaySettings mirror +/// this panel used to read/write, which had silently diverged into a +/// second, disconnected copy of the same three options (register row +/// AP-196). +/// /// public sealed class CombatUiController : IRetainedPanelController { + /// The live read/write seam for the three combat LEDs — + /// identical shape to . + public sealed record Bindings( + Func CurrentValue, + Action SetOption); + public const uint LayoutId = 0x21000073u; public const uint BasicPanelId = 0x1000005Cu; public const uint SpellcastingPanelId = 0x10000061u; @@ -47,8 +64,7 @@ public sealed class CombatUiController : IRetainedPanelController private readonly UiButton _keepInView; private readonly CombatState _combat; private readonly RuntimeCombatAttackState _attacks; - private readonly Func _gameplay; - private readonly Action _setGameplay; + private readonly Bindings _bindings; private readonly Action _setWindowVisible; private bool _disposed; @@ -65,8 +81,7 @@ public sealed class CombatUiController : IRetainedPanelController UiButton keepInView, CombatState combat, RuntimeCombatAttackState attacks, - Func gameplay, - Action setGameplay, + Bindings bindings, CombatUiLabels labels, Action setWindowVisible) { @@ -82,8 +97,7 @@ public sealed class CombatUiController : IRetainedPanelController _keepInView = keepInView; _combat = combat; _attacks = attacks; - _gameplay = gameplay; - _setGameplay = setGameplay; + _bindings = bindings; _setWindowVisible = setWindowVisible; // Retail layout 0x21000073 contains two sibling pages: gmCombatUI's @@ -110,12 +124,18 @@ public sealed class CombatUiController : IRetainedPanelController SetStaticText(speedLabel, labels.Speed, rightAligned: false); SetStaticText(powerLabel, labels.Power, rightAligned: true); + // MUST-FIX 3 (OP4 review-fix round, 2026-08-11, blast M2): writes + // go through the SAME server-bit seam the Character tab's rows for + // these same retail PlayerOptions use — TrySetOption writes the + // local bit first, then either sends 0x0005 immediately + // (AutoRepeatAttack/AutoTarget are auto-save) or marks the batched + // module dirty (ViewCombatTarget is batched). _repeatAttacks.OnClick = () => - _setGameplay(_gameplay() with { AutoRepeatAttack = _repeatAttacks.Selected }); + _bindings.SetOption(CharacterOptionId.AutoRepeatAttack, _repeatAttacks.Selected); _autoTarget.OnClick = () => - _setGameplay(_gameplay() with { AutoTarget = _autoTarget.Selected }); + _bindings.SetOption(CharacterOptionId.AutoTarget, _autoTarget.Selected); _keepInView.OnClick = () => - _setGameplay(_gameplay() with { ViewCombatTarget = _keepInView.Selected }); + _bindings.SetOption(CharacterOptionId.ViewCombatTarget, _keepInView.Selected); _combat.CombatModeChanged += OnCombatModeChanged; _attacks.StateChanged += OnAttackStateChanged; @@ -126,16 +146,14 @@ public sealed class CombatUiController : IRetainedPanelController ImportedLayout layout, CombatState combat, RuntimeCombatAttackState attacks, - Func gameplay, - Action setGameplay, + Bindings bindings, CombatUiLabels labels, Action setWindowVisible) { ArgumentNullException.ThrowIfNull(layout); ArgumentNullException.ThrowIfNull(combat); ArgumentNullException.ThrowIfNull(attacks); - ArgumentNullException.ThrowIfNull(gameplay); - ArgumentNullException.ThrowIfNull(setGameplay); + ArgumentNullException.ThrowIfNull(bindings); ArgumentNullException.ThrowIfNull(labels); ArgumentNullException.ThrowIfNull(setWindowVisible); @@ -153,7 +171,7 @@ public sealed class CombatUiController : IRetainedPanelController return new CombatUiController( layout, basic, spellcasting, power, high, medium, low, repeatAttacks, autoTarget, keepInView, - combat, attacks, gameplay, setGameplay, labels, setWindowVisible); + combat, attacks, bindings, labels, setWindowVisible); } public void SyncVisibility() => OnCombatModeChanged(_combat.CurrentMode); @@ -188,10 +206,9 @@ public sealed class CombatUiController : IRetainedPanelController _high.Selected = _attacks.RequestedHeight == AttackHeight.High; _medium.Selected = _attacks.RequestedHeight == AttackHeight.Medium; _low.Selected = _attacks.RequestedHeight == AttackHeight.Low; - GameplaySettings gameplay = _gameplay(); - _repeatAttacks.Selected = gameplay.AutoRepeatAttack; - _autoTarget.Selected = gameplay.AutoTarget; - _keepInView.Selected = gameplay.ViewCombatTarget; + _repeatAttacks.Selected = _bindings.CurrentValue(CharacterOptionId.AutoRepeatAttack); + _autoTarget.Selected = _bindings.CurrentValue(CharacterOptionId.AutoTarget); + _keepInView.Selected = _bindings.CurrentValue(CharacterOptionId.ViewCombatTarget); } private static void SetStaticText(UiText? text, string value, bool rightAligned) diff --git a/src/AcDream.App/UI/Layout/OptionPageModel.cs b/src/AcDream.App/UI/Layout/OptionPageModel.cs index 9c72c03a..817e5f02 100644 --- a/src/AcDream.App/UI/Layout/OptionPageModel.cs +++ b/src/AcDream.App/UI/Layout/OptionPageModel.cs @@ -59,17 +59,43 @@ public interface IOptionRow public sealed class BoolOptionRow : IOptionRow { private readonly Action? _apply; + private readonly Func? _read; + private readonly Action? _refresh; private Action? _notifyPageOptionChanged; private bool _current; private bool _saved; private bool _default; - public BoolOptionRow(bool initial, bool defaultValue, Action? apply = null) + /// + /// MUST-FIX 1 (OP4 review-fix round, 2026-08-11 — converged mechanism + /// MF-1 / blast M1): retail's UIOption_Checkbox::GetValue + /// @0x00486f60PlayerModule::GetOption, the LIVE + /// server-synced option word, not a widget-local cache. Optional so + /// every pre-existing non-DAT-backed caller (the synthetic pages in + /// OptionPageModelTests) keeps working unchanged; when supplied, + /// re-reads through it instead of + /// trusting the row's own possibly-stale . + /// + /// + /// Retail's own Refresh() push of the re-read value onto the + /// widget (e.g. checkbox.Selected = value) — invoked ONLY from + /// 's re-read path, never through + /// , so a re-seed never sends the value back + /// out over the wire. + /// + public BoolOptionRow( + bool initial, + bool defaultValue, + Action? apply = null, + Func? read = null, + Action? refresh = null) { _current = initial; _saved = initial; _default = defaultValue; _apply = apply; + _read = read; + _refresh = refresh; } /// The live value — what the LED currently shows. @@ -106,7 +132,28 @@ public sealed class BoolOptionRow : IOptionRow public void AttachPageNotify(Action notify) => _notifyPageOptionChanged = notify; - public void SaveCurrentValue() => _saved = _current; + /// + /// MUST-FIX 1 (OP4 review-fix round, 2026-08-11): retail + /// SaveCurrentValue @0x004868E0 is m_current = GetValue(); + /// m_saved = m_current; — it re-reads the LIVE option word, not + /// just m_saved = m_current over whatever m_current + /// already held. calls + /// , which calls this on every row — + /// so every panel (re)open, tab switch in, and the initial default- + /// tab activation self-corrects this row from the CURRENT server + /// truth, exactly on retail's own schedule. When no read + /// delegate was supplied (the synthetic non-DAT-backed test pages), + /// this degrades to the pre-fix m_saved = m_current shape. + /// + public void SaveCurrentValue() + { + if (_read is not null) + { + _current = _read(); + _refresh?.Invoke(_current); + } + _saved = _current; + } public void RestoreSavedValue() { diff --git a/src/AcDream.App/UI/Layout/OptionsPanelController.cs b/src/AcDream.App/UI/Layout/OptionsPanelController.cs index c900b621..dec99cca 100644 --- a/src/AcDream.App/UI/Layout/OptionsPanelController.cs +++ b/src/AcDream.App/UI/Layout/OptionsPanelController.cs @@ -217,22 +217,47 @@ public sealed class OptionsPanelController : IRetainedPanelController UiElement? pageRoot = UiElement.FindDescendant(tabPanel, pageId); if (pageRoot is null) continue; - BindPageButton(pageRoot, ApplyButtonId, page.Apply); - BindPageButton(pageRoot, ResetButtonId, page.Reset); + UiButton? apply = BindPageButton(pageRoot, ApplyButtonId, page.Apply); + UiButton? reset = BindPageButton(pageRoot, ResetButtonId, page.Reset); BindPageButton(pageRoot, DefaultsButtonId, page.Defaults); + + // MUST-FIX 2 (OP4 review-fix round, 2026-08-11): retail + // PlayerOptionPage::OnOptionChanged @0x004F27D0 — Apply/Reset + // Ghosted (disabled) when the page has nothing to commit/ + // revert, Normal (enabled) otherwise; Defaults is NEVER gated + // (retail's override never fetches its child id at all). + if (apply is not null && reset is not null) + { + page.OnOptionChanged = () => + { + uint state = page.Changed + ? UiButtonStateMachine.Normal + : UiButtonStateMachine.Ghosted; + apply.TrySetRetailState(state); + reset.TrySetRetailState(state); + }; + // Retail's PostInit calls InitOptions() then + // OnOptionChanged(0) so the pair starts disabled — run the + // gate once now, at bind time, for the same effect. + page.OnOptionChanged(); + } } return controller; } - private static void BindPageButton(UiElement pageRoot, uint elementId, Action onClick) + private static UiButton? BindPageButton(UiElement pageRoot, uint elementId, Action onClick) { if (UiElement.FindDescendant(pageRoot, elementId) is UiButton button) + { button.OnClick = onClick; - else - Console.WriteLine( - $"[D.2b] OptionsPanelController: page 0x{pageRoot.DatElementId:X8}'s button " - + $"0x{elementId:X8} not found — its handler was not wired."); + return button; + } + + Console.WriteLine( + $"[D.2b] OptionsPanelController: page 0x{pageRoot.DatElementId:X8}'s button " + + $"0x{elementId:X8} not found — its handler was not wired."); + return null; } /// diff --git a/src/AcDream.App/UI/RetailUiRuntime.cs b/src/AcDream.App/UI/RetailUiRuntime.cs index 9110cbcf..7a60e5a5 100644 --- a/src/AcDream.App/UI/RetailUiRuntime.cs +++ b/src/AcDream.App/UI/RetailUiRuntime.cs @@ -67,11 +67,14 @@ public sealed record RadarRuntimeBindings( SelectionState Selection, Action SetUiLocked); +// MUST-FIX 3 (OP4 review-fix round, 2026-08-11, blast M2): the +// GameplaySettings read/write pair was removed — MountCombat wires the +// three LEDs through the same OptionsRuntimeBindings server-bit seam the +// Character tab's rows use (CombatUiController.Bindings), not a +// client-local settings mirror. public sealed record CombatRuntimeBindings( CombatState State, - RuntimeCombatAttackState Attacks, - Func Gameplay, - Action SetGameplay); + RuntimeCombatAttackState Attacks); public sealed record MagicRuntimeBindings( Spellbook Spellbook, @@ -1190,12 +1193,17 @@ public sealed class RetailUiRuntime : IDisposable } float combatWidth = RetailCombatLayout.FitFavoriteSlots(layout); + // MUST-FIX 3 (OP4 review-fix round, 2026-08-11, blast M2): the SAME + // server-bit seam MountOptionsPanel wires the Character tab's rows + // through — see CharacterOptionsPageController.Bindings below. CombatUiController? controller = Layout.CombatUiController.Bind( layout, _bindings.Combat.State, _bindings.Combat.Attacks, - _bindings.Combat.Gameplay, - _bindings.Combat.SetGameplay, + new Layout.CombatUiController.Bindings( + CurrentValue: id => _bindings.Options.CurrentCharacterOption((uint)id), + SetOption: (id, value) => _bindings.Options.CommandBus().Publish( + new SetSingleCharacterOptionRuntimeCmd((uint)id, value))), labels, visible => { diff --git a/src/AcDream.Core/Chat/ChatLog.cs b/src/AcDream.Core/Chat/ChatLog.cs index 574e79c5..ab779fbf 100644 --- a/src/AcDream.Core/Chat/ChatLog.cs +++ b/src/AcDream.Core/Chat/ChatLog.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Collections.Concurrent; +using System.Globalization; using System.Threading; namespace AcDream.Core.Chat; @@ -45,6 +46,27 @@ public sealed class ChatLog _maxEntries = maxEntries; } + /// + /// OP4 review-fix round (2026-08-11, SHOULD-FIX S1): retail + /// PlayerOption DisplayTimeStampsClientSystem:: + /// AddTextToScroll @0x00563C50 — prefixes EVERY transcript line + /// with the timestamp, not just the subset that happens to route + /// through RuntimeCommunicationState.AddText. Moved HERE + /// (from AddText) because is the ONE seam every + /// chat producer (, , + /// , , + /// , , + /// , , + /// , ) funnels + /// through — AddText's own callers (ServerMessage/WeenieError) were a + /// strict subset, so heard speech, emotes, Turbine channels, and + /// combat text never gained the prefix. The transient SpewBox + /// (RetailLogTextType.ClientLocal) never touches this class at + /// all, so it stays exempt automatically — matching retail's own + /// exemption without a special case here. + /// + public Func? DisplayTimestampsSource { get; set; } + /// Fires every time a new entry is appended. public event Action? EntryAppended; @@ -413,6 +435,14 @@ public sealed class ChatLog private void Append(ChatEntry entry) { + if (DisplayTimestampsSource?.Invoke() == true) + { + entry = entry with + { + Text = FormatTimestampPrefix() + entry.Text, + }; + } + _buffer.Enqueue(entry); while (_buffer.Count > _maxEntries) _buffer.TryDequeue(out _); @@ -420,6 +450,24 @@ public sealed class ChatLog EntryAppended?.Invoke(entry); } + /// + /// SF-1/S4 (OP4 review-fix round, 2026-08-11): retail's constructor + /// default "%#H:%M:%S " (PlayerModule::PlayerModule + /// @0x005D51F0, BN-sourced string literal — wire research doc U6, + /// NOT byte-verified) is non-zero-padded 24h hour, then zero-padded + /// minute:second, trailing space. H\:mm\:ss with the colons + /// ESCAPED (not the culture-dependent DateTimeFormatInfo. + /// TimeSeparator placeholder) plus is the exact .NET equivalent — the CRT's + /// strftime always emits a literal colon regardless of locale. + /// acdream hardcodes this constructor default rather than reading the + /// per-character override retail sources from + /// GenericQualitiesData::InqString(m_pPlayerOptionsData, 1, ...) + /// — see register row AP-197. + /// + private static string FormatTimestampPrefix() => + DateTime.Now.ToString(@"H\:mm\:ss ", CultureInfo.InvariantCulture); + public void Clear() { while (_buffer.TryDequeue(out _)) { /* drain */ } diff --git a/src/AcDream.Headless/Hosting/HeadlessGameplayOperations.cs b/src/AcDream.Headless/Hosting/HeadlessGameplayOperations.cs index 99bada0b..acb349a2 100644 --- a/src/AcDream.Headless/Hosting/HeadlessGameplayOperations.cs +++ b/src/AcDream.Headless/Hosting/HeadlessGameplayOperations.cs @@ -1,6 +1,7 @@ using AcDream.Core.Combat; using AcDream.Core.Items; using AcDream.Core.Net; +using AcDream.Core.Net.Messages; using AcDream.Core.Spells; using AcDream.Content; using AcDream.Runtime; @@ -124,8 +125,17 @@ internal sealed class HeadlessGameplayOperations runtime.ActionOwner.Combat.CurrentMode); } } - public bool AutoRepeatAttack => false; - public bool AutoTarget => true; + // S5 (OP4 review-fix round, 2026-08-11, blast M2): OP7 gives headless + // bots a real `characterOptions` config that writes these very bits + // (HeadlessCharacterOptionsSeeder) — reading the hardcoded values here + // instead of the live server bit meant a bot configured with + // AutoRepeatAttack ON would set the server bit while its own combat + // loop kept reading `false`. Read the SAME RuntimeCharacterOptionsState + // seam the graphical host's CharacterOptionCombatSettingsSource uses. + public bool AutoRepeatAttack => + RequireRuntime().CharacterOwner.Options.GetOptionBit(CharacterOptionId.AutoRepeatAttack); + public bool AutoTarget => + RequireRuntime().CharacterOwner.Options.GetOptionBit(CharacterOptionId.AutoTarget); public uint? SelectClosestTarget() { diff --git a/src/AcDream.Runtime/Gameplay/RuntimeCommunicationState.cs b/src/AcDream.Runtime/Gameplay/RuntimeCommunicationState.cs index 5010d24f..f5956573 100644 --- a/src/AcDream.Runtime/Gameplay/RuntimeCommunicationState.cs +++ b/src/AcDream.Runtime/Gameplay/RuntimeCommunicationState.cs @@ -100,16 +100,32 @@ public sealed class RuntimeCommunicationState : IDisposable /// DisplayTimeStamps — ClientSystem::AddTextToScroll /// @0x00563C50 (character-options-map.md §2.2, "Display /// Timestamps") prefixes each transcript line with - /// PlayerModule::m_TimeStampFormat (ctor default the byte- - /// verified CRT strftime string "%#H:%M:%S " — non-zero-padded - /// 24h hour, then zero-padded minute:second, trailing space) when the - /// option is on. Polled — see for why. Applied only to - /// the persisted transcript (matching retail's own - /// ClientLocal exemption in below — the - /// transient is never timestamped). + /// PlayerModule::m_TimeStampFormat (ctor default the + /// BN-sourced CRT strftime string "%#H:%M:%S " — wire research + /// doc U6, NOT byte-verified — non-zero-padded 24h hour, then + /// zero-padded minute:second, trailing space) when the option is on. + /// Polled — see for why. + /// + /// + /// SHOULD-FIX S1 (OP4 review-fix round, 2026-08-11): the actual + /// prefixing now happens inside 's own + /// Append — the ONE seam every chat producer funnels through, + /// not just this class's own callers (a strict + /// subset that missed heard speech, emotes, Turbine channels, and + /// combat text). This property forwards to + /// so existing callers + /// (GameWindow's one-time bind) keep working unchanged. The + /// transient never touches + /// at all, so it stays exempt automatically — matching retail's own + /// ClientLocal exemption without a special case here. + /// /// - public Func? DisplayTimestampsSource { get; set; } + public Func? DisplayTimestampsSource + { + get => Chat.DisplayTimestampsSource; + set => Chat.DisplayTimestampsSource = value; + } public ChatCommandTargetState CommandTargets { get; } public TurbineChatState TurbineChat { get; } @@ -216,14 +232,9 @@ public sealed class RuntimeCommunicationState : IDisposable return; } - // OP4: DisplayTimeStamps — see DisplayTimestampsSource's doc - // comment. Prefixed AFTER the trim above (retail's own order: - // AddTextToScroll trims first, then ClientSystem's caller-side - // timestamp prepend applies to the transcript line, never to the - // ClientLocal/SpewBox branch already returned above). - if (DisplayTimestampsSource?.Invoke() == true) - text = DateTime.Now.ToString("H:mm:ss ") + text; - + // S1 (OP4 review-fix round, 2026-08-11): the DisplayTimeStamps + // prefix now applies inside Chat's own Append — see + // DisplayTimestampsSource's doc comment. Chat.OnSystemMessage(text, (uint)type); } diff --git a/src/AcDream.Runtime/Gameplay/RuntimeLocalPlayerMovementState.cs b/src/AcDream.Runtime/Gameplay/RuntimeLocalPlayerMovementState.cs index ac8d8edd..9c39d299 100644 --- a/src/AcDream.Runtime/Gameplay/RuntimeLocalPlayerMovementState.cs +++ b/src/AcDream.Runtime/Gameplay/RuntimeLocalPlayerMovementState.cs @@ -145,8 +145,11 @@ public sealed class RuntimeLocalPlayerMovementState public bool AutoRunActive => _autoRunActive; /// - /// Campaign OP slice OP4 (2026-08-11): retail PlayerOption - /// RunAsDefaultMovement (ACCmdInterp::UITogglesRun) — + /// Campaign OP slice OP4 (2026-08-11): retail PlayerOption id + /// 0xAacclient.h's own enumerator spelling is + /// ToggleRun_PlayerOption; "RunAsDefaultMovement" is ACE's name + /// for the same bit, corrected here at the OP4 review-fix round + /// (2026-08-11, N7) — read via ACCmdInterp::UITogglesRun — /// whether an ordinary held movement key runs by default (retail's own /// client default: ) or walks by default, /// requiring the walk-mode modifier to be held to invert. Distinct diff --git a/src/AcDream.UI.Abstractions/Panels/Settings/GameplaySettings.cs b/src/AcDream.UI.Abstractions/Panels/Settings/GameplaySettings.cs index 86949e5f..4e9bda58 100644 --- a/src/AcDream.UI.Abstractions/Panels/Settings/GameplaySettings.cs +++ b/src/AcDream.UI.Abstractions/Panels/Settings/GameplaySettings.cs @@ -24,9 +24,16 @@ namespace AcDream.UI.Abstractions.Panels.Settings; /// public sealed record GameplaySettings( // CharacterOption (32-bit) subset — most-used gameplay toggles. - bool AutoTarget, // 0x2000 — combat: auto-acquire target on attack - bool AutoRepeatAttack, // 0x2 — combat: keep attacking after first hit - bool ViewCombatTarget, // 0x80 — keep the current combat target in view + // + // OP4 review-fix round (2026-08-11, MUST-FIX 3 / blast M2): AutoTarget, + // AutoRepeatAttack, and ViewCombatTarget were REMOVED from this record + // — the Combat panel's three LEDs now read/write the SAME canonical + // server-bit seam (RuntimeCharacterOptionsState via + // CharacterOptionCombatSettingsSource) the Character tab's rows for + // these same retail PlayerOptions already used, closing the + // "two writable copies" divergence (register row AP-196). This record + // remains the client-local persistence/draft store for every OTHER + // gameplay preference that has no such server-authoritative seam. bool ToggleRun, // 0x400 — run-mode is tap-once vs hold-to-run bool AdvancedCombatUI, // 0x1000 — show extra combat tooltips/panels bool ShowTooltips, // 0x100 — show item tooltips on hover @@ -46,9 +53,6 @@ public sealed record GameplaySettings( /// to retail's Default_CharacterOption = 0x50C4A54A + /// Default_CharacterOptions2 = 0x948700 — see class remarks. public static GameplaySettings Default { get; } = new( - AutoTarget: true, - AutoRepeatAttack: true, - ViewCombatTarget: true, ToggleRun: true, AdvancedCombatUI: false, ShowTooltips: true, diff --git a/src/AcDream.UI.Abstractions/Panels/Settings/SettingsPanel.cs b/src/AcDream.UI.Abstractions/Panels/Settings/SettingsPanel.cs index dcba4c88..5c0870af 100644 --- a/src/AcDream.UI.Abstractions/Panels/Settings/SettingsPanel.cs +++ b/src/AcDream.UI.Abstractions/Panels/Settings/SettingsPanel.cs @@ -299,17 +299,15 @@ public sealed class SettingsPanel : IPanel renderer.Text("Combat"); renderer.Separator(); - bool autoTarget = g.AutoTarget; - if (renderer.Checkbox("Auto-target on attack", ref autoTarget)) - _vm.SetGameplay(g with { AutoTarget = autoTarget }); - - bool autoRepeat = g.AutoRepeatAttack; - if (renderer.Checkbox("Auto-repeat attacks", ref autoRepeat)) - _vm.SetGameplay(g with { AutoRepeatAttack = autoRepeat }); - - bool viewCombatTarget = g.ViewCombatTarget; - if (renderer.Checkbox("Keep combat target in view", ref viewCombatTarget)) - _vm.SetGameplay(g with { ViewCombatTarget = viewCombatTarget }); + // OP4 review-fix round (2026-08-11, MUST-FIX 3 / blast M2): + // Auto-target/Auto-repeat/Keep-in-view were removed from + // GameplaySettings — they now read/write the canonical server bit + // (RuntimeCharacterOptionsState) through CombatUiController, not + // this client-local draft. This panel has no production + // construction site (D.2b's retained UiHost/UiRoot stack is the + // one presentation surface — see CLAUDE.md's UI strategy section); + // the three checkboxes are simply retired rather than re-pointed + // to a seam this dat-free ImGui-shaped panel has no way to reach. bool toggleRun = g.ToggleRun; if (renderer.Checkbox("Run mode is toggle (vs hold)", ref toggleRun)) diff --git a/src/AcDream.UI.Abstractions/Panels/Settings/SettingsStore.cs b/src/AcDream.UI.Abstractions/Panels/Settings/SettingsStore.cs index 904c4237..d8f38fd1 100644 --- a/src/AcDream.UI.Abstractions/Panels/Settings/SettingsStore.cs +++ b/src/AcDream.UI.Abstractions/Panels/Settings/SettingsStore.cs @@ -144,9 +144,6 @@ public sealed class SettingsStore var d = GameplaySettings.Default; return new GameplaySettings( - AutoTarget: ReadBool(gp, "autoTarget", d.AutoTarget), - AutoRepeatAttack: ReadBool(gp, "autoRepeatAttack", d.AutoRepeatAttack), - ViewCombatTarget: ReadBool(gp, "viewCombatTarget", d.ViewCombatTarget), ToggleRun: ReadBool(gp, "toggleRun", d.ToggleRun), AdvancedCombatUI: ReadBool(gp, "advancedCombatUI", d.AdvancedCombatUI), ShowTooltips: ReadBool(gp, "showTooltips", d.ShowTooltips), @@ -603,9 +600,6 @@ public sealed class SettingsStore ["advancedCombatUI"] = g.AdvancedCombatUI, ["acceptLootPermits"] = g.AcceptLootPermits, ["allowGive"] = g.AllowGive, - ["autoRepeatAttack"] = g.AutoRepeatAttack, - ["autoTarget"] = g.AutoTarget, - ["viewCombatTarget"] = g.ViewCombatTarget, ["coordinatesOnRadar"] = g.CoordinatesOnRadar, ["lockUI"] = g.LockUI, ["showCloak"] = g.ShowCloak, diff --git a/tests/AcDream.App.Tests/Combat/CharacterOptionCombatSettingsSourceTests.cs b/tests/AcDream.App.Tests/Combat/CharacterOptionCombatSettingsSourceTests.cs index d60fdf70..38d5a1b4 100644 --- a/tests/AcDream.App.Tests/Combat/CharacterOptionCombatSettingsSourceTests.cs +++ b/tests/AcDream.App.Tests/Combat/CharacterOptionCombatSettingsSourceTests.cs @@ -5,12 +5,12 @@ using AcDream.Runtime.Gameplay; namespace AcDream.App.Tests.Combat; /// -/// Campaign OP slice OP4 (2026-08-11) — D7 Group-C re-point: -/// reads -/// AutoTarget/AutoRepeatAttack/ViewCombatTarget from the canonical -/// instead of the client-local -/// GameplaySettings record used -/// to be the only implementation of. +/// Campaign OP slice OP4 (2026-08-11) — D7 Group-C re-point, widened at the +/// OP4 review-fix round (2026-08-11, MUST-FIX 3): +/// reads AutoTarget/AutoRepeatAttack/ViewCombatTarget from the canonical +/// — the ONLY implementation of +/// ICombatGameplaySettingsSource now, since the dead client-local +/// GameplaySettingsState adapter was retired the same round. /// public sealed class CharacterOptionCombatSettingsSourceTests { diff --git a/tests/AcDream.App.Tests/Combat/CombatCameraTargetSourceTests.cs b/tests/AcDream.App.Tests/Combat/CombatCameraTargetSourceTests.cs index dac924f9..564ffe09 100644 --- a/tests/AcDream.App.Tests/Combat/CombatCameraTargetSourceTests.cs +++ b/tests/AcDream.App.Tests/Combat/CombatCameraTargetSourceTests.cs @@ -2,10 +2,19 @@ using System.Numerics; using AcDream.App.Combat; using AcDream.App.Interaction; using AcDream.Core.Combat; +using AcDream.Core.Net.Messages; using AcDream.Core.Selection; +using AcDream.Runtime.Gameplay; namespace AcDream.App.Tests.Combat; +/// +/// OP4 review-fix round (2026-08-11, MUST-FIX 3 / blast M2): the dead +/// GameplaySettingsState adapter over the client-local +/// GameplaySettings record is retired — +/// has exactly one implementation now, , +/// reading the canonical server-authoritative . +/// public sealed class CombatCameraTargetSourceTests { [Fact] @@ -13,7 +22,9 @@ public sealed class CombatCameraTargetSourceTests { const uint target = 0x70000001u; Vector3 point = new(10f, 20f, 30f); - var settings = new GameplaySettingsState(); + var options = new RuntimeCharacterOptionsState(); + options.SetOptionBit((uint)CharacterOptionId.ViewCombatTarget, false); + var settings = new CharacterOptionCombatSettingsSource(options); var combat = new CombatState(); var selection = new SelectionState(); var world = new TargetQuery(point); @@ -25,7 +36,7 @@ public sealed class CombatCameraTargetSourceTests Assert.Null(source.GetTrackedTargetPoint()); - settings.Value = settings.Value with { ViewCombatTarget = true }; + options.SetOptionBit((uint)CharacterOptionId.ViewCombatTarget, true); combat.SetCombatMode(CombatMode.Melee); selection.Select(target, SelectionChangeSource.World); diff --git a/tests/AcDream.App.Tests/Input/DispatcherMovementInputSourceTests.cs b/tests/AcDream.App.Tests/Input/DispatcherMovementInputSourceTests.cs index 96660bfa..e7047018 100644 --- a/tests/AcDream.App.Tests/Input/DispatcherMovementInputSourceTests.cs +++ b/tests/AcDream.App.Tests/Input/DispatcherMovementInputSourceTests.cs @@ -107,6 +107,58 @@ public sealed class DispatcherMovementInputSourceTests Assert.False(source.AutoRunActive); } + // ── S6 (OP4 review-fix round blast, 2026-08-11): the option × modifier + // truth table for Run — previously pinned only at option-unbound + // (⇒ true) + walk-held. ────────────────────────────────────────── + + [Theory] + [InlineData(true, false, true)] // run-by-default, no walk modifier -> runs + [InlineData(true, true, false)] // run-by-default, walk modifier held -> walks + [InlineData(false, false, false)] // walk-by-default, no modifier -> walks + [InlineData(false, true, true)] // walk-by-default, modifier held -> runs + public void Capture_RunReflectsOptionXorWalkModifier( + bool runAsDefault, bool walkModifierHeld, bool expectedRun) + { + var (dispatcher, _, _) = CreateDispatcher(); + var movement = new RuntimeLocalPlayerMovementState + { + RunAsDefaultMovementSource = () => runAsDefault, + }; + var source = new DispatcherMovementInputSource(movement); + source.Bind(dispatcher); + dispatcher.TrySetAutomationActionHeld(InputAction.MovementForward, held: true); + if (walkModifierHeld) + dispatcher.TrySetAutomationActionHeld(InputAction.MovementWalkMode, held: true); + + MovementInput captured = source.Capture(); + + Assert.Equal(expectedRun, captured.Run); + } + + [Fact] + public void Capture_AutoRunActive_ForcesRun_EvenWhenOptionIsOff() + { + // The `|| AutoRunActive` clause predates OP4 but was unobservable + // while the default was hardcoded true. Pinned as a + // CURRENT-BEHAVIOR test (not a retail-correctness claim — see + // mechanism review N6, which flags this as a possibly-divergent + // "|| AutoRunActive" that retail's SetAutoRun does not force) so a + // future change to this clause is deliberate, not accidental. + var (dispatcher, _, _) = CreateDispatcher(); + var movement = new RuntimeLocalPlayerMovementState + { + RunAsDefaultMovementSource = () => false, + }; + var source = new DispatcherMovementInputSource(movement); + source.Bind(dispatcher); + source.HandlePressedAction(InputAction.MovementRunLock); // arm autorun + Assert.True(source.AutoRunActive); + + MovementInput captured = source.Capture(); + + Assert.True(captured.Run); + } + [Fact] public void BindingIsIdempotentOnlyForTheSameDispatcher() { diff --git a/tests/AcDream.App.Tests/Settings/RuntimeSettingsControllerTests.cs b/tests/AcDream.App.Tests/Settings/RuntimeSettingsControllerTests.cs index dbf0c3b0..5f404375 100644 --- a/tests/AcDream.App.Tests/Settings/RuntimeSettingsControllerTests.cs +++ b/tests/AcDream.App.Tests/Settings/RuntimeSettingsControllerTests.cs @@ -23,12 +23,6 @@ public sealed class RuntimeSettingsControllerTests VSync = false, Quality = QualityPreset.Ultra, }, - GameplayValue = GameplaySettings.Default with - { - AutoTarget = true, - AutoRepeatAttack = true, - ViewCombatTarget = true, - }, }; var resolved = new QualitySettings(7, 18, 8, 16, true, 9); int resolveCount = 0; @@ -57,9 +51,6 @@ public sealed class RuntimeSettingsControllerTests Assert.Same(storage.DefaultCharacterValue, controller.Startup.Character); Assert.Equal(resolved, controller.Startup.Quality); Assert.Equal(resolved, controller.ResolvedQuality); - Assert.True(controller.AutoTarget); - Assert.True(controller.AutoRepeatAttack); - Assert.True(controller.ViewCombatTarget); Assert.Equal("default", controller.ActiveToonKey); } @@ -549,7 +540,7 @@ public sealed class RuntimeSettingsControllerTests ParticleRange = ParticleRange.Retail, }); viewModel.SetAudio(viewModel.AudioDraft with { Sfx = 0.33f }); - viewModel.SetGameplay(viewModel.GameplayDraft with { AutoTarget = true }); + viewModel.SetGameplay(viewModel.GameplayDraft with { ShowTooltips = false }); Assert.True(controller.HasDraftPreview); Assert.Equal(91f, controller.DisplayPreview.FieldOfView); @@ -565,7 +556,7 @@ public sealed class RuntimeSettingsControllerTests Assert.Equal(91f, controller.DisplayPreview.FieldOfView); Assert.True(controller.Gameplay.LockUI); Assert.True(controller.Gameplay.AcceptLootPermits); - Assert.True(viewModel.GameplayDraft.AutoTarget); + Assert.False(viewModel.GameplayDraft.ShowTooltips); Assert.True(viewModel.GameplayDraft.LockUI); Assert.True(viewModel.GameplayDraft.AcceptLootPermits); Assert.Contains("target-ui-lock:True", events); @@ -590,12 +581,9 @@ public sealed class RuntimeSettingsControllerTests CoordinatesOnRadar = false, }); - controller.SetCombatGameplay(controller.Gameplay with { AutoTarget = false }); controller.SetUiLocked(true); controller.SetAcceptLootPermits(true); - Assert.False(controller.Gameplay.AutoTarget); - Assert.False(viewModel.GameplayDraft.AutoTarget); Assert.False(viewModel.GameplayDraft.ShowTooltips); Assert.False(viewModel.GameplayDraft.CoordinatesOnRadar); Assert.True(viewModel.GameplayDraft.LockUI); @@ -609,14 +597,12 @@ public sealed class RuntimeSettingsControllerTests Assert.Equal( controller.Gameplay.CoordinatesOnRadar, viewModel.GameplayDraft.CoordinatesOnRadar); - Assert.False(viewModel.GameplayDraft.AutoTarget); Assert.True(viewModel.GameplayDraft.LockUI); Assert.True(viewModel.GameplayDraft.AcceptLootPermits); viewModel.SetGameplay(viewModel.GameplayDraft with { ShowHelm = false }); viewModel.Save(); - Assert.False(storage.GameplayValue.AutoTarget); Assert.True(storage.GameplayValue.LockUI); Assert.True(storage.GameplayValue.AcceptLootPermits); Assert.False(storage.GameplayValue.ShowHelm); @@ -638,26 +624,19 @@ public sealed class RuntimeSettingsControllerTests static _ => { }); controller.SetUiLocked(true); - controller.SetCombatGameplay(controller.Gameplay with { AutoTarget = false }); Assert.Throws(() => controller.SetAcceptLootPermits(true)); Assert.True(viewModel.GameplayDraft.LockUI); - Assert.False(viewModel.GameplayDraft.AutoTarget); Assert.True(viewModel.GameplayDraft.AcceptLootPermits); viewModel.Cancel(); Assert.Equal(GameplaySettings.Default.LockUI, viewModel.GameplayDraft.LockUI); - Assert.Equal( - GameplaySettings.Default.AutoTarget, - viewModel.GameplayDraft.AutoTarget); Assert.Equal( GameplaySettings.Default.AcceptLootPermits, viewModel.GameplayDraft.AcceptLootPermits); Assert.Contains(logs, line => line.Contains("radar lock save failed", StringComparison.Ordinal)); - Assert.Contains(logs, line => - line.Contains("combat option save failed", StringComparison.Ordinal)); } [Fact] @@ -928,6 +907,42 @@ public sealed class RuntimeSettingsControllerTests Assert.Equal(["target-quality"], events); } + [Fact] + public void SetUiLocked_AppliesEvenWhenGameplayLockUIAlreadyMatches_IfNeverActuallyApplied() + { + // MUST-FIX 4 (OP4 review-fix round, 2026-08-11, blast M3): the + // guard used to compare the requested value against + // `Gameplay.LockUI` — valid only while `ToggleUiLock` derived + // `locked` AS `!Gameplay.LockUI`. OP4 re-pointed `ToggleUiLock` to + // read the SERVER bit (RuntimeCharacterOptionsState) instead, a + // DIFFERENT store that can already equal a value this controller + // never actually pushed to `_runtimeTargets`. A pre-existing save + // seeds Gameplay.LockUI = true; the runtime target has never seen + // `true` — the FIRST SetUiLocked(true) call must still apply. + var events = new List(); + var storage = new FakeStorage(events) + { + GameplayValue = GameplaySettings.Default with { LockUI = true }, + }; + var controller = new RuntimeSettingsController( + storage, + static preset => QualitySettings.From(preset), + static _ => { }); + Assert.True(controller.Gameplay.LockUI); // already true, but never applied + var targets = new FakeRuntimeTargets(events); + controller.BindRuntimeTargets(targets); + + controller.SetUiLocked(true); + + Assert.Equal(1, targets.UiLockCalls); + + // A second call with the SAME value now correctly no-ops — this + // time it really was applied. + controller.SetUiLocked(true); + + Assert.Equal(1, targets.UiLockCalls); + } + [Fact] public void UiLockTargetFailureCanRetryTheSameRequestedValue() { diff --git a/tests/AcDream.App.Tests/UI/Layout/CharacterOptionsPageControllerTests.cs b/tests/AcDream.App.Tests/UI/Layout/CharacterOptionsPageControllerTests.cs index e20ef89b..4a6057cd 100644 --- a/tests/AcDream.App.Tests/UI/Layout/CharacterOptionsPageControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/CharacterOptionsPageControllerTests.cs @@ -142,6 +142,25 @@ public sealed class CharacterOptionsPageControllerTests DatStringResolver.ComputeHash("ID_PlayerOption_HearPKDeaths")); } + [Theory] + [InlineData("ID_CharacterOption_UIBehavior_Section", 0x06489B6Eu)] + [InlineData("ID_CharacterOption_UIDisplay_Section", 0x0A9BC99Eu)] + [InlineData("ID_CharacterOption_Grouping_Section", 0x0CBAAFAEu)] + [InlineData("ID_CharacterOption_OtherPlayers_Section", 0x0872DFFEu)] + [InlineData("ID_CharacterOption_CharacterBehavior_Section", 0x08674D5Eu)] + [InlineData("ID_CharacterOption_Chat_Section", 0x0987FE8Eu)] + public void HeaderKey_RetailNameHash_MatchesByteVerifiedStringId( + string headerKey, uint expectedHash) + { + // SF-2 (OP4 review-fix round, 2026-08-11): structure doc §7's six + // byte-verified header string ids — previously verified only by + // hand in the mechanism review, not pinned by a test. A typo in a + // HeaderKey literal would otherwise produce a silently blank + // header that only the user's eye catches. + Assert.Equal(expectedHash, DatStringResolver.ComputeHash(headerKey)); + Assert.Contains(headerKey, CharacterOptionsPageController.Groups.Select(g => g.HeaderKey)); + } + [Theory] [MemberData(nameof(RetailEnumNameCases))] public void RetailName_MatchesVerbatimAcclientEnumSpelling( @@ -479,6 +498,147 @@ public sealed class CharacterOptionsPageControllerTests Assert.False(controller.CharacterPage.Changed); } + // ── MUST-FIX 1 (OP4 review-fix round, 2026-08-11): the panel re-seeds + // from the live binding on OnShown instead of the pre-login + // constructor-default word it was constructed with. ───────────────── + + [Fact] + public void OnShown_ReSeedsRow_FromLiveBindingValue_ChangedBehindItsBack() + { + // Simulates a fresh PlayerDescription landing (or simply "the + // character's real server value differs from the word this row + // was constructed with") without ever calling SetCurrentValue. + (OptionsPanelController controller, FakeBindings bindings, _) = BindReal(); + CharacterOptionId id = AllRows().First().Id; + var row = Assert.IsType(controller.CharacterPage.Rows[0]); + bool initial = row.Current; + + bindings.Values[id] = !initial; + bindings.Sets.Clear(); + + controller.CharacterPage.OnShown(); + + Assert.Equal(!initial, row.Current); + Assert.Equal(!initial, row.Saved); + Assert.False(controller.CharacterPage.Changed); + // The re-read must never round-trip through SetOption — that would + // send the re-seeded value back out over the wire (retail's own + // GetValue()-into-SaveCurrentValue never calls SetPlayerOption). + Assert.Empty(bindings.Sets); + } + + [Fact] + public void OnShown_AllFiftyRows_ConvergeToTheLiveBindingSnapshot() + { + // The pre-login case: bind against a constructor-default word (the + // fake's dictionary starts empty -> every row seeds false), THEN + // the "server" state is populated (a PlayerDescription landing), + // THEN the page is shown — every one of the 50 rows must converge, + // matching retail's own InitOptions()+PostInit() / first tab- + // activation schedule (SaveCurrentValue re-reads GetValue() live). + var fakeBindings = new FakeBindings(); + var random = new Random(20260811); + foreach (CharacterOptionsPageController.RowSpec spec in AllRows()) + fakeBindings.Values[spec.Id] = random.Next(2) == 0; + + ImportedLayout layout = FixtureLoader.LoadOptionsPanelHost(); + OptionsPanelController controller = OptionsPanelController.Bind( + layout, + new OptionsPanelController.Callbacks( + Toggle: () => { }, + RequestExitToCharacterSelection: () => { }, + ExitGame: () => { }, + UseMouseTurningSettings: () => { }, + DisplaySystemMessage: _ => { }))!; + bool bound = CharacterOptionsPageController.Bind( + layout, + controller.CharacterPage, + MakeTemplateResolver(), + (_, _) => null, + fakeBindings.ToBindings()); + Assert.True(bound); + + controller.CharacterPage.OnShown(); + + int i = 0; + foreach (CharacterOptionsPageController.RowSpec spec in AllRows()) + { + var row = (BoolOptionRow)controller.CharacterPage.Rows[i++]; + Assert.Equal(fakeBindings.Values[spec.Id], row.Current); + Assert.Equal(fakeBindings.Values[spec.Id], row.Saved); + } + Assert.False(controller.CharacterPage.Changed); + } + + [Fact] + public void Reset_AfterReseed_RestoresTheLiveValue_NotTheStaleConstructionDefault() + { + // The historical bug MF-1 closes: before the fix, Reset/tab-switch + // could only ever restore whatever the row was seeded with AT BIND + // TIME (the pre-login constructor word) — visually-idempotent + // "toggle then cancel" could silently mutate the server bit in the + // wrong direction. After the fix, OnShown re-seeds _saved from the + // live bit first, so Reset can only revert to what was ACTUALLY + // live at the last show. + (OptionsPanelController controller, FakeBindings bindings, _) = BindReal(); + CharacterOptionId id = AllRows().First().Id; + var row = Assert.IsType(controller.CharacterPage.Rows[0]); + + bindings.Values[id] = true; + controller.CharacterPage.OnShown(); // re-seed: current == saved == true + bindings.Sets.Clear(); + + row.SetCurrentValue(false); // user toggles it off, never clicks Apply + controller.CharacterPage.Reset(); + + Assert.True(row.Current); + Assert.Contains(bindings.Sets, s => s.Id == id && s.Value); + } + + [Fact] + public void ClickingTheRealCheckboxWidget_PublishesSetOption_ViaMouseDownUpClick() + { + // SF-2/S6 (OP4 review-fix round, 2026-08-11): every other test in + // this suite drives BoolOptionRow.SetCurrentValue directly, which + // would stay green even if the toggle template's checkbox + // (0x10000219) ever lost its authored DAT property 0x0B + // (UiButton.ToggleBehavior) — the mechanism the whole LED click + // interaction rests on (mechanism review §1.5). This drives the + // REAL MouseDown/MouseUp/Click sequence: MouseUp flips + // UiButton.Selected FIRST (ToggleBehavior), then Click invokes + // checkbox.OnClick, which reads the NEW Selected value. + ImportedLayout layout = FixtureLoader.LoadOptionsPanelHost(); + OptionsPanelController controller = OptionsPanelController.Bind( + layout, + new OptionsPanelController.Callbacks( + Toggle: () => { }, + RequestExitToCharacterSelection: () => { }, + ExitGame: () => { }, + UseMouseTurningSettings: () => { }, + DisplaySystemMessage: _ => { }))!; + var fakeBindings = new FakeBindings(); + bool bound = CharacterOptionsPageController.Bind( + layout, controller.CharacterPage, MakeTemplateResolver(), (_, _) => null, + fakeBindings.ToBindings()); + Assert.True(bound); + + var listBox = Assert.IsType( + layout.FindElement(CharacterOptionsPageController.ListBoxElementId)); + var checkbox = Assert.IsType( + UiElement.FindDescendant(listBox, 0x10000219u)); + CharacterOptionId id = AllRows().First().Id; + Assert.False(checkbox.Selected); + + checkbox.OnEvent(new UiEvent(0, checkbox, UiEventType.MouseDown, Data1: 0, Data2: 0)); + checkbox.OnEvent(new UiEvent(0, checkbox, UiEventType.MouseUp, Data1: 0, Data2: 0)); + checkbox.OnEvent(new UiEvent(0, checkbox, UiEventType.Click)); + + Assert.True(checkbox.Selected); + var set = Assert.Single(fakeBindings.Sets); + Assert.Equal(id, set.Id); + Assert.True(set.Value); + } + [Fact] public void ScrollbarLinkage_ModelPointsAtTheListBoxScroll() { diff --git a/tests/AcDream.App.Tests/UI/Layout/CombatUiControllerTests.cs b/tests/AcDream.App.Tests/UI/Layout/CombatUiControllerTests.cs index 5334ea81..faa7642d 100644 --- a/tests/AcDream.App.Tests/UI/Layout/CombatUiControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/CombatUiControllerTests.cs @@ -2,8 +2,7 @@ using AcDream.App.Combat; using AcDream.App.UI; using AcDream.App.UI.Layout; using AcDream.Core.Combat; -using AcDream.Runtime.Gameplay; -using AcDream.UI.Abstractions.Panels.Settings; +using AcDream.Core.Net.Messages; namespace AcDream.App.Tests.UI.Layout; @@ -11,6 +10,21 @@ public sealed class CombatUiControllerTests { private static (uint, int, int) NoTex(uint _) => (0u, 0, 0); + /// + /// OP4 review-fix round (2026-08-11, MUST-FIX 3 / blast M2): the three + /// combat LEDs now read/write through the SAME server-bit seam the + /// Character tab's rows use — this fake is the test-local stand-in for + /// the production RuntimeCharacterOptionsState-backed binding. + /// + private sealed class FakeOptionBindings + { + public Dictionary Values { get; } = new(); + + public CombatUiController.Bindings ToBindings() => new( + CurrentValue: id => Values.TryGetValue(id, out bool v) && v, + SetOption: (id, value) => Values[id] = value); + } + [Fact] public void CombatMode_ShowsPhysicalAndMagicPages_AndSelectsMediumByDefault() { @@ -19,9 +33,9 @@ public sealed class CombatUiControllerTests using var attacks = CreateAttacks(combat, () => now, []); var (layout, basic, spellcasting, power, high, medium, low) = BuildLayout(); var visibility = new List(); - GameplaySettings gameplay = GameplaySettings.Default; + var options = new FakeOptionBindings(); using var controller = CombatUiController.Bind( - layout, combat, attacks, () => gameplay, value => gameplay = value, + layout, combat, attacks, options.ToBindings(), Labels, visibility.Add)!; controller.SyncVisibility(); @@ -45,9 +59,9 @@ public sealed class CombatUiControllerTests var combat = new CombatState(); using var attacks = CreateAttacks(combat, () => now, sent); var (layout, basic, advanced, power, high, _, _) = BuildLayout(); - GameplaySettings gameplay = GameplaySettings.Default; + var options = new FakeOptionBindings(); using var controller = CombatUiController.Bind( - layout, combat, attacks, () => gameplay, value => gameplay = value, + layout, combat, attacks, options.ToBindings(), Labels, _ => { })!; combat.SetCombatMode(CombatMode.Melee); @@ -69,9 +83,10 @@ public sealed class CombatUiControllerTests var combat = new CombatState(); using var attacks = CreateAttacks(combat, () => 0d, []); var (layout, _, _, _, _, _, _) = BuildLayout(); - GameplaySettings gameplay = GameplaySettings.Default; + var options = new FakeOptionBindings(); + options.Values[CharacterOptionId.AutoTarget] = true; using var controller = CombatUiController.Bind( - layout, combat, attacks, () => gameplay, value => gameplay = value, + layout, combat, attacks, options.ToBindings(), Labels, _ => { })!; var autoTarget = Assert.IsType(layout.FindElement(CombatUiController.AutoTargetId)); @@ -79,7 +94,7 @@ public sealed class CombatUiControllerTests autoTarget.OnEvent(new UiEvent(0, autoTarget, UiEventType.MouseUp, Data1: 3, Data2: 3)); autoTarget.OnEvent(new UiEvent(0, autoTarget, UiEventType.Click, Data1: 3, Data2: 3)); - Assert.False(gameplay.AutoTarget); + Assert.False(options.Values[CharacterOptionId.AutoTarget]); } [Fact] @@ -88,9 +103,9 @@ public sealed class CombatUiControllerTests var combat = new CombatState(); using var attacks = CreateAttacks(combat, () => 0d, []); var (layout, _, _, power, _, _, _) = BuildLayout(); - GameplaySettings gameplay = GameplaySettings.Default; + var options = new FakeOptionBindings(); using var controller = CombatUiController.Bind( - layout, combat, attacks, () => gameplay, value => gameplay = value, + layout, combat, attacks, options.ToBindings(), Labels, _ => { })!; var speed = Assert.IsType(layout.FindElement(CombatUiController.SpeedLabelId)); @@ -108,9 +123,9 @@ public sealed class CombatUiControllerTests ImportedLayout layout = LayoutImporter.Build(info, NoTex, datFont: null); var combat = new CombatState(); using var attacks = CreateAttacks(combat, () => 0d, []); - GameplaySettings gameplay = GameplaySettings.Default; + var options = new FakeOptionBindings(); using var controller = CombatUiController.Bind( - layout, combat, attacks, () => gameplay, value => gameplay = value, + layout, combat, attacks, options.ToBindings(), Labels, _ => { })!; ApplyAnchors(layout.Root); diff --git a/tests/AcDream.App.Tests/UI/Layout/OptionsPanelControllerTests.cs b/tests/AcDream.App.Tests/UI/Layout/OptionsPanelControllerTests.cs index 02ca82c8..8c9875ac 100644 --- a/tests/AcDream.App.Tests/UI/Layout/OptionsPanelControllerTests.cs +++ b/tests/AcDream.App.Tests/UI/Layout/OptionsPanelControllerTests.cs @@ -158,6 +158,113 @@ public sealed class OptionsPanelControllerTests Assert.Equal(0, flushCount); } + // ── MUST-FIX 2 (OP4 review-fix round, 2026-08-11): Apply/Reset gating ────── + + private static (UiButton Apply, UiButton Reset, UiButton Defaults) GetCharacterPageButtons( + OptionsPanelController controller) + { + UiElement pageRoot = UiElement.FindDescendant(controller.Root, 0x10000211u)!; // Character page slot + var apply = Assert.IsType(UiElement.FindDescendant(pageRoot, 0x100001FCu)); + var reset = Assert.IsType(UiElement.FindDescendant(pageRoot, 0x100001FDu)); + var defaults = Assert.IsType(UiElement.FindDescendant(pageRoot, 0x100001FEu)); + return (apply, reset, defaults); + } + + [Fact] + public void CharacterPage_ApplyAndReset_StartDisabled_OnFreshBind() + { + // Retail's PostInit calls InitOptions() then OnOptionChanged(0), so + // the pair starts Ghosted before any row has ever been touched. + ImportedLayout layout = FixtureLoader.LoadOptionsPanelHost(); + var calls = new List(); + OptionsPanelController controller = + OptionsPanelController.Bind(layout, MakeCallbacks(calls))!; + + (UiButton apply, UiButton reset, _) = GetCharacterPageButtons(controller); + + Assert.False(apply.Enabled); + Assert.False(reset.Enabled); + } + + [Fact] + public void CharacterPage_OneLedClick_EnablesApplyAndReset() + { + ImportedLayout layout = FixtureLoader.LoadOptionsPanelHost(); + var calls = new List(); + OptionsPanelController controller = + OptionsPanelController.Bind(layout, MakeCallbacks(calls))!; + var row = new BoolOptionRow(initial: false, defaultValue: false); + controller.CharacterPage.Register(row); + (UiButton apply, UiButton reset, _) = GetCharacterPageButtons(controller); + + row.SetCurrentValue(true); + + Assert.True(apply.Enabled); + Assert.True(reset.Enabled); + } + + [Fact] + public void CharacterPage_Apply_DisablesApplyAndResetAgain() + { + ImportedLayout layout = FixtureLoader.LoadOptionsPanelHost(); + var calls = new List(); + OptionsPanelController controller = + OptionsPanelController.Bind(layout, MakeCallbacks(calls))!; + var row = new BoolOptionRow(initial: false, defaultValue: false); + controller.CharacterPage.Register(row); + (UiButton apply, UiButton reset, _) = GetCharacterPageButtons(controller); + row.SetCurrentValue(true); + + controller.CharacterPage.Apply(); + + Assert.False(apply.Enabled); + Assert.False(reset.Enabled); + } + + [Fact] + public void CharacterPage_Defaults_LeavesApplyAndResetEnabled_WhenSomethingActuallyChanged() + { + ImportedLayout layout = FixtureLoader.LoadOptionsPanelHost(); + var calls = new List(); + OptionsPanelController controller = + OptionsPanelController.Bind(layout, MakeCallbacks(calls))!; + var row = new BoolOptionRow(initial: false, defaultValue: true); + controller.CharacterPage.Register(row); + (UiButton apply, UiButton reset, _) = GetCharacterPageButtons(controller); + + controller.CharacterPage.Defaults(); + + Assert.True(row.Current); + Assert.True(controller.CharacterPage.Changed); + Assert.True(apply.Enabled); + Assert.True(reset.Enabled); + } + + [Fact] + public void CharacterPage_Defaults_IsNeverGated() + { + // Retail's Defaults override never fetches its own child id at all + // — it must never grey itself out, before OR after a real change. + ImportedLayout layout = FixtureLoader.LoadOptionsPanelHost(); + var calls = new List(); + OptionsPanelController controller = + OptionsPanelController.Bind(layout, MakeCallbacks(calls))!; + var row = new BoolOptionRow(initial: false, defaultValue: false); + controller.CharacterPage.Register(row); + (_, _, UiButton defaults) = GetCharacterPageButtons(controller); + + Assert.True(defaults.Enabled); + + row.SetCurrentValue(true); + Assert.True(defaults.Enabled); + + controller.CharacterPage.Apply(); + Assert.True(defaults.Enabled); + + controller.CharacterPage.Defaults(); + Assert.True(defaults.Enabled); + } + // ── Close button ───────────────────────────────────────────────────────── [Fact] diff --git a/tests/AcDream.Core.Tests/Chat/ChatLogTests.cs b/tests/AcDream.Core.Tests/Chat/ChatLogTests.cs index d2281292..eef5a26d 100644 --- a/tests/AcDream.Core.Tests/Chat/ChatLogTests.cs +++ b/tests/AcDream.Core.Tests/Chat/ChatLogTests.cs @@ -1,3 +1,5 @@ +using System.Globalization; +using System.Threading; using AcDream.Core.Chat; using AcDream.Core.Combat; using Xunit; @@ -336,4 +338,83 @@ public sealed class ChatLogTests log.OnCombatLine("You hit Mosswart for 5 slashing damage (50.0%).", logTextType: 0x06u); Assert.Equal(0x06u, log.Snapshot()[0].LogTextType); } + + // ── SF-1/S1 (OP4 review-fix round, 2026-08-11): DisplayTimestampsSource + // prefixes EVERY producer through the shared Append seam, not just + // RuntimeCommunicationState.AddText's own subset of callers. ──────── + + [Fact] + public void DisplayTimestampsSource_Unbound_NoPrefix() + { + var log = new ChatLog(); + log.OnLocalSpeech("Alice", "hi", 0xAAu, isRanged: false, logTextType: 0x02u); + Assert.Equal("hi", log.Snapshot()[0].Text); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void DisplayTimestampsSource_GatesThePrefix(bool timestampsOn) + { + var log = new ChatLog { DisplayTimestampsSource = () => timestampsOn }; + log.OnLocalSpeech("Alice", "hi", 0xAAu, isRanged: false, logTextType: 0x02u); + + string text = log.Snapshot()[0].Text; + if (timestampsOn) + Assert.Matches(@"^\d{1,2}:\d{2}:\d{2} hi$", text); + else + Assert.Equal("hi", text); + } + + [Theory] + // Every public ingestion method — proving the prefix applies at the + // ONE shared Append seam, not per-caller. + [InlineData("OnEmote")] + [InlineData("OnSoulEmote")] + [InlineData("OnChannelBroadcast")] + [InlineData("OnTellReceived")] + [InlineData("OnSystemMessage")] + [InlineData("OnPopup")] + [InlineData("OnCombatLine")] + [InlineData("OnSelfSent")] + [InlineData("OnPlayerKilled")] + public void DisplayTimestampsSource_AppliesToEveryProducer(string method) + { + var log = new ChatLog { DisplayTimestampsSource = () => true }; + + switch (method) + { + case "OnEmote": log.OnEmote("Caith", "waves", 0xCAFEu); break; + case "OnSoulEmote": log.OnSoulEmote("Bob", "dances", 0xBEEFu); break; + case "OnChannelBroadcast": log.OnChannelBroadcast(42u, "Alice", "motd"); break; + case "OnTellReceived": log.OnTellReceived("Alice", "psst", 0xAAu, logTextType: 0x03u); break; + case "OnSystemMessage": log.OnSystemMessage("fizzled", chatType: 5); break; + case "OnPopup": log.OnPopup("modal"); break; + case "OnCombatLine": log.OnCombatLine("hit", logTextType: 0x06u); break; + case "OnSelfSent": log.OnSelfSent(ChatKind.Tell, "hey", logTextType: 0x04u, targetOrChannel: "Alice"); break; + case "OnPlayerKilled": log.OnPlayerKilled("died", 0x1u, 0x2u); break; + } + + string text = log.Snapshot()[0].Text; + Assert.Matches(@"^\d{1,2}:\d{2}:\d{2} .+$", text); + } + + [Fact] + public void DisplayTimestampsSource_UsesLiteralColons_RegardlessOfCurrentCulture() + { + CultureInfo original = Thread.CurrentThread.CurrentCulture; + try + { + Thread.CurrentThread.CurrentCulture = CultureInfo.GetCultureInfo("fi-FI"); + var log = new ChatLog { DisplayTimestampsSource = () => true }; + + log.OnSystemMessage("fizzled", chatType: 0); + + Assert.Matches(@"^\d{1,2}:\d{2}:\d{2} fizzled$", log.Snapshot()[0].Text); + } + finally + { + Thread.CurrentThread.CurrentCulture = original; + } + } } diff --git a/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCommunicationStateTests.cs b/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCommunicationStateTests.cs index 1f070f9d..b771bf3f 100644 --- a/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCommunicationStateTests.cs +++ b/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCommunicationStateTests.cs @@ -1,3 +1,5 @@ +using System.Globalization; +using System.Threading; using AcDream.Core.Chat; using AcDream.Core.Social; using AcDream.Runtime.Gameplay; @@ -276,10 +278,56 @@ public sealed class RuntimeCommunicationStateTests Assert.NotEqual("Your spell fizzled.", text); // Retail ctor default format "%#H:%M:%S " (non-zero-padded 24h // hour, zero-padded minute:second, trailing space before the - // text) — .NET "H:mm:ss " is the exact equivalent. + // text) — .NET "H\:mm\:ss " (colons ESCAPED, not the + // culture-dependent TimeSeparator placeholder) is the exact + // equivalent (SF-1/S4, OP4 review-fix round, 2026-08-11). Assert.Matches(@"^\d{1,2}:\d{2}:\d{2} Your spell fizzled\.$", text); } + [Fact] + public void AddText_TimestampsTrue_UsesLiteralColons_RegardlessOfCurrentCulture() + { + // SF-1/S4 (OP4 review-fix round, 2026-08-11): an unescaped "H:mm:ss" + // format string renders ':' as CurrentCulture.DateTimeFormat. + // TimeSeparator, which is NOT ':' on cultures like fi-FI ("."). The + // fix escapes the colons and forces InvariantCulture — prove the + // output stays colon-separated even under a culture that would + // otherwise substitute a different separator. + CultureInfo original = Thread.CurrentThread.CurrentCulture; + try + { + Thread.CurrentThread.CurrentCulture = CultureInfo.GetCultureInfo("fi-FI"); + using var state = new RuntimeCommunicationState { DisplayTimestampsSource = () => true }; + + state.AddText("Your spell fizzled.", RetailLogTextType.Default); + + string text = state.Chat.Snapshot()[0].Text; + Assert.Matches(@"^\d{1,2}:\d{2}:\d{2} Your spell fizzled\.$", text); + } + finally + { + Thread.CurrentThread.CurrentCulture = original; + } + } + + [Fact] + public void DisplayTimestampsSource_ForwardsToChat_TimestampingEveryProducer_NotJustAddText() + { + // S1 (OP4 review-fix round, 2026-08-11, blast lens): AddText's own + // callers (ServerMessage/WeenieError) were a strict SUBSET of + // every chat producer — heard speech, emotes, Turbine channels, + // and combat text all bypassed it by calling ChatLog's own OnXxx + // methods directly. Setting DisplayTimestampsSource on this class + // must timestamp lines that never go through AddText at all. + using var state = new RuntimeCommunicationState { DisplayTimestampsSource = () => true }; + + state.Chat.OnLocalSpeech("Alice", "hi", 0xAAu, isRanged: false, logTextType: 0x02u); + + string text = state.Chat.Snapshot()[0].Text; + Assert.EndsWith("hi", text); + Assert.Matches(@"^\d{1,2}:\d{2}:\d{2} hi$", text); + } + [Fact] public void AddText_TimestampsTrue_NeverAppliedToClientLocalSpewBox() { diff --git a/tests/AcDream.UI.Abstractions.Tests/Panels/Settings/GameplaySettingsTests.cs b/tests/AcDream.UI.Abstractions.Tests/Panels/Settings/GameplaySettingsTests.cs index 9cf2a3e5..b0230b83 100644 --- a/tests/AcDream.UI.Abstractions.Tests/Panels/Settings/GameplaySettingsTests.cs +++ b/tests/AcDream.UI.Abstractions.Tests/Panels/Settings/GameplaySettingsTests.cs @@ -16,8 +16,6 @@ public sealed class GameplaySettingsTests // starting point, not retail-bitmask. A change to any of these // should be a deliberate decision, not a drive-by. var d = GameplaySettings.Default; - Assert.True(d.AutoTarget); - Assert.True(d.AutoRepeatAttack); Assert.True(d.ToggleRun); Assert.False(d.AdvancedCombatUI); Assert.True(d.ShowTooltips); @@ -36,8 +34,8 @@ public sealed class GameplaySettingsTests public void Equality_is_value_based() { var a = GameplaySettings.Default; - var b = GameplaySettings.Default with { AutoTarget = false }; - var c = GameplaySettings.Default with { AutoTarget = false }; + var b = GameplaySettings.Default with { ToggleRun = false }; + var c = GameplaySettings.Default with { ToggleRun = false }; Assert.NotEqual(a, b); Assert.Equal(b, c); } @@ -48,7 +46,7 @@ public sealed class GameplaySettingsTests var d = GameplaySettings.Default with { LockUI = true }; Assert.True(d.LockUI); // Other fields untouched. - Assert.Equal(GameplaySettings.Default.AutoTarget, d.AutoTarget); + Assert.Equal(GameplaySettings.Default.ToggleRun, d.ToggleRun); Assert.Equal(GameplaySettings.Default.ShowHelm, d.ShowHelm); } } diff --git a/tests/AcDream.UI.Abstractions.Tests/Panels/Settings/SettingsPanelTests.cs b/tests/AcDream.UI.Abstractions.Tests/Panels/Settings/SettingsPanelTests.cs index 60243e3a..656856df 100644 --- a/tests/AcDream.UI.Abstractions.Tests/Panels/Settings/SettingsPanelTests.cs +++ b/tests/AcDream.UI.Abstractions.Tests/Panels/Settings/SettingsPanelTests.cs @@ -395,8 +395,15 @@ public sealed class SettingsPanelTests .Select(c => (string)c.Args[0]!).ToList(); // Spot check the major retail-named toggles. Don't assert exact // count — adding new toggles shouldn't break this test. - Assert.Contains("Auto-target on attack", checks); - Assert.Contains("Auto-repeat attacks", checks); + // + // OP4 review-fix round (2026-08-11, MUST-FIX 3 / blast M2): + // "Auto-target on attack" / "Auto-repeat attacks" were retired + // from this panel — those two options (plus "Keep combat target + // in view") now read/write the canonical server bit through + // CombatUiController, not the client-local GameplaySettings + // record this dat-free panel edits. + Assert.DoesNotContain("Auto-target on attack", checks); + Assert.DoesNotContain("Auto-repeat attacks", checks); Assert.Contains("Run mode is toggle (vs hold)", checks); Assert.Contains("Show item tooltips", checks); Assert.Contains("Show helm on character", checks); @@ -415,7 +422,7 @@ public sealed class SettingsPanelTests var checks = r.Calls.Where(c => c.Method == "Checkbox") .Select(c => (string)c.Args[0]!).ToList(); - Assert.DoesNotContain("Auto-target on attack", checks); + Assert.DoesNotContain("Run mode is toggle (vs hold)", checks); Assert.DoesNotContain("Lock UI (disable panel drag/resize)", checks); } diff --git a/tests/AcDream.UI.Abstractions.Tests/Panels/Settings/SettingsStoreTests.cs b/tests/AcDream.UI.Abstractions.Tests/Panels/Settings/SettingsStoreTests.cs index 8fba1869..c88989af 100644 --- a/tests/AcDream.UI.Abstractions.Tests/Panels/Settings/SettingsStoreTests.cs +++ b/tests/AcDream.UI.Abstractions.Tests/Panels/Settings/SettingsStoreTests.cs @@ -221,7 +221,6 @@ public sealed class SettingsStoreTests : System.IDisposable var store = new SettingsStore(_tempPath); var original = GameplaySettings.Default with { - AutoTarget = false, AdvancedCombatUI = true, ShowHelm = false, LockUI = true, @@ -249,7 +248,7 @@ public sealed class SettingsStoreTests : System.IDisposable var loaded = store.LoadGameplay(); Assert.True(loaded.LockUI); - Assert.Equal(GameplaySettings.Default.AutoTarget, loaded.AutoTarget); + Assert.Equal(GameplaySettings.Default.ToggleRun, loaded.ToggleRun); Assert.Equal(GameplaySettings.Default.ShowHelm, loaded.ShowHelm); } diff --git a/tests/AcDream.UI.Abstractions.Tests/Panels/Settings/SettingsVMTests.cs b/tests/AcDream.UI.Abstractions.Tests/Panels/Settings/SettingsVMTests.cs index 7dafceeb..c7e19ebf 100644 --- a/tests/AcDream.UI.Abstractions.Tests/Panels/Settings/SettingsVMTests.cs +++ b/tests/AcDream.UI.Abstractions.Tests/Panels/Settings/SettingsVMTests.cs @@ -420,7 +420,7 @@ public sealed class SettingsVMTests [Fact] public void GameplayDraft_initial_value_matches_persisted() { - var custom = GameplaySettings.Default with { AutoTarget = false, LockUI = true }; + var custom = GameplaySettings.Default with { LockUI = true }; var (vm, _, _, _, _, _, _, _, _, _) = Build(persistedGameplay: custom); Assert.Equal(custom, vm.GameplayDraft); Assert.False(vm.HasUnsavedChanges); @@ -438,7 +438,7 @@ public sealed class SettingsVMTests public void ApplyExternalGameplayChange_updates_both_snapshots_and_preserves_other_drafts() { var (vm, _, _, _, _, _, _, _, _, _) = Build(); - bool persistedAutoTarget = !GameplaySettings.Default.AutoTarget; + bool persistedLockUI = !GameplaySettings.Default.LockUI; vm.SetGameplay(vm.GameplayDraft with { @@ -448,10 +448,10 @@ public sealed class SettingsVMTests vm.ApplyExternalGameplayChange(gameplay => gameplay with { - AutoTarget = persistedAutoTarget, + LockUI = persistedLockUI, }); - Assert.Equal(persistedAutoTarget, vm.GameplayDraft.AutoTarget); + Assert.Equal(persistedLockUI, vm.GameplayDraft.LockUI); Assert.Equal( !GameplaySettings.Default.ShowTooltips, vm.GameplayDraft.ShowTooltips); @@ -462,7 +462,7 @@ public sealed class SettingsVMTests vm.Cancel(); - Assert.Equal(persistedAutoTarget, vm.GameplayDraft.AutoTarget); + Assert.Equal(persistedLockUI, vm.GameplayDraft.LockUI); Assert.Equal( GameplaySettings.Default.ShowTooltips, vm.GameplayDraft.ShowTooltips); @@ -478,7 +478,7 @@ public sealed class SettingsVMTests var (vm, _, _, _, _, _, _, savedGameplayHistory, _, _) = Build(); vm.SetGameplay(vm.GameplayDraft with { - AutoTarget = false, + LockUI = true, ShowTooltips = false, UseMouseTurning = true, }); @@ -486,7 +486,7 @@ public sealed class SettingsVMTests vm.Save(); Assert.Single(savedGameplayHistory); - Assert.False(savedGameplayHistory[0].AutoTarget); + Assert.True(savedGameplayHistory[0].LockUI); Assert.False(savedGameplayHistory[0].ShowTooltips); Assert.True(savedGameplayHistory[0].UseMouseTurning); Assert.False(vm.HasUnsavedChanges); @@ -509,7 +509,7 @@ public sealed class SettingsVMTests [Fact] public void ResetAllToDefaults_resets_gameplay_to_default() { - var custom = GameplaySettings.Default with { AutoTarget = false, LockUI = true }; + var custom = GameplaySettings.Default with { LockUI = true }; var (vm, _, _, _, _, _, _, _, _, _) = Build(persistedGameplay: custom); Assert.NotEqual(GameplaySettings.Default, vm.GameplayDraft);