fix(runtime,net): OP1 review fixes — server-seed gate, tick-wired auto-save/logout flush, fellowship mutual exclusion
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<uint,bool> 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 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
26b119354d
commit
09029f9f4b
12 changed files with 1043 additions and 51 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
|||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The one flush body shared by every trigger that can send the batched
|
||||
/// <c>SetCharacterOptions (0x01A1)</c> blob: the explicit
|
||||
/// <c>SaveOptions</c> command (both <c>IRuntimeCharacterCommands</c>
|
||||
/// adapters), the 480 s auto-save timer, and the pre-logoff flush (the
|
||||
/// latter two wired via <see cref="LiveSessionController.
|
||||
/// ConfigureAutoSaveTick"/>/<see cref="LiveSessionController.
|
||||
/// ConfigurePreLogoffFlush"/> in the constructor above). Reads
|
||||
/// <see cref="CharacterOwner"/>/<see cref="InventoryOwner"/> 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.
|
||||
/// <para>
|
||||
/// The <see cref="RuntimeCharacterOptionsState.IsDirty"/> pre-check
|
||||
/// below is retail-faithful (<c>CPlayerModule::UseTime @0x0059A710</c>
|
||||
/// opens with the identical <c>m_bDirty</c> 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 <see cref="LiveSessionController.Tick"/> 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.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
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; }
|
||||
|
|
|
|||
|
|
@ -21,7 +21,16 @@ public readonly record struct RuntimeCharacterOwnershipSnapshot(
|
|||
bool OptionsAreDefaults,
|
||||
bool MovementSkillsAreReset,
|
||||
/// <summary>C0-2: <see cref="RuntimeCharacterState.AutonomyLevel"/> is back at retail's default (<see cref="RuntimeCharacterState.FullAutonomyLevel"/>).</summary>
|
||||
bool AutonomyIsDefault = true)
|
||||
bool AutonomyIsDefault = true,
|
||||
/// <summary>
|
||||
/// S3 (Campaign OP OP1 review fix, 2026-08-11): <c>!Options.IsDirty</c>.
|
||||
/// <see cref="OptionsAreDefaults"/> 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 <c>m_bDirty</c> is
|
||||
/// still set — the ledger exists precisely to catch state a reset must
|
||||
/// clear but a value-only comparison would miss.
|
||||
/// </summary>
|
||||
bool OptionsAreClean = true)
|
||||
{
|
||||
public bool IsConverged =>
|
||||
IsDisposed
|
||||
|
|
@ -37,7 +46,8 @@ public readonly record struct RuntimeCharacterOwnershipSnapshot(
|
|||
&& PropertyCount == 0
|
||||
&& OptionsAreDefaults
|
||||
&& MovementSkillsAreReset
|
||||
&& AutonomyIsDefault;
|
||||
&& AutonomyIsDefault
|
||||
&& OptionsAreClean;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
|
@ -221,7 +231,8 @@ public sealed class RuntimeCharacterState : IDisposable
|
|||
&& _runSkillBase == -1
|
||||
&& _jumpSkillBase == -1
|
||||
&& _movementSkillAugmentations == default,
|
||||
AutonomyLevel == FullAutonomyLevel);
|
||||
AutonomyLevel == FullAutonomyLevel,
|
||||
OptionsAreClean: !Options.IsDirty);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
|
@ -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; }
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// MUST-FIX M1 (Campaign OP OP1 review fix, 2026-08-11): has a real
|
||||
/// <c>PlayerDescription</c> ever landed via <see cref="Replace"/> since
|
||||
/// construction / the last <see cref="ResetSession"/>? Starts
|
||||
/// <c>false</c> — this module's two words start at the CLIENT
|
||||
/// constructor defaults (<see cref="DefaultOptions1"/>/
|
||||
/// <see cref="DefaultOptions2"/>), not the character's server-side
|
||||
/// options, so flushing the batched <c>0x01A1</c> blob before the seed
|
||||
/// arrives would ship client defaults over whatever ACE actually has
|
||||
/// stored — the wipe class this gate exists to close. See
|
||||
/// <see cref="TryFlush"/>/<see cref="TryFlushIfAutoSaveDue"/>.
|
||||
/// </summary>
|
||||
public bool HasServerSeed
|
||||
{
|
||||
get { lock (_dirtyGate) return _hasServerSeed; }
|
||||
}
|
||||
|
||||
public bool DragItemOnPlayerOpensSecureTrade =>
|
||||
Snapshot.DragItemOnPlayerOpensSecureTrade;
|
||||
|
||||
/// <summary>
|
||||
/// Installs a fresh <c>PlayerDescription</c>'s two option words as
|
||||
/// server truth and arms <see cref="HasServerSeed"/> (MUST-FIX M1).
|
||||
/// S5 (OP1 review fix, blast lens): also clears <see cref="IsDirty"/>/
|
||||
/// <see cref="FirstDirtiedAt"/> — a re-seed WHOLESALE overwrites both
|
||||
/// words with no partial-merge path (retail's own
|
||||
/// <c>PlayerModule::UnPack</c> 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.
|
||||
/// </summary>
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
|
@ -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
|
||||
/// <c>IRuntimeCharacterCommands.SetSingleOption</c> host adapters.
|
||||
/// Mirrors <c>CPlayerModule::OnChanged(PlayerOption) @0x0059A8E0</c>
|
||||
/// exactly: write the bit into this LOCAL copy FIRST (so a same-session
|
||||
/// consumer like <see cref="TurbineChatMembershipGate"/> is correct
|
||||
/// before any round trip), THEN either invoke
|
||||
/// <paramref name="sendAutoSave"/> immediately (retail's
|
||||
/// Mirrors <c>CPlayerModule::OnChanged(PlayerOption) @0x0059A8E0</c>'s
|
||||
/// four-step body: write the bit into this LOCAL copy FIRST (so a
|
||||
/// same-session consumer like <see cref="TurbineChatMembershipGate"/> 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
|
||||
/// <paramref name="sendAutoSave"/> immediately (step 3, retail's
|
||||
/// <c>IsAutoSaveOption</c> branch — <c>Event_PlayerOptionChangedEvent</c>,
|
||||
/// the <c>0x0005</c> send) or <see cref="MarkDirty"/> for the batched
|
||||
/// <c>0x01A1</c> flush (the else branch). Matches retail's own
|
||||
/// <c>0x01A1</c> 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 <paramref name="value"/> already holds. Returns
|
||||
|
|
@ -708,8 +758,31 @@ public sealed class RuntimeCharacterOptionsState
|
|||
/// (retail's own <c>IsAutoSaveOption</c>/id-cast bounds check would
|
||||
/// reject it too) — callers turn that into a
|
||||
/// <see cref="RuntimeCommandStatus.Rejected"/>, never a silent send.
|
||||
/// <paramref name="sendAutoSave"/> 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
|
||||
/// <c>WorldSession.SendSetSingleCharacterOption</c> directly as the
|
||||
/// method group.
|
||||
/// </summary>
|
||||
public bool TrySetOption(uint characterOptionId, bool value, Action sendAutoSave)
|
||||
/// <remarks>
|
||||
/// 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 <c>PlayerModule</c> state itself —
|
||||
/// <c>case 2 IgnoreFellowshipRequests</c> and
|
||||
/// <c>case 0x12 FellowshipAutoAcceptRequests</c> 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 <c>0x0005</c> 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
|
||||
/// <see cref="MarkDirty"/> and always terminates after one level (the
|
||||
/// clear passes <c>value: false</c>, which never re-triggers either
|
||||
/// case's own "if now true" guard).
|
||||
/// </remarks>
|
||||
public bool TrySetOption(uint characterOptionId, bool value, Action<uint, bool> 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 <c>CPlayerModule::SaveToServer(force: 0) @0x0059A660</c> —
|
||||
/// both production call sites (Apply, logout) pass <c>force = 0</c>, so
|
||||
/// a clean module sends nothing. The explicit <c>SaveOptions</c>
|
||||
/// Runtime command flushes through here.
|
||||
/// Runtime command flushes through here. MUST-FIX M1 (Campaign OP OP1
|
||||
/// review fix, 2026-08-11): also refuses before <see cref="HasServerSeed"/>
|
||||
/// — 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 <see cref="Replace"/>
|
||||
/// arrives. S2 (blast lens): the decision (dirty AND seeded) is made and
|
||||
/// cleared under <c>_dirtyGate</c>, but <paramref name="flush"/> 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
|
||||
/// <c>LiveSessionCommandRouter</c> (which holds ITS OWN <c>_gate</c>
|
||||
/// before reaching <see cref="MarkDirty"/>'s <c>_dirtyGate</c>); holding
|
||||
/// <c>_dirtyGate</c> across a caller-supplied callback that could
|
||||
/// transitively want <c>_gate</c> is the lock-inversion shape that
|
||||
/// finding named. A throw from <paramref name="flush"/> propagates with
|
||||
/// the module left dirty (nothing cleared) — the correct direction, since
|
||||
/// nothing actually reached the wire.
|
||||
/// </summary>
|
||||
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;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Retail's <c>CPlayerModule::UseTime @0x0059A710</c>: flush iff dirty
|
||||
/// AND at least <see cref="AutoSaveDelay"/> (480 s, BYTE-VERIFIED) has
|
||||
/// elapsed since <see cref="FirstDirtiedAt"/>. 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 <see cref="TryFlush"/>) AND at least
|
||||
/// <see cref="AutoSaveDelay"/> (480 s, BYTE-VERIFIED) has elapsed since
|
||||
/// <see cref="FirstDirtiedAt"/>. 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
|
||||
/// <see cref="TryFlush"/> exactly, for the same lock-order reason.
|
||||
/// </summary>
|
||||
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;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Restores the client-constructor defaults AND clears
|
||||
/// <see cref="HasServerSeed"/> (MUST-FIX M1) — a reconnect's fresh
|
||||
/// <c>PlayerDescription</c> must re-arm the seed via a NEW
|
||||
/// <see cref="Replace"/> 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.
|
||||
/// </summary>
|
||||
public void ResetSession()
|
||||
{
|
||||
Volatile.Write(ref _options1, DefaultOptions1);
|
||||
Volatile.Write(ref _options2, DefaultOptions2);
|
||||
Interlocked.Increment(ref _revision);
|
||||
lock (_dirtyGate)
|
||||
{
|
||||
_isDirty = false;
|
||||
_hasServerSeed = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -266,6 +266,8 @@ public sealed class LiveSessionController
|
|||
private ulong _generation;
|
||||
private RuntimeTeardownStage _lastTeardownStages;
|
||||
private LiveSessionCharacterSelection? _activeSelection;
|
||||
private Action<WorldSession>? _autoSaveTickHook;
|
||||
private Action<WorldSession>? _preLogoffFlushHook;
|
||||
|
||||
public LiveSessionController()
|
||||
: this(ProductionLiveSessionOperations.Instance)
|
||||
|
|
@ -297,6 +299,38 @@ public sealed class LiveSessionController
|
|||
get { lock (_gate) return new RuntimeGenerationToken(_generation); }
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// MUST-FIX 1 (Campaign OP OP1 review fix, 2026-08-11 — mechanism lens
|
||||
/// finding): reaches retail's <c>CPlayerModule::UseTime</c> (the 480 s
|
||||
/// batched character-option auto-save) from the SAME per-session tick
|
||||
/// both the graphical host (<c>RetailLiveFrameCoordinator</c>) and the
|
||||
/// no-window host (<c>HeadlessSessionHost.Tick</c>) already call —
|
||||
/// <see cref="Tick()"/> — with ZERO host edits. <c>GameRuntime</c> wires
|
||||
/// this once, after constructing <c>CharacterOwner</c>/
|
||||
/// <c>InventoryOwner</c>, with a flush body that talks to the
|
||||
/// <see cref="WorldSession"/> directly rather than through App's
|
||||
/// <c>LiveSessionCommandRouter</c>/<c>LiveCommandBus</c> — 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 (<see cref="Tick()"/>), never treated
|
||||
/// as a tick failure — a transient send error on a background auto-save
|
||||
/// must not tear down the whole live session.
|
||||
/// </summary>
|
||||
internal void ConfigureAutoSaveTick(Action<WorldSession> hook) =>
|
||||
_autoSaveTickHook = hook ?? throw new ArgumentNullException(nameof(hook));
|
||||
|
||||
/// <summary>
|
||||
/// MUST-FIX 1: reaches retail's <c>CPlayerSystem::LogOffCharacter</c> →
|
||||
/// <c>SaveToServer</c> ordering — flush the batched option module BEFORE
|
||||
/// the character-logoff wire request goes out — from Runtime's own
|
||||
/// Stop/teardown transaction (<see cref="StopCore"/>), 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.
|
||||
/// </summary>
|
||||
internal void ConfigurePreLogoffFlush(Action<WorldSession> 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)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue