fix: DA client connection flow

Author: EmandM

com.unity.netcode.gameobjects/Runtime/Configuration/SessionConfig.cs

@@ -10,9 +10,10 @@ internal class SessionConfig
         public const uint ServerDistributionCompatible = 2;
         public const uint SessionStateToken = 3;
         public const uint NetworkBehaviourSerializationSafety = 4;
+        public const uint FixConnectionFlow = 5;
 
         // The most current session version (!!!!set this when you increment!!!!!)
-        public static uint PackageSessionVersion => NetworkBehaviourSerializationSafety;
+        public static uint PackageSessionVersion => FixConnectionFlow;
 
         internal uint SessionVersion;
 

com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs

@@ -28,13 +28,15 @@ public enum ConnectionEvent
         /// On the server side, the <see cref="ConnectionEventData.ClientId"/> will be the ID of the client that just connected.
         /// </remarks>
         ClientConnected,
+
         /// <summary>
         /// This event is set on clients that are already connected to the session.
         /// </summary>
         /// <remarks>
         /// The <see cref="ConnectionEventData.ClientId"/> will be the ID of the client that just connected.
         /// </remarks>
         PeerConnected,
+
         /// <summary>
         /// This event is set on the client-side of the client that disconnected client and on the server-side.
         /// </summary>
@@ -43,6 +45,7 @@ public enum ConnectionEvent
         /// On the server side, this will be the ID of the client that disconnected.
         /// </remarks>
         ClientDisconnected,
+
         /// <summary>
         /// This event is set on clients that are already connected to the session.
         /// </summary>
@@ -133,48 +136,49 @@ internal void InvokeOnClientConnectedCallback(ulong clientId)
                 Debug.LogException(exception);
             }
 
-            if (!NetworkManager.IsServer)
+            if (NetworkManager.IsServer || NetworkManager.LocalClient.IsSessionOwner)
             {
-                var peerClientIds = new NativeArray<ulong>(Math.Max(NetworkManager.ConnectedClientsIds.Count - 1, 0), Allocator.Temp);
-                // `using var peerClientIds` or `using(peerClientIds)` renders it immutable...
-                using var sentinel = peerClientIds;
-
-                var idx = 0;
-                foreach (var peerId in NetworkManager.ConnectedClientsIds)
-                {
-                    if (peerId == NetworkManager.LocalClientId)
-                    {
-                        continue;
-                    }
-
-                    // This assures if the server has not timed out prior to the client synchronizing that it doesn't exceed the allocated peer count.
-                    if (peerClientIds.Length > idx)
-                    {
-                        peerClientIds[idx] = peerId;
-                        ++idx;
-                    }
-                }
-
                 try
                 {
-                    OnConnectionEvent?.Invoke(NetworkManager, new ConnectionEventData { ClientId = NetworkManager.LocalClientId, EventType = ConnectionEvent.ClientConnected, PeerClientIds = peerClientIds });
+                    OnConnectionEvent?.Invoke(NetworkManager, new ConnectionEventData { ClientId = clientId, EventType = ConnectionEvent.ClientConnected });
                 }
                 catch (Exception exception)
                 {
                     Debug.LogException(exception);
                 }
+
+                return;
             }
-            else
+
+            // Invoking connection event on NonAuthority client. Need to calculate PeerIds.
+            var peerClientIds = new NativeArray<ulong>(Math.Max(NetworkManager.ConnectedClientsIds.Count - 1, 0), Allocator.Temp);
+            // `using var peerClientIds` or `using(peerClientIds)` renders it immutable...
+            using var sentinel = peerClientIds;
+
+            var idx = 0;
+            foreach (var peerId in NetworkManager.ConnectedClientsIds)
             {
-                try
+                if (peerId == NetworkManager.LocalClientId)
                 {
-                    OnConnectionEvent?.Invoke(NetworkManager, new ConnectionEventData { ClientId = clientId, EventType = ConnectionEvent.ClientConnected });
+                    continue;
                 }
-                catch (Exception exception)
+
+                // This assures if the server has not timed out prior to the client synchronizing that it doesn't exceed the allocated peer count.
+                if (peerClientIds.Length > idx)
                 {
-                    Debug.LogException(exception);
+                    peerClientIds[idx] = peerId;
+                    ++idx;
                 }
             }
+
+            try
+            {
+                OnConnectionEvent?.Invoke(NetworkManager, new ConnectionEventData { ClientId = NetworkManager.LocalClientId, EventType = ConnectionEvent.ClientConnected, PeerClientIds = peerClientIds });
+            }
+            catch (Exception exception)
+            {
+                Debug.LogException(exception);
+            }
         }
 
         internal void InvokeOnClientDisconnectCallback(ulong clientId)
@@ -853,6 +857,7 @@ internal void HandleConnectionApproval(ulong ownerClientId, NetworkManager.Conne
                             };
                         }
                     }
+
                     if (!MockSkippingApproval)
                     {
                         SendMessage(ref message, NetworkDelivery.ReliableFragmentedSequenced, ownerClientId);
@@ -861,6 +866,7 @@ internal void HandleConnectionApproval(ulong ownerClientId, NetworkManager.Conne
                     {
                         NetworkLog.LogInfo("Mocking server not responding with connection approved...");
                     }
+
                     message.MessageVersions.Dispose();
                     message.ConnectedClientIds.Dispose();
                     if (MockSkippingApproval)
@@ -877,8 +883,8 @@ internal void HandleConnectionApproval(ulong ownerClientId, NetworkManager.Conne
                         {
                             InvokeOnPeerConnectedCallback(ownerClientId);
                         }
-                        NetworkManager.SpawnManager.DistributeNetworkObjects(ownerClientId);
 
+                        NetworkManager.SpawnManager.DistributeNetworkObjects(ownerClientId);
                     }
                     else // Otherwise, let NetworkSceneManager handle the initial scene and NetworkObject synchronization
                     {
@@ -921,6 +927,7 @@ internal void HandleConnectionApproval(ulong ownerClientId, NetworkManager.Conne
                 {
                     return;
                 }
+
                 // Separating this into a contained function call for potential further future separation of when this notification is sent.
                 ApprovedPlayerSpawn(ownerClientId, response.PlayerPrefabHash ?? NetworkManager.NetworkConfig.PlayerPrefab.GetComponent<NetworkObject>().GlobalObjectIdHash);
             }
@@ -935,6 +942,7 @@ internal void HandleConnectionApproval(ulong ownerClientId, NetworkManager.Conne
                     SendMessage(ref disconnectReason, NetworkDelivery.Reliable, ownerClientId);
                     MessageManager.ProcessSendQueues();
                 }
+
                 DisconnectRemoteClient(ownerClientId);
             }
         }
@@ -1009,12 +1017,14 @@ internal NetworkClient AddClient(ulong clientId)
             {
                 networkClient = new NetworkClient();
             }
+
             networkClient.SetRole(clientId == NetworkManager.ServerClientId, isClient: true, NetworkManager);
             networkClient.ClientId = clientId;
             if (!ConnectedClients.ContainsKey(clientId))
             {
                 ConnectedClients.Add(clientId, networkClient);
             }
+
             if (!ConnectedClientsList.Contains(networkClient))
             {
                 ConnectedClientsList.Add(networkClient);
@@ -1038,6 +1048,7 @@ internal NetworkClient AddClient(ulong clientId)
                     NetworkManager.MessageManager.SendMessage(ref message, NetworkDelivery.ReliableSequenced, NetworkManager.CurrentSessionOwner);
                 }
             }
+
             if (!ConnectedClientIds.Contains(clientId))
             {
                 ConnectedClientIds.Add(clientId);
@@ -1063,6 +1074,7 @@ internal NetworkClient AddClient(ulong clientId)
                     {
                         continue;
                     }
+
                     networkObject.Observers.Add(clientId);
                 }
             }
@@ -1080,6 +1092,7 @@ internal void RemoveClient(ulong clientId)
             {
                 ConnectedClientIds.Remove(clientId);
             }
+
             if (ConnectedClients.ContainsKey(clientId))
             {
                 ConnectedClientsList.Remove(ConnectedClients[clientId]);
@@ -1186,6 +1199,7 @@ internal void OnClientDisconnectFromServer(ulong clientId)
                                     // Don't destroy (prefab handler will determine this, but always notify
                                     NetworkManager.SpawnManager.DespawnObject(ownedObject, false, true);
                                 }
+
                                 NetworkManager.PrefabHandler.HandleNetworkPrefabDestroy(ownedObject);
                             }
                             else
@@ -1225,6 +1239,7 @@ internal void OnClientDisconnectFromServer(ulong clientId)
                                         break;
                                     }
                                 }
+
                                 if (EnableDistributeLogging)
                                 {
                                     Debug.Log($"[Disconnected][Client-{clientId}][NetworkObjectId-{ownedObject.NetworkObjectId} Distributed to Client-{targetOwner}");
@@ -1247,6 +1262,7 @@ internal void OnClientDisconnectFromServer(ulong clientId)
                                     {
                                         continue;
                                     }
+
                                     // If the client owner disconnected, it is ok to unlock this at this point in time.
                                     if (childObject.IsOwnershipLocked)
                                     {

com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs

@@ -2603,14 +2603,6 @@ private void HandleSessionOwnerEvent(uint sceneEventId, ulong clientId)
                         // Mark this client as being connected
                         NetworkManager.ConnectedClients[clientId].IsConnected = true;
 
-                        // Notify the local server that a client has finished synchronizing
-                        OnSceneEvent?.Invoke(new SceneEvent()
-                        {
-                            SceneEventType = sceneEventData.SceneEventType,
-                            SceneName = string.Empty,
-                            ClientId = clientId
-                        });
-
                         // For non-authority clients in a distributed authority session, we show hidden objects,
                         // we distribute NetworkObjects, and then we end the scene event.
                         if (NetworkManager.DistributedAuthorityMode && !NetworkManager.LocalClient.IsSessionOwner)
@@ -2630,14 +2622,21 @@ private void HandleSessionOwnerEvent(uint sceneEventId, ulong clientId)
                         }
 
                         // All scenes are synchronized, let the server know we are done synchronizing
+
+                        // Notify the local server that a client has finished synchronizing
+                        OnSceneEvent?.Invoke(new SceneEvent()
+                        {
+                            SceneEventType = sceneEventData.SceneEventType,
+                            ClientId = clientId
+                        });
                         OnSynchronizeComplete?.Invoke(clientId);
 
                         // At this time the client is fully synchronized with all loaded scenes and
                         // NetworkObjects and should be considered "fully connected". Send the
                         // notification that the client is connected.
                         NetworkManager.ConnectionManager.InvokeOnClientConnectedCallback(clientId);
 
-                        if (NetworkManager.IsHost)
+                        if (NetworkManager.IsHost || NetworkManager.LocalClient.IsSessionOwner)
                         {
                             NetworkManager.ConnectionManager.InvokeOnPeerConnectedCallback(clientId);
                         }

com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs

@@ -206,6 +206,12 @@ protected NetworkManager GetAuthorityNetworkManager()
         /// <returns>A <see cref="NetworkManager"/> instance that will not be the session owner</returns>
         protected NetworkManager GetNonAuthorityNetworkManager()
         {
+            // If we haven't even started any NetworkManager, then return the assumed first non-authority NetworkManager
+            if (!NetcodeIntegrationTestHelpers.IsStarted)
+            {
+                return m_UseCmbService ? m_ClientNetworkManagers[1] : m_ClientNetworkManagers[0];
+            }
+
             return m_ClientNetworkManagers.First(client => !client.LocalClient.IsSessionOwner);
         }