fix: DA client connection flow

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -18,6 +18,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 - Fixed issue where non-authority `NetworkTransform` instances would not allow non-synchronized axis values to be updated locally. (#3471)
 - Fixed issue where invoking `NetworkObject.NetworkShow` and `NetworkObject.ChangeOwnership` consecutively within the same call stack location could result in an unnecessary change in ownership error message generated on the target client side. (#3468)
 - Fixed issue where `NetworkVariable`s on a `NetworkBehaviour` could fail to synchronize changes if one has `NetworkVariableUpdateTraits` set and is dirty but is not ready to send. (#3466)
+- Fixed issue with the Distributed Authority connection sequence with scene management enabled where the `ClientConnected` event was fired before the client was synchronized. (#3459)
 - Fixed inconsistencies in the `OnSceneEvent` callback. (#3458)
 - Fixed issues with the `NetworkBehaviour` and `NetworkVariable` length safety checks. (#3405)
 - Fixed memory leaks when domain reload is disabled. (#3427)

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

@@ -133,48 +133,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 non-authority local 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)

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ClientConnectedMessage.cs

@@ -31,7 +31,7 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int
         public void Handle(ref NetworkContext context)
         {
             var networkManager = (NetworkManager)context.SystemOwner;
-            if (ShouldSynchronize && networkManager.NetworkConfig.EnableSceneManagement && networkManager.DistributedAuthorityMode && networkManager.LocalClient.IsSessionOwner)
+            if (ShouldSynchronize && networkManager.NetworkConfig.EnableSceneManagement && networkManager.DistributedAuthorityMode && !networkManager.CMBServiceConnection && networkManager.LocalClient.IsSessionOwner)
             {
                 networkManager.SceneManager.SynchronizeNetworkObjects(ClientId);
             }

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ClientDisconnectedMessage.cs

@@ -25,6 +25,10 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int
         public void Handle(ref NetworkContext context)
         {
             var networkManager = (NetworkManager)context.SystemOwner;
+            if (networkManager.DistributedAuthorityMode && networkManager.CMBServiceConnection && networkManager.LocalClient.IsSessionOwner && networkManager.NetworkConfig.EnableSceneManagement)
+            {
+                networkManager.SceneManager.ClientConnectionQueue.Remove(ClientId);
+            }
             // All modes support removing NetworkClients
             networkManager.ConnectionManager.RemoveClient(ClientId);
             networkManager.ConnectionManager.ConnectedClientIds.Remove(ClientId);

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs

@@ -246,10 +246,30 @@ public bool Deserialize(FastBufferReader reader, ref NetworkContext context, int
         public void Handle(ref NetworkContext context)
         {
             var networkManager = (NetworkManager)context.SystemOwner;
+
+            if (networkManager.CMBServiceConnection && networkManager.LocalClient.IsSessionOwner && networkManager.NetworkConfig.EnableSceneManagement)
+            {
+                if (networkManager.LocalClientId != OwnerClientId)
+                {
+                    if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
+                    {
+                        NetworkLog.LogInfo($"[Session Owner] Received connection approved for Client-{OwnerClientId}! Synchronizing...");
+                    }
+
+                    networkManager.SceneManager.SynchronizeNetworkObjects(OwnerClientId);
+                }
+                else
+                {
+                    NetworkLog.LogWarning($"[Client-{OwnerClientId}] Receiving duplicate connection approved. Client is already connected!");
+                }
+                return;
+            }
+
             if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
             {
                 NetworkLog.LogInfo($"[Client-{OwnerClientId}] Connection approved! Synchronizing...");
             }
+
             networkManager.LocalClientId = OwnerClientId;
             networkManager.MessageManager.SetLocalClientId(networkManager.LocalClientId);
             networkManager.NetworkMetrics.SetConnectionId(networkManager.LocalClientId);

com.unity.netcode.gameobjects/Runtime/Messaging/NetworkManagerHooks.cs

@@ -98,7 +98,8 @@ public bool OnVerifyCanReceive(ulong senderId, Type messageType, FastBufferReade
                     return false;
                 }
 
-                if (m_NetworkManager.IsConnectedClient && messageType == typeof(ConnectionApprovedMessage))
+                if (m_NetworkManager.IsConnectedClient && messageType == typeof(ConnectionApprovedMessage) &&
+                    !(m_NetworkManager.CMBServiceConnection && m_NetworkManager.LocalClient.IsSessionOwner && m_NetworkManager.NetworkConfig.EnableSceneManagement))
                 {
                     if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
                     {

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

@@ -1925,6 +1925,10 @@ private void OnClientLoadedScene(uint sceneEventId, Scene scene)
         /// when many clients attempt to connect at the same time they will be
         /// handled sequentially so as to not saturate the session owner's maximum
         /// reliable messages.
+        /// DANGO-TODO: Get clients to track their synchronization status (if they haven't finished synchronizing)
+        ///     - pending clients can listen to SessionOwnerChanged messages
+        ///     - If the session owner changes, fire "some sort of event" to the service
+        ///     - service can fire ConnectionApproved at new session owner
         /// </summary>
         internal List<ulong> ClientConnectionQueue = new List<ulong>();
 
@@ -2332,37 +2336,15 @@ private void HandleClientSceneEvent(uint sceneEventId)
                             sceneEventData.SceneEventType = SceneEventType.SynchronizeComplete;
                             if (NetworkManager.DistributedAuthorityMode)
                             {
-                                if (NetworkManager.CMBServiceConnection)
+                                sceneEventData.TargetClientId = NetworkManager.CurrentSessionOwner;
+                                sceneEventData.SenderClientId = NetworkManager.LocalClientId;
+                                var message = new SceneEventMessage
                                 {
-                                    foreach (var clientId in NetworkManager.ConnectedClientsIds)
-                                    {
-                                        if (clientId == NetworkManager.LocalClientId)
-                                        {
-                                            continue;
-                                        }
-                                        sceneEventData.TargetClientId = clientId;
-                                        sceneEventData.SenderClientId = NetworkManager.LocalClientId;
-                                        var message = new SceneEventMessage
-                                        {
-                                            EventData = sceneEventData,
-                                        };
-                                        var target = NetworkManager.DAHost ? NetworkManager.CurrentSessionOwner : NetworkManager.ServerClientId;
-                                        var size = NetworkManager.ConnectionManager.SendMessage(ref message, k_DeliveryType, target);
-                                        NetworkManager.NetworkMetrics.TrackSceneEventSent(target, (uint)sceneEventData.SceneEventType, SceneNameFromHash(sceneEventData.SceneHash), size);
-                                    }
-                                }
-                                else
-                                {
-                                    sceneEventData.TargetClientId = NetworkManager.CurrentSessionOwner;
-                                    sceneEventData.SenderClientId = NetworkManager.LocalClientId;
-                                    var message = new SceneEventMessage
-                                    {
-                                        EventData = sceneEventData,
-                                    };
-                                    var target = NetworkManager.DAHost ? NetworkManager.CurrentSessionOwner : NetworkManager.ServerClientId;
-                                    var size = NetworkManager.ConnectionManager.SendMessage(ref message, k_DeliveryType, target);
-                                    NetworkManager.NetworkMetrics.TrackSceneEventSent(target, (uint)sceneEventData.SceneEventType, SceneNameFromHash(sceneEventData.SceneHash), size);
-                                }
+                                    EventData = sceneEventData,
+                                };
+                                var target = NetworkManager.DAHost ? NetworkManager.CurrentSessionOwner : NetworkManager.ServerClientId;
+                                var size = NetworkManager.ConnectionManager.SendMessage(ref message, k_DeliveryType, target);
+                                NetworkManager.NetworkMetrics.TrackSceneEventSent(target, (uint)sceneEventData.SceneEventType, SceneNameFromHash(sceneEventData.SceneHash), size);
                             }
                             else
                             {
@@ -2485,13 +2467,13 @@ 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
+                        // Notify that a client has finished synchronizing
                         OnSceneEvent?.Invoke(new SceneEvent()
                         {
                             SceneEventType = sceneEventData.SceneEventType,
-                            SceneName = string.Empty,
                             ClientId = clientId
                         });
+                        OnSynchronizeComplete?.Invoke(clientId);
 
                         // For non-authority clients in a distributed authority session, we show hidden objects,
                         // we distribute NetworkObjects, and then we end the scene event.
@@ -2511,9 +2493,6 @@ private void HandleSessionOwnerEvent(uint sceneEventId, ulong clientId)
                             return;
                         }
 
-                        // All scenes are synchronized, let the server know we are done synchronizing
-                        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.
@@ -2555,26 +2534,12 @@ private void HandleSessionOwnerEvent(uint sceneEventId, ulong clientId)
                             // Remove the client that just synchronized
                             ClientConnectionQueue.Remove(clientId);
 
-                            // If we have pending clients to synchronize, then make sure they are still connected
-                            while (ClientConnectionQueue.Count > 0)
-                            {
-                                // If the next client is no longer connected then remove it from the list
-                                if (!NetworkManager.ConnectedClientsIds.Contains(ClientConnectionQueue[0]))
-                                {
-                                    ClientConnectionQueue.RemoveAt(0);
-                                }
-                                else
-                                {
-                                    break;
-                                }
-                            }
-
                             // If we still have any pending clients waiting, then synchronize the next one
                             if (ClientConnectionQueue.Count > 0)
                             {
                                 if (NetworkManager.LogLevel <= LogLevel.Developer)
                                 {
-                                    Debug.Log($"Synchronizing Client-{ClientConnectionQueue[0]}...");
+                                    Debug.Log($"Synchronizing deferred Client-{ClientConnectionQueue[0]}...");
                                 }
                                 SynchronizeNetworkObjects(ClientConnectionQueue[0]);
                             }