fix: distributed authority client synchronization issues

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -18,6 +18,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed issue when using a distributed authority network topology and many clients attempt to connect simultaneously the session owner could max-out the maximum in-flight reliable messages allowed, start dropping packets, and some of the connecting clients would fail to fully synchronize. (#3350)
+- Fixed issue when using a distributed authority network topology and scene management was disabled clients would not be able to spawn any new network prefab instances until synchronization was complete. (#3350)
 - Fixed issue where the `MaximumInterpolationTime` could not be modified from within the inspector view or runtime. (#3337)
 - Fixed `ChangeOwnership` changing ownership to clients that are not observers. This also happened with automated object distribution. (#3323)
 - Fixed issue where `AnticipatedNetworkVariable` previous value returned by `AnticipatedNetworkVariable.OnAuthoritativeValueChanged` is updated correctly on the non-authoritative side. (#3306)

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

@@ -73,6 +73,8 @@ public void Handle(ref NetworkContext context)
                     /// DANGO-TODO: Determine if this needs to be removed once the service handles object distribution
                     networkManager.RedistributeToClients = true;
                     networkManager.ClientsToRedistribute.Add(ClientId);
+
+                    // TODO: We need a client synchronized message or something like that here
                 }
             }
         }

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

@@ -270,6 +270,13 @@ public void Handle(ref NetworkContext context)
             // Only if scene management is disabled do we handle NetworkObject synchronization at this point
             if (!networkManager.NetworkConfig.EnableSceneManagement)
             {
+                /// Mark the client being connected before running through the spawning synchronization so we
+                /// can assure that if a user attempts to spawn something when an already spawned NetworkObject
+                /// is spawned (during the initial synchronization just below) it will not error out complaining
+                /// about the player not being connected.
+                /// The check for this is done within <see cref="NetworkObject.SpawnInternal(bool, ulong, bool)"/>
+                networkManager.IsConnectedClient = true;
+
                 // DANGO-TODO: This is a temporary fix for no DA CMB scene event handling.
                 // We will either use this same concept or provide some way for the CMB state plugin to handle it.
                 if (networkManager.DistributedAuthorityMode && networkManager.LocalClient.IsSessionOwner)
@@ -292,9 +299,6 @@ public void Handle(ref NetworkContext context)
                     NetworkObject.AddSceneObject(sceneObject, m_ReceivedSceneObjectData, networkManager);
                 }
 
-                // Mark the client being connected
-                networkManager.IsConnectedClient = true;
-
                 if (networkManager.AutoSpawnPlayerPrefabClientSide)
                 {
                     networkManager.ConnectionManager.CreateAndSpawnPlayer(OwnerClientId);
@@ -315,14 +319,14 @@ public void Handle(ref NetworkContext context)
                 if (networkManager.DistributedAuthorityMode && networkManager.CMBServiceConnection && networkManager.LocalClient.IsSessionOwner && networkManager.NetworkConfig.EnableSceneManagement)
                 {
                     // Mark the client being connected
-                    networkManager.IsConnectedClient = true;
+                    networkManager.IsConnectedClient = networkManager.ConnectionManager.LocalClient.IsApproved;
 
                     networkManager.SceneManager.IsRestoringSession = GetIsSessionRestor();
 
                     if (!networkManager.SceneManager.IsRestoringSession)
                     {
                         // Synchronize the service with the initial session owner's loaded scenes and spawned objects
-                        networkManager.SceneManager.SynchronizeNetworkObjects(NetworkManager.ServerClientId);
+                        networkManager.SceneManager.SynchronizeNetworkObjects(NetworkManager.ServerClientId, true);
 
                         // Spawn any in-scene placed NetworkObjects
                         networkManager.SpawnManager.ServerSpawnSceneObjectsOnStartSweep();
@@ -334,9 +338,9 @@ public void Handle(ref NetworkContext context)
                         }
 
                         // Synchronize the service with the initial session owner's loaded scenes and spawned objects
-                        networkManager.SceneManager.SynchronizeNetworkObjects(NetworkManager.ServerClientId);
+                        networkManager.SceneManager.SynchronizeNetworkObjects(NetworkManager.ServerClientId, true);
 
-                        // With scene management enabled and since the session owner doesn't send a Synchronize scene event synchronize itself,
+                        // With scene management enabled and since the session owner doesn't send a scene event synchronize to itself,
                         // we need to notify the session owner that everything should be synchronized/spawned at this time.
                         networkManager.SpawnManager.NotifyNetworkObjectsSynchronized();
 

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

@@ -1986,6 +1986,8 @@ private void OnClientLoadedScene(uint sceneEventId, Scene scene)
         /// </summary>
         internal Func<Scene, bool> ExcludeSceneFromSychronization;
 
+        internal List<ulong> ClientConnectionQueue = new List<ulong>();
+
         /// <summary>
         /// Server Side:
         /// This is used for players that have just had their connection approved and will assure they are synchronized
@@ -1994,8 +1996,29 @@ private void OnClientLoadedScene(uint sceneEventId, Scene scene)
         /// synchronized.
         /// </summary>
         /// <param name="clientId">newly joined client identifier</param>
-        internal void SynchronizeNetworkObjects(ulong clientId)
+        internal void SynchronizeNetworkObjects(ulong clientId, bool synchronizingService = false)
         {
+            // If we are connected to a live service hosted session and we are not doing the initial synchronization for the service...
+            if (NetworkManager.CMBServiceConnection && !synchronizingService)
+            {
+                // then as long as this is a newly connecting client add it to the connecting client queue.
+                // Otherwise, if this is not a newly connecting client (i.e. it is already in the queue), then go ahead and synchronize
+                // that client.
+                if (!ClientConnectionQueue.Contains(clientId))
+                {
+                    ClientConnectionQueue.Add(clientId);
+                    // If we are already synchronizing one or more clients, then add this client to the queue and exit.
+                    if (ClientConnectionQueue.Count > 1)
+                    {
+                        if (NetworkManager.LogLevel <= LogLevel.Developer)
+                        {
+                            Debug.Log($"Deferring Client-{clientId} synchrnization.");
+                        }
+                        return;
+                    }
+                }
+            }
+
             // Update the clients
             NetworkManager.SpawnManager.UpdateObservedNetworkObjects(clientId);
 
@@ -2623,6 +2646,44 @@ private void HandleSessionOwnerEvent(uint sceneEventId, ulong clientId)
                         // DANGO-EXP TODO: Remove this once service distributes objects
                         NetworkManager.SpawnManager.DistributeNetworkObjects(clientId);
                         EndSceneEvent(sceneEventId);
+
+                        // Exit early if not a distributed authority session or this is a DAHost
+                        // (DAHost has a unique connection per client, so no need to queue synchronization)
+                        if (!NetworkManager.DistributedAuthorityMode || NetworkManager.DAHost)
+                        {
+                            return;
+                        }
+
+                        // Otherwise, this is a session owner that could have pending clients to synchronize
+                        if (NetworkManager.DistributedAuthorityMode && NetworkManager.CMBServiceConnection)
+                        {
+                            // 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]}...");
+                                }
+                                SynchronizeNetworkObjects(ClientConnectionQueue[0]);
+                            }
+                        }
                         break;
                     }
                 default:

com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs

@@ -1815,7 +1815,7 @@ internal void Shutdown()
         /// </summary>
         /// <param name="objectByTypeAndOwner">the table to populate</param>
         /// <param name="objectTypeCount">the total number of the specific object type to distribute</param>
-        internal void GetObjectDistribution(ref Dictionary<uint, Dictionary<ulong, List<NetworkObject>>> objectByTypeAndOwner, ref Dictionary<uint, int> objectTypeCount)
+        internal void GetObjectDistribution(ulong clientId, ref Dictionary<uint, Dictionary<ulong, List<NetworkObject>>> objectByTypeAndOwner, ref Dictionary<uint, int> objectTypeCount)
         {
             // DANGO-TODO-MVP: Remove this once the service handles object distribution
             var onlyIncludeOwnedObjects = NetworkManager.CMBServiceConnection;
@@ -1844,6 +1844,11 @@ internal void GetObjectDistribution(ref Dictionary<uint, Dictionary<ulong, List<
                 // At this point we only allow things marked with the distributable permission and is not locked to be distributed
                 if (networkObject.IsOwnershipDistributable && !networkObject.IsOwnershipLocked)
                 {
+                    // Don't include anything that is not visible to the new client
+                    if (!networkObject.Observers.Contains(clientId))
+                    {
+                        continue;
+                    }
 
                     // We have to check if it is an in-scene placed NetworkObject and if it is get the source prefab asset GlobalObjectIdHash value of the in-scene placed instance
                     // since all in-scene placed instances use unique GlobalObjectIdHash values.
@@ -1913,7 +1918,7 @@ internal void DistributeNetworkObjects(ulong clientId)
             var objectTypeCount = new Dictionary<uint, int>();
 
             // Get all spawned objects by type and then by client owner that are spawned and can be distributed
-            GetObjectDistribution(ref distributedNetworkObjects, ref objectTypeCount);
+            GetObjectDistribution(clientId, ref distributedNetworkObjects, ref objectTypeCount);
 
             var clientCount = NetworkManager.ConnectedClientsIds.Count;
 
@@ -1951,7 +1956,6 @@ internal void DistributeNetworkObjects(ulong clientId)
 
                     var maxDistributeCount = Mathf.Max(ownerList.Value.Count - objPerClient, 1);
                     var distributed = 0;
-
                     // For now when we have more players then distributed NetworkObjects that
                     // a specific client owns, just assign half of the NetworkObjects to the new client
                     var offsetCount = Mathf.Max((int)Math.Round((float)(ownerList.Value.Count / objPerClient)), 1);
@@ -1964,11 +1968,6 @@ internal void DistributeNetworkObjects(ulong clientId)
                     {
                         if ((i % offsetCount) == 0)
                         {
-                            while (!ownerList.Value[i].Observers.Contains(clientId))
-                            {
-                                i++;
-                            }
-
                             var children = ownerList.Value[i].GetComponentsInChildren<NetworkObject>();
                             // Since the ownerList.Value[i] has to be distributable, then transfer all child NetworkObjects
                             // with the same owner clientId and are marked as distributable also to the same client to keep
@@ -2011,7 +2010,7 @@ internal void DistributeNetworkObjects(ulong clientId)
                 var builder = new StringBuilder();
                 distributedNetworkObjects.Clear();
                 objectTypeCount.Clear();
-                GetObjectDistribution(ref distributedNetworkObjects, ref objectTypeCount);
+                GetObjectDistribution(clientId, ref distributedNetworkObjects, ref objectTypeCount);
                 builder.AppendLine($"Client Relative Distributed Object Count: (distribution follows)");
                 // Cycle through each prefab type
                 foreach (var objectTypeEntry in distributedNetworkObjects)
@@ -2026,7 +2025,6 @@ internal void DistributeNetworkObjects(ulong clientId)
                 }
                 Debug.Log(builder.ToString());
             }
-
         }
 
         internal struct DeferredDespawnObject

com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/SpawnDuringSynchronizationTests.cs

@@ -0,0 +1,147 @@
+using System.Collections;
+using System.Collections.Generic;
+using System.Text;
+using NUnit.Framework;
+using Unity.Netcode.TestHelpers.Runtime;
+using UnityEngine;
+using UnityEngine.TestTools;
+
+namespace Unity.Netcode.RuntimeTests
+{
+    [TestFixture(NetworkBehaviourSpawnTimes.OnNetworkSpawn, SceneManagement.Enabled)]
+    [TestFixture(NetworkBehaviourSpawnTimes.OnNetworkPostSpawn, SceneManagement.Enabled)]
+    [TestFixture(NetworkBehaviourSpawnTimes.OnSynchronized, SceneManagement.Enabled)]
+    [TestFixture(NetworkBehaviourSpawnTimes.OnNetworkSpawn, SceneManagement.Disabled)]
+    [TestFixture(NetworkBehaviourSpawnTimes.OnNetworkPostSpawn, SceneManagement.Disabled)]
+    [TestFixture(NetworkBehaviourSpawnTimes.OnSynchronized, SceneManagement.Disabled)]
+    internal class SpawnDuringSynchronizationTests : NetcodeIntegrationTest
+    {
+        protected override int NumberOfClients => 2;
+
+        private List<NetworkManager> m_AllNetworkManagers = new List<NetworkManager>();
+        private StringBuilder m_ErrorLog = new StringBuilder();
+
+        private NetworkBehaviourSpawnTimes m_SpawnTime;
+        private bool m_EnableSceneManagement;
+        internal enum NetworkBehaviourSpawnTimes
+        {
+            OnNetworkSpawn,
+            OnNetworkPostSpawn,
+            OnSynchronized
+        }
+
+        internal enum SceneManagement
+        {
+            Enabled,
+            Disabled,
+        }
+
+        internal class SpawnTestComponent : NetworkBehaviour
+        {
+            public GameObject PrefabToSpawn;
+            public NetworkObject SpawnedObject { get; private set; }
+            public NetworkBehaviourSpawnTimes SpawnTime;
+
+            private void SpawnObject(NetworkBehaviourSpawnTimes spawnTime)
+            {
+                if (IsOwner && SpawnTime == spawnTime)
+                {
+                    SpawnedObject = NetworkObject.InstantiateAndSpawn(PrefabToSpawn, NetworkManager, OwnerClientId);
+                }
+            }
+
+            public override void OnNetworkSpawn()
+            {
+                SpawnObject(NetworkBehaviourSpawnTimes.OnNetworkSpawn);
+                base.OnNetworkSpawn();
+            }
+
+            protected override void OnNetworkPostSpawn()
+            {
+                SpawnObject(NetworkBehaviourSpawnTimes.OnNetworkPostSpawn);
+                base.OnNetworkPostSpawn();
+            }
+
+            protected override void OnNetworkSessionSynchronized()
+            {
+                SpawnObject(NetworkBehaviourSpawnTimes.OnSynchronized);
+                base.OnNetworkSessionSynchronized();
+            }
+        }
+
+
+        public SpawnDuringSynchronizationTests(NetworkBehaviourSpawnTimes spawnTime, SceneManagement sceneManagement) : base(NetworkTopologyTypes.DistributedAuthority, HostOrServer.DAHost)
+        {
+            m_SpawnTime = spawnTime;
+            m_EnableSceneManagement = sceneManagement == SceneManagement.Enabled;
+        }
+
+        protected override void OnCreatePlayerPrefab()
+        {
+            var spawnTestComponent = m_PlayerPrefab.AddComponent<SpawnTestComponent>();
+            spawnTestComponent.SpawnTime = m_SpawnTime;
+            spawnTestComponent.NetworkObject.SetOwnershipStatus(NetworkObject.OwnershipStatus.None);
+            base.OnCreatePlayerPrefab();
+        }
+
+        protected override void OnServerAndClientsCreated()
+        {
+            var spawnTestComponent = m_PlayerPrefab.GetComponent<SpawnTestComponent>();
+            spawnTestComponent.PrefabToSpawn = CreateNetworkObjectPrefab("ObjToSpawn");
+            spawnTestComponent.PrefabToSpawn.GetComponent<NetworkObject>().SetOwnershipStatus(NetworkObject.OwnershipStatus.Transferable);
+            if (!UseCMBService())
+            {
+                m_ServerNetworkManager.NetworkConfig.EnableSceneManagement = m_EnableSceneManagement;
+            }
+            foreach (var networkManager in m_ClientNetworkManagers)
+            {
+                networkManager.NetworkConfig.EnableSceneManagement = m_EnableSceneManagement;
+            }
+            base.OnServerAndClientsCreated();
+        }
+
+        private bool AllClientsSpawnedObject()
+        {
+            m_ErrorLog.Clear();
+            var spawnTestComponent = (SpawnTestComponent)null;
+            foreach (var networkManager in m_AllNetworkManagers)
+            {
+                spawnTestComponent = networkManager.LocalClient.PlayerObject.GetComponent<SpawnTestComponent>();
+                if (spawnTestComponent.SpawnedObject == null || !spawnTestComponent.SpawnedObject.IsSpawned)
+                {
+                    m_ErrorLog.AppendLine($"{networkManager.name}'s player failed to spawn the network prefab!");
+                    break;
+                }
+                foreach (var networkManagerToCheck in m_AllNetworkManagers)
+                {
+                    if (networkManagerToCheck == networkManager)
+                    {
+                        continue;
+                    }
+                    if (!networkManagerToCheck.SpawnManager.SpawnedObjects.ContainsKey(spawnTestComponent.NetworkObjectId))
+                    {
+                        m_ErrorLog.AppendLine($"{networkManager.name}'s player failed to spawn the network prefab!");
+                    }
+                }
+            }
+            return m_ErrorLog.Length == 0;
+        }
+
+        /// <summary>
+        /// Validates that a client can spawn network prefabs during OnNetworkSpawn, OnNetworkPostSpawn, and OnNetworkSessionSynchronized.
+        /// </summary>
+        [UnityTest]
+        public IEnumerator SpawnDuringSynchronization()
+        {
+            m_AllNetworkManagers.Clear();
+            m_AllNetworkManagers.AddRange(m_ClientNetworkManagers);
+            if (!UseCMBService())
+            {
+                m_AllNetworkManagers.Add(m_ServerNetworkManager);
+            }
+
+            yield return WaitForConditionOrTimeOut(AllClientsSpawnedObject);
+            AssertOnTimeout(m_ErrorLog.ToString());
+        }
+    }
+}