fix(net,runtime): FA2 fix-round MUST-FIX -- allegiance clears at reset, 0x027C stops seeding
Two MUST-FIX findings from the FA2 mechanism/blast reviews (docs/research/2026-08-12-fa2-review-mechanism.md, docs/research/2026-08-12-fa2-review-blast.md): MF-1 (mechanism) -- RuntimeAllegianceState survived a generation reset, contradicting retail (ClientAllegianceSystem::OnEndCharacterSession @0x00569FA0 tail-calls AllegianceProfile::Clear at the same boundary Fellowship already clears at), contradicting the precedent it cited (RuntimeCharacterOptionsState.ResetSession clears-and-relatches, it does not persist), and pinned by a test asserting the wrong behavior. Fixed: RuntimeAllegianceState.ResetSession() clears the profile and drops HasServerSeed; a new RuntimeGenerationResetStage.Allegiance stage runs it on every generation reset, mirroring RuntimeFellowshipState exactly. RuntimeGenerationResetTests' FellowshipClearsAtResetButAllegianceSurvivesReconnect inverted to FellowshipAndAllegianceBothClearAtGenerationReset. MF-2 (mechanism) / blast MF-2 -- 0x027C AllegianceInfoResponse fed the Runtime allegiance owner (self-gated). Retail's own handler for 0x027C (CM_Allegiance::DispatchUI_AllegianceInfoResponseEvent @0x006a7470) unpacks into a stack-local profile destroyed on return; the consumer (Handle_Allegiance__AllegianceInfoResponseEvent @0x0056a1d0) only prints AddTextToScroll lines. Retail's panel is fed exclusively by 0x0020 AllegianceUpdate. The removed seeding also fabricated RuntimeAllegianceSnapshot.Rank (0x027C carries no rank field) on any client whose first allegiance message was a self @allegiance info query. Fixed: dropped ApplyInfoResponseSelf, the onAllegianceInfoResponseSelf delegate hole, and the self-gate; 0x027C is text-only again, matching retail and the pre-FA2 shape. Also covers blast SHOULD-FIX 1 in the same edit to LiveSessionEventRouter.cs: the fellowship/allegiance delegate holes are now passed conditionally on the owner being supplied, so GameEventDispatcher.GetUnhandledCount reads correctly for callers without an owner (bare-ChatLog tests, a future partial host) instead of silently reading 0 for 9 event types whose parse result was discarded. RuntimeAllegianceState.cs and the two owners' Apply* mutators also move their ObjectDisposedException.ThrowIf checks inside the lock they already take (mechanism SHOULD-FIX 2) -- the prior check-then-lock shape let an inbound event on the decode thread race Dispose on the host thread and repopulate state after _disposed = true, permanently falsifying CaptureOwnership().IsConverged at teardown. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
63649c8053
commit
4272ad0ea4
9 changed files with 425 additions and 229 deletions
|
|
@ -1,3 +1,4 @@
|
|||
using System.Reflection;
|
||||
using AcDream.Core.Combat;
|
||||
using AcDream.Core.Items;
|
||||
using AcDream.Core.Selection;
|
||||
|
|
@ -207,6 +208,54 @@ public sealed class GameRuntimeTests
|
|||
Assert.Equal(0u, ownership.PlayerIdentity.ServerGuid);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void CompletedTeardownStagesAccumulatesExactlyOneFlagPerStage()
|
||||
{
|
||||
// Blast MF-1 (docs/research/2026-08-12-fa2-review-blast.md): the
|
||||
// FA2 rewrite of case 9 in the private CompletedTeardownStages
|
||||
// switch claimed FellowshipDisposed one stage early (10 flags
|
||||
// instead of 9) — invisible to both existing tests, which only
|
||||
// sample the endpoints (stage 0 and Complete). This test walks
|
||||
// every intermediate stage directly via reflection so the next
|
||||
// owner insertion cannot repeat the class of bug: at
|
||||
// _disposeStage == N, EXACTLY the first N stages' flags must be
|
||||
// set, in DrainCurrentStage's disposal order.
|
||||
using var runtime = Create();
|
||||
FieldInfo stageField = typeof(GameRuntime).GetField(
|
||||
"_disposeStage", BindingFlags.NonPublic | BindingFlags.Instance)!;
|
||||
PropertyInfo completedProperty = typeof(GameRuntime).GetProperty(
|
||||
"CompletedTeardownStages", BindingFlags.NonPublic | BindingFlags.Instance)!;
|
||||
|
||||
GameRuntimeTeardownStage[] orderedFlags =
|
||||
[
|
||||
GameRuntimeTeardownStage.HostLeasesReleased,
|
||||
GameRuntimeTeardownStage.EventsDetached,
|
||||
GameRuntimeTeardownStage.SessionDisposed,
|
||||
GameRuntimeTeardownStage.TransitReset,
|
||||
GameRuntimeTeardownStage.ActionsDisposed,
|
||||
GameRuntimeTeardownStage.MovementDisposed,
|
||||
GameRuntimeTeardownStage.CharacterDisposed,
|
||||
GameRuntimeTeardownStage.InventoryDisposed,
|
||||
GameRuntimeTeardownStage.CommunicationDisposed,
|
||||
GameRuntimeTeardownStage.FellowshipDisposed,
|
||||
GameRuntimeTeardownStage.AllegianceDisposed,
|
||||
GameRuntimeTeardownStage.IdentityDisposed,
|
||||
GameRuntimeTeardownStage.EntityObjectsDisposed,
|
||||
];
|
||||
|
||||
GameRuntimeTeardownStage expected = GameRuntimeTeardownStage.None;
|
||||
for (int stage = 0; stage <= orderedFlags.Length; stage++)
|
||||
{
|
||||
stageField.SetValue(runtime, stage);
|
||||
var actual = (GameRuntimeTeardownStage)completedProperty.GetValue(runtime)!;
|
||||
Assert.Equal(expected, actual);
|
||||
if (stage < orderedFlags.Length)
|
||||
expected |= orderedFlags[stage];
|
||||
}
|
||||
|
||||
Assert.Equal(GameRuntimeTeardownStage.Complete, expected);
|
||||
}
|
||||
|
||||
private static GameRuntime Create() => new(Dependencies());
|
||||
|
||||
private static GameRuntimeDependencies Dependencies()
|
||||
|
|
|
|||
|
|
@ -6,12 +6,19 @@ namespace AcDream.Runtime.Tests.Gameplay;
|
|||
/// <summary>
|
||||
/// Campaign FA slice FA2 (2026-08-12): lifecycle rules for
|
||||
/// <see cref="RuntimeAllegianceState"/> — seeding from the unsolicited
|
||||
/// <c>AllegianceUpdate</c> push and from a self-gated
|
||||
/// <c>AllegianceInfoResponse</c>, the <c>HasServerSeed</c>-style latch, the
|
||||
/// monarch/patron/vassal walk, revision monotonicity, and the "survives
|
||||
/// reconnect" ownership contract (no <c>RuntimeGenerationReset</c> stage —
|
||||
/// see <see cref="RuntimeGenerationResetTests"/> for the sibling assertion
|
||||
/// that Fellowship IS a reset stage and Allegiance is not).
|
||||
/// <c>AllegianceUpdate</c> push, the <c>HasServerSeed</c>-style latch, the
|
||||
/// monarch/patron/vassal walk, revision monotonicity, and the
|
||||
/// generation-reset ownership contract.
|
||||
///
|
||||
/// <para>
|
||||
/// FA2 fix-round correction (2026-08-12,
|
||||
/// docs/research/2026-08-12-fa2-review-mechanism.md MF-1/MF-2): this owner
|
||||
/// is now a <c>RuntimeGenerationReset</c> stage (see
|
||||
/// <see cref="RuntimeGenerationResetTests"/> for the sibling assertion that
|
||||
/// BOTH Fellowship and Allegiance clear at reset), and no longer seeds from
|
||||
/// <c>0x027C AllegianceInfoResponse</c> — retail's own handler for that
|
||||
/// response is print-only over a stack-local profile.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public sealed class RuntimeAllegianceStateTests
|
||||
{
|
||||
|
|
@ -63,35 +70,6 @@ public sealed class RuntimeAllegianceStateTests
|
|||
Assert.Equal(4, snapshot.RecordCount);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ApplyInfoResponseSelf_SeedsTheProfileButLeavesRankUntouched()
|
||||
{
|
||||
var state = new RuntimeAllegianceState();
|
||||
state.ApplyUpdate(Update(rank: 9u));
|
||||
|
||||
var response = new ClientCommandResponses.AllegianceInfoResponse(
|
||||
TargetGuid: SelfGuid,
|
||||
TotalMembers: 5u,
|
||||
TotalVassals: 3u,
|
||||
RecordCount: 5,
|
||||
AllegianceName: "The Order",
|
||||
Monarch: Record(MonarchGuid, 0u, "Monarch"),
|
||||
Records:
|
||||
[
|
||||
Record(PatronGuid, MonarchGuid, "Patron"),
|
||||
Record(SelfGuid, PatronGuid, "Self"),
|
||||
]);
|
||||
|
||||
state.ApplyInfoResponseSelf(response);
|
||||
|
||||
RuntimeAllegianceSnapshot snapshot = state.View.Snapshot;
|
||||
Assert.True(snapshot.HasProfile);
|
||||
// AllegianceInfoResponse carries no rank field on the wire — the
|
||||
// last known rank from the earlier AllegianceUpdate is retained.
|
||||
Assert.Equal(9u, snapshot.Rank);
|
||||
Assert.Equal(2, snapshot.RecordCount);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void TryGetMonarchPatronAndVassals_WalkTheFlatRecordList()
|
||||
{
|
||||
|
|
@ -182,4 +160,60 @@ public sealed class RuntimeAllegianceStateTests
|
|||
Assert.Throws<ObjectDisposedException>(
|
||||
() => state.ApplyLoginNotification(SelfGuid, true));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ResetSession_ClearsProfileAndDropsHasServerSeed_MatchingOnEndCharacterSession()
|
||||
{
|
||||
// MF-1 (docs/research/2026-08-12-fa2-review-mechanism.md): retail's
|
||||
// ClientAllegianceSystem::OnEndCharacterSession @0x00569FA0
|
||||
// tail-calls AllegianceProfile::Clear at exactly this boundary —
|
||||
// the fix-round replaces the prior (inverted) "survives reconnect"
|
||||
// test with this one.
|
||||
var state = new RuntimeAllegianceState();
|
||||
state.ApplyUpdate(Update(rank: 7u));
|
||||
Assert.True(state.HasServerSeed);
|
||||
Assert.True(state.View.Snapshot.HasProfile);
|
||||
|
||||
state.ResetSession();
|
||||
|
||||
RuntimeAllegianceSnapshot snapshot = state.View.Snapshot;
|
||||
Assert.False(state.HasServerSeed);
|
||||
Assert.False(snapshot.HasServerSeed);
|
||||
Assert.False(snapshot.HasProfile);
|
||||
Assert.Equal(0u, snapshot.Rank);
|
||||
Assert.Equal(string.Empty, snapshot.AllegianceName);
|
||||
Assert.Equal(0u, snapshot.MonarchGuid);
|
||||
Assert.Equal(0, snapshot.RecordCount);
|
||||
Assert.False(state.IsDisposed);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ResetSession_OnAnUnseededOwner_IsANoOpThatDoesNotBumpRevision()
|
||||
{
|
||||
var state = new RuntimeAllegianceState();
|
||||
long before = state.View.Snapshot.Revision;
|
||||
|
||||
state.ResetSession();
|
||||
|
||||
Assert.Equal(before, state.View.Snapshot.Revision);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ResetSession_AfterDispose_IsANoOpAndDoesNotThrow()
|
||||
{
|
||||
// Blast SF-5: RuntimeFellowshipState.ResetSession has no disposed
|
||||
// guard (matching RuntimeInventoryState.ResetExternalContainer /
|
||||
// RuntimeCommunicationState.ResetNegotiatedChannels) because the
|
||||
// reset transaction is retryable and disposal is terminal — a
|
||||
// throwing guard could never converge on retry. Allegiance's new
|
||||
// ResetSession matches that shape.
|
||||
var state = new RuntimeAllegianceState();
|
||||
state.ApplyUpdate(Update());
|
||||
state.Dispose();
|
||||
|
||||
Exception? thrown = Xunit.Record.Exception(state.ResetSession);
|
||||
|
||||
Assert.Null(thrown);
|
||||
Assert.False(state.View.Snapshot.HasProfile);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -200,13 +200,20 @@ public sealed class RuntimeGenerationResetTests
|
|||
}
|
||||
|
||||
[Fact]
|
||||
public void FellowshipClearsAtResetButAllegianceSurvivesReconnect()
|
||||
public void FellowshipAndAllegianceBothClearAtGenerationReset()
|
||||
{
|
||||
// Campaign FA slice FA2 (2026-08-12), D2: fellowship is
|
||||
// session-scoped (cleared at every generation reset, matching the
|
||||
// ExternalContainer precedent); allegiance is NOT a reset stage at
|
||||
// all — it survives reconnect exactly like a real disconnect does
|
||||
// not sever your character's allegiance membership.
|
||||
// Campaign FA slice FA2 (2026-08-12), D2, CORRECTED by the FA2
|
||||
// fix-round MUST-FIX 1 (2026-08-12,
|
||||
// docs/research/2026-08-12-fa2-review-mechanism.md): this test
|
||||
// previously asserted the opposite of retail's behavior —
|
||||
// "allegiance survives reconnect". Retail's
|
||||
// ClientAllegianceSystem::OnEndCharacterSession @0x00569FA0
|
||||
// tail-calls AllegianceProfile::Clear at exactly this boundary,
|
||||
// mirroring the sibling ClientFellowshipSystem::
|
||||
// OnEndCharacterSession @0x005690A0 fellowship already honored, and
|
||||
// the cited precedent (RuntimeCharacterOptionsState.ResetSession)
|
||||
// CLEARS and re-latches, it does not persist. Both owners are now
|
||||
// RuntimeGenerationReset stages and both clear here.
|
||||
using var runtime = Create();
|
||||
runtime.PlayerIdentity.ServerGuid = 0x50000001u;
|
||||
runtime.FellowshipOwner.ApplyFullUpdate(new GameEvents.FellowshipFullUpdate(
|
||||
|
|
@ -231,16 +238,18 @@ public sealed class RuntimeGenerationResetTests
|
|||
|
||||
Assert.True(runtime.Fellowship.Snapshot.IsInFellowship);
|
||||
Assert.True(runtime.Allegiance.Snapshot.HasProfile);
|
||||
Assert.True(runtime.AllegianceOwner.HasServerSeed);
|
||||
|
||||
var host = new RecordingResetHost(runtime);
|
||||
runtime.ResetGeneration(new RuntimeGenerationToken(3), host);
|
||||
|
||||
Assert.False(runtime.Fellowship.Snapshot.IsInFellowship);
|
||||
Assert.Equal(0, runtime.Fellowship.Snapshot.MemberCount);
|
||||
// Allegiance is untouched by the reset — the data survives.
|
||||
Assert.True(runtime.Allegiance.Snapshot.HasProfile);
|
||||
Assert.True(runtime.AllegianceOwner.HasServerSeed);
|
||||
Assert.Equal("The Order", runtime.Allegiance.Snapshot.AllegianceName);
|
||||
Assert.False(runtime.Allegiance.Snapshot.HasProfile);
|
||||
Assert.False(runtime.AllegianceOwner.HasServerSeed);
|
||||
Assert.Equal(string.Empty, runtime.Allegiance.Snapshot.AllegianceName);
|
||||
Assert.Equal(0u, runtime.Allegiance.Snapshot.MonarchGuid);
|
||||
Assert.Equal(0, runtime.Allegiance.Snapshot.RecordCount);
|
||||
}
|
||||
|
||||
private static GameRuntime Create()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue