From ded23067aa70ca98ae48458fea2e158ec59cf31c Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 12 Aug 2026 02:17:56 +0200 Subject: [PATCH] fix(net,runtime): FA2 fix-round SHOULD-FIX -- fellowship mechanism parity, lookup reuse, router test, checkpoint defaults Remaining SHOULD-FIX findings from the FA2 mechanism/blast reviews: Mechanism SF-3/SF-4 -- RuntimeFellowshipState.ApplyUpdateFellow now ports Fellowship::RecalculateEvenXPSplitting @0x005B92E0 (called from retail's AddFellow/UpdateFellow/RemoveFellow on every upsert/removal, but never from a full update -- that carries the server's own authoritative flag verbatim, lane B 6.2) and Fellowship::AddFellow @0x005B9480's locked/departed admission gate (a brand-new guid is refused while _locked unless it appears in the 0x02BE field-8 _fellows_departed table within 900s, @0x005B94A5). ApplyFullUpdate now stores update.Departed instead of discarding it. A TimeProvider dependency (defaulting to TimeProvider.System, matching the RuntimeCharacterOptionsState precedent) makes the 900s grace window testable. Mechanism SF-5 -- RuntimeAllegianceState's TryGetMember/TryGetPatron/ GetVassals now reuse ClientCommandResponses.AllegianceProfileLookups (promoted private -> internal, AcDream.Runtime added to Core.Net's InternalsVisibleTo) instead of re-implementing the retail walk a second time. Mechanism SF-6 -- RuntimeStateCheckpoint's Fellowship/Allegiance parameters are no longer trailing-optional. `default(RuntimeFellowshipSnapshot)`/ `default(RuntimeAllegianceSnapshot)` zero-init Name/AllegianceName to null, and C# does not allow a non-constant `new(...)` as an optional parameter's default value (CS1736) even when the struct declares an explicit parameterless constructor -- so the only way to guarantee a non-null default was to make the parameters required. Both snapshot types still gained an explicit parameterless constructor for callers that want an empty-but-safe `new()`. Blast SF-4 -- LiveSessionEventRouterTests gains FellowshipQuit_RoutesSelfGuidToClearAndOtherGuidToRemove, wiring real RuntimeFellowshipState/RuntimeAllegianceState owners through the one production registration site and dispatching a real 0x00A3 envelope for both a self-quit and an other-quit -- the one non-trivial lambda in the slice (the self-guid source that decides "remove one member" vs "clear the whole snapshot") was previously untested; every other router test defaults Fellowship/Allegiance to null. Blast SF-5 -- RuntimeFellowshipState.ResetSession dropped its disposed guard to match the precedent its own doc comment names (RuntimeInventoryState.ResetExternalContainer, RuntimeCommunicationState.ResetNegotiatedChannels -- both bare delegations with no disposal guard); the reset transaction is retryable and disposal is terminal, so a throwing guard could never converge on retry. RuntimeAllegianceState.ResetSession (new this fix round) matches the same shape from the start. Blast SF-7 -- IRuntimeAllegianceView.GetVassals' per-call List<> allocation is now documented as an intentional exception to the file's "Snapshot + TryGet*, no allocation" view convention (C# cannot yield-return from inside a lock). Co-Authored-By: Claude Fable 5 --- src/AcDream.Core.Net/AcDream.Core.Net.csproj | 1 + .../Messages/ClientCommandResponses.cs | 12 +- .../GameRuntimeGameplayViews.cs | 21 +- src/AcDream.Runtime/GameRuntimeViews.cs | 19 +- .../Gameplay/RuntimeFellowshipState.cs | 154 ++++++++++++- .../GameRuntimeContractTests.cs | 9 +- .../Gameplay/RuntimeFellowshipStateTests.cs | 207 +++++++++++++++++- .../Session/LiveSessionEventRouterTests.cs | 96 ++++++++ 8 files changed, 499 insertions(+), 20 deletions(-) diff --git a/src/AcDream.Core.Net/AcDream.Core.Net.csproj b/src/AcDream.Core.Net/AcDream.Core.Net.csproj index fc204382..707e2468 100644 --- a/src/AcDream.Core.Net/AcDream.Core.Net.csproj +++ b/src/AcDream.Core.Net/AcDream.Core.Net.csproj @@ -13,6 +13,7 @@ + diff --git a/src/AcDream.Core.Net/Messages/ClientCommandResponses.cs b/src/AcDream.Core.Net/Messages/ClientCommandResponses.cs index 1159f295..7e35855d 100644 --- a/src/AcDream.Core.Net/Messages/ClientCommandResponses.cs +++ b/src/AcDream.Core.Net/Messages/ClientCommandResponses.cs @@ -246,8 +246,18 @@ public static class ClientCommandResponses /// ParentGuid tags IS the tree (lane C §0's DELETE verdict on /// Core/Allegiance/AllegianceTree.cs); these are ports of /// retail's own pointer-walk accessors (lane C §1.5). + /// + /// + /// FA2 fix-round SHOULD-FIX 5 (2026-08-12, + /// docs/research/2026-08-12-fa2-review-mechanism.md): promoted from + /// to (with + /// AcDream.Runtime added to this project's + /// InternalsVisibleTo) so + /// AcDream.Runtime.Gameplay.RuntimeAllegianceState can reuse this + /// walk instead of re-implementing it a second time. + /// /// - private static class AllegianceProfileLookups + internal static class AllegianceProfileLookups { /// Port of AllegianceProfile::GetData. public static AllegianceMemberRecord? FindData( diff --git a/src/AcDream.Runtime/GameRuntimeGameplayViews.cs b/src/AcDream.Runtime/GameRuntimeGameplayViews.cs index 831f5283..1695b7d2 100644 --- a/src/AcDream.Runtime/GameRuntimeGameplayViews.cs +++ b/src/AcDream.Runtime/GameRuntimeGameplayViews.cs @@ -136,7 +136,18 @@ public readonly record struct RuntimeFellowshipSnapshot( bool EvenXpSplit, bool IsOpen, bool Locked, - int MemberCount); + int MemberCount) +{ + // FA2 fix-round SHOULD-FIX 6 (blast review): an explicit parameterless + // constructor gives `new()` (used as RuntimeStateCheckpoint's default, + // GameRuntimeViews.cs) a non-null Name instead of `default`'s bitwise + // zero-init (structs never run field initializers or this constructor + // for `default(T)` — only `new S()` invokes it). + public RuntimeFellowshipSnapshot() + : this(0, false, string.Empty, 0u, false, false, false, false, 0) + { + } +} public interface IRuntimeFellowshipView { @@ -172,6 +183,14 @@ public readonly record struct RuntimeAllegianceSnapshot( int RecordCount) { public bool HasMonarch => MonarchGuid != 0u; + + // FA2 fix-round SHOULD-FIX 6 (blast review): see + // RuntimeFellowshipSnapshot's parameterless constructor — same + // non-null-default reasoning, for AllegianceName. + public RuntimeAllegianceSnapshot() + : this(0, false, false, 0u, 0u, 0u, string.Empty, 0u, 0) + { + } } public interface IRuntimeAllegianceView diff --git a/src/AcDream.Runtime/GameRuntimeViews.cs b/src/AcDream.Runtime/GameRuntimeViews.cs index 3a18ec28..ce17bcc9 100644 --- a/src/AcDream.Runtime/GameRuntimeViews.cs +++ b/src/AcDream.Runtime/GameRuntimeViews.cs @@ -239,10 +239,21 @@ public readonly record struct RuntimeStateCheckpoint( RuntimeWorldEnvironmentOwnershipSnapshot EnvironmentOwnership, RuntimePortalSnapshot Portal, RuntimeWorldTransitOwnershipSnapshot TransitOwnership, - // Campaign FA slice FA2 (2026-08-12): default so every existing - // positional construction site (tests) compiles unchanged. - RuntimeFellowshipSnapshot Fellowship = default, - RuntimeAllegianceSnapshot Allegiance = default); + // Campaign FA slice FA2 (2026-08-12): originally trailing-optional + // (`= default`) so every existing positional construction site (tests) + // compiled unchanged. FA2 fix-round SHOULD-FIX 6 (blast review) made + // both parameters REQUIRED instead: `default(RuntimeFellowshipSnapshot)`/ + // `default(RuntimeAllegianceSnapshot)` zero-init every field — + // including Name/AllegianceName to null, not string.Empty — and C# + // does not allow a non-constant expression (a real `new(...)` call) as + // an optional-parameter default, so there is no way to give a + // TRAILING-OPTIONAL parameter here a non-null default. Both snapshot + // types still declare an explicit parameterless constructor + // (`RuntimeFellowshipSnapshot()`/`RuntimeAllegianceSnapshot()`, + // GameRuntimeGameplayViews.cs) so callers that want an empty-but-safe + // snapshot can pass `new()` explicitly. + RuntimeFellowshipSnapshot Fellowship, + RuntimeAllegianceSnapshot Allegiance); public interface IGameRuntimeView { diff --git a/src/AcDream.Runtime/Gameplay/RuntimeFellowshipState.cs b/src/AcDream.Runtime/Gameplay/RuntimeFellowshipState.cs index c6f907fb..9e800647 100644 --- a/src/AcDream.Runtime/Gameplay/RuntimeFellowshipState.cs +++ b/src/AcDream.Runtime/Gameplay/RuntimeFellowshipState.cs @@ -18,9 +18,14 @@ public readonly record struct RuntimeFellowshipOwnershipSnapshot( /// fellowship roster — Campaign FA slice FA2 (2026-08-12). Session-scoped: /// a disconnect drops you from the fellowship server-side, so this clears /// at generation reset exactly like the external-container precedent -/// (), unlike -/// which survives reconnect -/// (docs/research/2026-08-11-fa-acdream-seams.md §1.3). +/// (). FA2 +/// fix-round correction (2026-08-12, MUST-FIX 1 in +/// docs/research/2026-08-12-fa2-review-mechanism.md): the class doc +/// previously contrasted this with +/// "surviving reconnect" — that citation was inverted (retail clears BOTH +/// at OnEndCharacterSession); +/// is now ALSO a stage with the same +/// clear-at-reset shape. /// /// /// Assembled from the FA1 parsers: a full update (0x02BE) REPLACES @@ -33,11 +38,32 @@ public readonly record struct RuntimeFellowshipOwnershipSnapshot( /// rather than subscribing to a push event (D2 — no /// member). /// +/// +/// +/// FA2 fix-round SHOULD-FIX 3/4 (mechanism review): the incremental upsert +/// path () now also ports +/// Fellowship::RecalculateEvenXPSplitting @0x005B92E0 (called from +/// retail's AddFellow/UpdateFellow/RemoveFellow — NEVER +/// from a full update, which carries the server's own authoritative flag +/// verbatim) and Fellowship::AddFellow @0x005B9480's locked/departed +/// admission gate (a brand-new guid is refused while +/// unless it appears in the 0x02BE field-8 departed-members table +/// within 900 s). +/// /// public sealed class RuntimeFellowshipState : IDisposable { + /// + /// Port of the 900 s (0x384) grace window in + /// Fellowship::AddFellow @0x005B94A5 — a guid readmitted while + /// only if it departed within this many seconds. + /// + private const int DepartedGraceSeconds = 900; + private readonly object _gate = new(); + private readonly TimeProvider _timeProvider; private readonly Dictionary _members = []; + private readonly Dictionary _fellowsDeparted = []; private string _name = string.Empty; private uint _leaderGuid; private bool _shareXp; @@ -48,7 +74,11 @@ public sealed class RuntimeFellowshipState : IDisposable private long _revision; private bool _disposed; - public RuntimeFellowshipState() => View = new FellowshipView(this); + public RuntimeFellowshipState(TimeProvider? timeProvider = null) + { + _timeProvider = timeProvider ?? TimeProvider.System; + View = new FellowshipView(this); + } public IRuntimeFellowshipView View { get; } @@ -66,15 +96,26 @@ public sealed class RuntimeFellowshipState : IDisposable /// public void ApplyFullUpdate(GameEvents.FellowshipFullUpdate update) { - ObjectDisposedException.ThrowIf(IsDisposed, this); lock (_gate) { + ObjectDisposedException.ThrowIf(_disposed, this); _members.Clear(); foreach (GameEvents.FellowMember member in update.Members) _members[member.Guid] = member; + // SHOULD-FIX 4 (mechanism review): field 8 of 0x02BE — the + // _fellows_departed table AddFellow's locked-admission gate + // consults (see ApplyUpdateFellow below). Previously discarded. + _fellowsDeparted.Clear(); + foreach (GameEvents.FellowshipDepartedMember departed in update.Departed) + _fellowsDeparted[departed.Guid] = departed.DepartedTimestamp; _name = update.Name; _leaderGuid = update.LeaderGuid; _shareXp = update.ShareXp; + // Store the wire flag verbatim — do NOT re-derive via + // RecalculateEvenXpSplit here. Lane B §6.2: the full update + // carries the server's own authoritative flag; the client-side + // recompute (SHOULD-FIX 3, below) is a display-only optimistic + // estimate for BETWEEN full updates and must never override it. _evenXpSplit = update.EvenXpSplit; _isOpen = update.OpenFellow; _locked = update.Locked; @@ -88,18 +129,88 @@ public sealed class RuntimeFellowshipState : IDisposable /// guid (vitals/level/name refresh). A no-op before any full update has /// ever established that the local player IS in a fellowship — retail /// never sends this to a client outside one either. + /// + /// + /// SHOULD-FIX 4 (mechanism review): retail's + /// Fellowship::UpdateFellow @0x005B9730 falls through to + /// Fellowship::AddFellow @0x005B9480 when the guid is absent from + /// the table, and AddFellow refuses a brand-new guid while + /// unless it appears in + /// within + /// seconds — ported below as + /// . An existing member's own + /// refresh is never gated (only the "is this guid NEW" branch is). + /// /// public void ApplyUpdateFellow(GameEvents.FellowshipUpdateFellow update) { - ObjectDisposedException.ThrowIf(IsDisposed, this); lock (_gate) { + ObjectDisposedException.ThrowIf(_disposed, this); if (!_isInFellowship) return; + bool isNewMember = !_members.ContainsKey(update.MemberGuid); + if (isNewMember && _locked && !IsAdmissibleWhileLocked(update.MemberGuid)) + return; _members[update.MemberGuid] = update.Member; + // SHOULD-FIX 3 (mechanism review): Fellowship::UpdateFellow + // @0x005B9785 calls RecalculateEvenXPSplitting on every upsert + // (both the AddFellow and the existing-member-refresh branch). + RecalculateEvenXpSplit(); Bump(); } } + /// + /// Port of Fellowship::AddFellow @0x005B94A5's locked-admission + /// check. + /// + private bool IsAdmissibleWhileLocked(uint guid) + { + if (!_fellowsDeparted.TryGetValue(guid, out int departedTimestamp)) + return false; + long nowSeconds = _timeProvider.GetUtcNow().ToUnixTimeSeconds(); + return nowSeconds - departedTimestamp <= DepartedGraceSeconds; + } + + /// + /// Port of Fellowship::RecalculateEvenXPSplitting @0x005B92E0 + /// (lane B §2.10/§7.4), called from AddFellow/UpdateFellow/ + /// RemoveFellow only — NEVER from a full update, which carries + /// the server's own authoritative flag (see 's + /// comment). A local optimistic recompute for DISPLAY between updates; a + /// later 0x02BE always overrides it. + /// + private void RecalculateEvenXpSplit() + { + if (!_shareXp) return; // leaves _evenXpSplit untouched, matching retail + + uint minLevel = uint.MaxValue; + uint maxLevel = 0u; + foreach (GameEvents.FellowMember member in _members.Values) + { + if (member.Level < minLevel) minLevel = member.Level; + if (member.Level > maxLevel) maxLevel = member.Level; + } + + if (!_members.TryGetValue(_leaderGuid, out GameEvents.FellowMember leader)) + { + // Fellowship::GetLeadersLevel @0x005B91B0 returns the + // 0xFFFFFFFF sentinel when the leader isn't in the table; lane B + // §7.4's byte-decode note directs treating that case as "leave + // _even_xp_split at 1" rather than replaying the + // unsigned-wraparound comparison against a sentinel. + _evenXpSplit = true; + return; + } + + _evenXpSplit = true; + if (minLevel < 50u) + { + if (maxLevel > leader.Level + 5u) _evenXpSplit = false; + if (minLevel + 5u < leader.Level) _evenXpSplit = false; + } + } + /// /// 0x00A3 FellowshipQuit (S→C direction) — sent both to the /// quitter and to every remaining member. Self-removal ( public void ApplyQuit(uint quitterGuid, uint selfGuid) { - ObjectDisposedException.ThrowIf(IsDisposed, this); lock (_gate) { + ObjectDisposedException.ThrowIf(_disposed, this); if (!_isInFellowship) return; if (quitterGuid == selfGuid) { @@ -119,7 +230,10 @@ public sealed class RuntimeFellowshipState : IDisposable return; } if (_members.Remove(quitterGuid)) + { + RecalculateEvenXpSplit(); Bump(); + } } } @@ -129,9 +243,9 @@ public sealed class RuntimeFellowshipState : IDisposable /// public void ApplyDismiss(uint dismissedGuid, uint selfGuid) { - ObjectDisposedException.ThrowIf(IsDisposed, this); lock (_gate) { + ObjectDisposedException.ThrowIf(_disposed, this); if (!_isInFellowship) return; if (dismissedGuid == selfGuid) { @@ -139,15 +253,21 @@ public sealed class RuntimeFellowshipState : IDisposable return; } if (_members.Remove(dismissedGuid)) + { + RecalculateEvenXpSplit(); Bump(); + } } } /// 0x02BF FellowshipDisband — always clears. public void ApplyDisband() { - ObjectDisposedException.ThrowIf(IsDisposed, this); - lock (_gate) ClearLocked(); + lock (_gate) + { + ObjectDisposedException.ThrowIf(_disposed, this); + ClearLocked(); + } } /// The local player's own current leader guid, or 0 when not in a fellowship. @@ -201,10 +321,19 @@ public sealed class RuntimeFellowshipState : IDisposable _members.Count); } - /// Session-scoped: cleared at every generation reset (reconnect). + /// + /// Session-scoped: cleared at every generation reset (reconnect). + /// Blast SHOULD-FIX 5: no disposed guard, matching the precedent this + /// class's doc comment cites + /// (, + /// — both + /// bare delegations with no disposal guard). The reset transaction is + /// retryable and disposal is terminal; a throwing guard here could never + /// converge on retry. A no-op after — the fields + /// are already cleared. + /// public void ResetSession() { - ObjectDisposedException.ThrowIf(IsDisposed, this); lock (_gate) ClearLocked(); } @@ -229,6 +358,7 @@ public sealed class RuntimeFellowshipState : IDisposable || _isOpen || _locked; _members.Clear(); + _fellowsDeparted.Clear(); _name = string.Empty; _leaderGuid = 0u; _shareXp = false; diff --git a/tests/AcDream.Runtime.Tests/GameRuntimeContractTests.cs b/tests/AcDream.Runtime.Tests/GameRuntimeContractTests.cs index b56f9289..e8592713 100644 --- a/tests/AcDream.Runtime.Tests/GameRuntimeContractTests.cs +++ b/tests/AcDream.Runtime.Tests/GameRuntimeContractTests.cs @@ -197,7 +197,14 @@ public sealed class GameRuntimeContractTests ActiveRevealCount: 1, PendingDestinationReadinessCount: 1, HostProjectionCount: 1, - PendingHostAcknowledgementCount: 2)); + PendingHostAcknowledgementCount: 2), + // FA2 fix-round SHOULD-FIX 6: Fellowship/Allegiance are no + // longer trailing-optional (see RuntimeStateCheckpoint's doc + // comment) — `new()` uses each snapshot's explicit + // parameterless constructor, which gives a non-null Name/ + // AllegianceName instead of `default`'s bitwise zero-init. + new(), + new()); recorder.AddCheckpoint(stamp, checkpoint); diff --git a/tests/AcDream.Runtime.Tests/Gameplay/RuntimeFellowshipStateTests.cs b/tests/AcDream.Runtime.Tests/Gameplay/RuntimeFellowshipStateTests.cs index d7359332..2ef2bd64 100644 --- a/tests/AcDream.Runtime.Tests/Gameplay/RuntimeFellowshipStateTests.cs +++ b/tests/AcDream.Runtime.Tests/Gameplay/RuntimeFellowshipStateTests.cs @@ -283,6 +283,211 @@ public sealed class RuntimeFellowshipStateTests Assert.Throws( () => state.ApplyQuit(SelfGuid, SelfGuid)); Assert.Throws(state.ApplyDisband); - Assert.Throws(state.ResetSession); + } + + [Fact] + public void ResetSession_AfterDispose_IsANoOpAndDoesNotThrow() + { + // Blast SHOULD-FIX 5: ResetSession has NO disposed guard, matching + // the precedent its own doc comment cites + // (RuntimeInventoryState.ResetExternalContainer, + // RuntimeCommunicationState.ResetNegotiatedChannels — both bare + // delegations with no disposal guard). The reset transaction is + // retryable and disposal is terminal; a throwing guard could never + // converge on retry. + var state = new RuntimeFellowshipState(); + state.ApplyFullUpdate(FullUpdate(Member(SelfGuid))); + state.Dispose(); + + Exception? thrown = Record.Exception(state.ResetSession); + + Assert.Null(thrown); + Assert.False(state.View.Snapshot.IsInFellowship); + } + + // ── SHOULD-FIX 3 (mechanism review): RecalculateEvenXPSplitting ──────── + + [Theory] + [InlineData(10u, 10u, true)] // within +/-5 of the leader -> stays even + [InlineData(10u, 20u, false)] // 10 spread, minLevel<50 -> not even + [InlineData(60u, 200u, true)] // minLevel >= 50 -> the spread check never runs, stays even + public void ApplyUpdateFellow_RecalculatesEvenXpSplit_MatchingRetailWideSpreadRule( + uint memberLevel, + uint leaderLevel, + bool expectedEvenXpSplit) + { + var state = new RuntimeFellowshipState(); + state.ApplyFullUpdate(new GameEvents.FellowshipFullUpdate( + [Member(LeaderGuid)], + "The Fellows", + LeaderGuid, + ShareXp: true, + EvenXpSplit: true, + OpenFellow: true, + Locked: false, + Departed: [])); + // Overwrite the leader's own level to the test's value via a + // same-guid upsert (an existing-member refresh, never gated). + state.ApplyUpdateFellow(new GameEvents.FellowshipUpdateFellow( + LeaderGuid, + Member(LeaderGuid) with { Level = leaderLevel }, + UpdateType: 3u)); + + state.ApplyUpdateFellow(new GameEvents.FellowshipUpdateFellow( + SelfGuid, + Member(SelfGuid) with { Level = memberLevel }, + UpdateType: 1u)); + + Assert.Equal(expectedEvenXpSplit, state.View.Snapshot.EvenXpSplit); + } + + [Fact] + public void ApplyUpdateFellow_ShareXpOff_LeavesEvenXpSplitUntouched() + { + var state = new RuntimeFellowshipState(); + state.ApplyFullUpdate(new GameEvents.FellowshipFullUpdate( + [Member(LeaderGuid)], + "The Fellows", + LeaderGuid, + ShareXp: false, + EvenXpSplit: true, // deliberately mismatched with ShareXp: false + OpenFellow: true, + Locked: false, + Departed: [])); + + state.ApplyUpdateFellow(new GameEvents.FellowshipUpdateFellow( + SelfGuid, Member(SelfGuid) with { Level = 999u }, UpdateType: 1u)); + + // ShareXp == false -> RecalculateEvenXPSplitting's retail body + // returns immediately, leaving the wire-supplied flag alone. + Assert.True(state.View.Snapshot.EvenXpSplit); + } + + [Fact] + public void ApplyFullUpdate_NeverRecomputesEvenXpSplit_StoresTheWireFlagVerbatim() + { + // Lane B §6.2: the full update carries the server's own + // authoritative flag; ApplyFullUpdate must store it as-is even when + // the client-side recompute would disagree (leader/member levels + // far enough apart that RecalculateEvenXPSplitting would say false). + var state = new RuntimeFellowshipState(); + + state.ApplyFullUpdate(new GameEvents.FellowshipFullUpdate( + [ + Member(LeaderGuid) with { Level = 5u }, + Member(SelfGuid) with { Level = 500u }, + ], + "The Fellows", + LeaderGuid, + ShareXp: true, + EvenXpSplit: true, // server says even despite the huge spread + OpenFellow: true, + Locked: false, + Departed: [])); + + Assert.True(state.View.Snapshot.EvenXpSplit); + } + + // ── SHOULD-FIX 4 (mechanism review): locked/departed admission gate ─── + + [Fact] + public void ApplyUpdateFellow_LockedFellowship_RefusesABrandNewGuidNotInDeparted() + { + var state = new RuntimeFellowshipState(); + state.ApplyFullUpdate(new GameEvents.FellowshipFullUpdate( + [Member(LeaderGuid)], + "The Fellows", + LeaderGuid, + ShareXp: false, + EvenXpSplit: false, + OpenFellow: false, + Locked: true, + Departed: [])); + long before = state.View.Snapshot.Revision; + + state.ApplyUpdateFellow(new GameEvents.FellowshipUpdateFellow( + OtherGuid, Member(OtherGuid), UpdateType: 1u)); + + Assert.False(state.View.TryGetMember(OtherGuid, out _)); + Assert.Equal(1, state.View.Snapshot.MemberCount); + Assert.Equal(before, state.View.Snapshot.Revision); + } + + [Fact] + public void ApplyUpdateFellow_LockedFellowship_AdmitsAGuidThatDepartedWithinTheGraceWindow() + { + var clock = new ManualTimeProvider(); + var state = new RuntimeFellowshipState(clock); + state.ApplyFullUpdate(new GameEvents.FellowshipFullUpdate( + [Member(LeaderGuid)], + "The Fellows", + LeaderGuid, + ShareXp: false, + EvenXpSplit: false, + OpenFellow: false, + Locked: true, + Departed: [new GameEvents.FellowshipDepartedMember( + OtherGuid, (int)clock.GetUtcNow().ToUnixTimeSeconds())])); + clock.Advance(TimeSpan.FromSeconds(899)); + + state.ApplyUpdateFellow(new GameEvents.FellowshipUpdateFellow( + OtherGuid, Member(OtherGuid), UpdateType: 1u)); + + Assert.True(state.View.TryGetMember(OtherGuid, out _)); + Assert.Equal(2, state.View.Snapshot.MemberCount); + } + + [Fact] + public void ApplyUpdateFellow_LockedFellowship_RefusesAGuidThatDepartedOutsideTheGraceWindow() + { + var clock = new ManualTimeProvider(); + var state = new RuntimeFellowshipState(clock); + state.ApplyFullUpdate(new GameEvents.FellowshipFullUpdate( + [Member(LeaderGuid)], + "The Fellows", + LeaderGuid, + ShareXp: false, + EvenXpSplit: false, + OpenFellow: false, + Locked: true, + Departed: [new GameEvents.FellowshipDepartedMember( + OtherGuid, (int)clock.GetUtcNow().ToUnixTimeSeconds())])); + clock.Advance(TimeSpan.FromSeconds(901)); + + state.ApplyUpdateFellow(new GameEvents.FellowshipUpdateFellow( + OtherGuid, Member(OtherGuid), UpdateType: 1u)); + + Assert.False(state.View.TryGetMember(OtherGuid, out _)); + Assert.Equal(1, state.View.Snapshot.MemberCount); + } + + [Fact] + public void ApplyUpdateFellow_LockedFellowship_NeverGatesAnExistingMembersOwnRefresh() + { + var state = new RuntimeFellowshipState(); + state.ApplyFullUpdate(new GameEvents.FellowshipFullUpdate( + [Member(LeaderGuid), Member(SelfGuid, currentHealth: 100u)], + "The Fellows", + LeaderGuid, + ShareXp: false, + EvenXpSplit: false, + OpenFellow: false, + Locked: true, + Departed: [])); + + state.ApplyUpdateFellow(new GameEvents.FellowshipUpdateFellow( + SelfGuid, Member(SelfGuid, currentHealth: 42u), UpdateType: 3u)); + + Assert.True(state.View.TryGetMember(SelfGuid, out RuntimeFellowMemberSnapshot self)); + Assert.Equal(42u, self.CurrentHealth); + } + + private sealed class ManualTimeProvider : TimeProvider + { + private DateTimeOffset _now = new(2026, 8, 12, 0, 0, 0, TimeSpan.Zero); + + public override DateTimeOffset GetUtcNow() => _now; + + public void Advance(TimeSpan elapsed) => _now += elapsed; } } diff --git a/tests/AcDream.Runtime.Tests/Session/LiveSessionEventRouterTests.cs b/tests/AcDream.Runtime.Tests/Session/LiveSessionEventRouterTests.cs index 5f5b4693..c30fd2c0 100644 --- a/tests/AcDream.Runtime.Tests/Session/LiveSessionEventRouterTests.cs +++ b/tests/AcDream.Runtime.Tests/Session/LiveSessionEventRouterTests.cs @@ -156,6 +156,102 @@ public sealed class LiveSessionEventRouterTests router.Dispose(); } + // ── Campaign FA slice FA2 fix-round SHOULD-FIX 4 (blast review) ──────── + // The single production registration site (LiveSessionEventRouter.cs) + // was untested — both router-test factories default Fellowship/ + // Allegiance to null, so every FA2 lambda no-ops through `?.` in every + // OTHER test in this file. The self-vs-other guid routing + // (onFellowshipQuit/onFellowshipDismiss supply inventory.PlayerGuid() + // as the self-guid, which selects "remove one member" vs "clear the + // whole snapshot") is the one non-trivial lambda in the whole slice — + // a transposed argument or wrong guid source there was invisible to + // every test until now. + + [Fact] + public void FellowshipQuit_RoutesSelfGuidToClearAndOtherGuidToRemove() + { + const uint self = 0x50000001u; + const uint other = 0x50000002u; + using var session = NewSession(); + var fellowship = new RuntimeFellowshipState(); + var allegiance = new RuntimeAllegianceState(); + try + { + fellowship.ApplyFullUpdate(new GameEvents.FellowshipFullUpdate( + [ + new GameEvents.FellowMember( + self, 0u, 0u, 1u, 100u, 100u, 100u, 100u, 100u, 100u, 0u, "Self"), + new GameEvents.FellowMember( + other, 0u, 0u, 1u, 100u, 100u, 100u, 100u, 100u, 100u, 0u, "Other"), + ], + "The Fellows", + LeaderGuid: self, + ShareXp: true, + EvenXpSplit: false, + OpenFellow: true, + Locked: false, + Departed: [])); + + var router = new LiveSessionEventRouter( + session, + NoOpEntitySink(), + NoOpEnvironmentSink(), + new LiveInventorySessionBindings( + new ClientObjectTable(), + PlayerGuid: () => self, + OnShortcuts: null, + OnUseDone: null, + ItemMana: new ItemManaState(), + ExternalContainers: new ExternalContainerState()), + NewCharacterBindings(), + new LiveSocialSessionBindings( + new ChatLog(), + new TurbineChatState(), + new FriendsState(), + new SquelchState(), + Fellowship: fellowship, + Allegiance: allegiance)); + router.Attach(); + + // Someone ELSE quits -- removes exactly that one member. + session.GameEvents.Dispatch( + GameEventEnvelope.TryParse(WrapFellowshipQuitEnvelope(other))!.Value); + + Assert.True(fellowship.View.Snapshot.IsInFellowship); + Assert.Equal(1, fellowship.View.Snapshot.MemberCount); + Assert.False(fellowship.View.TryGetMember(other, out _)); + Assert.True(fellowship.View.TryGetMember(self, out _)); + + // The LOCAL PLAYER quits -- clears the whole snapshot, not just + // one member. + session.GameEvents.Dispatch( + GameEventEnvelope.TryParse(WrapFellowshipQuitEnvelope(self))!.Value); + + Assert.False(fellowship.View.Snapshot.IsInFellowship); + Assert.Equal(0, fellowship.View.Snapshot.MemberCount); + + router.Dispose(); + } + finally + { + fellowship.Dispose(); + allegiance.Dispose(); + } + } + + private static byte[] WrapFellowshipQuitEnvelope(uint quitterGuid) + { + byte[] payload = new byte[4]; + BinaryPrimitives.WriteUInt32LittleEndian(payload, quitterGuid); + byte[] body = new byte[GameEventEnvelope.HeaderSize + payload.Length]; + BinaryPrimitives.WriteUInt32LittleEndian(body, GameEventEnvelope.Opcode); + BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(4), 0u); + BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(8), 0u); + BinaryPrimitives.WriteUInt32LittleEndian(body.AsSpan(12), (uint)GameEventType.FellowshipQuit); + Array.Copy(payload, 0, body, GameEventEnvelope.HeaderSize, payload.Length); + return body; + } + // ── Campaign CH slice CH3: TurbineChat ack HResult surfacing ── [Fact]