fix: NetworksSenemanager order of operations and scene migration preprocess

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -15,6 +15,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 - Fixed issue where some temporary debug console logging was left in a merged PR. (#2562)
 - Fixed the "Generate Default Network Prefabs List" setting not loading correctly and always reverting to being checked. (#2545)
 - Fixed missing value on `NetworkListEvent` for `EventType.RemoveAt` events.  (#2542,#2543)
+- Fixed issue where a server would include scene migrated and then despawned NetworkObjects to a client that was being synchronized. (#2532)
 - Fixed the inspector throwing exceptions when attempting to render `NetworkVariable`s of enum types. (#2529)
 - Making a `NetworkVariable` with an `INetworkSerializable` type that doesn't meet the `new()` constraint will now create a compile-time error instead of an editor crash (#2528)
 - Fixed Multiplayer Tools package installation docs page link on the NetworkManager popup. (#2526)
@@ -26,10 +27,10 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ## Changed
 
+- Connecting clients being synchronized now switch to the server's active scene before spawning and synchronizing NetworkObjects. (#2532)
 - Updated `UnityTransport` dependency on `com.unity.transport` to 1.3.4. (#2533)
 - Improved performance of NetworkBehaviour initialization by replacing reflection when initializing NetworkVariables with compile-time code generation, which should help reduce hitching during additive scene loads. (#2522)
 
-
 ## [1.4.0] - 2023-04-10
 
 ### Added

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

@@ -337,7 +337,7 @@ public void SetClientSynchronizationMode(ref NetworkManager networkManager, Load
                 return;
             }
             else // Warn users if they are changing this after there are clients already connected and synchronized
-            if (networkManager.ConnectedClientsIds.Count > (networkManager.IsServer ? 0 : 1) && sceneManager.ClientSynchronizationMode != mode)
+            if (networkManager.ConnectedClientsIds.Count > (networkManager.IsHost ? 1 : 0) && sceneManager.ClientSynchronizationMode != mode)
             {
                 if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
                 {

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

@@ -2026,8 +2026,6 @@ private void HandleClientSceneEvent(uint sceneEventId)
                         {
                             // Include anything in the DDOL scene
                             PopulateScenePlacedObjects(DontDestroyOnLoadScene, false);
-                            // Synchronize the NetworkObjects for this scene
-                            sceneEventData.SynchronizeSceneNetworkObjects(NetworkManager);
 
                             // If needed, set the currently active scene
                             if (HashToBuildIndex.ContainsKey(sceneEventData.ActiveSceneHash))
@@ -2039,7 +2037,10 @@ private void HandleClientSceneEvent(uint sceneEventId)
                                 }
                             }
 
-                            // If needed, migrate dynamically spawned NetworkObjects to the same scene as on the server side
+                            // Spawn and Synchronize all NetworkObjects
+                            sceneEventData.SynchronizeSceneNetworkObjects(NetworkManager);
+
+                            // If needed, migrate dynamically spawned NetworkObjects to the same scene as they are on the server
                             SynchronizeNetworkObjectScene();
 
                             sceneEventData.SceneEventType = SceneEventType.SynchronizeComplete;
@@ -2468,6 +2469,9 @@ internal void MigrateNetworkObjectsIntoScenes()
             ObjectsMigratedIntoNewScene.Clear();
         }
 
+
+        private List<int> m_ScenesToRemoveFromObjectMigration = new List<int>();
+
         /// <summary>
         /// Should be invoked during PostLateUpdate just prior to the NetworkMessageManager processes its outbound message queue.
         /// </summary>
@@ -2479,6 +2483,40 @@ internal void CheckForAndSendNetworkObjectSceneChanged()
                 return;
             }
 
+            // Double check that the NetworkObjects to migrate still exist
+            m_ScenesToRemoveFromObjectMigration.Clear();
+            foreach (var sceneEntry in ObjectsMigratedIntoNewScene)
+            {
+                for (int i = sceneEntry.Value.Count - 1; i >= 0; i--)
+                {
+                    // Remove NetworkObjects that are no longer spawned
+                    if (!sceneEntry.Value[i].IsSpawned)
+                    {
+                        sceneEntry.Value.RemoveAt(i);
+                    }
+                }
+                // If the scene entry no longer has any NetworkObjects to migrate
+                // then add it to the list of scenes to be removed from the table
+                // of scenes containing NetworkObjects to migrate.
+                if (sceneEntry.Value.Count == 0)
+                {
+                    m_ScenesToRemoveFromObjectMigration.Add(sceneEntry.Key);
+                }
+            }
+
+            // Remove sceneHandle entries that no longer have any NetworkObjects remaining
+            foreach (var sceneHandle in m_ScenesToRemoveFromObjectMigration)
+            {
+                ObjectsMigratedIntoNewScene.Remove(sceneHandle);
+            }
+
+            // If there is nothing to send a migration notification for then exit
+            if (ObjectsMigratedIntoNewScene.Count == 0)
+            {
+                return;
+            }
+
+            // Some NetworkObjects still exist, send the message
             var sceneEvent = BeginSceneEvent();
             sceneEvent.SceneEventType = SceneEventType.ObjectSceneChanged;
             SendSceneEventData(sceneEvent.SceneEventId, NetworkManager.ConnectedClientsIds.Where(c => c != NetworkManager.ServerClientId).ToArray());

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

@@ -207,21 +207,51 @@ public IEnumerator MigrateIntoNewSceneTest()
 
             // 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;
+            m_ServerNetworkManager.SceneManager.OnSynchronize += MigrateObjects_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!");
+
+            // Verify that a late joining client synchronizes properly even if we migrate
+            // during its synchronization and despawn some of the NetworkObjects migrated.
+            m_ServerNetworkManager.SceneManager.OnSynchronize += MigrateAndDespawnObjects_OnSynchronize;
+            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>
+        /// Part of NetworkObject scene migration tests to verify that a NetworkObject
+        /// migrated to a scene and then despawned will be handled properly for clients
+        /// in the middle of synchronization.
+        /// </summary>
+        private void MigrateAndDespawnObjects_OnSynchronize(ulong clientId)
+        {
+            var objectCount = 0;
+            // Migrate the NetworkObjects into different scenes than they originally were migrated into
+            for (int i = m_ServerSpawnedPrefabInstances.Count - 1; i >= 0; i--)
+            {
+                var scene = m_ScenesLoaded[i % m_ScenesLoaded.Count];
+                SceneManager.MoveGameObjectToScene(m_ServerSpawnedPrefabInstances[i].gameObject, scene);
+                // De-spawn every-other object
+                if (i % 2 == 0)
+                {
+                    m_ServerSpawnedPrefabInstances[objectCount + i].Despawn();
+                    m_ServerSpawnedPrefabInstances.RemoveAt(i);
+                }
+            }
         }
 
         /// <summary>
         /// Migrate objects into other scenes when a client begins synchronization
         /// </summary>
         /// <param name="clientId"></param>
-        private void SceneManager_OnSynchronize(ulong clientId)
+        private void MigrateObjects_OnSynchronize(ulong clientId)
         {
             var objectCount = k_MaxObjectsToSpawn - 1;
 
@@ -233,6 +263,9 @@ private void SceneManager_OnSynchronize(ulong clientId)
                 SceneManager.MoveGameObjectToScene(m_ServerSpawnedPrefabInstances[objectCount - 2].gameObject, scene);
                 objectCount -= 3;
             }
+
+            // Unsubscribe to this event for this part of the test
+            m_ServerNetworkManager.SceneManager.OnSynchronize -= MigrateObjects_OnSynchronize;
         }
 
         /// <summary>