fix(net): conform character entry and session shutdown

src/AcDream.Core.Net/WorldSession.cs

@@ -627,6 +627,7 @@ public sealed class WorldSession : IDisposable
     private IsaacRandom? _outboundIsaac;
     private ushort _sessionClientId;
     private ushort _sessionIteration;
+    private bool _transportNegotiated;
     private uint _clientPacketSequence;
     private uint _fragmentSequence = 1;
 
@@ -732,9 +733,6 @@ public sealed class WorldSession : IDisposable
         // in the ConnectRequest optional section. Publish it to
         // subscribers so WorldTimeService.SyncFromServer can seed the
         // client clock.
-        LastServerTimeTicks = opt.ConnectRequestServerTime;
-        ServerTimeUpdated?.Invoke(opt.ConnectRequestServerTime);
-
         byte[] serverSeedBytes = new byte[4];
         BinaryPrimitives.WriteUInt32LittleEndian(serverSeedBytes, opt.ConnectRequestServerSeed);
         byte[] clientSeedBytes = new byte[4];
@@ -746,8 +744,15 @@ public sealed class WorldSession : IDisposable
         // generation into connection-level control packets, including the
         // final disconnect. ACE currently emits iteration 1.
         _sessionIteration = cr.Header.Iteration;
+        _transportNegotiated = true;
         _clientPacketSequence = 2;
 
+        // Publish only after the receiver identity and crypto state are fully
+        // committed. A synchronous App callback may throw or request teardown;
+        // disposal must still send the negotiated Disconnect.
+        LastServerTimeTicks = opt.ConnectRequestServerTime;
+        ServerTimeUpdated?.Invoke(opt.ConnectRequestServerTime);
+
         byte[] crBody = new byte[8];
         BinaryPrimitives.WriteUInt64LittleEndian(crBody, opt.ConnectRequestCookie);
         var crHeader = new PacketHeader { Sequence = 1, Flags = PacketHeaderFlags.ConnectResponse, Id = 0 };
@@ -770,15 +775,15 @@ public sealed class WorldSession : IDisposable
     /// Returns once the server starts sending CreateObjects (at which point
     /// callers should poll <see cref="Tick"/> to stream events).
     /// </summary>
-    public void EnterWorld(string account, int characterIndex = 0, TimeSpan? timeout = null)
+    public void EnterWorld(int characterIndex = 0, TimeSpan? timeout = null)
     {
         if (Characters is null || Characters.Characters.Count == 0)
             throw new InvalidOperationException("Connect() must complete with a non-empty CharacterList");
-        if (characterIndex < 0 || characterIndex >= Characters.Characters.Count)
-            throw new ArgumentOutOfRangeException(nameof(characterIndex));
-
         var deadline = DateTime.UtcNow + (timeout ?? TimeSpan.FromSeconds(10));
-        var chosen = Characters.Characters[characterIndex];
+        EnterWorldSelection selection = SelectCharacterForEnterWorld(
+            Characters,
+            characterIndex);
+        CharacterList.Character chosen = selection.Character;
         _activeCharacterId = chosen.Id;
         Transition(State.EnteringWorld);
 
@@ -795,7 +800,10 @@ public sealed class WorldSession : IDisposable
         }
         if (!serverReady) { Transition(State.Failed); throw new TimeoutException("ServerReady not received"); }
 
-        SendGameMessage(CharacterEnterWorld.BuildEnterWorldBody(chosen.Id, account));
+        // CPlayerSystem::LogOnCharacter @ 0x0055F890 passes the account
+        // populated by CharacterSet::UnPack, not the spelling supplied to the
+        // login form. ACE validates this canonical account value.
+        SendGameMessage(selection.EnterWorldBody);
 
         // NOTE: LoginComplete used to be sent here unconditionally. That was
         // wrong — per holtburger's flow (see references/holtburger/.../client/
@@ -819,6 +827,35 @@ public sealed class WorldSession : IDisposable
         _netThread.Start();
     }
 
+    internal readonly record struct EnterWorldSelection(
+        CharacterList.Character Character,
+        byte[] EnterWorldBody);
+
+    /// <summary>
+    /// Applies retail's selected-active-character gate and binds the F657
+    /// account field to the canonical CharacterSet account. Kept pure so the
+    /// public session invariant is conformance-tested without opening a socket.
+    /// </summary>
+    internal static EnterWorldSelection SelectCharacterForEnterWorld(
+        CharacterList.Parsed characters,
+        int characterIndex)
+    {
+        ArgumentNullException.ThrowIfNull(characters);
+        if (characterIndex < 0 || characterIndex >= characters.Characters.Count)
+            throw new ArgumentOutOfRangeException(nameof(characterIndex));
+
+        CharacterList.Character chosen = characters.Characters[characterIndex];
+        if (!CharacterList.IsAvailableActiveIdentity(chosen))
+            throw new InvalidOperationException(
+                "selected character must be an active, non-greyed identity");
+
+        return new EnterWorldSelection(
+            chosen,
+            CharacterEnterWorld.BuildEnterWorldBody(
+                chosen.Id,
+                characters.AccountName));
+    }
+
     // Per-frame inbound time budget (#2 flood timeslice). On a teleport arrival ACE floods
     // a town's CreateObjects; draining them ALL in one Tick (each hydrates mesh + textures
     // under _datLock on the render thread) monopolized the update loop for ~a minute and
@@ -1951,38 +1988,38 @@ public sealed class WorldSession : IDisposable
         if (Interlocked.Exchange(ref _disposeStarted, 1) != 0)
             return;
 
-        if (CurrentState == State.InWorld)
+        SessionShutdownPlan shutdown = BuildShutdownPlan(
+            CurrentState,
+            _transportNegotiated,
+            _activeCharacterId);
+
+        Interlocked.Exchange(ref _characterLogOffConfirmed, 0);
+        ShutdownExecutionResult result = ExecuteShutdownWire(
+            shutdown,
+            _activeCharacterId,
+            _sessionClientId,
+            _sessionIteration,
+            SendGameMessage,
+            WaitForCharacterLogOffConfirmation,
+            packet => _net.Send(packet),
+            TimeSpan.FromSeconds(35));
+
+        if (result.CharacterLogOffSent)
         {
-            try
-            {
-                // Proto_UI::LogOffCharacter @ 0x00546A20 sends the active id.
-                Interlocked.Exchange(ref _characterLogOffConfirmed, 0);
-                SendGameMessage(CharacterLogOff.BuildRequestBody(_activeCharacterId));
-                Console.WriteLine(
-                    $"[session] graceful logout requested character=0x{_activeCharacterId:X8}");
-
-                if (WaitForCharacterLogOffConfirmation(TimeSpan.FromSeconds(35)))
-                    Console.WriteLine("[session] graceful logout confirmed");
-                else
-                    Console.Error.WriteLine(
-                        "[session] graceful logout confirmation timed out; disconnecting transport");
-            }
-            catch (Exception error)
-            {
+            Console.WriteLine(
+                $"[session] graceful logout requested character=0x{_activeCharacterId:X8}");
+            if (result.ConfirmationReceived)
+                Console.WriteLine("[session] graceful logout confirmed");
+            else if (result.CharacterLogOffError is null)
                 Console.Error.WriteLine(
-                    $"[session] graceful logout failed: {error.Message}");
-            }
-
-            try
-            {
-                SendTransportDisconnect();
-            }
-            catch (Exception error)
-            {
-                Console.Error.WriteLine(
-                    $"[session] transport disconnect failed: {error.Message}");
-            }
+                    "[session] graceful logout confirmation timed out; disconnecting transport");
         }
+        if (result.CharacterLogOffError is not null)
+            Console.Error.WriteLine(
+                $"[session] graceful logout failed: {result.CharacterLogOffError.Message}");
+        if (result.TransportDisconnectError is not null)
+            Console.Error.WriteLine(
+                $"[session] transport disconnect failed: {result.TransportDisconnectError.Message}");
 
         // Phase A.3: shut down the background receive thread. Cancel the
         // token → the 250ms receive timeout fires → loop exits → join.
@@ -1995,25 +2032,132 @@ public sealed class WorldSession : IDisposable
         Transition(State.Disconnected);
     }
 
-    private bool WaitForCharacterLogOffConfirmation(TimeSpan timeout)
+    internal readonly record struct SessionShutdownPlan(
+        bool RequestCharacterLogOff,
+        bool SendTransportDisconnect);
+
+    internal readonly record struct ShutdownExecutionResult(
+        bool CharacterLogOffSent,
+        bool ConfirmationReceived,
+        bool TransportDisconnectSent,
+        Exception? CharacterLogOffError,
+        Exception? TransportDisconnectError);
+
+    internal static SessionShutdownPlan BuildShutdownPlan(
+        State state,
+        bool transportNegotiated,
+        uint activeCharacterId) =>
+        new(
+            RequestCharacterLogOff:
+                transportNegotiated
+                && state == State.InWorld
+                && activeCharacterId != 0,
+            SendTransportDisconnect: transportNegotiated);
+
+    /// <summary>
+    /// Executes the retail wire order while keeping request and transport
+    /// failures independent. The delegates are the production send/wait path;
+    /// their narrow shape also makes exact packet ordering deterministic in
+    /// tests without duplicating shutdown logic.
+    /// </summary>
+    internal static ShutdownExecutionResult ExecuteShutdownWire(
+        SessionShutdownPlan plan,
+        uint activeCharacterId,
+        ushort sessionClientId,
+        ushort sessionIteration,
+        Action<byte[]> sendGameMessage,
+        Func<TimeSpan, bool> waitForConfirmation,
+        Action<byte[]> sendTransportDatagram,
+        TimeSpan confirmationTimeout)
     {
+        ArgumentNullException.ThrowIfNull(sendGameMessage);
+        ArgumentNullException.ThrowIfNull(waitForConfirmation);
+        ArgumentNullException.ThrowIfNull(sendTransportDatagram);
+
+        bool characterLogOffSent = false;
+        bool confirmationReceived = false;
+        bool transportDisconnectSent = false;
+        Exception? characterError = null;
+        Exception? transportError = null;
+
+        if (plan.RequestCharacterLogOff)
+        {
+            try
+            {
+                // Proto_UI::LogOffCharacter @ 0x00546A20 sends the active id.
+                sendGameMessage(CharacterLogOff.BuildRequestBody(activeCharacterId));
+                characterLogOffSent = true;
+                confirmationReceived = waitForConfirmation(confirmationTimeout);
+            }
+            catch (Exception error)
+            {
+                characterError = error;
+            }
+        }
+
+        if (plan.SendTransportDisconnect)
+        {
+            try
+            {
+                // ClientNet::LogOffServer @ 0x00543EF0 sends Disconnect for
+                // every negotiated ReceiverData, independent of character
+                // entry state.
+                sendTransportDatagram(TransportDisconnect.Build(
+                    sessionClientId,
+                    sessionIteration));
+                transportDisconnectSent = true;
+            }
+            catch (Exception error)
+            {
+                transportError = error;
+            }
+        }
+
+        return new ShutdownExecutionResult(
+            characterLogOffSent,
+            confirmationReceived,
+            transportDisconnectSent,
+            characterError,
+            transportError);
+    }
+
+    private bool WaitForCharacterLogOffConfirmation(TimeSpan timeout) =>
+        WaitForCharacterLogOffConfirmation(
+            _inboundQueue.Reader,
+            timeout,
+            bytes =>
+            {
+                ProcessDatagram(bytes, dispatchWorldEvents: false);
+                return Volatile.Read(ref _characterLogOffConfirmed) != 0;
+            });
+
+    internal static bool WaitForCharacterLogOffConfirmation(
+        ChannelReader<byte[]> reader,
+        TimeSpan timeout,
+        Func<byte[], bool> processAndCheckConfirmation)
+    {
+        ArgumentNullException.ThrowIfNull(reader);
+        ArgumentNullException.ThrowIfNull(processAndCheckConfirmation);
         using var timeoutSource = new CancellationTokenSource(timeout);
 
         try
         {
-            while (Volatile.Read(ref _characterLogOffConfirmed) == 0)
+            while (!timeoutSource.IsCancellationRequested)
             {
-                while (_inboundQueue.Reader.TryRead(out byte[]? bytes))
+                while (reader.TryRead(out byte[]? bytes))
                 {
-                    ProcessDatagram(bytes, dispatchWorldEvents: false);
-                    if (Volatile.Read(ref _characterLogOffConfirmed) != 0)
+                    if (timeoutSource.IsCancellationRequested)
+                        return false;
+                    if (processAndCheckConfirmation(bytes))
                         return true;
                 }
 
-                _inboundQueue.Reader.WaitToReadAsync(timeoutSource.Token)
+                bool canRead = reader.WaitToReadAsync(timeoutSource.Token)
                     .AsTask()
                     .GetAwaiter()
                     .GetResult();
+                if (!canRead)
+                    return false;
             }
         }
         catch (OperationCanceledException)
@@ -2025,14 +2169,6 @@ public sealed class WorldSession : IDisposable
             return false;
         }
 
-        return true;
-    }
-
-    private void SendTransportDisconnect()
-    {
-        byte[] disconnectPacket = TransportDisconnect.Build(
-            _sessionClientId,
-            _sessionIteration);
-        _net.Send(disconnectPacket);
+        return false;
     }
 }