refactor: decouple PendingSoftSyncObjects from NetworkSpawnManager (#913)

* refactor

This removes NetworkSpawnManager.PendingSoftSynchObjects and replaces it with NetworkSceneManager.ScenePlacedObjects.
The primary difference between the two is that the ScenePlacedObjects is populated immediately after a level is loaded or when being approved and already within the current scene.

* style and refactor

Migrating the ScenePlacedObjects dictionary up to the rest of the NetworkkSceneManager's dictionary definitions.
Removing the PendingSoftSyncObjects dictionary from NetworkSpawnManager.

* refactor

Use the ScenePlacedObjects dictionary to generate the client(s) switch scene stream buffer.

Refactored PopulateScenePlacedObjects method to not set the IsSceneObject to true and to also check and make sure the NetworkObject instance's NetworkManager reference is the same NetworkManager instance as the NetworkSceneManager's NetworkManager instance in question in order to maintain compatibility with the MultiInstanceHelper class for Mult-Instance unit tests.

* refactor

Renaming OnSceneUnloadClient and OnSceneUnloadServer to OnClientLoadedScene and OnServerLoadedScene

* refactor and fix

Fixed some remaining checks (player prefab) as to whether the rigid body should be kinematic or not to prevent the mega spam.

During the revert changes of the NetworkTransform changes to better reflect what is in the develop branch, missed the else if.

Fixed a minor issue with the slider in the SceneTransitioning manual test to better visually reflect what the value is as well as handle the scenario if it starts at 0 boxes per second it needed to start the coroutine and if set to zero and the coroutine was running it terminates the coroutine.

* Fix

This is a fix for the NetworkObjectOnSpawnTest where it was spawning a GameObject twice that contained a component that would throw an exception if you spawned a NetworkObject.

* fix

An unintended project version change

* bug test

Checking to see if Yamato fails on this...

* test

This test verifies that "cleaning in-scene placed network objects" within the connection approved method was hiding potential bugs from other unit tests.

* fix

Removing the unit test duplicated GameObject instance and all of the associated code to verify this was how the bug was missed by TestRunner and Yamato.

* fix

reverting back to the original values for these 3 files that were mistakenly committed.

* Update com.unity.multiplayer.mlapi/Runtime/Spawning/NetworkSpawnManager.cs

Co-authored-by: M. Fatih MAR <mfatihmar@gmail.com>

Co-authored-by: M. Fatih MAR <mfatihmar@gmail.com>

Author: NoelStephensUnity

com.unity.multiplayer.mlapi/Runtime/Core/NetworkObject.cs

@@ -50,7 +50,7 @@ internal void GenerateGlobalObjectIdHash()
                 return;
             }
 
-            // do NOT regenerate GlobalObjectIdHash if Editor is transitining into or out of PlayMode
+            // do NOT regenerate GlobalObjectIdHash if Editor is transitioning into or out of PlayMode
             if (!UnityEditor.EditorApplication.isPlaying && UnityEditor.EditorApplication.isPlayingOrWillChangePlaymode)
             {
                 return;

com.unity.multiplayer.mlapi/Runtime/Messaging/InternalMessageHandler.cs

@@ -86,6 +86,7 @@ public void HandleConnectionApproved(ulong clientId, Stream stream, float receiv
 
                 void DelayedSpawnAction(Stream continuationStream)
                 {
+                    
                     using (var continuationReader = PooledNetworkReader.Get(continuationStream))
                     {
                         if (!NetworkManager.NetworkConfig.EnableSceneManagement)
@@ -94,7 +95,7 @@ void DelayedSpawnAction(Stream continuationStream)
                         }
                         else
                         {
-                            NetworkManager.SpawnManager.ClientCollectSoftSyncSceneObjectSweep(null);
+                            NetworkManager.SceneManager.PopulateScenePlacedObjects();
                         }
 
                         var objectCount = continuationReader.ReadUInt32Packed();
@@ -103,7 +104,6 @@ void DelayedSpawnAction(Stream continuationStream)
                             NetworkObject.DeserializeSceneObject(continuationStream as NetworkBuffer, continuationReader, m_NetworkManager);
                         }
 
-                        NetworkManager.SpawnManager.CleanDiffedSceneObjects();
                         NetworkManager.IsConnectedClient = true;
                         NetworkManager.InvokeOnClientConnectedCallback(NetworkManager.LocalClientId);
                     }

com.unity.multiplayer.mlapi/Runtime/SceneManagement/NetworkSceneManager.cs

@@ -79,6 +79,7 @@ public class NetworkSceneManager
         internal readonly Dictionary<string, uint> SceneNameToIndex = new Dictionary<string, uint>();
         internal readonly Dictionary<uint, string> SceneIndexToString = new Dictionary<uint, string>();
         internal readonly Dictionary<Guid, SceneSwitchProgress> SceneSwitchProgresses = new Dictionary<Guid, SceneSwitchProgress>();
+        internal readonly Dictionary<uint, NetworkObject> ScenePlacedObjects = new Dictionary<uint, NetworkObject>();
 
         private static Scene s_LastScene;
         private static string s_NextSceneName;
@@ -273,12 +274,49 @@ internal void OnFirstSceneSwitchSync(uint sceneIndex, Guid switchSceneGuid)
             s_IsSwitching = false;
         }
 
+
+
+
+        /// <summary>
+        /// Should be invoked on both the client and server side after:
+        /// -- A new scene has been loaded
+        /// -- Before any "DontDestroyOnLoad" NetworkObjects have been added back into the scene.
+        /// Added the ability to choose not to clear the scene placed objects for additive scene loading.
+        /// </summary>
+        internal void PopulateScenePlacedObjects(bool clearScenePlacedObjects = true)
+        {
+            if (clearScenePlacedObjects)
+            {
+                ScenePlacedObjects.Clear();
+            }
+
+            var networkObjects = UnityEngine.Object.FindObjectsOfType<NetworkObject>();
+
+            // Just add every NetworkObject found that isn't already in the list
+            // If any "non-in-scene placed NetworkObjects" are added to this list it shouldn't matter
+            // The only thing that matters is making sure each NetworkObject is keyed off of their GlobalObjectIdHash            
+            foreach (var networkObjectInstance in networkObjects)
+            {
+                if (!ScenePlacedObjects.ContainsKey(networkObjectInstance.GlobalObjectIdHash))
+                {
+                    // We check to make sure the NetworkManager instance is the same one to be "MultiInstanceHelpers" compatible
+                    if (networkObjectInstance.IsSceneObject == null && networkObjectInstance.NetworkManager == m_NetworkManager)
+                    {
+                        ScenePlacedObjects.Add(networkObjectInstance.GlobalObjectIdHash, networkObjectInstance);
+                    }
+                }
+            }
+        }
+
         private void OnSceneLoaded(Guid switchSceneGuid, Stream objectStream)
         {
             CurrentActiveSceneIndex = SceneNameToIndex[s_NextSceneName];
             var nextScene = SceneManager.GetSceneByName(s_NextSceneName);
             SceneManager.SetActiveScene(nextScene);
 
+            //Get all NetworkObjects loaded by the scene
+            PopulateScenePlacedObjects();
+
             // Move all objects to the new scene
             MoveObjectsToScene(nextScene);
 
@@ -288,31 +326,22 @@ private void OnSceneLoaded(Guid switchSceneGuid, Stream objectStream)
 
             if (m_NetworkManager.IsServer)
             {
-                OnSceneUnloadServer(switchSceneGuid);
+                OnServerLoadedScene(switchSceneGuid);
             }
             else
             {
-                OnSceneUnloadClient(switchSceneGuid, objectStream);
+                OnClientLoadedScene(switchSceneGuid, objectStream);
             }
         }
 
-        private void OnSceneUnloadServer(Guid switchSceneGuid)
+        private void OnServerLoadedScene(Guid switchSceneGuid)
         {
-            // Justification: Rare alloc, could(should?) reuse
-            var newSceneObjects = new List<NetworkObject>();
+            // Register in-scene placed NetworkObjects with MLAPI
+            foreach (var keyValuePair in ScenePlacedObjects)
             {
-                var networkObjects = UnityEngine.Object.FindObjectsOfType<NetworkObject>();
-
-                for (int i = 0; i < networkObjects.Length; i++)
+                if (!keyValuePair.Value.IsPlayerObject)
                 {
-                    if (networkObjects[i].NetworkManager == m_NetworkManager)
-                    {
-                        if (networkObjects[i].IsSceneObject == null)
-                        {
-                            m_NetworkManager.SpawnManager.SpawnNetworkObjectLocally(networkObjects[i], m_NetworkManager.SpawnManager.GetNetworkObjectId(), true, false, null, null, false, 0, false, true);
-                            newSceneObjects.Add(networkObjects[i]);
-                        }
-                    }
+                    m_NetworkManager.SpawnManager.SpawnNetworkObjectLocally(keyValuePair.Value, m_NetworkManager.SpawnManager.GetNetworkObjectId(), true, false, null, null, false, 0, false, true);
                 }
             }
 
@@ -327,25 +356,24 @@ private void OnSceneUnloadServer(Guid switchSceneGuid)
                         writer.WriteByteArray(switchSceneGuid.ToByteArray());
 
                         uint sceneObjectsToSpawn = 0;
-                        for (int i = 0; i < newSceneObjects.Count; i++)
+
+                        foreach (var keyValuePair in ScenePlacedObjects)
                         {
-                            if (newSceneObjects[i].Observers.Contains(m_NetworkManager.ConnectedClientsList[j].ClientId))
+                            if (keyValuePair.Value.Observers.Contains(m_NetworkManager.ConnectedClientsList[j].ClientId))
                             {
                                 sceneObjectsToSpawn++;
                             }
                         }
 
                         // Write number of scene objects to spawn
                         writer.WriteUInt32Packed(sceneObjectsToSpawn);
-
-                        for (int i = 0; i < newSceneObjects.Count; i++)
+                        foreach (var keyValuePair in ScenePlacedObjects)
                         {
-                            if (newSceneObjects[i].Observers.Contains(m_NetworkManager.ConnectedClientsList[j].ClientId))
+                            if (keyValuePair.Value.Observers.Contains(m_NetworkManager.ConnectedClientsList[j].ClientId))
                             {
-                                newSceneObjects[i].SerializeSceneObject(writer, m_NetworkManager.ConnectedClientsList[j].ClientId);
+                                keyValuePair.Value.SerializeSceneObject(writer, m_NetworkManager.ConnectedClientsList[j].ClientId);
                             }
                         }
-
                         m_NetworkManager.MessageSender.Send(m_NetworkManager.ConnectedClientsList[j].ClientId, NetworkConstants.SWITCH_SCENE, NetworkChannel.Internal, buffer);
                     }
                 }
@@ -362,12 +390,10 @@ private void OnSceneUnloadServer(Guid switchSceneGuid)
             OnSceneSwitched?.Invoke();
         }
 
-        private void OnSceneUnloadClient(Guid switchSceneGuid, Stream objectStream)
+        private void OnClientLoadedScene(Guid switchSceneGuid, Stream objectStream)
         {
             var networkObjects = UnityEngine.Object.FindObjectsOfType<NetworkObject>();
 
-            m_NetworkManager.SpawnManager.ClientCollectSoftSyncSceneObjectSweep(networkObjects);
-
             using (var reader = PooledNetworkReader.Get(objectStream))
             {
                 var newObjectsCount = reader.ReadUInt32Packed();

com.unity.multiplayer.mlapi/Runtime/Spawning/NetworkSpawnManager.cs

@@ -25,9 +25,6 @@ public class NetworkSpawnManager
         /// </summary>
         public readonly Dictionary<ulong, NetworkObject> SpawnedObjects = new Dictionary<ulong, NetworkObject>();
 
-        // Pending SoftSync objects
-        internal readonly Dictionary<ulong, NetworkObject> PendingSoftSyncObjects = new Dictionary<ulong, NetworkObject>();
-
         /// <summary>
         /// A list of the spawned objects
         /// </summary>
@@ -246,17 +243,19 @@ internal NetworkObject CreateLocalNetworkObject(bool isSceneObject, uint globalO
             }
             else
             {
-                // SoftSync them by mapping
-                if (!PendingSoftSyncObjects.TryGetValue(globalObjectIdHash, out NetworkObject networkObject))
+                if (!NetworkManager.SceneManager.ScenePlacedObjects.TryGetValue(globalObjectIdHash, out NetworkObject networkObject))
                 {
                     if (NetworkLog.CurrentLogLevel <= LogLevel.Error)
                     {
                         NetworkLog.LogError($"{nameof(NetworkPrefab)} hash was not found! In-Scene placed {nameof(NetworkObject)} soft synchronization failure for Hash: {globalObjectIdHash}!");
                     }
+
                     return null;
                 }
-
-                PendingSoftSyncObjects.Remove(globalObjectIdHash);
+                else
+                {
+                    NetworkManager.SceneManager.ScenePlacedObjects.Remove(globalObjectIdHash);
+                }
 
                 if (parentNetworkObject != null)
                 {
@@ -557,20 +556,6 @@ internal void DestroySceneObjects()
             }
         }
 
-        internal void CleanDiffedSceneObjects()
-        {
-            // Clean up any in-scene objects that had been destroyed
-            if (PendingSoftSyncObjects.Count > 0)
-            {
-                foreach (var pair in PendingSoftSyncObjects)
-                {
-                    UnityEngine.Object.Destroy(pair.Value.gameObject);
-                }
-
-                // Make sure to clear this once done destroying all remaining NetworkObjects
-                PendingSoftSyncObjects.Clear();
-            }
-        }
 
         internal void ServerSpawnSceneObjectsOnStartSweep()
         {
@@ -588,25 +573,6 @@ internal void ServerSpawnSceneObjectsOnStartSweep()
             }
         }
 
-        internal void ClientCollectSoftSyncSceneObjectSweep(NetworkObject[] networkObjects)
-        {
-            if (networkObjects == null)
-            {
-                networkObjects = UnityEngine.Object.FindObjectsOfType<NetworkObject>();
-            }
-
-            for (int i = 0; i < networkObjects.Length; i++)
-            {
-                if (networkObjects[i].NetworkManager == NetworkManager)
-                {
-                    if (networkObjects[i].IsSceneObject == null)
-                    {
-                        PendingSoftSyncObjects.Add(networkObjects[i].GlobalObjectIdHash, networkObjects[i]);
-                    }
-                }
-            }
-        }
-
         internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObject)
         {
             if (NetworkManager == null)

com.unity.multiplayer.mlapi/Tests/Runtime/NetworkObject/NetworkObjectOnSpawnTests.cs

@@ -26,12 +26,9 @@ public IEnumerator InstantiateDestroySpawnNotCalled()
 
             yield return null;
 
-            // instantiate
-            var instance = Object.Instantiate(gameObject);
-            yield return null;
-
             // destroy
-            Object.Destroy(instance);
+            Object.Destroy(gameObject);
+            
             yield return null;
         }
 

com.unity.multiplayer.mlapi/Tests/Runtime/NetworkObject/NetworkObjectSceneSerializationTests.cs

@@ -84,8 +84,8 @@ public void NetworkObjectSceneSerializationFailure()
                     // Serialize the valid NetworkObject
                     networkObject.SerializeSceneObject(writer, 0);
 
-                    // Add this valid NetworkObject into the PendinigSoftSyncObjects list
-                    NetworkManagerHelper.NetworkManagerObject.SpawnManager.PendingSoftSyncObjects.Add(networkObject.GlobalObjectIdHash, networkObject);
+                    // Add this valid NetworkObject into the ScenePlacedObjects list
+                    NetworkManagerHelper.NetworkManagerObject.SceneManager.ScenePlacedObjects.Add(networkObject.GlobalObjectIdHash, networkObject);
                 }
             }
 

testproject/Assets/Prefabs/Player.prefab

@@ -52,14 +52,16 @@ MonoBehaviour:
   m_Name: 
   m_EditorClassIdentifier: 
   TransformAuthority: 1
-  FixedSendsPerSecond: 5
+  FixedSendsPerSecond: 15
   InterpolatePosition: 1
   SnapDistance: 10
   InterpolateServer: 1
-  MinMeters: 0.16
+  MinMeters: 0.15
   MinDegrees: 1.5
   MinSize: 0.15
-  Channel: 0
+  Channel: 2
+  m_UseLocal:
+    m_InternalValue: 0
 --- !u!114 &-3775814466963834669
 MonoBehaviour:
   m_ObjectHideFlags: 0
@@ -73,6 +75,7 @@ MonoBehaviour:
   m_Name: 
   m_EditorClassIdentifier: 
   GlobalObjectIdHash: 951099334
+  m_MarkedAsSceneObject: 0
   AlwaysReplicateAsRoot: 0
   DontDestroyWithOwner: 0
 --- !u!33 &4079352819444256610

testproject/Assets/Tests/Manual/Scripts/NetworkPrefabPool.cs

@@ -21,7 +21,7 @@ public class NetworkPrefabPool : NetworkBehaviour
         [Tooltip("When enabled, this will utilize the NetworkPrefabHandler to register a custom INetworkPrefabInstanceHandler")]
         public bool EnableHandler;
 
-        [Tooltip("When enabled, this will register register a custom INetworkPrefabInstanceHandler using a NetworkObject reference")]
+        [Tooltip("When enabled, this will register a custom INetworkPrefabInstanceHandler using a NetworkObject reference")]
         public bool RegisterUsingNetworkObject;
 
         [Tooltip("What is going to be spawned on the server side from the pool.")]
@@ -167,6 +167,9 @@ public override void OnNetworkSpawn()
                 {
                     m_DelaySpawning = Time.realtimeSinceStartup + InitialSpawnDelay;
                     StartSpawningBoxes();
+
+                    //Make sure our slider reflects the current spawn rate
+                    UpdateSpawnsPerSecond();
                 }
             }
         }
@@ -233,7 +236,6 @@ private GameObject AddNewInstance()
         /// </summary>
         private void StartSpawningBoxes()
         {
-
             if (NetworkManager.IsHost && SpawnSlider != null)
             {
                 SpawnSlider.gameObject.SetActive(true);
@@ -246,14 +248,26 @@ private void StartSpawningBoxes()
         }
 
         /// <summary>
-        /// Checks to detemrine if we need to update our spawns per second calculations
+        /// Checks to determine if we need to update our spawns per second calculations
         /// </summary>
         public void UpdateSpawnsPerSecond()
         {
             if (SpawnSlider != null)
             {
                 SpawnsPerSecond = (int)SpawnSlider.value;
                 SpawnSliderValueText.text = SpawnsPerSecond.ToString();
+
+                // Handle case where the initial value is set to zero and so coroutine needs to be started
+                if(SpawnsPerSecond > 0 && !m_IsSpawningObjects)
+                {
+                    StartSpawningBoxes();
+                }
+                else //Handle case where spawning coroutine is running but we set our spawn rate to zero
+                if (SpawnsPerSecond == 0 && m_IsSpawningObjects)
+                {
+                    m_IsSpawningObjects = false;
+                    StopCoroutine(SpawnObjects());
+                }
             }
         }