fix(runtime): close Campaign LA7b review findings

2026-08-14 18:55:48 +02:00 · 2026-08-14 18:55:48 +02:00 · 1b9e7e41f9
commit 1b9e7e41f9
parent 0e82cbf700
11 changed files with 700 additions and 28 deletions
--- a/tests/AcDream.App.Tests/Runtime/CurrentGameRuntimeAdapterTests.cs
+++ b/tests/AcDream.App.Tests/Runtime/CurrentGameRuntimeAdapterTests.cs
@ -211,6 +211,74 @@ public sealed class CurrentGameRuntimeAdapterTests
        Assert.Equal(Harness.PlayerGuid, harness.Identity.ServerGuid);
    }

+    [Fact]
+    public void DisposedAdapterMakesRetainedSelectionRoutesInertWhileRuntimeLives()
+    {
+        using var harness = new Harness(awaitCharacterSelection: true);
+        using CurrentGameRuntimeAdapter survivor =
+            harness.CreateAdditionalAdapter();
+        Assert.Equal(
+            RuntimeSessionStartStatus.AwaitingCharacterSelection,
+            harness.Runtime.Session.Start(harness.Runtime.Generation).Status);
+
+        RuntimeGenerationToken generation = harness.Runtime.Generation;
+        IRuntimeCharacterSelectionView retainedView =
+            harness.Runtime.CharacterSelection;
+        IRuntimeCharacterSelectionCommands publicCommands =
+            harness.Runtime.CharacterSelectionCommands;
+        IRuntimeCharacterSelectionCommands interfaceCommands =
+            ((IGameRuntimeCommands)harness.Runtime).CharacterSelection;
+        var observer = new CharacterSelectionObserver();
+        using IDisposable subscription = retainedView.Subscribe(observer);
+        RuntimeCharacterSelectionSnapshot before =
+            survivor.CharacterSelection.Snapshot;
+
+        harness.Runtime.Dispose();
+
+        Assert.False(harness.Runtime.Lifecycle.HasTransport);
+        Assert.Equal(
+            RuntimeCharacterSelectionLifecycle.Inactive,
+            retainedView.Snapshot.Lifecycle);
+        Assert.Equal(0, retainedView.Snapshot.RosterCount);
+        Assert.False(retainedView.TryGetAt(0, out _));
+        Assert.False(retainedView.TryGet(Harness.PlayerGuid, out _));
+        var visitor = new CharacterSelectionVisitor();
+        retainedView.Visit(visitor);
+        Assert.Empty(visitor.Entries);
+        Assert.Throws<ObjectDisposedException>(() =>
+            retainedView.Subscribe(new CharacterSelectionObserver()));
+
+        RuntimeCommandStatus[] statuses =
+        [
+            publicCommands.Highlight(generation, 0x50000001u).Status,
+            publicCommands.Enter(generation).Status,
+            publicCommands.RequestDelete(generation).Status,
+            interfaceCommands.ConfirmDelete(generation).Status,
+            interfaceCommands.Restore(generation).Status,
+            interfaceCommands.Cancel(generation).Status,
+        ];
+        Assert.All(statuses, status =>
+            Assert.Equal(RuntimeCommandStatus.Inactive, status));
+
+        RuntimeCharacterSelectionSnapshot after =
+            survivor.CharacterSelection.Snapshot;
+        Assert.Equal(before.Revision, after.Revision);
+        Assert.Equal(before.HighlightedCharacterId, after.HighlightedCharacterId);
+        Assert.Equal(before.Operation, after.Operation);
+
+        // The second lease proves the canonical Runtime is still live, while
+        // the disposed adapter's already-returned observer stays detached.
+        Assert.True(survivor.CharacterSelectionCommands.Highlight(
+            generation,
+            0x50000001u).Accepted);
+        Assert.Equal(0x50000001u,
+            survivor.CharacterSelection.Snapshot.HighlightedCharacterId);
+        Assert.Empty(observer.Deltas);
+        Assert.Equal(
+            RuntimeCharacterSelectionLifecycle.Inactive,
+            harness.Runtime.CharacterSelection.Snapshot.Lifecycle);
+    }
+
    [Fact]
    public void GenerationGateRejectsStaleCommandsAndStopAcknowledgesTeardown()
    {
@ -735,6 +803,7 @@ public sealed class CurrentGameRuntimeAdapterTests
        private readonly LiveSessionController _session;
        private readonly IDisposable _combatModeBinding;
        private readonly GameRuntime _gameRuntime;
+        private readonly SelectionInteractionController _selectionController;

        public Harness(bool awaitCharacterSelection = false)
        {
@ -789,7 +858,7 @@ public sealed class CurrentGameRuntimeAdapterTests
                sendWield: null,
                sendDrop: null);
            var query = new SelectionQuery(TargetGuid);
-            var selectionController = new SelectionInteractionController(
+            _selectionController = new SelectionInteractionController(
                Selection,
                query,
                _items,
@ -805,7 +874,7 @@ public sealed class CurrentGameRuntimeAdapterTests
                _gameRuntime,
                Host,
                Commands,
-                selectionController);
+                _selectionController);
        }

        public RuntimeOptions Options { get; }
@ -842,6 +911,13 @@ public sealed class CurrentGameRuntimeAdapterTests
        public LiveSessionHost Host { get; }
        public CurrentGameRuntimeAdapter Runtime { get; }

+        public CurrentGameRuntimeAdapter CreateAdditionalAdapter() =>
+            new(
+                _gameRuntime,
+                Host,
+                Commands,
+                _selectionController);
+
        public void Dispose()
        {
            Runtime.Dispose();
@ -853,6 +929,25 @@ public sealed class CurrentGameRuntimeAdapterTests
        }
    }

+    private sealed class CharacterSelectionVisitor
+        : IRuntimeCharacterSelectionVisitor
+    {
+        public List<RuntimeCharacterSelectionEntry> Entries { get; } = [];
+
+        public void Visit(in RuntimeCharacterSelectionEntry character) =>
+            Entries.Add(character);
+    }
+
+    private sealed class CharacterSelectionObserver
+        : IRuntimeCharacterSelectionObserver
+    {
+        public List<RuntimeCharacterSelectionDelta> Deltas { get; } = [];
+
+        public void OnCharacterSelectionChanged(
+            in RuntimeCharacterSelectionDelta delta) =>
+            Deltas.Add(delta);
+    }
+
    private static void PrepareCombatAndMagic(Harness harness)
    {
        harness.Actions.Combat.SetCombatMode(AcDream.Core.Combat.CombatMode.Melee);
--- a/tests/AcDream.Core.Net.Tests/Transport/FakeAceTransport.cs
+++ b/tests/AcDream.Core.Net.Tests/Transport/FakeAceTransport.cs
@ -74,6 +74,12 @@ internal sealed class FakeAceTransport : IWorldSessionTransport
    /// </summary>
    public TimeSpan AutoAdvanceOnBlockingReceive { get; set; }

+    /// <summary>
+    /// Keeps the default ACE script intact while allowing focused handshake
+    /// tests to enqueue messages before the valid ServerReady response.
+    /// </summary>
+    public bool AutoReplyServerReady { get; set; } = true;
+
    public FakeAceTransport(VirtualClock? clock = null, LossyLink? link = null)
    {
        Clock = clock ?? new VirtualClock();
@ -101,7 +107,12 @@ internal sealed class FakeAceTransport : IWorldSessionTransport
            case CharacterEnterWorld.EnterWorldRequestOpcode: // 0xF7C8
                // Server replies CharacterEnterWorldServerReady (0xF7DF) —
                // WorldSession.EnterWorld blocks on this opcode.
-                Model.EnqueueGameMessage(BuildOpcodeOnlyBody(0xF7DFu), GameMessageGroup.UIQueue);
+                if (AutoReplyServerReady)
+                {
+                    Model.EnqueueGameMessage(
+                        BuildOpcodeOnlyBody(0xF7DFu),
+                        GameMessageGroup.UIQueue);
+                }
                break;
            case CharacterLogOff.Opcode: // 0xF653 request (opcode + character id)
                // ACE echoes the opcode-only confirmation; WorldSession.Dispose
@ -142,6 +153,22 @@ internal sealed class FakeAceTransport : IWorldSessionTransport
        }
    }

+    /// <summary>
+    /// Enqueue and flush one model game message under the transport's model
+    /// lock. This is the race-free test seam for a server follower emitted
+    /// while the real WorldSession background receiver is active.
+    /// </summary>
+    public void EnqueueServerGameMessage(
+        byte[] body,
+        GameMessageGroup group)
+    {
+        lock (_gate)
+        {
+            Model.EnqueueGameMessage(body, group);
+            PumpServerLocked();
+        }
+    }
+
    /// <summary>
    /// N2 test hook: deliver raw bytes straight into the client's receive
    /// queue, bypassing both the model and the link. Used for late
--- a/tests/AcDream.Core.Net.Tests/Transport/FakeAceTransportTests.cs
+++ b/tests/AcDream.Core.Net.Tests/Transport/FakeAceTransportTests.cs
@ -2,6 +2,7 @@ using System.Buffers.Binary;
 using System.Net;
 using AcDream.Core.Net.Messages;
 using AcDream.Core.Net.Packets;
+using AcDream.Core.Net.Transport;

 namespace AcDream.Core.Net.Tests.Transport;

@ -165,6 +166,105 @@ public sealed class FakeAceTransportTests
        Assert.Equal(0, transport.Model.CrcDropCount);
    }

+    [Fact]
+    public async Task PausedSelector_SeededDroppedServerReady_RecoversOnIdleSweep()
+    {
+        var fake = new FakeAceTransport();
+        // Random(~13) yields 4, 92, 55, 54: at 50%, only the first
+        // post-arm inbound datagram (ServerReady) is dropped; the follower,
+        // recovered resend, and graceful-logoff confirmation all land.
+        var lossy = new LossyTransportDecorator(
+            fake,
+            dropPercent: 50,
+            seed: 13,
+            NetDropDirection.In);
+        var session = new WorldSession(
+            new IPEndPoint(IPAddress.Loopback, 9000),
+            lossy)
+        {
+            TransportClockSource =
+                (fake.Clock.GetTimestamp, fake.Clock.Frequency),
+        };
+        using var enterRequest = new ManualResetEventSlim();
+        fake.Model.MessageDispatched += body =>
+        {
+            if (ReadOpcode(body)
+                == CharacterEnterWorld.EnterWorldRequestOpcode)
+            {
+                enterRequest.Set();
+            }
+        };
+
+        try
+        {
+            session.Connect(
+                FakeAceTransport.DefaultAccountName,
+                "testpassword",
+                TimeSpan.FromSeconds(5));
+            session.StartCharacterSelectionReceive();
+
+            // Graphical selector frames continue before the user enters.
+            for (int i = 0; i < 3; i++)
+            {
+                fake.Clock.Advance(TimeSpan.FromMilliseconds(50));
+                session.Tick();
+            }
+
+            Task gapDriver = Task.Run(() =>
+            {
+                Assert.True(enterRequest.Wait(TimeSpan.FromSeconds(2)));
+                // This later sequenced packet passes the seeded loss gate,
+                // exposing the missing ServerReady and parking behind it.
+                fake.EnqueueServerGameMessage(
+                    BuildServerMessage("post-ready follower"),
+                    GameMessageGroup.UIQueue);
+                Assert.True(SpinWait.SpinUntil(
+                    () => session.Transport?.Inbound.NakCount > 0,
+                    TimeSpan.FromSeconds(2)));
+
+                // No datagram follows this virtual-time edge. Recovery now
+                // requires paused EnterWorld's independent periodic sweep.
+                fake.Clock.Advance(TimeSpan.FromSeconds(1));
+            });
+
+            session.EnterWorld(0, TimeSpan.FromSeconds(5));
+            await gapDriver;
+
+            Assert.Equal(WorldSession.State.InWorld, session.CurrentState);
+            Assert.Equal(1, lossy.InboundDropped);
+            Assert.True(session.Transport!.Stats.NaksSent > 0);
+            Assert.True(fake.Model.RetransmitsServed > 0);
+            Assert.Equal(0, session.Transport.Inbound.NakCount);
+            Assert.Equal(0, session.Transport.Outbound.PendingResendCount);
+            Assert.Equal(
+                new[]
+                {
+                    CharacterEnterWorld.EnterWorldRequestOpcode,
+                    CharacterEnterWorld.EnterWorldOpcode,
+                },
+                fake.Model.DispatchedMessages.Select(ReadOpcode).ToArray());
+            Assert.Equal(256, fake.Model.Crypto.Headroom);
+            Assert.Equal(0, fake.Model.Crypto.OrphanCount);
+            Assert.Equal(0, fake.Model.CrcDropCount);
+            Assert.False(fake.Model.IsTerminated);
+        }
+        finally
+        {
+            session.Dispose();
+        }
+
+        Assert.Equal(WorldSession.State.Disconnected, session.CurrentState);
+        Assert.True(fake.Model.IsTerminated);
+        Assert.Equal(
+            AceTerminationReason.PacketHeaderDisconnect,
+            fake.Model.TerminationReason);
+        Assert.Equal(
+            CharacterLogOff.Opcode,
+            ReadOpcode(fake.Model.DispatchedMessages[^1]));
+        Assert.Equal(256, fake.Model.Crypto.Headroom);
+        Assert.Equal(0, fake.Model.Crypto.OrphanCount);
+    }
+
    private static uint ReadOpcode(byte[] messageBody) =>
        BinaryPrimitives.ReadUInt32LittleEndian(messageBody);

--- a/tests/AcDream.Core.Net.Tests/WorldSessionCharacterSelectionTests.cs
+++ b/tests/AcDream.Core.Net.Tests/WorldSessionCharacterSelectionTests.cs
@ -3,6 +3,7 @@ using System.Net;
 using System.Reflection;
 using AcDream.Core.Net.Messages;
 using AcDream.Core.Net.Packets;
+using AcDream.Core.Net.Tests.Transport;

 namespace AcDream.Core.Net.Tests;

@ -91,11 +92,80 @@ public sealed class WorldSessionCharacterSelectionTests
        Assert.Equal(1u, current.SecondsGreyedOut);
    }

+    [Fact]
+    public void ImmediateEnterWorld_IgnoresNumErrorsSentinelBeforeServerReady()
+    {
+        var transport = new FakeAceTransport
+        {
+            AutoReplyServerReady = false,
+        };
+        using var session = new WorldSession(
+            new IPEndPoint(IPAddress.Loopback, 9000),
+            transport);
+        int errors = 0;
+        session.CharacterErrorReceived += _ => errors++;
+        ConfigureSentinelThenServerReady(transport);
+
+        session.Connect(
+            FakeAceTransport.DefaultAccountName,
+            "testpassword",
+            TimeSpan.FromSeconds(5));
+        session.EnterWorld(0, TimeSpan.FromSeconds(5));
+
+        Assert.Equal(WorldSession.State.InWorld, session.CurrentState);
+        Assert.Equal(0, errors);
+    }
+
+    [Fact]
+    public void PausedEnterWorld_IgnoresNumErrorsSentinelBeforeServerReady()
+    {
+        var transport = new FakeAceTransport
+        {
+            AutoReplyServerReady = false,
+        };
+        using var session = new WorldSession(
+            new IPEndPoint(IPAddress.Loopback, 9000),
+            transport);
+        int errors = 0;
+        session.CharacterErrorReceived += _ => errors++;
+        ConfigureSentinelThenServerReady(transport);
+
+        session.Connect(
+            FakeAceTransport.DefaultAccountName,
+            "testpassword",
+            TimeSpan.FromSeconds(5));
+        session.StartCharacterSelectionReceive();
+        session.EnterWorld(0, TimeSpan.FromSeconds(5));
+
+        Assert.Equal(WorldSession.State.InWorld, session.CurrentState);
+        Assert.Equal(0, errors);
+    }
+
    private static WorldSession CreateSession() =>
        new(
            new IPEndPoint(IPAddress.Loopback, 9000),
            new NullTransport());

+    private static void ConfigureSentinelThenServerReady(
+        FakeAceTransport transport)
+    {
+        transport.Model.MessageDispatched += body =>
+        {
+            if (BinaryPrimitives.ReadUInt32LittleEndian(body)
+                != CharacterEnterWorld.EnterWorldRequestOpcode)
+            {
+                return;
+            }
+
+            transport.Model.EnqueueGameMessage(
+                BuildCharacterError(CharacterError.Code.NumErrors),
+                GameMessageGroup.UIQueue);
+            transport.Model.EnqueueGameMessage(
+                BitConverter.GetBytes(0xF7DFu),
+                GameMessageGroup.UIQueue);
+        };
+    }
+
    private static byte[] BuildRoster(uint secondsGreyedOut)
    {
        var writer = new PacketWriter(96);
--- a/tests/AcDream.Runtime.Tests/Session/LiveSessionControllerTests.cs
+++ b/tests/AcDream.Runtime.Tests/Session/LiveSessionControllerTests.cs
@ -8,6 +8,18 @@ namespace AcDream.Runtime.Tests.Session;

 public sealed class LiveSessionControllerTests
 {
+    private sealed class ManualTimeProvider : TimeProvider
+    {
+        private long _timestamp;
+
+        public override long TimestampFrequency => TimeSpan.TicksPerSecond;
+
+        public override long GetTimestamp() => _timestamp;
+
+        public void Advance(TimeSpan elapsed) =>
+            _timestamp += elapsed.Ticks;
+    }
+
    private sealed class TestTransport : IWorldSessionTransport
    {
        public void Send(ReadOnlySpan<byte> datagram) { }
@ -466,7 +478,8 @@ public sealed class LiveSessionControllerTests
        var calls = new List<string>();
        var operations = new TestOperations(calls);
        var host = new TestHost(calls);
-        var controller = new LiveSessionController(operations);
+        var time = new ManualTimeProvider();
+        var controller = new LiveSessionController(operations, time);
        Assert.Equal(
            LiveSessionStartStatus.AwaitingCharacterSelection,
            controller.Start(LiveOptions(awaitSelection: true), host).Status);
@ -499,8 +512,21 @@ public sealed class LiveSessionControllerTests
        // ACE may send no response for an unknown guid. The synchronous
        // command has already returned and does not gate later commands.
        Assert.True(controller.Highlight(generation, 0x50000002u).Accepted);
+        Assert.Equal(
+            RuntimeCommandStatus.Rejected,
+            controller.Restore(generation).Status);
        controller.Tick();
        Assert.Equal(1, operations.TickCount);
+        time.Advance(RuntimeCharacterSelectionState.RestoreCorrelationTimeout);
+        controller.Tick();
+        Assert.Equal(
+            RuntimeCharacterSelectionOperation.None,
+            controller.CharacterSelection.Snapshot.Operation);
+        Assert.True(controller.CharacterSelection.Snapshot.Buttons.CanRestore);
+        Assert.True(controller.Restore(generation).Accepted);
+        Assert.Equal(
+            [0x50000001u, 0x50000002u],
+            operations.RestoreRequests);
        Assert.False(controller.IsInWorld);
    }

--- a/tests/AcDream.Runtime.Tests/Session/RuntimeCharacterSelectionStateTests.cs
+++ b/tests/AcDream.Runtime.Tests/Session/RuntimeCharacterSelectionStateTests.cs
@ -23,6 +23,18 @@ public sealed class RuntimeCharacterSelectionStateTests
            onDelta(delta);
    }

+    private sealed class ManualTimeProvider : TimeProvider
+    {
+        private long _timestamp;
+
+        public override long TimestampFrequency => TimeSpan.TicksPerSecond;
+
+        public override long GetTimestamp() => _timestamp;
+
+        public void Advance(TimeSpan elapsed) =>
+            _timestamp += elapsed.Ticks;
+    }
+
    [Fact]
    public void ApplyRoster_PortsRetailOrderFallbackAndDisabledButtonMatrix()
    {
@ -154,9 +166,10 @@ public sealed class RuntimeCharacterSelectionStateTests
    }

    [Fact]
-    public void RestoreResponse_FromSupersededRequestCannotCompleteTheCurrentRequest()
+    public void DelayedFlagOnlyRestoreResponse_CannotCompleteNewerRequest()
    {
-        using var state = new RuntimeCharacterSelectionState();
+        var time = new ManualTimeProvider();
+        using var state = new RuntimeCharacterSelectionState(time);
        state.Begin(new RuntimeGenerationToken(12));
        state.ApplyRoster(Roster(
            new(0x50000001u, "First", 1u),
@ -164,18 +177,55 @@ public sealed class RuntimeCharacterSelectionStateTests

        Assert.True(state.TryBeginRestore(out _));
        Assert.True(state.TryHighlight(0x50000002u));
+        Assert.False(state.TryBeginRestore(out _));
+        time.Advance(RuntimeCharacterSelectionState.RestoreCorrelationTimeout);
+        Assert.True(state.SweepRestoreCorrelation());
        Assert.True(state.TryBeginRestore(out _));
+        long revision = state.Snapshot.Revision;
        state.ApplyRestore(new CharacterRestore.Parsed(
-            1u,
-            0x50000001u,
-            "First",
-            0u));
+            VerificationFlag: 2u,
+            Guid: null,
+            Name: null,
+            SecondsGreyedOut: null));

        Assert.Equal(
            RuntimeCharacterSelectionOperation.RestoreRequested,
            state.Snapshot.Operation);
+        Assert.Equal(0x50000002u, state.Snapshot.LastRestoreRequestedCharacterId);
+        Assert.Equal(revision, state.Snapshot.Revision);
        Assert.True(state.View.TryGet(0x50000001u, out var first));
        Assert.True(first.IsPendingDelete);
+        Assert.True(state.View.TryGet(0x50000002u, out var second));
+        Assert.True(second.IsPendingDelete);
+        Assert.False(state.Snapshot.Buttons.CanRestore);
+    }
+
+    [Fact]
+    public void NoReplyRestoreTimeout_ReleasesOnlyRestoreGate()
+    {
+        var time = new ManualTimeProvider();
+        using var state = new RuntimeCharacterSelectionState(time);
+        state.Begin(new RuntimeGenerationToken(13));
+        state.ApplyRoster(Roster(
+            new(0x50000001u, "Pending", 1u),
+            new(0x50000002u, "Ready", 0u)));
+
+        Assert.True(state.TryHighlight(0x50000001u));
+        Assert.True(state.TryBeginRestore(out _));
+        Assert.False(state.Snapshot.Buttons.CanRestore);
+        Assert.True(state.TryHighlight(0x50000002u));
+        Assert.True(state.BeginEnter(out var ready));
+        Assert.Equal(0x50000002u, ready.CharacterId);
+
+        state.ReturnToSelection();
+        Assert.True(state.TryHighlight(0x50000001u));
+        Assert.True(state.TryBeginRestore(out _));
+        time.Advance(RuntimeCharacterSelectionState.RestoreCorrelationTimeout);
+        Assert.True(state.SweepRestoreCorrelation());
+        Assert.Equal(
+            RuntimeCharacterSelectionOperation.None,
+            state.Snapshot.Operation);
+        Assert.True(state.Snapshot.Buttons.CanRestore);
    }

    [Fact]