fix: DA client connection flow

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

testproject/Assets/Tests/Runtime/NetworkSceneManager/SceneManagementSynchronizationTests.cs

@@ -0,0 +1,244 @@
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Linq;
+using NUnit.Framework;
+using Unity.Collections;
+using Unity.Netcode;
+using Unity.Netcode.TestHelpers.Runtime;
+using UnityEngine.TestTools;
+
+namespace TestProject.RuntimeTests
+{
+    [TestFixture(HostOrServer.Host)]
+    [TestFixture(HostOrServer.Server)]
+    [TestFixture(HostOrServer.DAHost)]
+    public class SceneManagementSynchronizationTests : NetcodeIntegrationTest
+    {
+        protected override int NumberOfClients => 1;
+
+
+        public SceneManagementSynchronizationTests(HostOrServer hostOrServer) : base(hostOrServer)
+        {
+        }
+
+        private struct ExpectedEvent
+        {
+            public SceneEvent SceneEvent;
+            public ConnectionEventData ConnectionEvent;
+        }
+
+        private readonly Queue<ExpectedEvent> m_ExpectedEventQueue = new();
+
+        private static int s_NumEventsProcessed;
+
+        private void OnSceneEvent(SceneEvent sceneEvent)
+        {
+            VerboseDebug($"OnSceneEvent! Type: {sceneEvent.SceneEventType}.");
+            AssertEventMatchesExpectedEvent(expectedEvent =>
+                ValidateSceneEventsAreEqual(expectedEvent, sceneEvent), sceneEvent.SceneEventType);
+        }
+
+        private void OnConnectionEvent(NetworkManager manager, ConnectionEventData eventData)
+        {
+            VerboseDebug($"OnConnectionEvent! Type: {eventData.EventType} -  Client-{eventData.ClientId}");
+            AssertEventMatchesExpectedEvent(expectedEvent =>
+                ValidateConnectionEventsAreEqual(expectedEvent, eventData), eventData.EventType);
+        }
+
+        private void AssertEventMatchesExpectedEvent<T>(Action<ExpectedEvent> predicate, T eventType)
+        {
+            if (m_ExpectedEventQueue.Count > 0)
+            {
+                var expectedEvent = m_ExpectedEventQueue.Dequeue();
+                predicate(expectedEvent);
+            }
+            else
+            {
+                Assert.Fail($"Received unexpected event at index {s_NumEventsProcessed}: {eventType}");
+            }
+
+            s_NumEventsProcessed++;
+        }
+
+        private NetworkManager m_ManagerToTest;
+
+        private void SetManagerToTest(NetworkManager manager)
+        {
+            m_ManagerToTest = manager;
+            m_ManagerToTest.OnConnectionEvent += OnConnectionEvent;
+            m_ManagerToTest.SceneManager.OnSceneEvent += OnSceneEvent;
+        }
+
+        protected override void OnNewClientStarted(NetworkManager networkManager)
+        {
+            // If m_ManagerToTest isn't set at this point, it means we are testing the newly created NetworkManager
+            if (m_ManagerToTest == null)
+            {
+                SetManagerToTest(networkManager);
+            }
+            base.OnNewClientCreated(networkManager);
+        }
+
+        protected override IEnumerator OnTearDown()
+        {
+            m_ManagerToTest.OnConnectionEvent -= OnConnectionEvent;
+            m_ManagerToTest.SceneManager.OnSceneEvent -= OnSceneEvent;
+            m_ManagerToTest = null;
+            m_ExpectedEventQueue.Clear();
+            s_NumEventsProcessed = 0;
+
+            yield return base.OnTearDown();
+        }
+
+        [UnityTest]
+        public IEnumerator SynchronizationCallbacks_Authority()
+        {
+            SetManagerToTest(GetAuthorityNetworkManager());
+
+            // Calculate the expected ID of the newly connecting networkManager
+            var expectedClientId = GetNonAuthorityNetworkManager().LocalClientId + 1;
+
+            // Setup expected events
+            m_ExpectedEventQueue.Enqueue(new ExpectedEvent()
+            {
+                SceneEvent = new SceneEvent()
+                {
+                    SceneEventType = SceneEventType.Synchronize,
+                    ClientId = expectedClientId
+                },
+            });
+
+            m_ExpectedEventQueue.Enqueue(new ExpectedEvent()
+            {
+                SceneEvent = new SceneEvent()
+                {
+                    SceneEventType = SceneEventType.SynchronizeComplete,
+                    ClientId = expectedClientId,
+                },
+            });
+
+            m_ExpectedEventQueue.Enqueue(new ExpectedEvent()
+            {
+                ConnectionEvent = new ConnectionEventData()
+                {
+                    EventType = ConnectionEvent.ClientConnected,
+                    ClientId = expectedClientId,
+                }
+            });
+
+            if (m_UseHost)
+            {
+                m_ExpectedEventQueue.Enqueue(new ExpectedEvent()
+                {
+                    ConnectionEvent = new ConnectionEventData()
+                    {
+                        EventType = ConnectionEvent.PeerConnected,
+                        ClientId = expectedClientId,
+                    }
+                });
+            }
+
+            m_EnableVerboseDebug = true;
+            //////////////////////////////////////////
+            // Testing event notifications
+            yield return CreateAndStartNewClient();
+            yield return null;
+
+            Assert.IsEmpty(m_ExpectedEventQueue, "Not all expected callbacks were received");
+        }
+
+        [UnityTest]
+        public IEnumerator SynchronizationCallbacks_NonAuthority()
+        {
+            var authorityId = GetAuthorityNetworkManager().LocalClientId;
+            var peerClientId = GetNonAuthorityNetworkManager().LocalClientId;
+            var expectedClientId = peerClientId + 1;
+
+            var expectedPeerClientIds = m_UseHost ? new[] { authorityId, peerClientId } : new[] { peerClientId };
+
+            // Setup expected events
+            m_ExpectedEventQueue.Enqueue(new ExpectedEvent()
+            {
+                ConnectionEvent = new ConnectionEventData()
+                {
+                    EventType = ConnectionEvent.ClientConnected,
+                    ClientId = expectedClientId,
+                    PeerClientIds = new NativeArray<ulong>(expectedPeerClientIds.ToArray(), Allocator.Persistent),
+                }
+            });
+
+            m_ExpectedEventQueue.Enqueue(new ExpectedEvent()
+            {
+                SceneEvent = new SceneEvent()
+                {
+                    SceneEventType = SceneEventType.SynchronizeComplete,
+                    ClientId = expectedClientId,
+                },
+            });
+
+            Assert.Null(m_ManagerToTest, "m_ManagerToTest should be null as we should be testing newly created client");
+
+            //////////////////////////////////////////
+            // Testing event notifications
+
+            // CreateAndStartNewClient will configure m_ManagerToTest inside OnNewClientStarted
+            yield return CreateAndStartNewClient();
+            yield return null;
+
+            Assert.IsEmpty(m_ExpectedEventQueue, "Not all expected callbacks were received");
+        }
+
+        [UnityTest]
+        public IEnumerator LateJoiningClient_PeerCallbacks()
+        {
+            SetManagerToTest(GetNonAuthorityNetworkManager());
+            // Setup expected events
+            m_ExpectedEventQueue.Enqueue(new ExpectedEvent()
+            {
+                ConnectionEvent = new ConnectionEventData()
+                {
+                    EventType = ConnectionEvent.PeerConnected,
+                    ClientId = m_ManagerToTest.LocalClientId + 1,
+                }
+            });
+
+            //////////////////////////////////////////
+            // Testing event notifications
+            yield return CreateAndStartNewClient();
+            yield return null;
+
+            Assert.IsEmpty(m_ExpectedEventQueue, "Not all expected callbacks were received");
+        }
+
+        private static void ValidateSceneEventsAreEqual(ExpectedEvent expectedEvent, SceneEvent sceneEvent)
+        {
+            Assert.NotNull(expectedEvent.SceneEvent, $"Received unexpected scene event {sceneEvent.SceneEventType} at index {s_NumEventsProcessed}");
+            AssertField(expectedEvent.SceneEvent.SceneEventType, sceneEvent.SceneEventType, nameof(sceneEvent.SceneEventType), sceneEvent.SceneEventType);
+            AssertField(expectedEvent.SceneEvent.ClientId, sceneEvent.ClientId, nameof(sceneEvent.ClientId), sceneEvent.SceneEventType);
+        }
+
+        private static void ValidateConnectionEventsAreEqual(ExpectedEvent expectedEvent, ConnectionEventData eventData)
+        {
+            Assert.NotNull(expectedEvent.ConnectionEvent, $"Received unexpected connection event {eventData.EventType} at index {s_NumEventsProcessed}");
+            AssertField(expectedEvent.ConnectionEvent.EventType, eventData.EventType, nameof(eventData.EventType), eventData.EventType);
+            AssertField(expectedEvent.ConnectionEvent.ClientId, eventData.ClientId, nameof(eventData.ClientId), eventData.EventType);
+
+            AssertField(expectedEvent.ConnectionEvent.PeerClientIds.Length, eventData.PeerClientIds.Length, "length of PeerClientIds", eventData.EventType);
+            if (eventData.PeerClientIds.Length > 0)
+            {
+                var peerIds = eventData.PeerClientIds.ToArray();
+                foreach (var expectedClientId in expectedEvent.ConnectionEvent.PeerClientIds)
+                {
+                    Assert.Contains(expectedClientId, peerIds, "PeerClientIds does not contain all expected client IDs.");
+                }
+            }
+        }
+
+        private static void AssertField<T, TK>(T expected, T actual, string fieldName, TK type)
+        {
+            Assert.AreEqual(expected, actual, $"Failed on event {s_NumEventsProcessed} - {type}. Incorrect {fieldName}");
+        }
+
+    }
+}