fix: defer ObjectSceneChanged events to occur after client has finished synchronizing (#2502)

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -41,6 +41,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed issue where during client synchronization the synchronizing client could receive a ObjectSceneChanged message before the client-side NetworkObject instance had been instantiated and spawned. (#2502)
 - Fixed issue where `NetworkAnimator` was building client RPC parameters to exclude the host from sending itself messages but was not including it in the ClientRpc parameters. (#2492)
 - Fixed issue where `NetworkAnimator` was not properly detecting and synchronizing cross fade initiated transitions. (#2481)
 - Fixed issue where `NetworkAnimator` was not properly synchronizing animation state updates. (#2481)

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

@@ -2058,6 +2058,11 @@ private void HandleClientSceneEvent(uint sceneEventId)
                                 ClientId = m_NetworkManager.LocalClientId, // Client sent this to the server
                             });
 
+                            // Process any SceneEventType.ObjectSceneChanged messages that
+                            // were deferred while synchronizing and migrate the associated
+                            // NetworkObjects to their newly assigned scenes.
+                            sceneEventData.ProcessDeferredObjectSceneChangedEvents();
+
                             // Only if PostSynchronizationSceneUnloading is set and we are running in client synchronization
                             // mode additive do we unload any remaining scene that was not synchronized (otherwise any loaded
                             // scene not synchronized by the server will remain loaded)
@@ -2412,6 +2417,7 @@ internal void NotifyNetworkObjectSceneChanged(NetworkObject networkObject)
             }
 
             // Don't notify if a scene event is in progress
+            // Note: This does not apply to SceneEventType.Synchronize since synchronization isn't a global connected client event.
             foreach (var sceneEventEntry in SceneEventProgressTracking)
             {
                 if (!sceneEventEntry.Value.HasTimedOut() && sceneEventEntry.Value.Status == SceneEventProgressStatus.Started)
@@ -2430,8 +2436,10 @@ internal void NotifyNetworkObjectSceneChanged(NetworkObject networkObject)
 
         /// <summary>
         /// Invoked by clients when processing a <see cref="SceneEventType.ObjectSceneChanged"/> event
+        /// or invoked by <see cref="SceneEventData.ProcessDeferredObjectSceneChangedEvents"/> when a client finishes
+        /// synchronization.
         /// </summary>
-        private void MigrateNetworkObjectsIntoScenes()
+        internal void MigrateNetworkObjectsIntoScenes()
         {
             try
             {
@@ -2476,5 +2484,13 @@ internal void CheckForAndSendNetworkObjectSceneChanged()
             SendSceneEventData(sceneEvent.SceneEventId, m_NetworkManager.ConnectedClientsIds.Where(c => c != NetworkManager.ServerClientId).ToArray());
             EndSceneEvent(sceneEvent.SceneEventId);
         }
+
+        // Used to handle client-side scene migration messages received while
+        // a client is synchronizing
+        internal struct DeferredObjectsMovedEvent
+        {
+            internal Dictionary<int, List<ulong>> ObjectsMigratedTable;
+        }
+        internal List<DeferredObjectsMovedEvent> DeferredObjectsMovedEvents = new List<DeferredObjectsMovedEvent>();
     }
 }

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

@@ -575,7 +575,15 @@ internal void Deserialize(FastBufferReader reader)
 
             if (SceneEventType == SceneEventType.ObjectSceneChanged)
             {
-                DeserializeObjectsMovedIntoNewScene(reader);
+                // Defer these scene event types if a client hasn't finished synchronizing
+                if (!m_NetworkManager.IsConnectedClient)
+                {
+                    DeferObjectsMovedIntoNewScene(reader);
+                }
+                else
+                {
+                    DeserializeObjectsMovedIntoNewScene(reader);
+                }
                 return;
             }
 
@@ -1036,14 +1044,94 @@ private void DeserializeObjectsMovedIntoNewScene(FastBufferReader reader)
                     reader.ReadValueSafe(out networkObjectId);
                     if (!spawnManager.SpawnedObjects.ContainsKey(networkObjectId))
                     {
-                        throw new Exception($"[Object Scene Migration] Trying to synchronize NetworkObjectId ({networkObjectId}) but it no longer exists!");
+                        NetworkLog.LogError($"[Object Scene Migration] Trying to synchronize NetworkObjectId ({networkObjectId}) but it was not spawned or no longer exists!!");
+                        continue;
                     }
+                    // Add NetworkObject scene migration to ObjectsMigratedIntoNewScene dictionary that is processed
+                    //
                     sceneManager.ObjectsMigratedIntoNewScene[sceneHandle].Add(spawnManager.SpawnedObjects[networkObjectId]);
                 }
             }
         }
 
 
+        /// <summary>
+        /// While a client is synchronizing ObjectSceneChanged messages could be received.
+        /// This defers any ObjectSceneChanged message processing to occur after the client
+        /// has completed synchronization to assure the associated NetworkObjects being
+        /// migrated to a new scene are instantiated and spawned.
+        /// </summary>
+        private void DeferObjectsMovedIntoNewScene(FastBufferReader reader)
+        {
+            var sceneManager = m_NetworkManager.SceneManager;
+            var spawnManager = m_NetworkManager.SpawnManager;
+            var numberOfScenes = 0;
+            var sceneHandle = 0;
+            var objectCount = 0;
+            var networkObjectId = (ulong)0;
+
+            var deferredObjectsMovedEvent = new NetworkSceneManager.DeferredObjectsMovedEvent()
+            {
+                ObjectsMigratedTable = new Dictionary<int, List<ulong>>()
+            };
+
+            reader.ReadValueSafe(out numberOfScenes);
+            for (int i = 0; i < numberOfScenes; i++)
+            {
+                reader.ReadValueSafe(out sceneHandle);
+                deferredObjectsMovedEvent.ObjectsMigratedTable.Add(sceneHandle, new List<ulong>());
+                reader.ReadValueSafe(out objectCount);
+                for (int j = 0; j < objectCount; j++)
+                {
+                    reader.ReadValueSafe(out networkObjectId);
+                    deferredObjectsMovedEvent.ObjectsMigratedTable[sceneHandle].Add(networkObjectId);
+                }
+            }
+            sceneManager.DeferredObjectsMovedEvents.Add(deferredObjectsMovedEvent);
+        }
+
+        internal void ProcessDeferredObjectSceneChangedEvents()
+        {
+            var sceneManager = m_NetworkManager.SceneManager;
+            var spawnManager = m_NetworkManager.SpawnManager;
+            if (sceneManager.DeferredObjectsMovedEvents.Count == 0)
+            {
+                return;
+            }
+            foreach (var objectsMovedEvent in sceneManager.DeferredObjectsMovedEvents)
+            {
+                foreach (var keyEntry in objectsMovedEvent.ObjectsMigratedTable)
+                {
+                    if (!sceneManager.ObjectsMigratedIntoNewScene.ContainsKey(keyEntry.Key))
+                    {
+                        sceneManager.ObjectsMigratedIntoNewScene.Add(keyEntry.Key, new List<NetworkObject>());
+                    }
+                    foreach (var objectId in keyEntry.Value)
+                    {
+                        if (!spawnManager.SpawnedObjects.ContainsKey(objectId))
+                        {
+                            NetworkLog.LogWarning($"[Deferred][Object Scene Migration] Trying to synchronize NetworkObjectId ({objectId}) but it was not spawned or no longer exists!");
+                            continue;
+                        }
+                        var networkObject = spawnManager.SpawnedObjects[objectId];
+                        if (!sceneManager.ObjectsMigratedIntoNewScene[keyEntry.Key].Contains(networkObject))
+                        {
+                            sceneManager.ObjectsMigratedIntoNewScene[keyEntry.Key].Add(networkObject);
+                        }
+                    }
+                }
+                objectsMovedEvent.ObjectsMigratedTable.Clear();
+            }
+
+            sceneManager.DeferredObjectsMovedEvents.Clear();
+
+            // If there are any pending objects to migrate, then migrate them
+            if (sceneManager.ObjectsMigratedIntoNewScene.Count > 0)
+            {
+                sceneManager.MigrateNetworkObjectsIntoScenes();
+            }
+        }
+
         /// <summary>
         /// Used to release the pooled network buffer
         /// </summary>

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

@@ -157,6 +157,7 @@ private bool VerifySpawnedObjectsMigrated()
             return true;
         }
 
+        private const int k_MaxObjectsToSpawn = 9;
         /// <summary>
         /// Integration test to verify that migrating NetworkObjects
         /// into different scenes (in the same frame) is synchronized
@@ -167,7 +168,7 @@ private bool VerifySpawnedObjectsMigrated()
         public IEnumerator MigrateIntoNewSceneTest()
         {
             // Spawn 9 NetworkObject instances
-            for (int i = 0; i < 9; i++)
+            for (int i = 0; i < k_MaxObjectsToSpawn; i++)
             {
                 var serverInstance = Object.Instantiate(m_TestPrefab);
                 var serverNetworkObject = serverInstance.GetComponent<NetworkObject>();
@@ -204,12 +205,36 @@ public IEnumerator MigrateIntoNewSceneTest()
             yield return WaitForConditionOrTimeOut(VerifySpawnedObjectsMigrated);
             AssertOnTimeout($"Timed out waiting for all clients to migrate all NetworkObjects into the appropriate scenes!");
 
-            // Verify that a late joining client synchronizes properly
+            // Register for the server-side client synchronization so we can send an object scene migration event at the same time
+            // the new client begins to synchronize
+            m_ServerNetworkManager.SceneManager.OnSynchronize += SceneManager_OnSynchronize;
+
+            // Verify that a late joining client synchronizes properly even while new scene migrations occur
+            // during its synchronization
             yield return CreateAndStartNewClient();
             yield return WaitForConditionOrTimeOut(VerifySpawnedObjectsMigrated);
+
             AssertOnTimeout($"[Late Joined Client] Timed out waiting for all clients to migrate all NetworkObjects into the appropriate scenes!");
         }
 
+        /// <summary>
+        /// Migrate objects into other scenes when a client begins synchronization
+        /// </summary>
+        /// <param name="clientId"></param>
+        private void SceneManager_OnSynchronize(ulong clientId)
+        {
+            var objectCount = k_MaxObjectsToSpawn - 1;
+
+            // Migrate the NetworkObjects into different scenes than they originally were migrated into
+            foreach (var scene in m_ScenesLoaded)
+            {
+                SceneManager.MoveGameObjectToScene(m_ServerSpawnedPrefabInstances[objectCount].gameObject, scene);
+                SceneManager.MoveGameObjectToScene(m_ServerSpawnedPrefabInstances[objectCount - 1].gameObject, scene);
+                SceneManager.MoveGameObjectToScene(m_ServerSpawnedPrefabInstances[objectCount - 2].gameObject, scene);
+                objectCount -= 3;
+            }
+        }
+
         /// <summary>
         /// Integration test to verify changing the currently active scene
         /// will migrate NetworkObjects with ActiveSceneSynchronization set