diff --git a/docs/architecture/retail-divergence-register.md b/docs/architecture/retail-divergence-register.md index 905747b5..240a0096 100644 --- a/docs/architecture/retail-divergence-register.md +++ b/docs/architecture/retail-divergence-register.md @@ -346,11 +346,11 @@ AP-94..AP-112 for the confirmed retail-UI completion gaps. | AP-191 | **Filed 2026-08-10 (Campaign CH round 4, user-gate items 1+2 — retail two-plane glyph outline + authored SpewBox/chat text style, `docs/research/2026-08-10-retail-ui-text-style.md`).** The chat transcript's authored BASE STYLE (`0x10000372` in layout `0x2100003F`) carries a `0x1C`/`0x1D` pair alongside its `0x1A`/`0x1B` — `0x1D` (`TagFontColor[]`) is confirmed authored `ARGB(255,0,178,0)` (green), and `0x1C` is UNVERIFIED but most likely `TagFontDID` by symmetry with `0x1D` (both are pull-based, no `OnSetAttribute` case, unlike `0x1A`/`0x1B`/`0x21`/`0x22` which this round's commit DOES import). Retail's `AppendTextWithFont` selects a font/colour PAIR per appended run via `SetFontDIDNum`/`SetFontColorNum`, so a message's `[General]`-style channel tag can render in a distinct colour/font from the rest of the line — a capability `UiText.Line` does not have (one `Color` per whole line, no sub-line run concept). Landing this needs a per-run tag boundary threaded from `ChatTranscriptRenderer.BuildLines` through `UiText`'s line model into `UiRenderContext.DrawStringDat`, deliberately out of this round's scope (Fix 5 only changed the DEFAULT/uncolored-run seed, not the run model). `src/AcDream.App/UI/Layout/ChatTranscriptRenderer.cs` (`BuildLines`); `src/AcDream.App/UI/UiText.cs` (`Line`) | The default-fill fix (this same commit) is the higher-value, lower-risk half of retail's text-style gap for the transcript; a per-run tag concept is a larger structural change (touches the line model every transcript consumer reads) better landed as its own reviewed slice than folded into a text-style bugfix commit | Retail's `[General]`/channel-name tag prefix on a chat line renders the SAME colour as the rest of the line in acdream instead of green, and any authored tag-specific font goes unused — cosmetic only, the message text itself is unaffected | `UIElement_Text::AppendTextWithFont @0x00469de0`; `UIElement_Text::SetFontColorHelper @0x00466ac0`; `docs/research/2026-08-10-retail-ui-text-style.md` §2.3/§2.6 | | AP-192 | **Filed 2026-08-10 (Campaign CH round-5 polish, review item S2 — non-UiText outline paths).** Authored glyph outline `0x21`/outline color `0x22` now reach every text-bearing retained widget (`UiText`, `UiButton`, `UiDatElement`, `UiField`, `UiMeter`, `UiMenu`, `UiCatalogSlot` — the last two settable-only, having no authored build path), seeded ONCE from the element's effective-default state via `ElementReader.ApplyCanonicalLegacyProjection`'s `TryGetEffectiveProperty` (DirectState-then-effective-default rule). Retail instead re-resolves text properties on every UI STATE CHANGE — a button entering state `0x3` whose StateDesc authors `0x21=true` gains the outline for the duration of that state. The authored data hits this today: the dialog panel's two buttons (`0x2100003C` elements `0x17`/`0x19`), the character panel button `0x10000535`, and the combat panel button `0x100000B2` each author `0x21=true` in state `0x3` ONLY (DefaultStateId=1 → no outline at effective-default; `0x100000B2` also authors DirectState `0x21=true`, which the canonical rule DOES honor). The same seed-once shape already governs `UiText` (its `ApplyDatState` re-resolves `0x1B` FontColor per state but not `0x21`/`0x22`). `src/AcDream.App/UI/Layout/DatWidgetFactory.cs` (BuildButton/BuildCheckbox/BuildMeter/BuildText + the editable-field branch); `src/AcDream.App/UI/UiText.cs` (`ApplyDatState`) | Seed-once from the canonical effective state is strictly closer to retail than the pre-round-5 any-state first-wins scan (which lit those state-`0x3` outlines PERMANENTLY); the widening this row rides in on makes every ALWAYS-outlined authored element (DirectState/default-state authors) render retail-correct, and per-state re-resolution needs a property-application pass on the existing `TrySetRetailState` path — a reviewed slice of its own, not a polish-commit fold-in | A button that retail outlines only in a specific UI state (the four state-`0x3` authors above — state 3 is a hover/highlight-class state) never shows that transient outline in acdream; conversely nothing over-renders, since the effective-default resolution correctly yields outline-off for those elements | `UIElement_Text::SetOutline @0x0046a81c` (`m_bitField & 0x10`); `UIElement_Text::DrawSelf @0x00467aa0` (two-pass outline+fill); LayoutDesc fixtures `dialogs_2100003C.json` (`0x17`/`0x19`), `character_2100002E.json` (`0x10000535`), `combat_21000073.json` (`0x100000B2`) | -## 4. Temporary stopgap (TS) — 41 active rows (TS-72 filed 2026-08-10 at Campaign OP slice OP2 — `UiCheckboxBitfield64`'s click-toggle bit math is a conservative AND/OR approximation, flagged for OP5 to verify against the real `UIOption_CheckboxBitfield64` click-handler decomp before wiring the authoritative transaction; the widget is not yet reachable by any user (see the row below); TS-71 filed 2026-08-10 at Campaign OP slice OP1 — only the explicit SaveOptions flush trigger is production-wired; the 480 s timer and logout flush exist as tested pure state-machine logic but are not yet called from either host's live loop (see the row below); TS-70 RETIRED 2026-08-09 at Campaign CH user-gate round 1, item E (#362) — `ClientCommandResponses.cs` now parses and renders all four named inbound GameEvents (`ChannelIndex 0x0149`, `ChannelList 0x0148`, `AvailableHouses 0x0271`, `AllegianceInfoResponse 0x027C`), each wired into `GameEventWiring.cs` and rendering retail-shaped `LogTextType 0x00` lines ported from the named-retail decomp (`Handle_Communication__ChannelIndex`/`ChannelList` @0x0057d0c0/@0x0057d230, `Handle_House__Recv_AvailableHouses` + `DisplayListOfCoords` @0x00585d50/@0x00585c20, `Handle_Allegiance__AllegianceInfoResponseEvent` @0x0056a1d0); the row's `@on`/`@off` mention was never itself missing a handler (both already resolve through the pre-existing `WeenieErrorWithString` registration) so nothing there needed a fix; TS-68/TS-69 filed 2026-08-09, Campaign CH slice CH4 — the deferred allegiance/house subcommand dispatchers, the three unported pure-local commands (day/log/render); TS-66/TS-67 filed and TS-29 retired 2026-08-08, Campaign A slice A5 — the region ambient system landed, so TS-29's ambient half is ported and its music half turned out to have nothing to port; TS-66 is the omitted `seen_outside` interior case and TS-67 the in-plane contribution weight. TS-64/TS-65 filed 2026-08-08, Campaign A slice A2 — TS-64 the two unimplemented retail sound preferences (unfocused-app silence, pan disable) plus the three enable bools; TS-65 the volume-squared quirk, applied on the ambient path where two lanes byte-confirmed it and deliberately NOT on the hook path where the pre-multiplying overload is unpinned. TS-62/TS-63 filed 2026-08-02, continuation-executor slice; TS-4 and TS-8 retired 2026-07-31; Campaign P's goal-enumerated physics stopgaps are now zero. TS-4's graph/flat Path-6 branches match retail's foot SetCollide/Adjusted and head CollisionNormal/Collided split with no BSP-layer sliding-normal write; TS-8's live 0x02C2 carries its complete StatMod through the canonical enchantment record and updates effective stats immediately. Campaign P P7 2026-07-30: TS-25 retired — outbound stance has shipped via RawState.CurrentStyle since #219; TS-24 re-argued to AD-57; TS-40 re-argued to AD-58; TS-35 retired at P5; earlier same campaign: TS-1/TS-5/TS-23/TS-46 retired by ports; TS-23 retired 2026-07-30 at Campaign P Slice P3 — every mover-flags call site (local player world-entry ×2, remote DR sweep ×2, remote teleport, ordinary movers) now ORs in the mover's real PK/PKLite/Impenetrable `ObjectInfoState` bits via the new `ClientObjectTable`-backed `EntityCollisionFlagsExt.ResolveMoverPvpState` — **narrative corrected 2026-08-03 (#297): "real" only became true at #297. Until then the bits existed but the source `PublicWeenieBitfield` was frozen at CreateObject, so every one of those sites read a stale value for the whole session. The site enumeration is also incomplete: `RuntimeSetPositionMoverPreparation.cs:183-188` is a SEVENTH mover-flags site that decodes `record.Snapshot.ObjectDescriptionFlags` directly rather than calling `ResolveMoverPvpState`, and it also derives `ObjectInfoState.IsPlayer` from the PWD bit, contradicting `EntityCollisionFlags.cs:119-123`'s claim that every site uses a GUID-prefix heuristic. See AP-134.** — and `PlayerWeenie.JumpStaminaCost`'s `pk` parameter reads the real `PlayerKillerStatus`/`LastPkAttackTimestamp` pair against a 20-second window instead of a hardcoded `false`; the non-PK invariant (every ACE default-created character) is bit-identical to the pre-P3 value since `ResolveMoverPvpState` and the PK-timer predicate both resolve to a no-op for `PublicWeenieBitfield` absent/0; TS-46 retired 2026-07-30 at Campaign P Slice P3 — the Setup's verbatim ≤2-sphere list (`CPhysicsObj::transition` 0x00512dc0 → `SPHEREPATH::init_sphere` 0x0050c670) now seeds the sweep for the local player, remote dead-reckoning, and ordinary movers alike, replacing the two-scalar (radius, height) capsule reconstruction; remote/ordinary step-up/step-down are now Setup-derived (`CPartArray::GetStepUpHeight`/`GetStepDownHeight`, 0x005180d0/0x005180f0, ×ObjScale) instead of a hardcoded 0.4 m, closing both residuals the row named; TS-5 retired 2026-07-30 at Campaign P Slice P1 — real burden-gated CanJump + real JumpStaminaCost, both decomp-verbatim; TS-1 retired 2026-07-30 at Campaign P Slice P2 — the row was stale; the EdgeSlide → PrecipiceSlide/CliffSlide chain is already a real, tested port; TS-57..TS-61 filed 2026-07-29 during Campaign N — no outbound RejectRetransmit; TS-27 narrowed same slice to the inbound direction) + TS-37 historical note (TS-20 retired 2026-07-16 — the later named-retail audit disproved the proposed DrawingBSP polygon filter; TS-37 is a retired-row historical note, not an active count; TS-39 retired R5-V3 — sticky seams bound to the ported PositionManager/StickyManager, radii threaded; TS-45 retired 2026-07-07 — hand-rolled `SphereCollision` replaced by the faithful CSphere family port, fixing the player-vs-monster crowd wedge; TS-3 retired 2026-07-07 — `frames_stationary_fall` accounting ported in the #182 verbatim UpdateObjectInternal rebuild, fixing the airborne falling-animation wedge; TS-41 retired 2026-07-07 — SERVERVEL synth-velocity remote body-drive replaced by the retail interp catch-up + unconditional MovementManager::UseTime, the remote-creature de-overlap #184; TS-42 retired 2026-07-19 — semantic animation completion now precedes the ordered Target/Movement/PartArray/Position tail; TS-44 narrowed again 2026-07-19 — complete orientation joined interpolation, only during-stick enqueue suppression remains) +## 4. Temporary stopgap (TS) — 41 active rows (TS-73 filed 2026-08-11 at the Campaign OP OP1 review-fix round — `RuntimeCharacterOptionsState.TrySetOption`'s port of `CPlayerModule::OnChanged`'s local side-effect switch (MF-2) covers only the two `PlayerModule`-state-mutating cases (0x02/0x12 fellowship mutual exclusion); the four presentation-binding cases (weather/day/combat-target/fog) remain unmodeled, pre-anchored to Campaign OP OP4's Group B consumer binds (see the row below); TS-71 RETIRED 2026-08-11 at the same round — both remaining `SetCharacterOptions (0x01A1)` flush triggers (the 480 s auto-save timer, the pre-logoff flush) are now wired through `LiveSessionController`'s own tick/stop transaction (`ConfigureAutoSaveTick`/`ConfigurePreLogoffFlush`, wired once by `GameRuntime`'s constructor), matching the plan's stated target; TS-72 filed 2026-08-10 at Campaign OP slice OP2 — `UiCheckboxBitfield64`'s click-toggle bit math is a conservative AND/OR approximation, flagged for OP5 to verify against the real `UIOption_CheckboxBitfield64` click-handler decomp before wiring the authoritative transaction; the widget is not yet reachable by any user (see the row below); TS-70 RETIRED 2026-08-09 at Campaign CH user-gate round 1, item E (#362) — `ClientCommandResponses.cs` now parses and renders all four named inbound GameEvents (`ChannelIndex 0x0149`, `ChannelList 0x0148`, `AvailableHouses 0x0271`, `AllegianceInfoResponse 0x027C`), each wired into `GameEventWiring.cs` and rendering retail-shaped `LogTextType 0x00` lines ported from the named-retail decomp (`Handle_Communication__ChannelIndex`/`ChannelList` @0x0057d0c0/@0x0057d230, `Handle_House__Recv_AvailableHouses` + `DisplayListOfCoords` @0x00585d50/@0x00585c20, `Handle_Allegiance__AllegianceInfoResponseEvent` @0x0056a1d0); the row's `@on`/`@off` mention was never itself missing a handler (both already resolve through the pre-existing `WeenieErrorWithString` registration) so nothing there needed a fix; TS-68/TS-69 filed 2026-08-09, Campaign CH slice CH4 — the deferred allegiance/house subcommand dispatchers, the three unported pure-local commands (day/log/render); TS-66/TS-67 filed and TS-29 retired 2026-08-08, Campaign A slice A5 — the region ambient system landed, so TS-29's ambient half is ported and its music half turned out to have nothing to port; TS-66 is the omitted `seen_outside` interior case and TS-67 the in-plane contribution weight. TS-64/TS-65 filed 2026-08-08, Campaign A slice A2 — TS-64 the two unimplemented retail sound preferences (unfocused-app silence, pan disable) plus the three enable bools; TS-65 the volume-squared quirk, applied on the ambient path where two lanes byte-confirmed it and deliberately NOT on the hook path where the pre-multiplying overload is unpinned. TS-62/TS-63 filed 2026-08-02, continuation-executor slice; TS-4 and TS-8 retired 2026-07-31; Campaign P's goal-enumerated physics stopgaps are now zero. TS-4's graph/flat Path-6 branches match retail's foot SetCollide/Adjusted and head CollisionNormal/Collided split with no BSP-layer sliding-normal write; TS-8's live 0x02C2 carries its complete StatMod through the canonical enchantment record and updates effective stats immediately. Campaign P P7 2026-07-30: TS-25 retired — outbound stance has shipped via RawState.CurrentStyle since #219; TS-24 re-argued to AD-57; TS-40 re-argued to AD-58; TS-35 retired at P5; earlier same campaign: TS-1/TS-5/TS-23/TS-46 retired by ports; TS-23 retired 2026-07-30 at Campaign P Slice P3 — every mover-flags call site (local player world-entry ×2, remote DR sweep ×2, remote teleport, ordinary movers) now ORs in the mover's real PK/PKLite/Impenetrable `ObjectInfoState` bits via the new `ClientObjectTable`-backed `EntityCollisionFlagsExt.ResolveMoverPvpState` — **narrative corrected 2026-08-03 (#297): "real" only became true at #297. Until then the bits existed but the source `PublicWeenieBitfield` was frozen at CreateObject, so every one of those sites read a stale value for the whole session. The site enumeration is also incomplete: `RuntimeSetPositionMoverPreparation.cs:183-188` is a SEVENTH mover-flags site that decodes `record.Snapshot.ObjectDescriptionFlags` directly rather than calling `ResolveMoverPvpState`, and it also derives `ObjectInfoState.IsPlayer` from the PWD bit, contradicting `EntityCollisionFlags.cs:119-123`'s claim that every site uses a GUID-prefix heuristic. See AP-134.** — and `PlayerWeenie.JumpStaminaCost`'s `pk` parameter reads the real `PlayerKillerStatus`/`LastPkAttackTimestamp` pair against a 20-second window instead of a hardcoded `false`; the non-PK invariant (every ACE default-created character) is bit-identical to the pre-P3 value since `ResolveMoverPvpState` and the PK-timer predicate both resolve to a no-op for `PublicWeenieBitfield` absent/0; TS-46 retired 2026-07-30 at Campaign P Slice P3 — the Setup's verbatim ≤2-sphere list (`CPhysicsObj::transition` 0x00512dc0 → `SPHEREPATH::init_sphere` 0x0050c670) now seeds the sweep for the local player, remote dead-reckoning, and ordinary movers alike, replacing the two-scalar (radius, height) capsule reconstruction; remote/ordinary step-up/step-down are now Setup-derived (`CPartArray::GetStepUpHeight`/`GetStepDownHeight`, 0x005180d0/0x005180f0, ×ObjScale) instead of a hardcoded 0.4 m, closing both residuals the row named; TS-5 retired 2026-07-30 at Campaign P Slice P1 — real burden-gated CanJump + real JumpStaminaCost, both decomp-verbatim; TS-1 retired 2026-07-30 at Campaign P Slice P2 — the row was stale; the EdgeSlide → PrecipiceSlide/CliffSlide chain is already a real, tested port; TS-57..TS-61 filed 2026-07-29 during Campaign N — no outbound RejectRetransmit; TS-27 narrowed same slice to the inbound direction) + TS-37 historical note (TS-20 retired 2026-07-16 — the later named-retail audit disproved the proposed DrawingBSP polygon filter; TS-37 is a retired-row historical note, not an active count; TS-39 retired R5-V3 — sticky seams bound to the ported PositionManager/StickyManager, radii threaded; TS-45 retired 2026-07-07 — hand-rolled `SphereCollision` replaced by the faithful CSphere family port, fixing the player-vs-monster crowd wedge; TS-3 retired 2026-07-07 — `frames_stationary_fall` accounting ported in the #182 verbatim UpdateObjectInternal rebuild, fixing the airborne falling-animation wedge; TS-41 retired 2026-07-07 — SERVERVEL synth-velocity remote body-drive replaced by the retail interp catch-up + unconditional MovementManager::UseTime, the remote-creature de-overlap #184; TS-42 retired 2026-07-19 — semantic animation completion now precedes the ordered Target/Movement/PartArray/Position tail; TS-44 narrowed again 2026-07-19 — complete orientation joined interpolation, only during-stick enqueue suppression remains) | # | Divergence | Where (file:line) | Why it is safe / justified | Risk if assumption breaks | Retail oracle | |---|---|---|---|---|---| -| TS-71 | Only ONE of retail's three `SetCharacterOptions (0x01A1)` flush triggers is production-wired: the explicit `SaveOptions` Runtime command (`IRuntimeCharacterCommands.SaveOptions`, both host adapters). The 480 s auto-save timer (`CPlayerModule::UseTime`) and the flush-before-logout trigger (`CPlayerSystem::LogOffCharacter` calling `SaveToServer`) exist as fully-tested, pure Runtime state-machine logic (`RuntimeCharacterOptionsState.TryFlushIfAutoSaveDue` / `TryFlush`) but are not yet called from either host's live per-frame tick loop or graceful-shutdown sequence — a batched (non-auto-save) option change today reaches the server only via an explicit `SaveOptions` call (e.g. the future Options panel's Apply button), a subsequent option change that IS auto-save (which never touches the blob), or a fresh `PlayerDescription` overwriting the local draft on reconnect. | `src/AcDream.Runtime/Gameplay/RuntimeCharacterState.cs` (`RuntimeCharacterOptionsState.TryFlushIfAutoSaveDue`, `TryFlush`) | Wiring the timer requires a new per-frame phase touching both `AcDream.App`'s `UpdateFrameOrchestrator` graph and `AcDream.Headless`'s `HeadlessSessionHost.Tick` — outside Campaign OP slice OP1's Runtime/wire-layer scope. Wiring logout requires a live `WorldSession` reference at the moment either adapter's `Stop` command runs, ahead of `IRuntimeSessionCommands.Stop`'s teardown; deferred rather than risking the already-fragile graceful-shutdown sequence CLAUDE.md flags (ACE session-cleanup timing sensitivity) for an untested addition in this slice. | Until a follow-up slice wires these, a player who toggles ONLY batched (non-auto-save) Character-tab options and then disconnects without pressing Apply/calling `SaveOptions` loses those toggles — ACE never receives the blob. Auto-save ids (the 21-id table) are unaffected; they always send immediately regardless. | `CPlayerModule::UseTime @0x0059A710`; `CPlayerSystem::LogOffCharacter @0x00563520`; `docs/research/2026-08-10-set-character-options-wire.md` §3.3 | +| TS-73 | `RuntimeCharacterOptionsState.TrySetOption`'s port of `CPlayerModule::OnChanged @0x0059A8E0`'s local side-effect switch (step 2) covers only the two cases that mutate `PlayerModule` state itself — `case 2 IgnoreFellowshipRequests` / `case 0x12 FellowshipAutoAcceptRequests`, the fellowship mutual-exclusion pair (MF-2, OP1 review fix). The other four cases (`0x04 DisableMostWeatherEffects`, `0x05 PersistentAtDay`, `0x07 ViewCombatTarget`, `0x30 DisableDistanceFog`) are presentation bindings retail wires to `SmartBox::EnableWeather`/`LScape::SetDay`/`ClientCombatSystem::TrackTarget`/`LScape::m_fFogEnabled` — none of which this Runtime-only seam can reach today. | `src/AcDream.Runtime/Gameplay/RuntimeCharacterState.cs` (`RuntimeCharacterOptionsState.TrySetOption`) | Pre-anchored, not newly discovered: Campaign OP's own slice map already assigns these four options' consumers to OP4's "Group B one-line binds" (weather/daylight/fog) and OP3's mouse-turning row — the campaign plan (`docs/plans/2026-08-10-options-panel-campaign.md` §4 OP4) is the tracking mechanism, this row is the auditable pointer to it from the code that actually omits them. | Until OP4 lands, toggling `DisableMostWeatherEffects`/`PersistentAtDay`/`ViewCombatTarget`/`DisableDistanceFog` writes the bit and dirties/auto-saves it correctly, but produces NONE of retail's immediate local presentation change (weather doesn't stop, distance fog doesn't toggle, day/night doesn't force, combat-target tracking doesn't engage) until the next full reconnect re-derives it from some other path. | `CPlayerModule::OnChanged @0x0059A8E0`; `docs/research/2026-08-10-character-options-map.md` §1.5 | | TS-72 | `UiCheckboxBitfield64`'s click handler (the Chat tab's per-window text-filter block) applies a conservative AND/OR-mask toggle — a row is "on" when ALL its mask bits are set in the current value, and a click SETS (turns on) or CLEARS (turns off) exactly those bits — because the decompiled excerpt this campaign pulled documents `UIOption_CheckboxBitfield64::Apply`'s WRITE side (the property write into `PlayerModule::SetChatWindowOption`) but not the CLICK handler's own bit-toggle math. The widget itself is not yet wired into any page controller (Campaign OP slice OP2 ships the primitive only), so nothing user-reachable can observe a divergence yet. | `src/AcDream.App/UI/UiCheckboxBitfield64.cs` (`ToggleRow`) | Filed rather than guessed silently: the class doc/remarks flag the exact gap and name the follow-up slice (OP5, the Chat tab controller) that must verify/replace the algorithm against `UIOption_CheckboxBitfield64`'s real click-handler decomp before wiring the authoritative `PlayerModule::SetChatWindowOption` transaction. | If the real algorithm differs (e.g. XOR instead of set/clear, or partial-overlap handling), a later slice that wires this widget without re-verifying ships a subtly wrong per-row toggle for the Chat tab's text filters — a checkbox that doesn't match retail's exact on/off transition for masks with partially-overlapping bits. | `docs/research/2026-08-10-options-panel-structure.md` §3.7 (documents `Apply`'s write side only); `UIOption_CheckboxBitfield64::Apply @0x00485FF0` | | ~~TS-1~~ | **RETIRED 2026-07-30 (Campaign P Slice P2) — the row was stale, not the code.** The cited `:1254` line is unrelated stepping-loop code; the file moved substantially since the row was written. Retail's `EdgeSlide → PrecipiceSlide / CliffSlide` chain is already a real, tested port: `SpherePath.PrecipiceSlide` (`TransitionTypes.cs:943-970`, retail `SPHEREPATH::precipice_slide` pc:274316), `Transition.CliffSlide` (`:2080-2164`, retail `CTransition::cliff_slide` pc:272397, return-value mapping verified against `acclient.h:6100-6108`), and `Transition.EdgeSlideAfterStepDownFailed` (`:1907-2078`, mirrors `CTransition::edge_slide` pc:273001-273090). The one real gap (back-probe fallback skipping retail's `walkable_check_pos`/`localspace_sphere` recache, pc:274318-274326) needed no code change: acdream's `WalkableVertices`/`GlobalSphere` are populated in unified world space at assignment time (`SetWalkable`/`SetWalkableTransformed`, `SetCheckPos`/`RestoreCheckPos`), so both operands `BSPQuery.FindCrossedEdge` compares are already commensurable — retail's per-cell local-frame reprojection is a no-op correction here. Documented in-code at the back-probe site and pinned by `EdgeSlideBackProbePrecipiceSlideTests`. The chain's two acdream-only compensating branches (CliffSlide's three-source reference-normal fallback; the walkable-steepness reroute to CliffSlide before PrecipiceSlide) are real, non-retail additions — filed as AD-53 / AD-54 rather than folded into this row. | `src/AcDream.Core/Physics/TransitionTypes.cs` (`SpherePath.PrecipiceSlide`, `Transition.CliffSlide`, `Transition.EdgeSlideAfterStepDownFailed`); `tests/AcDream.Core.Tests/Physics/EdgeSlideBackProbePrecipiceSlideTests.cs` | — | — | `SPHEREPATH::precipice_slide` pc:274316 (0050cc80); `CTransition::cliff_slide` pc:272397 (0050a6d0); `CTransition::edge_slide` pc:273001-273090 (0050b3d0); `SPHEREPATH::get_walkable_pos`/`cache_localspace_sphere`/`set_walkable_check_pos` pc:274318-274326 (0050a8f0/0050c9d0/00509ce0); `docs/research/2026-07-30-response-layer-edge-family-pseudocode.md` §2, §6 Step 1 | | ~~TS-4~~ | **RETIRED 2026-07-31 (Campaign P Slice 2B; corrective acceptance complete).** The graph and prepared-flat Path-6 implementations now match retail's exact two-sphere split: every primary/foot polygon hit calls `SetCollide`, sets `WalkableAllowance=LandingZ`, and returns `Adjusted`; only a secondary/head hit writes `CollisionNormal` and returns `Collided`. The steep tangent shortcut and every BSP-layer `SetSlidingNormal` write are deleted. Exact site tests pin all changed and preserved fields plus raw-bit graph/flat parity. A corrective 90-tick already-airborne, zero-root-motion Core suite executes acceleration, body integration, transition resolution, exact commit, and `handle_all_collisions` while retaining every behavior-bearing collision/body field used by that specialized quantum. Vertical, inward, tangential, downhill, and positive-Z uphill-jump traces match graph/flat by raw bits, reject penetration/fixed points/second launches, and pin exact terminal velocity, contact, sliding, and contact-plane state. The older resolver-only capture is explicitly historical and restored to its three-second bound. | `src/AcDream.Core/Physics/BSPQuery.cs`; `src/AcDream.Core/Physics/FlatBspQuery.cs`; `tests/AcDream.Core.Tests/Physics/Ts4Path6ConformanceTests.cs`; `tests/AcDream.Core.Tests/Physics/Ts4ProductionQuantumConformanceTests.cs`; `tests/AcDream.Core.Tests/Physics/Ts4SteepRoofWedgeCaptureTests.cs` | — | — | `BSPTREE::find_collisions` 0x0053A440: head `0x0053A793..0x0053A7A4`, foot `0x0053A7B3..0x0053A7DC`; research §10 | diff --git a/src/AcDream.App/Net/LiveSessionRuntimeFactory.cs b/src/AcDream.App/Net/LiveSessionRuntimeFactory.cs index cdb9cf8b..71d3ab60 100644 --- a/src/AcDream.App/Net/LiveSessionRuntimeFactory.cs +++ b/src/AcDream.App/Net/LiveSessionRuntimeFactory.cs @@ -348,8 +348,10 @@ internal sealed class LiveSessionRuntimeFactory _domain.Character.Options.TrySetOption( optionId, value, - sendAutoSave: () => - session.SendSetSingleCharacterOption(optionId, value)); + // MF-2 (OP1 review fix, 2026-08-11): TrySetOption now takes + // (id, value) so its fellowship mutual-exclusion recursion + // can send a DIFFERENT id/value than this call's own. + sendAutoSave: session.SendSetSingleCharacterOption); // OP1: the explicit SaveOptions verb — retail's // CPlayerModule::SaveToServer(force: 0). No-ops when the batched diff --git a/src/AcDream.App/Runtime/CurrentGameRuntimeCommandAdapter.cs b/src/AcDream.App/Runtime/CurrentGameRuntimeCommandAdapter.cs index 74ccf66b..dd4d87df 100644 --- a/src/AcDream.App/Runtime/CurrentGameRuntimeCommandAdapter.cs +++ b/src/AcDream.App/Runtime/CurrentGameRuntimeCommandAdapter.cs @@ -701,6 +701,11 @@ internal sealed class CurrentGameRuntimeCommandAdapter if (gate != RuntimeCommandStatus.Accepted) return Result(gate); _commands.Publish(new SaveCharacterOptionsRuntimeCmd()); + // S4 (OP1 review fix, blast lens, 2026-08-11): objectId 0 — the bus + // has no return channel to report whether the deferred flush + // actually fired, so this host reports the SAME shape + // DirectGameRuntimeCommandAdapter.SaveOptions now does (Accepted, + // objectId 0) rather than a host-specific encoding. return EmitResult( RuntimeCommandDomain.Character, operation: 5, diff --git a/src/AcDream.Runtime/GameRuntime.cs b/src/AcDream.Runtime/GameRuntime.cs index c9321334..e1523c63 100644 --- a/src/AcDream.Runtime/GameRuntime.cs +++ b/src/AcDream.Runtime/GameRuntime.cs @@ -1,4 +1,5 @@ using System.Numerics; +using AcDream.Core.Net; using AcDream.Runtime.Entities; using AcDream.Runtime.Gameplay; using AcDream.Runtime.Physics; @@ -323,6 +324,23 @@ public sealed class GameRuntime TransitOwner = transit; GenerationReset = generationReset; _events = context.Events; + + // MUST-FIX 1 (Campaign OP OP1 review fix, 2026-08-11): wire + // retail's two remaining CPlayerModule::SaveToServer trigger + // sites — the 480 s auto-save timer (CPlayerModule::UseTime) and + // the pre-logoff flush (CPlayerSystem::LogOffCharacter) — through + // LiveSessionController's own tick/stop transaction, closing + // TS-71. Both share the SAME flush body the explicit SaveOptions + // command already uses (CharacterOptionsBlobSource.Capture + + // WorldSession.SendSetCharacterOptions); this talks to the + // WorldSession directly rather than through App's + // LiveSessionCommandRouter/LiveCommandBus, which is what keeps + // it off the S2 lock-order hazard the blast-lens review named. + context.Session.ConfigureAutoSaveTick( + session => FlushCharacterOptions(session, ifAutoSaveDue: true)); + context.Session.ConfigurePreLogoffFlush( + session => FlushCharacterOptions(session, ifAutoSaveDue: false)); + construction.Complete(); } catch (Exception failure) @@ -332,6 +350,59 @@ public sealed class GameRuntime } } + /// + /// The one flush body shared by every trigger that can send the batched + /// SetCharacterOptions (0x01A1) blob: the explicit + /// SaveOptions command (both IRuntimeCharacterCommands + /// adapters), the 480 s auto-save timer, and the pre-logoff flush (the + /// latter two wired via / in the constructor above). Reads + /// / at + /// invocation time, not construction time, so this is safe to bind + /// before either property's backing value is technically "public" — + /// both are always populated long before the session can ever tick or + /// stop. + /// + /// The pre-check + /// below is retail-faithful (CPlayerModule::UseTime @0x0059A710 + /// opens with the identical m_bDirty byte compare before it ever + /// touches the FPU timer math) AND load-bearing for allocation: without + /// it, the auto-save-tick hook would allocate a fresh flush closure on + /// EVERY call — every frame, + /// for every live session, whether or not anything is actually + /// dirty — which is exactly the per-tick allocation the K4 headless + /// resource-envelope gate measures. Gating on the cheap flag first means + /// the closure below is only ever allocated on the rare tick where a + /// flush might really happen. + /// + /// + private void FlushCharacterOptions(WorldSession session, bool ifAutoSaveDue) + { + RuntimeCharacterOptionsState options = CharacterOwner.Options; + if (!options.IsDirty) + return; + + void SendBlob() + { + CharacterOptionsBlobEcho echo = CharacterOptionsBlobSource.Capture( + CharacterOwner, + InventoryOwner.Shortcuts); + session.SendSetCharacterOptions( + echo.Options1, + echo.Options2, + echo.Shortcuts, + echo.FavoriteSpells, + echo.DesiredComponents, + echo.SpellbookFilters); + } + + if (ifAutoSaveDue) + options.TryFlushIfAutoSaveDue(SendBlob); + else + options.TryFlush(SendBlob); + } + public GameRuntimeClock Clock { get; } public LiveSessionController Session { get; } public RuntimeLocalPlayerIdentityState PlayerIdentity { get; } diff --git a/src/AcDream.Runtime/Gameplay/RuntimeCharacterState.cs b/src/AcDream.Runtime/Gameplay/RuntimeCharacterState.cs index 895c0991..a52b3fe5 100644 --- a/src/AcDream.Runtime/Gameplay/RuntimeCharacterState.cs +++ b/src/AcDream.Runtime/Gameplay/RuntimeCharacterState.cs @@ -21,7 +21,16 @@ public readonly record struct RuntimeCharacterOwnershipSnapshot( bool OptionsAreDefaults, bool MovementSkillsAreReset, /// C0-2: is back at retail's default (). - bool AutonomyIsDefault = true) + bool AutonomyIsDefault = true, + /// + /// S3 (Campaign OP OP1 review fix, 2026-08-11): !Options.IsDirty. + /// alone cannot catch a module whose + /// two words happen to have cycled back to their default bit pattern + /// (e.g. an option flipped off then back on) while m_bDirty is + /// still set — the ledger exists precisely to catch state a reset must + /// clear but a value-only comparison would miss. + /// + bool OptionsAreClean = true) { public bool IsConverged => IsDisposed @@ -37,7 +46,8 @@ public readonly record struct RuntimeCharacterOwnershipSnapshot( && PropertyCount == 0 && OptionsAreDefaults && MovementSkillsAreReset - && AutonomyIsDefault; + && AutonomyIsDefault + && OptionsAreClean; } /// @@ -221,7 +231,8 @@ public sealed class RuntimeCharacterState : IDisposable && _runSkillBase == -1 && _jumpSkillBase == -1 && _movementSkillAugmentations == default, - AutonomyLevel == FullAutonomyLevel); + AutonomyLevel == FullAutonomyLevel, + OptionsAreClean: !Options.IsDirty); } /// @@ -651,6 +662,7 @@ public sealed class RuntimeCharacterOptionsState private long _revision; private bool _isDirty; private DateTimeOffset _firstDirtiedAt; + private bool _hasServerSeed; public RuntimeCharacterOptionsState(TimeProvider? timeProvider = null) { @@ -678,14 +690,49 @@ public sealed class RuntimeCharacterOptionsState get { lock (_dirtyGate) return _isDirty ? _firstDirtiedAt : null; } } + /// + /// MUST-FIX M1 (Campaign OP OP1 review fix, 2026-08-11): has a real + /// PlayerDescription ever landed via since + /// construction / the last ? Starts + /// false — this module's two words start at the CLIENT + /// constructor defaults (/ + /// ), not the character's server-side + /// options, so flushing the batched 0x01A1 blob before the seed + /// arrives would ship client defaults over whatever ACE actually has + /// stored — the wipe class this gate exists to close. See + /// /. + /// + public bool HasServerSeed + { + get { lock (_dirtyGate) return _hasServerSeed; } + } + public bool DragItemOnPlayerOpensSecureTrade => Snapshot.DragItemOnPlayerOpensSecureTrade; + /// + /// Installs a fresh PlayerDescription's two option words as + /// server truth and arms (MUST-FIX M1). + /// S5 (OP1 review fix, blast lens): also clears / + /// — a re-seed WHOLESALE overwrites both + /// words with no partial-merge path (retail's own + /// PlayerModule::UnPack has none either), so any batched-but- + /// unflushed local intent is superseded the instant this runs: the + /// server's own values are now current, and a later flush would only + /// echo them back. Continuing to report the module dirty after a + /// wholesale overwrite would let a stale pending-save appear to persist + /// a change that no longer exists locally. + /// public void Replace(uint options1, uint options2) { Volatile.Write(ref _options1, options1); Volatile.Write(ref _options2, options2); Interlocked.Increment(ref _revision); + lock (_dirtyGate) + { + _isDirty = false; + _hasServerSeed = true; + } } /// @@ -693,14 +740,17 @@ public sealed class RuntimeCharacterOptionsState /// flip a character option funnels through — @join/@leave, the Settings /// Chat toggles, the Options panel, a headless bot, both /// IRuntimeCharacterCommands.SetSingleOption host adapters. - /// Mirrors CPlayerModule::OnChanged(PlayerOption) @0x0059A8E0 - /// exactly: write the bit into this LOCAL copy FIRST (so a same-session - /// consumer like is correct - /// before any round trip), THEN either invoke - /// immediately (retail's + /// Mirrors CPlayerModule::OnChanged(PlayerOption) @0x0059A8E0's + /// four-step body: write the bit into this LOCAL copy FIRST (so a + /// same-session consumer like is + /// correct before any round trip; step 1's local UI broadcast has no + /// acdream consumer today), THEN run the PlayerModule-state-mutating + /// half of step 2's side-effect switch (MF-2, Campaign OP OP1 review + /// fix, 2026-08-11 — see below), THEN either invoke + /// immediately (step 3, retail's /// IsAutoSaveOption branch — Event_PlayerOptionChangedEvent, /// the 0x0005 send) or for the batched - /// 0x01A1 flush (the else branch). Matches retail's own + /// 0x01A1 flush (step 4, the else branch). Matches retail's own /// unchanged-value early return (wire research §3.1 — "an unchanged /// option produces no notice, no side effect, no message at all") by /// no-op'ing when already holds. Returns @@ -708,8 +758,31 @@ public sealed class RuntimeCharacterOptionsState /// (retail's own IsAutoSaveOption/id-cast bounds check would /// reject it too) — callers turn that into a /// , never a silent send. + /// takes the (id, value) actually being + /// sent rather than closing over a fixed pair, because MF-2's recursive + /// clear below needs to send a DIFFERENT id/value than the caller's own + /// — every production caller now passes + /// WorldSession.SendSetSingleCharacterOption directly as the + /// method group. /// - public bool TrySetOption(uint characterOptionId, bool value, Action sendAutoSave) + /// + /// MF-2: step 2's local side-effect switch has six cases. Four are + /// presentation bindings (weather/day/combat-target/fog — Campaign OP's + /// later slices own those consumers) and stay unmodeled here. The other + /// two are the ONLY cases that mutate PlayerModule state itself — + /// case 2 IgnoreFellowshipRequests and + /// case 0x12 FellowshipAutoAcceptRequests each clear the OTHER + /// option when turned ON, through a real recursive accessor call in + /// retail (not an inlined bit-twiddle) — so turning one on while the + /// other is set produces the clear's own 0x0005 BEFORE the + /// primary option's own send (the nested call's IsAutoSaveOption branch + /// fires and returns before the outer call resumes past its own switch). + /// Both ids are themselves auto-save, so the recursion never touches + /// and always terminates after one level (the + /// clear passes value: false, which never re-triggers either + /// case's own "if now true" guard). + /// + public bool TrySetOption(uint characterOptionId, bool value, Action sendAutoSave) { ArgumentNullException.ThrowIfNull(sendAutoSave); if (!CharacterOptionTable.TryGet(characterOptionId, out CharacterOptionTableEntry entry)) @@ -721,8 +794,26 @@ public sealed class RuntimeCharacterOptionsState SetOptionBit(characterOptionId, value); + // MF-2: OnChanged @0x0059A8E0, cases 2/0x12 — read AFTER the local + // write above, exactly like retail's post-write accessor jump, so + // checking `value` directly is equivalent to re-reading the bit. + if (value && characterOptionId == (uint)CharacterOptionId.IgnoreFellowshipRequests) + { + TrySetOption( + (uint)CharacterOptionId.FellowshipAutoAcceptRequests, + false, + sendAutoSave); + } + else if (value && characterOptionId == (uint)CharacterOptionId.FellowshipAutoAcceptRequests) + { + TrySetOption( + (uint)CharacterOptionId.IgnoreFellowshipRequests, + false, + sendAutoSave); + } + if (entry.IsAutoSave) - sendAutoSave(); + sendAutoSave(characterOptionId, value); else MarkDirty(); @@ -784,47 +875,82 @@ public sealed class RuntimeCharacterOptionsState /// Retail's CPlayerModule::SaveToServer(force: 0) @0x0059A660 — /// both production call sites (Apply, logout) pass force = 0, so /// a clean module sends nothing. The explicit SaveOptions - /// Runtime command flushes through here. + /// Runtime command flushes through here. MUST-FIX M1 (Campaign OP OP1 + /// review fix, 2026-08-11): also refuses before + /// — flushing client-default words over the character's real server-side + /// options is the exact wipe class this gate exists to close; the module + /// stays dirty (nothing is lost) until a real + /// arrives. S2 (blast lens): the decision (dirty AND seeded) is made and + /// cleared under _dirtyGate, but itself + /// runs OUTSIDE the lock — the natural TS-71 auto-save-timer wiring + /// invokes this from Runtime's own tick while a DIFFERENT thread may be + /// routing a same-tick option toggle through App's + /// LiveSessionCommandRouter (which holds ITS OWN _gate + /// before reaching 's _dirtyGate); holding + /// _dirtyGate across a caller-supplied callback that could + /// transitively want _gate is the lock-inversion shape that + /// finding named. A throw from propagates with + /// the module left dirty (nothing cleared) — the correct direction, since + /// nothing actually reached the wire. /// public bool TryFlush(Action flush) { ArgumentNullException.ThrowIfNull(flush); lock (_dirtyGate) { - if (!_isDirty) return false; - flush(); - _isDirty = false; - return true; + if (!_isDirty || !_hasServerSeed) return false; } + flush(); + lock (_dirtyGate) + { + _isDirty = false; + } + return true; } /// /// Retail's CPlayerModule::UseTime @0x0059A710: flush iff dirty - /// AND at least (480 s, BYTE-VERIFIED) has - /// elapsed since . A no-op host may call - /// this once per tick; it is cheap and inert unless the timer is - /// actually due. + /// AND seeded (MUST-FIX M1 — see ) AND at least + /// (480 s, BYTE-VERIFIED) has elapsed since + /// . A no-op host may call this once per + /// tick; it is cheap and inert unless the timer is actually due. S2: the + /// decide-and-clear/callback-outside-the-lock split matches + /// exactly, for the same lock-order reason. /// public bool TryFlushIfAutoSaveDue(Action flush) { ArgumentNullException.ThrowIfNull(flush); lock (_dirtyGate) { - if (!_isDirty) return false; + if (!_isDirty || !_hasServerSeed) return false; if (_timeProvider.GetUtcNow() - _firstDirtiedAt < AutoSaveDelay) return false; - flush(); - _isDirty = false; - return true; } + flush(); + lock (_dirtyGate) + { + _isDirty = false; + } + return true; } + /// + /// Restores the client-constructor defaults AND clears + /// (MUST-FIX M1) — a reconnect's fresh + /// PlayerDescription must re-arm the seed via a NEW + /// call before the next flush can succeed; a stale + /// seed surviving a session boundary could let a flush ship the PRIOR + /// character's words over the new one's. + /// public void ResetSession() { Volatile.Write(ref _options1, DefaultOptions1); Volatile.Write(ref _options2, DefaultOptions2); Interlocked.Increment(ref _revision); lock (_dirtyGate) + { _isDirty = false; + _hasServerSeed = false; + } } } diff --git a/src/AcDream.Runtime/Session/DirectGameRuntimeCommandAdapter.cs b/src/AcDream.Runtime/Session/DirectGameRuntimeCommandAdapter.cs index fd9c0662..e307d8d7 100644 --- a/src/AcDream.Runtime/Session/DirectGameRuntimeCommandAdapter.cs +++ b/src/AcDream.Runtime/Session/DirectGameRuntimeCommandAdapter.cs @@ -666,8 +666,11 @@ public sealed class DirectGameRuntimeCommandAdapter bool accepted = _runtime.CharacterOwner.Options.TrySetOption( optionId, value, - sendAutoSave: () => - session!.SendSetSingleCharacterOption(optionId, value)); + // MF-2 (OP1 review fix, 2026-08-11): TrySetOption now takes + // (id, value) rather than a fixed pair — its fellowship + // mutual-exclusion recursion needs to send a DIFFERENT id/value + // than this call's own. + sendAutoSave: session!.SendSetSingleCharacterOption); return EmitResult( RuntimeCommandDomain.Character, operation: 4, @@ -681,7 +684,11 @@ public sealed class DirectGameRuntimeCommandAdapter Validate(expectedGeneration, out WorldSession? session); if (gate != RuntimeCommandStatus.Accepted) return Result(gate); - bool flushed = _runtime.CharacterOwner.Options.TryFlush(() => + // MUST-FIX M1 (OP1 review fix, 2026-08-11): TryFlush itself now + // refuses before RuntimeCharacterOptionsState.HasServerSeed, so a + // SaveOptions called before the first PlayerDescription lands is a + // safe no-op rather than a wipe. + _runtime.CharacterOwner.Options.TryFlush(() => { CharacterOptionsBlobEcho echo = CharacterOptionsBlobSource.Capture( _runtime.CharacterOwner, @@ -694,11 +701,18 @@ public sealed class DirectGameRuntimeCommandAdapter echo.DesiredComponents, echo.SpellbookFilters); }); + // S4 (OP1 review fix, blast lens): PrimaryObjectId is a typed guid + // field in the K2 bot-facing event stream (GameRuntimeEventHub.cs / + // GameRuntimeEvents.cs) — the previous `flushed ? 1u : 0u` encoding + // read as object guid 0x00000001 there. + // CurrentGameRuntimeCommandAdapter's graphical route cannot report + // whether the flush actually fired at all (LiveCommandBus.Publish + // has no return channel), so both hosts now report the identical + // shape: Accepted, objectId 0. return EmitResult( RuntimeCommandDomain.Character, operation: 5, - RuntimeCommandStatus.Accepted, - primaryObjectId: flushed ? 1u : 0u); + RuntimeCommandStatus.Accepted); } public RuntimeCommandResult Execute( diff --git a/src/AcDream.Runtime/Session/LiveSessionController.cs b/src/AcDream.Runtime/Session/LiveSessionController.cs index ee352c97..d09f0ad9 100644 --- a/src/AcDream.Runtime/Session/LiveSessionController.cs +++ b/src/AcDream.Runtime/Session/LiveSessionController.cs @@ -266,6 +266,8 @@ public sealed class LiveSessionController private ulong _generation; private RuntimeTeardownStage _lastTeardownStages; private LiveSessionCharacterSelection? _activeSelection; + private Action? _autoSaveTickHook; + private Action? _preLogoffFlushHook; public LiveSessionController() : this(ProductionLiveSessionOperations.Instance) @@ -297,6 +299,38 @@ public sealed class LiveSessionController get { lock (_gate) return new RuntimeGenerationToken(_generation); } } + /// + /// MUST-FIX 1 (Campaign OP OP1 review fix, 2026-08-11 — mechanism lens + /// finding): reaches retail's CPlayerModule::UseTime (the 480 s + /// batched character-option auto-save) from the SAME per-session tick + /// both the graphical host (RetailLiveFrameCoordinator) and the + /// no-window host (HeadlessSessionHost.Tick) already call — + /// — with ZERO host edits. GameRuntime wires + /// this once, after constructing CharacterOwner/ + /// InventoryOwner, with a flush body that talks to the + /// directly rather than through App's + /// LiveSessionCommandRouter/LiveCommandBus — which is what + /// keeps this wiring off the S2 lock-order hazard the blast-lens finding + /// named (this hook never touches the router's own gate). A hook + /// throwing is caught and logged (), never treated + /// as a tick failure — a transient send error on a background auto-save + /// must not tear down the whole live session. + /// + internal void ConfigureAutoSaveTick(Action hook) => + _autoSaveTickHook = hook ?? throw new ArgumentNullException(nameof(hook)); + + /// + /// MUST-FIX 1: reaches retail's CPlayerSystem::LogOffCharacter → + /// SaveToServer ordering — flush the batched option module BEFORE + /// the character-logoff wire request goes out — from Runtime's own + /// Stop/teardown transaction (), using the + /// CURRENT (not-yet-retired) session. Caught and logged rather than + /// propagated: a failed flush must not block the graceful-shutdown + /// sequence CLAUDE.md flags as ACE-timing-sensitive. + /// + internal void ConfigurePreLogoffFlush(Action hook) => + _preLogoffFlushHook = hook ?? throw new ArgumentNullException(nameof(hook)); + public bool IsDisposalComplete { get { lock (_gate) return _disposed; } @@ -440,10 +474,31 @@ public sealed class LiveSessionController throw; throw error; } + + // MUST-FIX 1: TS-71's 480 s auto-save timer half. Runs after + // the protocol pump above and only when the scope/generation + // are still current — a reconnect that happened mid-tick + // must not flush against a retired session. + InvokeAutoSaveTick(scope.Session); }); } } + private void InvokeAutoSaveTick(WorldSession session) + { + if (_autoSaveTickHook is not { } hook) + return; + try + { + hook(session); + } + catch (Exception error) + { + Console.Error.WriteLine( + $"live: auto-save character-options tick failed: {error.Message}"); + } + } + public void Dispose() { lock (_gate) @@ -621,6 +676,16 @@ public sealed class LiveSessionController private void StopCore() { + // MUST-FIX 1: TS-71's logout-flush half — retail's + // CPlayerSystem::LogOffCharacter calls SaveToServer BEFORE the + // character-logoff wire request, so this runs before anything below + // touches the scope (teardown, generation bump). Gated on _inWorld + // so a failed/never-entered-world Stop (still connecting, no + // character session) never fires it — matching retail's own call + // site, which only exists on an actual in-world character. + if (_inWorld && _scope is { } activeScope) + InvokePreLogoffFlush(activeScope.Session); + ++_generation; _inWorld = false; _activeSelection = null; @@ -638,6 +703,21 @@ public sealed class LiveSessionController _lastTeardownStages = RuntimeTeardownStage.Complete; } + private void InvokePreLogoffFlush(WorldSession session) + { + if (_preLogoffFlushHook is not { } hook) + return; + try + { + hook(session); + } + catch (Exception error) + { + Console.Error.WriteLine( + $"live: pre-logoff character-options flush failed: {error.Message}"); + } + } + private void DrainRetiredScope() { if (_retiredScope is not { } retired) diff --git a/tests/AcDream.App.Tests/Net/LiveSessionCommandRouterTests.cs b/tests/AcDream.App.Tests/Net/LiveSessionCommandRouterTests.cs index f66b7596..d3d69480 100644 --- a/tests/AcDream.App.Tests/Net/LiveSessionCommandRouterTests.cs +++ b/tests/AcDream.App.Tests/Net/LiveSessionCommandRouterTests.cs @@ -487,16 +487,22 @@ public sealed class LiveSessionCommandRouterTests societyEldrytchWebRoom: 0u, societyRadiantBloodRoom: 0u); var sent = new List<(uint OptionId, bool Value)>(); - // Mirrors LiveSessionRuntimeFactory.CreateCommandBindings' shared - // SendSingleCharacterOption local function: local write FIRST, then - // the wire send. + // S1 (Campaign OP OP1 review fix, blast lens, 2026-08-11): drives the + // REAL production binding — RuntimeCharacterOptionsState. + // TrySetOption, the SAME shared local-write-then-send/dirty seam + // LiveSessionRuntimeFactory.CreateCommandBindings' SendSingleCharacterOption + // local function calls — instead of a hand-rolled substitute that had + // silently drifted from it after OP1 (the previous shape called + // SetOptionBit directly, which does not run TrySetOption's + // unchanged-value early return or its MF-2 fellowship + // mutual-exclusion side effect). LiveSessionCommandRouter router = NewRouter( characterState: characterState, sendSingleCharacterOption: (id, value) => - { - characterState.Options.SetOptionBit(id, value); - sent.Add((id, value)); - }); + characterState.Options.TrySetOption( + id, + value, + sendAutoSave: (sentId, sentValue) => sent.Add((sentId, sentValue)))); router.Activate(); Assert.Equal( diff --git a/tests/AcDream.Runtime.Tests/Gameplay/CharacterOptionTableTests.cs b/tests/AcDream.Runtime.Tests/Gameplay/CharacterOptionTableTests.cs index 7254e1db..edb8728e 100644 --- a/tests/AcDream.Runtime.Tests/Gameplay/CharacterOptionTableTests.cs +++ b/tests/AcDream.Runtime.Tests/Gameplay/CharacterOptionTableTests.cs @@ -185,4 +185,114 @@ public sealed class CharacterOptionTableTests for (uint id = 0x00; id <= 0x34; id++) yield return [(CharacterOptionId)id]; } + + // ── SHOULD-FIX MF-3 (Campaign OP OP1 review fix, mechanism lens, + // 2026-08-11): a hand-transcribed 53-row (word, mask) pin, the SAME + // shape as AutoSaveIds/ClientDefaultOnIds above — a transposition among + // the 37 non-ClientDefault masks (e.g. swapping DisplayAge's O2 0x20 + // with DisplayNumberDeaths' O2 0x10) would previously pass the entire + // suite silently; this is the id-by-id guard against exactly that. + // Transcribed independently from named-retail/acclient.h:4162-4218 + // (`enum PlayerOption`, the id space) cross-referenced by NAME against + // :3404-3436 (`enum CharacterOption`, Options1) and :3451-3481 + // (`enum CharacterOptions2`) — not derived from CharacterOptionTable.cs. + [Theory] + [InlineData(CharacterOptionId.AutoRepeatAttack, true, 0x00000002u)] + [InlineData(CharacterOptionId.IgnoreAllegianceRequests, true, 0x00000004u)] + [InlineData(CharacterOptionId.IgnoreFellowshipRequests, true, 0x00000008u)] + [InlineData(CharacterOptionId.IgnoreTradeRequests, true, 0x00020000u)] + [InlineData(CharacterOptionId.DisableMostWeatherEffects, true, 0x00010000u)] + [InlineData(CharacterOptionId.PersistentAtDay, false, 0x00000001u)] + [InlineData(CharacterOptionId.AllowGive, true, 0x00000040u)] + [InlineData(CharacterOptionId.ViewCombatTarget, true, 0x00000080u)] + [InlineData(CharacterOptionId.ShowTooltips, true, 0x00000100u)] + [InlineData(CharacterOptionId.UseDeception, true, 0x00000200u)] + [InlineData(CharacterOptionId.ToggleRun, true, 0x00000400u)] + [InlineData(CharacterOptionId.StayInChatMode, true, 0x00000800u)] + [InlineData(CharacterOptionId.AdvancedCombatUI, true, 0x00001000u)] + [InlineData(CharacterOptionId.AutoTarget, true, 0x00002000u)] + [InlineData(CharacterOptionId.VividTargetingIndicator, true, 0x00008000u)] + [InlineData(CharacterOptionId.FellowshipShareXP, true, 0x00040000u)] + [InlineData(CharacterOptionId.AcceptLootPermits, true, 0x00080000u)] + [InlineData(CharacterOptionId.FellowshipShareLoot, true, 0x00100000u)] + [InlineData(CharacterOptionId.FellowshipAutoAcceptRequests, true, 0x20000000u)] + [InlineData(CharacterOptionId.SideBySideVitals, true, 0x00200000u)] + [InlineData(CharacterOptionId.CoordinatesOnRadar, true, 0x00400000u)] + [InlineData(CharacterOptionId.SpellDuration, true, 0x00800000u)] + [InlineData(CharacterOptionId.DisableHouseRestrictionEffects, true, 0x02000000u)] + [InlineData(CharacterOptionId.DragItemOnPlayerOpensSecureTrade, true, 0x04000000u)] + [InlineData(CharacterOptionId.DisplayAllegianceLogonNotifications, true, 0x08000000u)] + [InlineData(CharacterOptionId.UseChargeAttack, true, 0x10000000u)] + [InlineData(CharacterOptionId.UseCraftSuccessDialog, true, 0x80000000u)] + [InlineData(CharacterOptionId.ListenToAllegianceChat, true, 0x40000000u)] + [InlineData(CharacterOptionId.DisplayDateOfBirth, false, 0x00000002u)] + [InlineData(CharacterOptionId.DisplayAge, false, 0x00000020u)] + [InlineData(CharacterOptionId.DisplayChessRank, false, 0x00000004u)] + [InlineData(CharacterOptionId.DisplayFishingSkill, false, 0x00000008u)] + [InlineData(CharacterOptionId.DisplayNumberDeaths, false, 0x00000010u)] + [InlineData(CharacterOptionId.DisplayTimeStamps, false, 0x00000040u)] + [InlineData(CharacterOptionId.SalvageMultiple, false, 0x00000080u)] + [InlineData(CharacterOptionId.ListenToGeneralChat, false, 0x00000100u)] + [InlineData(CharacterOptionId.ListenToTradeChat, false, 0x00000200u)] + [InlineData(CharacterOptionId.ListenToLFGChat, false, 0x00000400u)] + [InlineData(CharacterOptionId.ListenToRoleplayChat, false, 0x00000800u)] + [InlineData(CharacterOptionId.AppearOffline, false, 0x00001000u)] + [InlineData(CharacterOptionId.DisplayNumberCharacterTitles, false, 0x00002000u)] + [InlineData(CharacterOptionId.MainPackPreferred, false, 0x00004000u)] + [InlineData(CharacterOptionId.LeadMissileTargets, false, 0x00008000u)] + [InlineData(CharacterOptionId.UseFastMissiles, false, 0x00010000u)] + [InlineData(CharacterOptionId.FilterLanguage, false, 0x00020000u)] + [InlineData(CharacterOptionId.ConfirmVolatileRareUse, false, 0x00040000u)] + [InlineData(CharacterOptionId.ListenToSocietyChat, false, 0x00080000u)] + [InlineData(CharacterOptionId.ShowHelm, false, 0x00100000u)] + [InlineData(CharacterOptionId.DisableDistanceFog, false, 0x00200000u)] + [InlineData(CharacterOptionId.UseMouseTurning, false, 0x00400000u)] + [InlineData(CharacterOptionId.ShowCloak, false, 0x00800000u)] + [InlineData(CharacterOptionId.LockUI, false, 0x01000000u)] + // D3 / register row: ACE-sourced (ListenToPKDeathMessages), unverifiable + // against the 2013 binary — see the type doc on CharacterOptionTable. + [InlineData(CharacterOptionId.HearPkDeathMessages, false, 0x02000000u)] + public void WordAndMask_MatchesIndependentTranscriptionOfVerbatimAcclientEnums( + CharacterOptionId id, bool expectedIsOptions1, uint expectedMask) + { + Assert.True(CharacterOptionTable.TryGet(id, out CharacterOptionTableEntry entry)); + Assert.Equal(expectedIsOptions1, entry.IsOptions1); + Assert.Equal(expectedMask, entry.Mask); + } + + [Fact] + public void WordAndMask_AreAllPairwiseDistinct() + { + // N7 (blast lens): the reconstruction test above cannot catch a + // duplicate because OR is idempotent — this is the direct guard. + var pairs = CharacterOptionTable.All + .Select(static e => (e.IsOptions1, e.Mask)) + .ToList(); + Assert.Equal(53, pairs.Distinct().Count()); + } + + // ── S6 (Campaign OP OP1 review fix, blast lens, 2026-08-11): the write + // path (this table) and the read path (PlayerDescriptionParser. + // CharacterOptions1/2, consumed by TurbineChatMembershipGate and + // RuntimeSettingsController) define the SAME retail bits independently, + // in different projects, with nothing else asserting they agree — an + // edit to one without the other silently diverges the write path from + // the membership gate (the exact CH3 failure class). Covers every + // non-None/Default member of both parser enums. + [Theory] + [InlineData(CharacterOptionId.AllowGive, true, (uint)PlayerDescriptionParser.CharacterOptions1.AllowGive)] + [InlineData(CharacterOptionId.ListenToAllegianceChat, true, (uint)PlayerDescriptionParser.CharacterOptions1.HearAllegianceChat)] + [InlineData(CharacterOptionId.DragItemOnPlayerOpensSecureTrade, true, (uint)PlayerDescriptionParser.CharacterOptions1.DragItemOnPlayerOpensSecureTrade)] + [InlineData(CharacterOptionId.ListenToGeneralChat, false, (uint)PlayerDescriptionParser.CharacterOptions2.HearGeneralChat)] + [InlineData(CharacterOptionId.ListenToTradeChat, false, (uint)PlayerDescriptionParser.CharacterOptions2.HearTradeChat)] + [InlineData(CharacterOptionId.ListenToLFGChat, false, (uint)PlayerDescriptionParser.CharacterOptions2.HearLFGChat)] + [InlineData(CharacterOptionId.ListenToRoleplayChat, false, (uint)PlayerDescriptionParser.CharacterOptions2.HearRoleplayChat)] + [InlineData(CharacterOptionId.ListenToSocietyChat, false, (uint)PlayerDescriptionParser.CharacterOptions2.HearSocietyChat)] + public void CharacterOptionTable_AgreesWithPlayerDescriptionParserEnums( + CharacterOptionId id, bool expectedIsOptions1, uint expectedMask) + { + Assert.True(CharacterOptionTable.TryGet(id, out CharacterOptionTableEntry entry)); + Assert.Equal(expectedIsOptions1, entry.IsOptions1); + Assert.Equal(expectedMask, entry.Mask); + } } diff --git a/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCharacterStateTests.cs b/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCharacterStateTests.cs index 8d33593d..45c2a1f0 100644 --- a/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCharacterStateTests.cs +++ b/tests/AcDream.Runtime.Tests/Gameplay/RuntimeCharacterStateTests.cs @@ -299,8 +299,7 @@ public sealed class RuntimeCharacterStateTests bool accepted = options.TrySetOption( (uint)CharacterOptionId.ListenToGeneralChat, true, - sendAutoSave: () => sent.Add( - ((uint)CharacterOptionId.ListenToGeneralChat, true))); + sendAutoSave: (id, value) => sent.Add((id, value))); Assert.True(accepted); Assert.Equal( @@ -323,7 +322,7 @@ public sealed class RuntimeCharacterStateTests bool accepted = options.TrySetOption( (uint)CharacterOptionId.AutoTarget, false, - sendAutoSave: () => sent.Add(((uint)CharacterOptionId.AutoTarget, false))); + sendAutoSave: (id, value) => sent.Add((id, value))); Assert.True(accepted); // AutoTarget_CharacterOption = 0x2000 (acclient.h:3417). @@ -343,7 +342,7 @@ public sealed class RuntimeCharacterStateTests bool accepted = options.TrySetOption( (uint)CharacterOptionId.AutoTarget, true, - sendAutoSave: () => sent.Add(((uint)CharacterOptionId.AutoTarget, true))); + sendAutoSave: (id, value) => sent.Add((id, value))); Assert.True(accepted); Assert.Empty(sent); @@ -356,13 +355,118 @@ public sealed class RuntimeCharacterStateTests var options = new RuntimeCharacterOptionsState(); bool invoked = false; - bool accepted = options.TrySetOption(0x35u, true, () => invoked = true); + bool accepted = options.TrySetOption(0x35u, true, (_, _) => invoked = true); Assert.False(accepted); Assert.False(invoked); Assert.False(options.IsDirty); } + // ── MF-2 (Campaign OP OP1 review fix, 2026-08-11): CPlayerModule:: + // OnChanged @0x0059A8E0's fellowship mutual-exclusion side effect ────── + + [Fact] + public void TrySetOption_TurningOnIgnoreFellowshipRequests_ClearsAutoAccept_ClearSendsBeforePrimary() + { + var options = new RuntimeCharacterOptionsState(); + // Arm AutoAcceptFellowshipRequests ON first so there is something + // for the recursive clear to actually clear. + options.TrySetOption( + (uint)CharacterOptionId.FellowshipAutoAcceptRequests, true, (_, _) => { }); + var sent = new List<(uint OptionId, bool Value)>(); + + bool accepted = options.TrySetOption( + (uint)CharacterOptionId.IgnoreFellowshipRequests, + true, + sendAutoSave: (id, value) => sent.Add((id, value))); + + Assert.True(accepted); + // Retail's OnChanged runs the recursive clear (a REAL nested + // accessor call, complete with its own immediate 0x0005) BEFORE + // returning to finish the outer call's own IsAutoSaveOption branch + // — so the clear reaches the wire FIRST. + Assert.Equal( + [ + ((uint)CharacterOptionId.FellowshipAutoAcceptRequests, false), + ((uint)CharacterOptionId.IgnoreFellowshipRequests, true), + ], + sent); + Assert.NotEqual(0u, options.Options1 & 0x00000008u); // IgnoreFellowshipRequests set + Assert.Equal(0u, options.Options1 & 0x20000000u); // AutoAccept cleared + } + + [Fact] + public void TrySetOption_TurningOnAutoAcceptFellowship_ClearsIgnoreRequests_ClearSendsBeforePrimary() + { + var options = new RuntimeCharacterOptionsState(); + options.TrySetOption( + (uint)CharacterOptionId.IgnoreFellowshipRequests, true, (_, _) => { }); + var sent = new List<(uint OptionId, bool Value)>(); + + bool accepted = options.TrySetOption( + (uint)CharacterOptionId.FellowshipAutoAcceptRequests, + true, + sendAutoSave: (id, value) => sent.Add((id, value))); + + Assert.True(accepted); + Assert.Equal( + [ + ((uint)CharacterOptionId.IgnoreFellowshipRequests, false), + ((uint)CharacterOptionId.FellowshipAutoAcceptRequests, true), + ], + sent); + Assert.NotEqual(0u, options.Options1 & 0x20000000u); // AutoAccept set + Assert.Equal(0u, options.Options1 & 0x00000008u); // IgnoreFellowshipRequests cleared + } + + [Fact] + public void TrySetOption_TurningOnFellowshipOption_WhenTheOtherIsAlreadyOff_SendsOnlyThePrimary() + { + var options = new RuntimeCharacterOptionsState(); + // IgnoreFellowshipRequests defaults ON (ClientDefault=true) — flip + // it off first so the "turn on" below is a REAL transition. + options.TrySetOption( + (uint)CharacterOptionId.IgnoreFellowshipRequests, false, (_, _) => { }); + var sent = new List<(uint OptionId, bool Value)>(); + + // FellowshipAutoAcceptRequests already off — the recursive clear's + // own TrySetOption call must early-return silently (retail's + // accessor's own unchanged-value early return), producing exactly + // ONE wire send, not two. + bool accepted = options.TrySetOption( + (uint)CharacterOptionId.IgnoreFellowshipRequests, + true, + sendAutoSave: (id, value) => sent.Add((id, value))); + + Assert.True(accepted); + Assert.Equal([((uint)CharacterOptionId.IgnoreFellowshipRequests, true)], sent); + } + + [Fact] + public void TrySetOption_TurningOffAFellowshipOption_NeverTriggersTheClear() + { + var options = new RuntimeCharacterOptionsState(); + // Force BOTH bits on directly — SetOptionBit bypasses OnChanged's + // side-effect switch entirely, so this reaches a state retail's OWN + // accessors (and TrySetOption) can never produce, but one a fresh + // PlayerDescription CAN carry (ACE performs no validation/clamping + // on these bits, wire research §5.1). + options.SetOptionBit((uint)CharacterOptionId.IgnoreFellowshipRequests, true); + options.SetOptionBit((uint)CharacterOptionId.FellowshipAutoAcceptRequests, true); + var sent = new List<(uint OptionId, bool Value)>(); + + // Retail's case 2/0x12 only fire "if now true" — turning ONE off + // must not touch the other. + bool accepted = options.TrySetOption( + (uint)CharacterOptionId.IgnoreFellowshipRequests, + false, + sendAutoSave: (id, value) => sent.Add((id, value))); + + Assert.True(accepted); + Assert.Equal([((uint)CharacterOptionId.IgnoreFellowshipRequests, false)], sent); + Assert.NotEqual(0u, options.Options1 & 0x20000000u); // AutoAccept untouched (still on) + } + [Fact] public void MarkDirty_OnlySecondCallDoesNotPushOutFirstDirtiedAt() { @@ -370,17 +474,174 @@ public sealed class RuntimeCharacterStateTests var options = new RuntimeCharacterOptionsState(clock); options.TrySetOption( - (uint)CharacterOptionId.AutoTarget, false, () => { }); + (uint)CharacterOptionId.AutoTarget, false, (_, _) => { }); DateTimeOffset? firstStamp = options.FirstDirtiedAt; Assert.NotNull(firstStamp); clock.Advance(TimeSpan.FromSeconds(10)); options.TrySetOption( - (uint)CharacterOptionId.ShowTooltips, false, () => { }); + (uint)CharacterOptionId.ShowTooltips, false, (_, _) => { }); Assert.Equal(firstStamp, options.FirstDirtiedAt); } + // ── MUST-FIX M1 (Campaign OP OP1 review fix, 2026-08-11): the server- + // seed latch guarding TryFlush/TryFlushIfAutoSaveDue ─────────────────── + + [Fact] + public void TryFlush_RefusesBeforeServerSeed_EvenWhenDirty_ThenSucceedsAfterSeed() + { + var options = new RuntimeCharacterOptionsState(); + Assert.False(options.HasServerSeed); + + options.TrySetOption( + (uint)CharacterOptionId.AutoTarget, false, (_, _) => { }); + Assert.True(options.IsDirty); + + int flushes = 0; + Assert.False(options.TryFlush(() => flushes++)); + Assert.Equal(0, flushes); + // Nothing lost, nothing sent — the pending change is still pending. + Assert.True(options.IsDirty); + + // A real PlayerDescription lands. S5: the seed supersedes the + // pending change (retail's own PlayerModule is likewise clobbered by + // a wholesale re-seed), so re-dirty AFTER the seed to prove the + // GATE (not the module) was what refused above. + options.Replace(options.Options1, options.Options2); + Assert.True(options.HasServerSeed); + Assert.False(options.IsDirty); + + options.TrySetOption( + (uint)CharacterOptionId.AutoTarget, true, (_, _) => { }); + Assert.True(options.TryFlush(() => flushes++)); + Assert.Equal(1, flushes); + } + + [Fact] + public void TryFlushIfAutoSaveDue_RefusesBeforeServerSeed_EvenAtThreshold() + { + var clock = new ManualTimeProvider(); + var options = new RuntimeCharacterOptionsState(clock); + options.TrySetOption( + (uint)CharacterOptionId.AutoTarget, false, (_, _) => { }); + + clock.Advance(RuntimeCharacterOptionsState.AutoSaveDelay + TimeSpan.FromSeconds(1)); + + int flushes = 0; + Assert.False(options.TryFlushIfAutoSaveDue(() => flushes++)); + Assert.Equal(0, flushes); + Assert.True(options.IsDirty); + } + + [Fact] + public void ReconnectSequence_ResetSessionClearsSeed_NewReplaceUnblocksFlushAgain() + { + var options = new RuntimeCharacterOptionsState(); + options.Replace(options.Options1, options.Options2); // first session's seed + options.TrySetOption( + (uint)CharacterOptionId.AutoTarget, false, (_, _) => { }); + + int flushes = 0; + Assert.True(options.TryFlush(() => flushes++)); + Assert.Equal(1, flushes); + + // Simulated reconnect: the generation-reset transaction clears the + // seed along with everything else. + options.ResetSession(); + Assert.False(options.HasServerSeed); + + // Anything that dirties the module BEFORE the new session's + // PlayerDescription arrives must not be flushable yet. + options.TrySetOption( + (uint)CharacterOptionId.AutoTarget, false, (_, _) => { }); + Assert.True(options.IsDirty); + Assert.False(options.TryFlush(() => flushes++)); + Assert.Equal(1, flushes); + Assert.True(options.IsDirty); + + // The new session's PlayerDescription lands — S5 supersedes the + // stale pending change; a FRESH change after the reseed flushes. + options.Replace(options.Options1, options.Options2); + Assert.True(options.HasServerSeed); + Assert.False(options.IsDirty); + options.TrySetOption( + (uint)CharacterOptionId.ShowTooltips, false, (_, _) => { }); + Assert.True(options.TryFlush(() => flushes++)); + Assert.Equal(2, flushes); + } + + // ── S5 (Campaign OP OP1 review fix, blast lens, 2026-08-11) ──────────── + + [Fact] + public void Replace_ClearsDirtyState_ServerTruthSupersedesPendingLocalIntent() + { + var options = new RuntimeCharacterOptionsState(); + options.TrySetOption( + (uint)CharacterOptionId.AutoTarget, false, (_, _) => { }); + Assert.True(options.IsDirty); + Assert.NotNull(options.FirstDirtiedAt); + + options.Replace(0x11111111u, 0x22222222u); + + Assert.False(options.IsDirty); + Assert.Null(options.FirstDirtiedAt); + Assert.Equal(0x11111111u, options.Options1); + Assert.Equal(0x22222222u, options.Options2); + } + + // ── S2 (Campaign OP OP1 review fix, blast lens, 2026-08-11): the + // decide-and-clear/callback-outside-the-lock split ───────────────────── + + [Fact] + public async Task TryFlush_ReleasesTheDirtyGate_DuringTheCallback_SoAConcurrentMarkDirtyDoesNotBlock() + { + var options = new RuntimeCharacterOptionsState(); + options.Replace(options.Options1, options.Options2); + options.TrySetOption( + (uint)CharacterOptionId.AutoTarget, false, (_, _) => { }); + + using var callbackEntered = new ManualResetEventSlim(false); + using var releaseCallback = new ManualResetEventSlim(false); + + Task flushTask = Task.Run(() => + options.TryFlush(() => + { + callbackEntered.Set(); + releaseCallback.Wait(TimeSpan.FromSeconds(10)); + })); + + Assert.True(callbackEntered.Wait(TimeSpan.FromSeconds(5))); + + // While the callback above is still blocked and holds NO lock (per + // the fix), a concurrent MarkDirty from another thread must + // complete promptly. Under the pre-fix shape (callback invoked + // INSIDE _dirtyGate) this would block until releaseCallback fires. + Task probe = Task.Run(options.MarkDirty); + Task probeCompletion = await Task.WhenAny(probe, Task.Delay(TimeSpan.FromSeconds(2))); + bool probeCompletedPromptly = ReferenceEquals(probeCompletion, probe); + + releaseCallback.Set(); + bool flushed = await flushTask.WaitAsync(TimeSpan.FromSeconds(5)); + Assert.True(flushed); + Assert.True(probeCompletedPromptly); + } + + [Fact] + public void TryFlush_PreservesDirtyState_WhenTheCallbackThrows() + { + var options = new RuntimeCharacterOptionsState(); + options.Replace(options.Options1, options.Options2); + options.TrySetOption( + (uint)CharacterOptionId.AutoTarget, false, (_, _) => { }); + Assert.True(options.IsDirty); + + Assert.Throws(() => + options.TryFlush(() => throw new InvalidOperationException("network down"))); + + Assert.True(options.IsDirty); + } + [Fact] public void TryFlush_NoOpWhenClean_FlushesAndClearsWhenDirty() { @@ -389,8 +650,9 @@ public sealed class RuntimeCharacterStateTests Assert.False(options.TryFlush(() => cleanFlushes++)); Assert.Equal(0, cleanFlushes); + options.Replace(options.Options1, options.Options2); // seed (M1) options.TrySetOption( - (uint)CharacterOptionId.AutoTarget, false, () => { }); + (uint)CharacterOptionId.AutoTarget, false, (_, _) => { }); Assert.True(options.IsDirty); int dirtyFlushes = 0; @@ -410,8 +672,9 @@ public sealed class RuntimeCharacterStateTests { var clock = new ManualTimeProvider(); var options = new RuntimeCharacterOptionsState(clock); + options.Replace(options.Options1, options.Options2); // seed (M1) options.TrySetOption( - (uint)CharacterOptionId.AutoTarget, false, () => { }); + (uint)CharacterOptionId.AutoTarget, false, (_, _) => { }); int flushes = 0; clock.Advance(RuntimeCharacterOptionsState.AutoSaveDelay - TimeSpan.FromSeconds(1)); @@ -429,7 +692,7 @@ public sealed class RuntimeCharacterStateTests { var options = new RuntimeCharacterOptionsState(); options.TrySetOption( - (uint)CharacterOptionId.AutoTarget, false, () => { }); + (uint)CharacterOptionId.AutoTarget, false, (_, _) => { }); Assert.True(options.IsDirty); options.ResetSession(); @@ -438,6 +701,63 @@ public sealed class RuntimeCharacterStateTests Assert.Null(options.FirstDirtiedAt); } + // ── S3 (Campaign OP OP1 review fix, blast lens, 2026-08-11): the + // combined ownership ledger observes IsDirty ──────────────────────────── + + [Fact] + public void CaptureOwnership_OptionsAreClean_ReflectsOptionsIsDirty_EvenWhenBitsReturnToDefault() + { + using var state = new RuntimeCharacterState(); + Assert.True(state.CaptureOwnership().OptionsAreClean); + + // AutoTarget (0x0D) defaults ON. Flip off then back on: the WORDS + // return to their default value, but m_bDirty was set on the first + // (real) transition and never cleared by a flush/reset — exactly + // the gap S3 flags: the pre-existing OptionsAreDefaults check alone + // cannot see this (it would read true here despite a real pending + // save being owed). + state.Options.TrySetOption( + (uint)CharacterOptionId.AutoTarget, false, (_, _) => { }); + state.Options.TrySetOption( + (uint)CharacterOptionId.AutoTarget, true, (_, _) => { }); + + Assert.Equal(RuntimeCharacterOptionsState.DefaultOptions1, state.Options.Options1); + Assert.True(state.Options.IsDirty); + Assert.False(state.CaptureOwnership().OptionsAreClean); + + state.ResetSession(); + + Assert.True(state.CaptureOwnership().OptionsAreClean); + } + + [Fact] + public void RuntimeCharacterOwnershipSnapshot_IsConverged_RequiresOptionsAreClean() + { + // Direct record-level pin: IsConverged must fail on OptionsAreClean + // alone, exactly like every other convergence field, even when + // every other field is in its converged shape. + var converged = new RuntimeCharacterOwnershipSnapshot( + IsDisposed: true, + InternalSubscriptionsAttached: false, + LearnedSpellCount: 0, + ActiveEnchantmentCount: 0, + DesiredComponentCount: 0, + FavoriteSpellCount: 0, + VitalCount: 0, + AttributeCount: 0, + SkillCount: 0, + PositionCount: 0, + PropertyCount: 0, + OptionsAreDefaults: true, + MovementSkillsAreReset: true, + AutonomyIsDefault: true, + OptionsAreClean: true); + Assert.True(converged.IsConverged); + + RuntimeCharacterOwnershipSnapshot dirty = converged with { OptionsAreClean = false }; + Assert.False(dirty.IsConverged); + } + private sealed class ManualTimeProvider : TimeProvider { private DateTimeOffset _now = new(2026, 8, 10, 0, 0, 0, TimeSpan.Zero); diff --git a/tests/AcDream.Runtime.Tests/Session/DirectGameRuntimeCommandAdapterTests.cs b/tests/AcDream.Runtime.Tests/Session/DirectGameRuntimeCommandAdapterTests.cs index 256f1efd..77249f14 100644 --- a/tests/AcDream.Runtime.Tests/Session/DirectGameRuntimeCommandAdapterTests.cs +++ b/tests/AcDream.Runtime.Tests/Session/DirectGameRuntimeCommandAdapterTests.cs @@ -350,6 +350,11 @@ public sealed class DirectGameRuntimeCommandAdapterTests CreateStartedHarness(); var gameActions = new List(); operations.Sessions[^1].GameActionCapture = body => gameActions.Add(body); + // MUST-FIX M1 (OP1 review fix, 2026-08-11): seed the server truth + // (as a real session's PlayerDescription would) before the flush — + // otherwise TryFlush now refuses outright (see + // SaveOptions_RefusesBeforeServerSeed_EvenWhenDirty below). + SeedServerOptions(runtime); adapter.Character.SetSingleOption( runtime.Generation, (uint)CharacterOptionId.AutoTarget, false); @@ -365,17 +370,142 @@ public sealed class DirectGameRuntimeCommandAdapterTests SocialActions.SetCharacterOptionsOpcode, System.Buffers.Binary.BinaryPrimitives.ReadUInt32LittleEndian( blob.AsSpan(8))); + // S4 (OP1 review fix, blast lens): PrimaryObjectId no longer encodes + // whether the flush actually fired — both hosts report the SAME + // shape (Accepted, objectId 0). + Assert.Equal(0u, saved.ResultObjectId); // A clean module's second SaveOptions sends nothing more. RuntimeCommandResult savedAgain = adapter.Character.SaveOptions(runtime.Generation); Assert.True(savedAgain.Accepted); + Assert.Equal(0u, savedAgain.ResultObjectId); Assert.Single(gameActions); runtime.Dispose(); } + [Fact] + public void SaveOptions_RefusesBeforeServerSeed_EvenWhenDirty_ThenSucceedsAfterSeed() + { + (GameRuntime runtime, DirectGameRuntimeCommandAdapter adapter, FixtureSessionOperations operations) = + CreateStartedHarness(); + var gameActions = new List(); + operations.Sessions[^1].GameActionCapture = body => gameActions.Add(body); + + // MUST-FIX M1 (OP1 review fix, 2026-08-11): CreateStartedHarness's + // RuntimeCharacterOptionsState starts at CLIENT constructor defaults + // — no PlayerDescription has landed yet (HasServerSeed is false). A + // blob flush here would ship acdream's defaults over the + // character's real server-side options — the wipe class M1 closes. + adapter.Character.SetSingleOption( + runtime.Generation, (uint)CharacterOptionId.AutoTarget, false); + Assert.True(runtime.CharacterOwner.Options.IsDirty); + + RuntimeCommandResult saved = adapter.Character.SaveOptions(runtime.Generation); + + Assert.True(saved.Accepted); + Assert.Empty(gameActions); + // Nothing lost, nothing sent — the pending change is still pending. + Assert.True(runtime.CharacterOwner.Options.IsDirty); + + // The server's real PlayerDescription lands. S5: the seed + // supersedes the (unsent) pending change, so dirty a FRESH change + // after the seed to prove the GATE — not the module — was what + // refused above. + SeedServerOptions(runtime); + Assert.False(runtime.CharacterOwner.Options.IsDirty); + adapter.Character.SetSingleOption( + runtime.Generation, (uint)CharacterOptionId.ShowTooltips, false); + + RuntimeCommandResult savedAfterSeed = + adapter.Character.SaveOptions(runtime.Generation); + + Assert.True(savedAfterSeed.Accepted); + Assert.Single(gameActions); + Assert.False(runtime.CharacterOwner.Options.IsDirty); + runtime.Dispose(); + } + + // ── MUST-FIX 1 (Campaign OP OP1 review fix, mechanism lens, 2026-08-11): + // end-to-end proof that GameRuntime's real wiring — not just + // LiveSessionController's hook mechanics in isolation + // (LiveSessionControllerTests.cs) — actually auto-flushes the batched + // blob from an ordinary Session.Tick() once the 480 s timer is due. + + [Fact] + public void Session_Tick_AutoFlushesTheDirtyBlob_OnceThe480sTimerIsDue() + { + var clock = new ManualTimeProvider(); + (GameRuntime runtime, DirectGameRuntimeCommandAdapter adapter, FixtureSessionOperations operations) = + CreateStartedHarness(clock); + var gameActions = new List(); + operations.Sessions[^1].GameActionCapture = body => gameActions.Add(body); + SeedServerOptions(runtime); + + adapter.Character.SetSingleOption( + runtime.Generation, (uint)CharacterOptionId.AutoTarget, false); + Assert.True(runtime.CharacterOwner.Options.IsDirty); + + // Before the timer: an ordinary tick must not flush. + runtime.Session.Tick(); + Assert.Empty(gameActions); + Assert.True(runtime.CharacterOwner.Options.IsDirty); + + clock.Advance(RuntimeCharacterOptionsState.AutoSaveDelay + TimeSpan.FromSeconds(1)); + + runtime.Session.Tick(); + + byte[] blob = Assert.Single(gameActions); + Assert.Equal( + SocialActions.SetCharacterOptionsOpcode, + System.Buffers.Binary.BinaryPrimitives.ReadUInt32LittleEndian( + blob.AsSpan(8))); + Assert.False(runtime.CharacterOwner.Options.IsDirty); + runtime.Dispose(); + } + + [Fact] + public void Stop_AutoFlushesTheDirtyBlob_BeforeTheCharacterLogoffRequest() + { + (GameRuntime runtime, DirectGameRuntimeCommandAdapter adapter, FixtureSessionOperations operations) = + CreateStartedHarness(); + var order = new List(); + operations.Sessions[^1].GameActionCapture = body => + { + uint opcode = System.Buffers.Binary.BinaryPrimitives.ReadUInt32LittleEndian( + body.AsSpan(8)); + if (opcode == SocialActions.SetCharacterOptionsOpcode) + order.Add("options-blob"); + }; + SeedServerOptions(runtime); + adapter.Character.SetSingleOption( + runtime.Generation, (uint)CharacterOptionId.AutoTarget, false); + Assert.True(runtime.CharacterOwner.Options.IsDirty); + + RuntimeTeardownAcknowledgement stopped = + adapter.Session.Stop(runtime.Generation); + + Assert.True(stopped.IsComplete); + Assert.Equal(["options-blob"], order); + runtime.Dispose(); + } + + private sealed class ManualTimeProvider : TimeProvider + { + private DateTimeOffset _now = new(2026, 8, 11, 0, 0, 0, TimeSpan.Zero); + + public override DateTimeOffset GetUtcNow() => _now; + + public void Advance(TimeSpan elapsed) => _now += elapsed; + } + + private static void SeedServerOptions(GameRuntime runtime) => + runtime.CharacterOwner.Options.Replace( + runtime.CharacterOwner.Options.Options1, + runtime.CharacterOwner.Options.Options2); + private static (GameRuntime Runtime, DirectGameRuntimeCommandAdapter Adapter, FixtureSessionOperations Operations) - CreateStartedHarness() + CreateStartedHarness(TimeProvider? timeProvider = null) { var operations = new FixtureSessionOperations(); var gameplay = new FixtureGameplayOperations(); @@ -384,6 +514,7 @@ public sealed class DirectGameRuntimeCommandAdapterTests gameplay, gameplay, gameplay, + TimeProvider: timeProvider, SessionOperations: operations)); gameplay.Bind(runtime); var resetHost = new FixtureResetHost(); diff --git a/tests/AcDream.Runtime.Tests/Session/LiveSessionControllerTests.cs b/tests/AcDream.Runtime.Tests/Session/LiveSessionControllerTests.cs index eaa6703d..adeaa109 100644 --- a/tests/AcDream.Runtime.Tests/Session/LiveSessionControllerTests.cs +++ b/tests/AcDream.Runtime.Tests/Session/LiveSessionControllerTests.cs @@ -897,6 +897,133 @@ public sealed class LiveSessionControllerTests StringComparison.Ordinal); } + // ── MUST-FIX 1 (Campaign OP OP1 review fix, mechanism lens, + // 2026-08-11): the TS-71 auto-save-timer and pre-logoff-flush hooks ──── + + [Fact] + public void Tick_InvokesConfiguredAutoSaveHook_WithTheCurrentSessionWhileInWorld() + { + var calls = new List(); + var operations = new TestOperations(calls); + var host = new TestHost(calls); + var controller = new LiveSessionController(operations); + controller.Start(LiveOptions(), host); + WorldSession session = operations.Sessions[0]; + WorldSession? seen = null; + controller.ConfigureAutoSaveTick(s => seen = s); + + controller.Tick(); + + Assert.Same(session, seen); + } + + [Fact] + public void Tick_DoesNotInvokeAutoSaveHook_WhenNotInWorld() + { + var calls = new List(); + var operations = new TestOperations(calls); + var controller = new LiveSessionController(operations); + bool invoked = false; + controller.ConfigureAutoSaveTick(_ => invoked = true); + + // Never started — Tick() early-returns before any hook can fire. + controller.Tick(); + + Assert.False(invoked); + Assert.Equal(0, operations.TickCount); + } + + [Fact] + public void Tick_AutoSaveHookThrowing_DoesNotFailTheTickOrTearDownTheSession() + { + var calls = new List(); + var operations = new TestOperations(calls); + var host = new TestHost(calls); + var controller = new LiveSessionController(operations); + controller.Start(LiveOptions(), host); + WorldSession session = operations.Sessions[0]; + controller.ConfigureAutoSaveTick( + _ => throw new InvalidOperationException("send failed")); + + // Must not throw — a transient send error on a background auto-save + // must not tear down the whole live session. + controller.Tick(); + + Assert.True(controller.IsInWorld); + Assert.Same(session, controller.CurrentSession); + Assert.False(operations.DisposeCounts.ContainsKey(session)); + } + + [Fact] + public void Stop_InvokesConfiguredPreLogoffFlushHook_BeforeSessionDisposed() + { + var calls = new List(); + var operations = new TestOperations(calls); + var host = new TestHost(calls); + var controller = new LiveSessionController(operations); + controller.Start(LiveOptions(), host); + WorldSession session = operations.Sessions[0]; + WorldSession? seen = null; + controller.ConfigurePreLogoffFlush(s => + { + seen = s; + calls.Add("pre-logoff-flush"); + }); + + controller.Stop(); + + Assert.Same(session, seen); + // Retail's CPlayerSystem::LogOffCharacter calls SaveToServer BEFORE + // the character-logoff wire request — the flush must precede + // WorldSession disposal. + int flushIndex = calls.IndexOf("pre-logoff-flush"); + int disposeIndex = calls.IndexOf("dispose-session"); + Assert.True(flushIndex >= 0); + Assert.True(disposeIndex >= 0); + Assert.True(flushIndex < disposeIndex); + } + + [Fact] + public void Stop_DoesNotInvokePreLogoffFlushHook_WhenNeverEnteredWorld() + { + var calls = new List(); + var operations = new TestOperations(calls) { ThrowOnEnterWorld = true }; + var host = new TestHost(calls); + var controller = new LiveSessionController(operations); + bool invoked = false; + controller.ConfigurePreLogoffFlush(_ => invoked = true); + + // Fails before _inWorld ever becomes true — StartCore's own + // StopAfterFailure -> StopCore() runs, but the hook must not fire; + // matches retail's own call site, which only exists on an actual + // in-world character. + controller.Start(LiveOptions(), host); + + Assert.False(invoked); + Assert.False(controller.IsInWorld); + } + + [Fact] + public void Stop_PreLogoffFlushHookThrowing_DoesNotBlockGracefulTeardown() + { + var calls = new List(); + var operations = new TestOperations(calls); + var host = new TestHost(calls); + var controller = new LiveSessionController(operations); + controller.Start(LiveOptions(), host); + WorldSession session = operations.Sessions[0]; + controller.ConfigurePreLogoffFlush( + _ => throw new InvalidOperationException("flush failed")); + + // Must not throw — a failed flush must not block the graceful- + // shutdown sequence. + controller.Stop(); + + Assert.False(controller.IsInWorld); + Assert.Null(controller.CurrentSession); + Assert.Equal(1, operations.DisposeCounts[session]); + } + [Fact] public void DisposeIsIdempotentAndMakesOldCommandsInert() {