fix

Session owner now starts and connects ahead of the other clients.
This exposed an issue with the connection sequence and the NetworkClient not getting the approved set to true when the client finished synchronizing.
FIxed an issue where NetworkManager names were skewed by 1 during the initial start up sequence (which can make things confusing if logging its name).
Fixed an issue with invoking the ClientNetworkManagerPostStartInit a 2nd time when connected to a CMB server.
Added some additional logging within the NetworkObjectOwnershipTests.TestOwnedObjectCounts.
Added a better logical check within the ConnectionApprovedMessage.

Author: NoelStephensUnity

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

@@ -1,4 +1,5 @@
 using System.Collections.Generic;
+using System.Diagnostics;
 using Unity.Collections;
 
 namespace Unity.Netcode
@@ -273,10 +274,10 @@ public void Handle(ref NetworkContext context)
             // Stop the client-side approval timeout coroutine since we are approved.
             networkManager.ConnectionManager.StopClientApprovalCoroutine();
 
-            networkManager.ConnectionManager.ConnectedClientIds.Clear();
             foreach (var clientId in ConnectedClientIds)
             {
-                if (!networkManager.ConnectionManager.ConnectedClientIds.Contains(clientId))
+                // If there is any disconnect between the connection sequence of Ids vs ConnectedClients, then add the client.
+                if (!networkManager.ConnectionManager.ConnectedClientIds.Contains(clientId) || !networkManager.ConnectionManager.ConnectedClients.ContainsKey(clientId))
                 {
                     networkManager.ConnectionManager.AddClient(clientId);
                 }

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

@@ -2604,8 +2604,15 @@ private void HandleSessionOwnerEvent(uint sceneEventId, ulong clientId)
                                 SceneName = string.Empty,
                                 ClientId = clientId
                             });
-                            if (NetworkManager.ConnectedClients.ContainsKey(clientId))
+
+                            if (NetworkManager.DistributedAuthorityMode)
                             {
+                                // Make sure we have a NetworkClient for this synchronized client
+                                if (!NetworkManager.ConnectedClients.ContainsKey(clientId))
+                                {
+                                    NetworkManager.ConnectionManager.AddClient(clientId);
+                                }
+                                // Mark this client as being connected
                                 NetworkManager.ConnectedClients[clientId].IsConnected = true;
                             }
                         }
@@ -2619,6 +2626,14 @@ private void HandleSessionOwnerEvent(uint sceneEventId, ulong clientId)
                                 ClientId = clientId
                             });
 
+                            // Make sure we have a NetworkClient for this synchronized client
+                            if (!NetworkManager.ConnectedClients.ContainsKey(clientId))
+                            {
+                                NetworkManager.ConnectionManager.AddClient(clientId);
+                            }
+                            // Mark this client as being connected
+                            NetworkManager.ConnectedClients[clientId].IsConnected = true;
+
                             // Show any NetworkObjects that are:
                             // - Hidden from the session owner
                             // - Owned by this client

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

@@ -180,24 +180,16 @@ protected NetworkManager GetAuthorityNetworkManager()
         {
             if (m_UseCmbService)
             {
-                // TODO: Because we start the NetworkManagers back-to-back, the service can make a NetworkManager
-                // (other than the very first one) the session owner. This could lead to test instabilities and
-                // we need to add additional code during the start process that will start the very 1st instance
-                // and wait for it to be approved and assigned the session owner before we assume anything.
-                // Otherwise, any pick from the m_NetworkManagers could result in that specific instance not being
-                // the first session owner.
-                // If we haven't even started any NetworkManager, then just return the first
-                // instance until we resolve the above issue.
+                // If we haven't even started any NetworkManager, then return the first instance
+                // since it will be the session owner.
                 if (!NetcodeIntegrationTestHelpers.IsStarted)
                 {
                     return m_NetworkManagers[0];
                 }
 
                 foreach (var client in m_NetworkManagers)
                 {
-                    // See above notes.
-                    // TODO: Once the above is resolved, just check for client.LocalClient.IsSessionOwner
-                    if (!client.LocalClient.IsApproved || client.LocalClient.IsSessionOwner)
+                    if (client.LocalClient.IsSessionOwner)
                     {
                         return client;
                     }
@@ -584,15 +576,6 @@ private void CreatePlayerPrefab()
             VerboseDebug($"Exiting {nameof(CreatePlayerPrefab)}");
         }
 
-        /// <summary>
-        /// This is invoked before the server and client(s) are started.
-        /// Override this method if you want to make any adjustments to their
-        /// NetworkManager instances.
-        /// </summary>
-        protected virtual void OnServerAndClientsCreated()
-        {
-        }
-
         private void AddRemoveNetworkManager(NetworkManager networkManager, bool addNetworkManager)
         {
             var clientNetworkManagersList = new List<NetworkManager>(m_ClientNetworkManagers);
@@ -615,6 +598,78 @@ private void AddRemoveNetworkManager(NetworkManager networkManager, bool addNetw
             m_NetworkManagers = clientNetworkManagersList.ToArray();
         }
 
+        /// <summary>
+        /// This is invoked before the server and client(s) are started.
+        /// Override this method if you want to make any adjustments to their
+        /// NetworkManager instances.
+        /// </summary>
+        protected virtual void OnServerAndClientsCreated()
+        {
+        }
+
+        /// <summary>
+        /// Will create <see cref="NumberOfClients"/> number of clients.
+        /// To create a specific number of clients <see cref="CreateServerAndClients(int)"/>
+        /// </summary>
+        protected void CreateServerAndClients()
+        {
+            CreateServerAndClients(NumberOfClients);
+        }
+
+        /// <summary>
+        /// Creates the server and clients
+        /// </summary>
+        /// <param name="numberOfClients">The number of client instances to create</param>
+        protected void CreateServerAndClients(int numberOfClients)
+        {
+            VerboseDebug($"Entering {nameof(CreateServerAndClients)}");
+
+            CreatePlayerPrefab();
+
+            if (m_EnableTimeTravel)
+            {
+                m_TargetFrameRate = -1;
+            }
+
+            // If we are connecting to a CMB server we add +1 for the session owner
+            if (m_UseCmbService)
+            {
+                numberOfClients++;
+            }
+
+            // Create multiple NetworkManager instances
+            if (!NetcodeIntegrationTestHelpers.Create(numberOfClients, out NetworkManager server, out NetworkManager[] clients, m_TargetFrameRate, m_CreateServerFirst, m_EnableTimeTravel, m_UseCmbService))
+            {
+                Debug.LogError("Failed to create instances");
+                Assert.Fail("Failed to create instances");
+            }
+            m_NumberOfClients = numberOfClients;
+            m_ClientNetworkManagers = clients;
+            m_ServerNetworkManager = server;
+
+            var managers = clients.ToList();
+            if (!m_UseCmbService)
+            {
+                managers.Insert(0, m_ServerNetworkManager);
+            }
+            m_NetworkManagers = managers.ToArray();
+
+            s_DefaultWaitForTick = new WaitForSecondsRealtime(1.0f / GetAuthorityNetworkManager().NetworkConfig.TickRate);
+
+            // Set the player prefab for the server and clients
+            foreach (var manager in m_NetworkManagers)
+            {
+                manager.NetworkConfig.PlayerPrefab = m_PlayerPrefab;
+                SetDistributedAuthorityProperties(manager);
+            }
+
+            // Provides opportunity to allow child derived classes to
+            // modify the NetworkManager's configuration before starting.
+            OnServerAndClientsCreated();
+
+            VerboseDebug($"Exiting {nameof(CreateServerAndClients)}");
+        }
+
         /// <summary>
         /// CreateAndStartNewClient Only
         /// Invoked when the newly created client has been created
@@ -863,69 +918,6 @@ protected void SetTimeTravelSimulatedLatencyJitter(float jitterSeconds)
             }
         }
 
-        /// <summary>
-        /// Will create <see cref="NumberOfClients"/> number of clients.
-        /// To create a specific number of clients <see cref="CreateServerAndClients(int)"/>
-        /// </summary>
-        protected void CreateServerAndClients()
-        {
-            CreateServerAndClients(NumberOfClients);
-        }
-
-        /// <summary>
-        /// Creates the server and clients
-        /// </summary>
-        /// <param name="numberOfClients">The number of client instances to create</param>
-        protected void CreateServerAndClients(int numberOfClients)
-        {
-            VerboseDebug($"Entering {nameof(CreateServerAndClients)}");
-
-            CreatePlayerPrefab();
-
-            if (m_EnableTimeTravel)
-            {
-                m_TargetFrameRate = -1;
-            }
-
-            // If we are connecting to a CMB server we add +1 for the session owner
-            if (m_UseCmbService)
-            {
-                numberOfClients++;
-            }
-
-            // Create multiple NetworkManager instances
-            if (!NetcodeIntegrationTestHelpers.Create(numberOfClients, out NetworkManager server, out NetworkManager[] clients, m_TargetFrameRate, m_CreateServerFirst, m_EnableTimeTravel, m_UseCmbService))
-            {
-                Debug.LogError("Failed to create instances");
-                Assert.Fail("Failed to create instances");
-            }
-            m_NumberOfClients = numberOfClients;
-            m_ClientNetworkManagers = clients;
-            m_ServerNetworkManager = server;
-
-            var managers = clients.ToList();
-            if (!m_UseCmbService)
-            {
-                managers.Insert(0, m_ServerNetworkManager);
-            }
-            m_NetworkManagers = managers.ToArray();
-
-            s_DefaultWaitForTick = new WaitForSecondsRealtime(1.0f / GetAuthorityNetworkManager().NetworkConfig.TickRate);
-
-            // Set the player prefab for the server and clients
-            foreach (var manager in m_NetworkManagers)
-            {
-                manager.NetworkConfig.PlayerPrefab = m_PlayerPrefab;
-                SetDistributedAuthorityProperties(manager);
-            }
-
-            // Provides opportunity to allow child derived classes to
-            // modify the NetworkManager's configuration before starting.
-            OnServerAndClientsCreated();
-
-            VerboseDebug($"Exiting {nameof(CreateServerAndClients)}");
-        }
-
         /// <summary>
         /// Override this method and return false in order to be able
         /// to manually control when the server and clients are started.
@@ -1028,11 +1020,7 @@ protected void ClientNetworkManagerPostStartInit()
 
             if (m_UseHost)
             {
-#if UNITY_2023_1_OR_NEWER
                 var clientSideServerPlayerClones = Object.FindObjectsByType<NetworkObject>(FindObjectsSortMode.None).Where((c) => c.IsPlayerObject && c.OwnerClientId == NetworkManager.ServerClientId);
-#else
-                var clientSideServerPlayerClones = Object.FindObjectsOfType<NetworkObject>().Where((c) => c.IsPlayerObject && c.OwnerClientId == m_ServerNetworkManager.LocalClientId);
-#endif
                 foreach (var playerNetworkObject in clientSideServerPlayerClones)
                 {
                     // When the server is not the host this needs to be done
@@ -1056,6 +1044,19 @@ protected virtual bool ShouldCheckForSpawnedPlayers()
             return true;
         }
 
+        /// <summary>
+        /// Starts the session owner and awaits for it to connect before starting the remaining clients.
+        /// </summary>
+        private IEnumerator StartSessionOwner()
+        {
+            VerboseDebug("Starting session owner...");
+            NetcodeIntegrationTestHelpers.StartOneClient(m_ClientNetworkManagers[0]);
+            yield return WaitForConditionOrTimeOut(() => m_ClientNetworkManagers[0].IsConnectedClient);
+            AssertOnTimeout($"Timed out waiting for the session owner to connect to CMB Server!");
+            Assert.True(m_ClientNetworkManagers[0].LocalClient.IsSessionOwner, $"Client-{m_ClientNetworkManagers[0].LocalClientId} started session but was not set to be the session owner!");
+            VerboseDebug("Session owner connected and approved.");
+        }
+
         /// <summary>
         /// This starts the server and clients as long as <see cref="CanStartServerAndClients"/>
         /// returns true.
@@ -1066,17 +1067,21 @@ protected IEnumerator StartServerAndClients()
             {
                 VerboseDebug($"Entering {nameof(StartServerAndClients)}");
 
+                if (m_UseCmbService)
+                {
+                    VerboseDebug("Using a distributed authority CMB Server for connection.");
+                    yield return StartSessionOwner();
+                }
+
                 // Start the instances and pass in our SceneManagerInitialization action that is invoked immediately after host-server
                 // is started and after each client is started.
-
-                VerboseDebug($"Starting with useCmbService: {m_UseCmbService}");
                 if (!NetcodeIntegrationTestHelpers.Start(m_UseHost, !m_UseCmbService, m_ServerNetworkManager, m_ClientNetworkManagers))
                 {
                     Debug.LogError("Failed to start instances");
                     Assert.Fail("Failed to start instances");
                 }
 
-                // When using the CMBService, we don't have a server, so get the appropriate authority network manager
+                // Get the authority NetworkMananger (Server, Host, or Session Owner)
                 var authorityManager = GetAuthorityNetworkManager();
 
                 // When scene management is enabled, we need to re-apply the scenes populated list since we have overriden the ISceneManagerHandler
@@ -1108,13 +1113,8 @@ protected IEnumerator StartServerAndClients()
 
                     if (m_UseHost || authorityManager.IsHost)
                     {
-#if UNITY_2023_1_OR_NEWER
                         // Add the server player instance to all m_ClientSidePlayerNetworkObjects entries
                         var serverPlayerClones = Object.FindObjectsByType<NetworkObject>(FindObjectsSortMode.None).Where((c) => c.IsPlayerObject && c.OwnerClientId == authorityManager.LocalClientId);
-#else
-                        // Add the server player instance to all m_ClientSidePlayerNetworkObjects entries
-                        var serverPlayerClones = Object.FindObjectsOfType<NetworkObject>().Where((c) => c.IsPlayerObject && c.OwnerClientId == authorityManager.LocalClientId);
-#endif
                         foreach (var playerNetworkObject in serverPlayerClones)
                         {
                             if (!m_PlayerNetworkObjects.ContainsKey(playerNetworkObject.NetworkManager.LocalClientId))
@@ -1128,6 +1128,8 @@ protected IEnumerator StartServerAndClients()
                             }
                         }
                     }
+
+                    // With distributed authority, we check that all players have spawned on all NetworkManager instances
                     if (m_DistributedAuthority)
                     {
                         foreach (var networkManager in m_NetworkManagers)
@@ -1137,8 +1139,10 @@ protected IEnumerator StartServerAndClients()
                         }
                     }
 
-                    if (ShouldCheckForSpawnedPlayers())
+                    // Client-Server or DAHost
+                    if (ShouldCheckForSpawnedPlayers() && !m_UseCmbService)
                     {
+                        // Check for players being spawned on server instance
                         ClientNetworkManagerPostStartInit();
                     }
 
@@ -1698,13 +1702,12 @@ private bool CheckClientsConnected(NetworkManager[] clientsToCheck)
             }
 
             var manager = GetAuthorityNetworkManager();
-            var expectedCount = manager.IsHost ? clientsToCheck.Length + 1 : clientsToCheck.Length;
             var currentCount = manager.ConnectedClients.Count;
 
-            if (currentCount != expectedCount)
+            if (currentCount != TotalClients)
             {
                 allClientsConnected = false;
-                m_InternalErrorLog.AppendLine($"[Server-Side] Expected {expectedCount} clients to connect but only {currentCount} connected!");
+                m_InternalErrorLog.AppendLine($"[Server-Side] Expected {TotalClients} clients to connect but only {currentCount} connected!");
             }
 
             return allClientsConnected;

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

@@ -327,10 +327,13 @@ internal static NetworkManager CreateNewClient(int identifier, bool mockTranspor
         public static bool CreateNewClients(int clientCount, out NetworkManager[] clients, bool useMockTransport = false, bool useCmbService = false)
         {
             clients = new NetworkManager[clientCount];
+            // Pre-identify NetworkManager identifiers based on network topology type
+            var startCount = useCmbService ? 1 : 0;
             for (int i = 0; i < clientCount; i++)
             {
                 // Create networkManager component
-                clients[i] = CreateNewClient(i, useMockTransport, useCmbService);
+                clients[i] = CreateNewClient(startCount, useMockTransport, useCmbService);
+                startCount++;
             }
 
             NetworkManagerInstances.AddRange(clients);
@@ -546,6 +549,18 @@ public static bool Start(bool host, NetworkManager server, NetworkManager[] clie
 
             foreach (var client in clients)
             {
+                if (client.IsConnectedClient)
+                {
+                    // Skip starting the session owner
+                    if (client.DistributedAuthorityMode && client.CMBServiceConnection && client.LocalClient.IsSessionOwner)
+                    {
+                        continue;
+                    }
+                    else
+                    {
+                        throw new Exception("Client NetworkManager is already connected when starting clients!");
+                    }
+                }
                 client.StartClient();
                 hooks = new MultiInstanceHooks();
                 client.ConnectionManager.MessageManager.Hook(hooks);