Ensure a goodbye packet is sent when a client is disconnected

Source/Client/Networking/NetworkingInMemory.cs

@@ -1,5 +1,6 @@
 using System;
 using Multiplayer.Common;
+using Multiplayer.Common.Networking.Packet;
 using Verse;
 
 namespace Multiplayer.Client.Networking
@@ -46,7 +47,7 @@ protected override void SendRaw(byte[] raw, bool reliable = true)
             });
         }
 
-        protected override void OnClose()
+        protected override void OnClose(ServerDisconnectPacket? goodbye)
         {
         }
 

Source/Client/Networking/NetworkingLiteNet.cs

@@ -4,6 +4,7 @@
 using LiteNetLib;
 using Multiplayer.Client.Util;
 using Multiplayer.Common;
+using Multiplayer.Common.Networking.Packet;
 using Verse;
 
 namespace Multiplayer.Client.Networking
@@ -42,16 +43,16 @@ public static ClientLiteNetConnection Connect(string address, int port)
 
         public void Tick() => netManager.PollEvents();
 
-        public void OnDisconnect(MpDisconnectReason reason, ByteReader data)
+        public void OnDisconnect(SessionDisconnectInfo disconnectInfo)
         {
             if (State == ConnectionStateEnum.Disconnected) return;
-            ConnectionStatusListeners.TryNotifyAll_Disconnected(SessionDisconnectInfo.From(reason, data));
+            ConnectionStatusListeners.TryNotifyAll_Disconnected(disconnectInfo);
             Multiplayer.StopMultiplayer();
         }
 
-        protected override void OnClose()
+        protected override void OnClose(ServerDisconnectPacket? goodbye)
         {
-            base.OnClose();
+            base.OnClose(goodbye);
             netManager.Stop();
         }
 
@@ -82,8 +83,7 @@ public void OnPeerDisconnected(NetPeer peer, DisconnectInfo info)
                 MpDisconnectReason reason;
                 ByteReader reader;
 
-                // Fallback: should generally be handled by ClientBaseState.HandleDisconnected.
-                if (info.AdditionalData.IsNull || info.AdditionalData.AvailableBytes == 0)
+                if (info.AdditionalData.EndOfData)
                 {
                     if (info.Reason is DisconnectReason.DisconnectPeerCalled or DisconnectReason.RemoteConnectionClose)
                         reason = MpDisconnectReason.Generic;
@@ -100,7 +100,7 @@ public void OnPeerDisconnected(NetPeer peer, DisconnectInfo info)
                     reason = reader.ReadEnum<MpDisconnectReason>();
                 }
 
-                GetConnection(peer).OnDisconnect(reason, reader);
+                GetConnection(peer).OnDisconnect(SessionDisconnectInfo.From(reason, reader));
                 MpLog.Log($"Net client disconnected {info.Reason}");
             }
 

Source/Client/Networking/NetworkingSteam.cs

@@ -3,6 +3,7 @@
 using System.Diagnostics;
 using System.Linq;
 using Multiplayer.Common;
+using Multiplayer.Common.Networking.Packet;
 using Steamworks;
 using Verse;
 using Verse.Steam;
@@ -41,8 +42,12 @@ public void SendRawSteam(byte[] raw, bool reliable)
 
         public abstract void OnError(EP2PSessionError error);
 
-        protected override void OnClose()
+        protected override void OnClose(ServerDisconnectPacket? goodbye)
         {
+            if (goodbye.HasValue) Send(goodbye.Value);
+            // TODO this should probably include SteamNetworking.CloseP2PSessionWithUser to free up any leftover
+            //   resources in the Steam API. The API docs are not clear whether the connection is closed instantly, or
+            //   are the queued packets sent.
         }
 
         public override string ToString() => $"SteamP2P ({remoteId}:{username})";

Source/Client/Networking/State/ClientBaseState.cs

@@ -28,11 +28,11 @@ public void HandleTimeControl(ServerTimeControlPacket packet)
         Multiplayer.session.ProcessTimeControl();
     }
 
+    // Currently handles disconnection only for Steam connections. See comment in ConnectionBase.Close for more info.
     [TypedPacketHandler]
     public void HandleDisconnected(ServerDisconnectPacket packet)
     {
-        ConnectionStatusListeners.TryNotifyAll_Disconnected(SessionDisconnectInfo.From(packet.reason,
-            new ByteReader(packet.data)));
+        ConnectionStatusListeners.TryNotifyAll_Disconnected(SessionDisconnectInfo.From(packet));
         Multiplayer.StopMultiplayer();
     }
 }

Source/Client/Saving/ReplayConnection.cs

@@ -1,5 +1,6 @@
 using System;
 using Multiplayer.Common;
+using Multiplayer.Common.Networking.Packet;
 
 namespace Multiplayer.Client;
 
@@ -30,7 +31,7 @@ public override void HandleReceiveRaw(ByteReader data, bool reliable)
     {
     }
 
-    protected override void OnClose()
+    protected override void OnClose(ServerDisconnectPacket? goodbye)
     {
     }
 }

Source/Client/Session/SessionDisconnectInfo.cs

@@ -1,6 +1,7 @@
 using System;
 using LiteNetLib;
 using Multiplayer.Common;
+using Multiplayer.Common.Networking.Packet;
 using Verse;
 
 namespace Multiplayer.Client;
@@ -13,6 +14,9 @@ public struct SessionDisconnectInfo
     public Action specialButtonAction;
     public bool wideWindow;
 
+    public static SessionDisconnectInfo From(ServerDisconnectPacket goodbye) =>
+        From(goodbye.reason, new ByteReader(goodbye.data));
+
     public static SessionDisconnectInfo From(MpDisconnectReason reason, ByteReader reader)
     {
         var disconnectInfo = new SessionDisconnectInfo();

Source/Common/Networking/ConnectionBase.cs

@@ -266,25 +266,22 @@ private void ExecuteMessageHandler(PacketHandlerInfo handler, Packets packet, By
 
         public void Close(MpDisconnectReason reason, byte[]? data = null)
         {
+            // Ideally, we'd send the final packet here and let OnClose handle only the connection teardown.
+            // However, LiteNetLib closes connections immediately, discarding any queued packets.
+            // To ensure reliable delivery before closing, data must be sent via the Disconnect method itself.
+
             // State.IsServer check only used when disconnecting from a self-hosted local server
             if (State != ConnectionStateEnum.Disconnected && State.IsServer())
-                Send(new ServerDisconnectPacket { reason = reason, data = data ?? [] });
-            OnClose();
+                OnClose(new ServerDisconnectPacket { reason = reason, data = data ?? [] });
+            else
+                OnClose(null);
         }
 
-        protected abstract void OnClose();
+        protected abstract void OnClose(ServerDisconnectPacket? goodbye);
 
         /// Invoked after a keep alive timer arrives. Only used by the server
         public virtual void OnKeepAliveArrived(bool idMatched)
         {
         }
-
-        public static byte[] GetDisconnectBytes(MpDisconnectReason reason, byte[]? data = null)
-        {
-            var writer = new ByteWriter();
-            writer.WriteEnum(reason);
-            writer.WriteRaw(data ?? []);
-            return writer.ToArray();
-        }
     }
 }

Source/Common/Networking/LiteNetConnection.cs

@@ -1,4 +1,5 @@
 using LiteNetLib;
+using Multiplayer.Common.Networking.Packet;
 
 namespace Multiplayer.Common
 {
@@ -14,10 +15,12 @@ protected override void SendRaw(byte[] raw, bool reliable)
                 ServerLog.Error($"SendRaw() called with invalid connection state ({peer}): {peer.ConnectionState}");
         }
 
-        protected override void OnClose()
+        protected override void OnClose(ServerDisconnectPacket? goodbye)
         {
-            peer.NetManager.TriggerUpdate(); // todo: is this needed?
-            peer.NetManager.DisconnectPeer(peer);
+            if (goodbye.HasValue)
+                peer.Disconnect(goodbye.Value.Serialize().data);
+            else
+                peer.Disconnect();
         }
 
         public override void OnKeepAliveArrived(bool idMatched)

Source/Common/Networking/NetworkingLiteNet.cs

@@ -1,17 +1,17 @@
 using System.Net;
 using System.Net.Sockets;
 using LiteNetLib;
+using Multiplayer.Common.Networking.Packet;
 
 namespace Multiplayer.Common
 {
     public class MpServerNetListener(MultiplayerServer server, bool arbiter) : INetEventListener
     {
         public void OnConnectionRequest(ConnectionRequest req)
         {
-            var result = server.playerManager.OnPreConnect(req.RemoteEndPoint.Address);
-            if (result != null)
+            if (server.playerManager.OnPreConnect(req.RemoteEndPoint.Address) is { } disconnectReason)
             {
-                req.Reject(ConnectionBase.GetDisconnectBytes(result.Value));
+                req.Reject(new ServerDisconnectPacket { reason = disconnectReason }.Serialize().data);
                 return;
             }