fix: raise the client's own disconnect event when a hybrid client disconnects

UnifiedNetcodeTransport.DisconnectLocalClient only requested the N4E disconnect. N4E reports it later, after NGO's shutdown has stopped listening, so the disconnecting client never received its ClientDisconnected event. It now notifies immediately, as UnityTransport does, and ignores N4E's later event. A server-initiated disconnect removes the connection before notifying, so the shutdown it triggers does not notify a second time.

DisconnectTests and PeerDisconnectCallbackTests now run in hybrid prefab mode.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/Runtime/Transports/Unified/UnifiedNetcodeTransport.cs

@@ -221,7 +221,11 @@ private class ConnectionInfo
 
         internal void DispatchMessage(int connectionId, in FixedBytes1280 buffer)
         {
-            var connectionInfo = m_Connections[connectionId];
+            // The connection is removed on disconnect, which can be ahead of its last messages.
+            if (!m_Connections.TryGetValue(connectionId, out ConnectionInfo connectionInfo))
+            {
+                return;
+            }
 
             using var arr = FixedBytes1280.ToNativeArray(buffer);
             var reader = new DataStreamReader(arr);
@@ -374,6 +378,8 @@ private void OnClientDisconnectFromServer(Connection connection, NetcodeConnecti
                 GetDisconnectEventFromNetworkStreamDisconnectReason(connectionEvent.DisconnectReason),
                 GetDisconnectMessageFromNetworkStreamDisconnectReason(connectionEvent.DisconnectReason)
             );
+            // Removed before notifying, so the shutdown this triggers does not notify again from DisconnectLocalClient.
+            m_Connections.Remove(connectionEvent.Id.Value);
             InvokeOnTransportEvent(NetworkEvent.Disconnect, (ulong)connectionEvent.Id.Value, default, m_RealTimeProvider.RealTimeSinceStartup);
         }
 
@@ -440,7 +446,7 @@ public override void DisconnectRemoteClient(ulong clientId)
         public override void DisconnectLocalClient()
         {
             // Remove the connection 1st (the world might not be available)
-            m_Connections.Remove((int)ServerClientId);
+            var wasConnected = m_Connections.Remove((int)ServerClientId);
 
             // TODO-FIX-REVIEW-ME:
             // This was causing errors to occur upon shutdown during an integration test.
@@ -462,6 +468,13 @@ public override void DisconnectLocalClient()
             }
             m_NetworkManager.NetcodeWorld.RequestDisconnectFromServer();
 
+            // N4E reports the disconnect a frame or more later, after NGO's shutdown has stopped listening, so the
+            // client would never be notified. Notify now, as UnityTransport does, and ignore N4E's later event.
+            m_NetworkManager.NetcodeWorld.OnConnectionEvent -= OnClientConnectionEvent;
+            if (wasConnected)
+            {
+                InvokeOnTransportEvent(NetworkEvent.Disconnect, ServerClientId, default, m_RealTimeProvider.RealTimeSinceStartup);
+            }
         }
 
         public override ulong GetCurrentRtt(ulong clientId)
@@ -479,7 +492,12 @@ public override void Initialize(NetworkManager networkManager = null)
 
         public override void Shutdown()
         {
-
+            var netcodeWorld = m_NetworkManager != null ? m_NetworkManager.NetcodeWorld : null;
+            if (netcodeWorld != null)
+            {
+                netcodeWorld.OnConnectionEvent -= OnClientConnectionEvent;
+                netcodeWorld.OnConnectionEvent -= OnServerConnectionEvent;
+            }
         }
     }
 }

com.unity.netcode.gameobjects/Tests/Runtime/Connection/DisconnectTests.cs

@@ -19,8 +19,12 @@ namespace Unity.Netcode.RuntimeTests
     /// - When <see cref="OwnerPersistence.DestroyWithOwner"/> the server-side player object is destroyed
     /// - When <see cref="OwnerPersistence.DontDestroyWithOwner"/> the server-side player object ownership is transferred back to the server
     /// </summary>
-    [TestFixture(OwnerPersistence.DestroyWithOwner)]
-    [TestFixture(OwnerPersistence.DontDestroyWithOwner)]
+    [TestFixture(OwnerPersistence.DestroyWithOwner, HostOrServer.Host)]
+    [TestFixture(OwnerPersistence.DontDestroyWithOwner, HostOrServer.Host)]
+#if UNIFIED_NETCODE
+    [TestFixture(OwnerPersistence.DestroyWithOwner, HostOrServer.UnifiedHost)]
+    [TestFixture(OwnerPersistence.DontDestroyWithOwner, HostOrServer.UnifiedHost)]
+#endif
     internal class DisconnectTests : NetcodeIntegrationTest
     {
         public enum OwnerPersistence
@@ -37,6 +41,13 @@ public enum ClientDisconnectType
 
         protected override int NumberOfClients => 2;
 
+#if UNIFIED_NETCODE
+        protected override bool UseUnifiedTests()
+        {
+            return true;
+        }
+#endif
+
         private OwnerPersistence m_OwnerPersistence;
         private ClientDisconnectType m_ClientDisconnectType;
         private bool m_ClientDisconnected;
@@ -46,7 +57,7 @@ public enum ClientDisconnectType
         private ulong m_ClientId;
 
 
-        public DisconnectTests(OwnerPersistence ownerPersistence) : base(HostOrServer.Host)
+        public DisconnectTests(OwnerPersistence ownerPersistence, HostOrServer hostOrServer) : base(hostOrServer)
         {
             m_OwnerPersistence = ownerPersistence;
         }

com.unity.netcode.gameobjects/Tests/Runtime/PeerDisconnectCallbackTests.cs

@@ -22,6 +22,10 @@ namespace Unity.Netcode.RuntimeTests
     /// </summary>
     [TestFixture(HostOrServer.Server)]
     [TestFixture(HostOrServer.Host)]
+#if UNIFIED_NETCODE
+    [TestFixture(HostOrServer.UnifiedServer)]
+    [TestFixture(HostOrServer.UnifiedHost)]
+#endif
     internal class PeerDisconnectCallbackTests : NetcodeIntegrationTest
     {
 
@@ -33,6 +37,13 @@ public enum ClientDisconnectType
 
         protected override int NumberOfClients => 3;
 
+#if UNIFIED_NETCODE
+        protected override bool UseUnifiedTests()
+        {
+            return true;
+        }
+#endif
+
         private int m_ClientDisconnectCount;
         private int m_PeerDisconnectCount;