From 09029f9f4b0dfca5a3cb380acd667dfc23af3b57 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 11 Aug 2026 00:39:51 +0200 Subject: [PATCH] =?UTF-8?q?fix(runtime,net):=20OP1=20review=20fixes=20?= =?UTF-8?q?=E2=80=94=20server-seed=20gate,=20tick-wired=20auto-save/logout?= =?UTF-8?q?=20flush,=20fellowship=20mutual=20exclusion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the two mechanism-lens and blast-lens dual reviews of Campaign OP slice OP1 (86c0a7e0): docs/research/2026-08-10-op1-review-mechanism.md and docs/research/2026-08-10-op1-review-blast.md. MUST-FIX M1 (blast): RuntimeCharacterOptionsState gains a HasServerSeed latch, set by Replace (the PlayerDescription seed) and cleared by ResetSession. TryFlush/TryFlushIfAutoSaveDue now refuse before the seed arrives — closing the window where a bot (or, after this commit, the timer/logout triggers) could flush client-default option words over a character's real server-side options before any PlayerDescription ever landed. MUST-FIX 1 (mechanism): the 480 s auto-save timer and the pre-logoff flush are now wired into production, closing TS-71 (retired). Both ride LiveSessionController's own tick/stop transaction via two new hooks (ConfigureAutoSaveTick/ConfigurePreLogoffFlush), wired once by GameRuntime's constructor — a Runtime-internal change requiring zero host edits, exactly as the review identified. The flush body talks to WorldSession directly rather than through App's LiveSessionCommandRouter, which is what keeps this off the S2 lock-order hazard (below). Filed TS-73 for the two OnChanged side-effect cases (weather/day/combat- target/fog) TrySetOption still doesn't model — pre-anchored to OP4's Group B consumer binds. SHOULD-FIX S2 (blast, prerequisite for MUST-FIX 1): TryFlush/ TryFlushIfAutoSaveDue no longer invoke the flush callback while holding _dirtyGate — the decision is made and cleared under the lock, but the callback itself runs outside it, closing the lock-inversion hazard the natural timer wiring would have hit (Runtime tick's _dirtyGate-then- _gate vs the router's _gate-then-_dirtyGate). SHOULD-FIX MF-2 (mechanism): TrySetOption now ports the two PlayerModule-state-mutating cases of CPlayerModule::OnChanged's local side-effect switch — turning ON IgnoreFellowshipRequests or FellowshipAutoAcceptRequests clears the other through a real recursive TrySetOption call, reproducing retail's second 0x0005 (the clear's send reaches the wire before the primary option's own send, matching the nested-call order in the decomp). The signature widened from Action sendAutoSave to Action so the recursion can send a different (id, value) than the caller's own; every production call site now passes WorldSession.SendSetSingleCharacterOption directly. SHOULD-FIX MF-3 (mechanism): a hand-transcribed 53-row (id, isOptions1, mask) theory in CharacterOptionTableTests, independently re-derived from acclient.h's PlayerOption/CharacterOption/CharacterOptions2 enums rather than copied from CharacterOptionTable.cs — closes the one column with no id-by-id pin. Also added the pairwise-distinctness check blast NOTE N7 named. SHOULD-FIX S1 (blast): LiveSessionCommandRouterTests' CH3/CH4 regression test now drives the REAL TrySetOption binding instead of a hand-rolled SetOptionBit substitute that had silently drifted from production after OP1. SHOULD-FIX S3 (blast): RuntimeCharacterOwnershipSnapshot gains OptionsAreClean (!Options.IsDirty), included in IsConverged — a module whose two words happen to cycle back to their default bit pattern while still dirty is now caught by the combined ownership ledger, not just by OptionsAreDefaults. SHOULD-FIX S4 (blast): SaveOptions no longer encodes "did it actually flush" as PrimaryObjectId 1u/0u (which read as object guid 0x00000001 in the K2 event stream). Both host adapters now report the identical shape (Accepted, objectId 0) — the graphical host never could report this anyway (LiveCommandBus.Publish has no return channel). SHOULD-FIX S5 (blast): Replace (the server-seed arrival) now also clears IsDirty/FirstDirtiedAt — a wholesale re-seed supersedes any pending batched-but-unflushed local intent (retail's own PlayerModule has no partial-merge path either), documented at the member. SHOULD-FIX S6 (blast): a cross-check theory asserting CharacterOptionTable's masks equal PlayerDescriptionParser.CharacterOptions1/2's independently (the write path vs the read path TurbineChatMembershipGate/ RuntimeSettingsController consume) — guards the exact CH3 failure class. Also fixed a real allocation regression found while landing MUST-FIX 1: the naive per-tick flush closure would have allocated on EVERY LiveSessionController.Tick() call regardless of dirty state, which broke the K4 headless 30-session resource-envelope gate. GameRuntime. FlushCharacterOptions now pre-checks Options.IsDirty (itself retail- faithful — CPlayerModule::UseTime opens with the identical m_bDirty byte compare) before allocating the flush closure, so the allocation only happens on the rare tick that might actually flush. Dispositions on findings not changed this round: - Mechanism NOTE 6 / not independently re-flagged: a re-entrant MarkDirty from inside a flush callback can still be erased by the trailing "_isDirty = false" — pre-existing, unchanged by the S2 lock restructure (same outcome whether the callback runs inside or outside the lock), not reachable from any current caller, not a one-liner to close correctly (needs a per-dirty-period generation token). Left as documented in the review; worth closing before the Options panel ever flushes from inside a change handler. - Mechanism NOTE 9, blast N2/N3/N4/N5/N6/N8: informational or require touching files this round doesn't otherwise edit (SocialActions.cs, CharacterOptionsBlobSource.cs, GameRuntimeContractTests.cs) — left per the "one-liner in a file already being edited" instruction. Register: TS-71 retired (both remaining SetCharacterOptions flush triggers now production-wired); TS-73 filed (the two unmodeled OnChanged presentation-binding cases, pre-anchored to OP4). Quality bar: Release build green; full solution suite 12,853 passed / 4 skipped / 0 failed (baseline 12,770/4/0 post-OP2 — 83 new tests added, zero regressions). Co-Authored-By: Claude Fable 5 --- .../retail-divergence-register.md | 4 +- .../Net/LiveSessionRuntimeFactory.cs | 6 +- .../CurrentGameRuntimeCommandAdapter.cs | 5 + src/AcDream.Runtime/GameRuntime.cs | 71 ++++ .../Gameplay/RuntimeCharacterState.cs | 174 +++++++-- .../DirectGameRuntimeCommandAdapter.cs | 24 +- .../Session/LiveSessionController.cs | 80 +++++ .../Net/LiveSessionCommandRouterTests.cs | 20 +- .../Gameplay/CharacterOptionTableTests.cs | 110 ++++++ .../Gameplay/RuntimeCharacterStateTests.cs | 340 +++++++++++++++++- .../DirectGameRuntimeCommandAdapterTests.cs | 133 ++++++- .../Session/LiveSessionControllerTests.cs | 127 +++++++ 12 files changed, 1043 insertions(+), 51 deletions(-) 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() {