fix: DA client connection flow (#3459)

<!-- Replace this block with what this PR does and why. Describe what
you'd like reviewers to know, how you applied the engineering
principles, and any interesting tradeoffs made. Delete bullet points
below that don't apply, and update the changelog section as appropriate.
-->

<!-- Add short version of the JIRA ticket to the PR title (e.g. "feat:
new shiny feature [MTT-123]") -->
[MTT-12308](https://jira.unity3d.com/browse/MTT-12308)

## Changelog

- Fixed: Initial connection flow with CMB Service and scene management
enabled

## Testing and Documentation

- Includes unit tests.

<!-- Uncomment and mark items off with a * if this PR deprecates any
API:
### Deprecated API
- [ ] An `[Obsolete]` attribute was added along with a `(RemovedAfter
yyyy-mm-dd)` entry.
- [ ] An [api updater] was added.
- [ ] Deprecation of the API is explained in the CHANGELOG.
- [ ] The users can understand why this API was removed and what they
should use instead.
-->

## Backport

<!-- If this is a backport:
 - Add the following to the PR title: "\[Backport\] ..." .
 - Link to the original PR.
If this needs a backport - state this here
If a backport is not needed please provide the reason why.
If the "Backports" section is not present it will lead to a CI test
failure.
-->

Not needed.

Fixes an issue with CMB Service so this is 2.0 only functionality.

---------

Co-authored-by: unity-renovate[bot] <120015202+unity-renovate[bot]@users.noreply.github.com>
Co-authored-by: Noel Stephens <[email protected]>

Author: 3 people

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]);
                             }