fix: PopulateScenePlacedObjects only checks IsSceneObject for null [MTT-3041] (#1850)

* fix

MTT-3041
This fixes the issue when using a single scene containing both a NetworkManager and one or more in-scene placed NetworkObjects where a client could disconnect but not unload the scene and reconnect only to receive a soft synchronization error.
The fix is to check for both HasValue and Value being true in order to account for already previously instantiated NetworkObjects.

* style

* Update CHANGELOG.md

MTT-3041 changelog update

* update

cleaned up PopulateScenePlacedObjects for clarity.
reduce the networkObjectInstance.IsSceneObject check.
made PopulateScenePlacedObjects internal for testing purposes.

* test

MTT-3041
This test verifies the changes made to PopulateScenePlacedObjects

* style

Adding some comments

* test

just spawning one of each type of simulated in-scene placed NetworkObject

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -23,6 +23,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 - Removed `com.unity.modules.animation`, `com.unity.modules.physics` and `com.unity.modules.physics2d` dependencies from the package (#1812)
 
 ### Fixed
+- Fixed in-scene placed NetworkObjects not being found/ignored after a client disconnects and then reconnects. (#1850)
 - Fixed issue where `UnityTransport` send queues were not flushed when calling `DisconnectLocalClient` or `DisconnectRemoteClient`. (#1847)
 - Fixed NetworkBehaviour dependency verification check for an existing NetworkObject not searching from root parent transform relative GameObject. (#1841)
 - Fixed issue where entries were not being removed from the NetworkSpawnManager.OwnershipToObjectsTable. (#1838)

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

@@ -1840,7 +1840,7 @@ internal void MoveObjectsToDontDestroyOnLoad()
         /// Using the local scene relative Scene.handle as a sub-key to the root dictionary allows us to
         /// distinguish between duplicate in-scene placed NetworkObjects
         /// </summary>
-        private void PopulateScenePlacedObjects(Scene sceneToFilterBy, bool clearScenePlacedObjects = true)
+        internal void PopulateScenePlacedObjects(Scene sceneToFilterBy, bool clearScenePlacedObjects = true)
         {
             if (clearScenePlacedObjects)
             {
@@ -1855,25 +1855,26 @@ private void PopulateScenePlacedObjects(Scene sceneToFilterBy, bool clearScenePl
             // at the end of scene loading we use this list to soft synchronize all in-scene placed NetworkObjects
             foreach (var networkObjectInstance in networkObjects)
             {
-                // We check to make sure the NetworkManager instance is the same one to be "NetcodeIntegrationTestHelpers" compatible and filter the list on a per scene basis (additive scenes)
-                if (networkObjectInstance.IsSceneObject == null && networkObjectInstance.NetworkManager == m_NetworkManager && networkObjectInstance.gameObject.scene == sceneToFilterBy &&
-                    networkObjectInstance.gameObject.scene.handle == sceneToFilterBy.handle)
+                var globalObjectIdHash = networkObjectInstance.GlobalObjectIdHash;
+                var sceneHandle = networkObjectInstance.gameObject.scene.handle;
+                // We check to make sure the NetworkManager instance is the same one to be "NetcodeIntegrationTestHelpers" compatible and filter the list on a per scene basis (for additive scenes)
+                if (networkObjectInstance.IsSceneObject != false && networkObjectInstance.NetworkManager == m_NetworkManager && networkObjectInstance.gameObject.scene == sceneToFilterBy &&
+                    sceneHandle == sceneToFilterBy.handle)
                 {
-                    if (!ScenePlacedObjects.ContainsKey(networkObjectInstance.GlobalObjectIdHash))
+                    if (!ScenePlacedObjects.ContainsKey(globalObjectIdHash))
                     {
-                        ScenePlacedObjects.Add(networkObjectInstance.GlobalObjectIdHash, new Dictionary<int, NetworkObject>());
+                        ScenePlacedObjects.Add(globalObjectIdHash, new Dictionary<int, NetworkObject>());
                     }
 
-                    if (!ScenePlacedObjects[networkObjectInstance.GlobalObjectIdHash].ContainsKey(networkObjectInstance.gameObject.scene.handle))
+                    if (!ScenePlacedObjects[globalObjectIdHash].ContainsKey(sceneHandle))
                     {
-                        ScenePlacedObjects[networkObjectInstance.GlobalObjectIdHash].Add(networkObjectInstance.gameObject.scene.handle, networkObjectInstance);
+                        ScenePlacedObjects[globalObjectIdHash].Add(sceneHandle, networkObjectInstance);
                     }
                     else
                     {
-                        var exitingEntryName = ScenePlacedObjects[networkObjectInstance.GlobalObjectIdHash][networkObjectInstance.gameObject.scene.handle] != null ?
-                            ScenePlacedObjects[networkObjectInstance.GlobalObjectIdHash][networkObjectInstance.gameObject.scene.handle].name : "Null Entry";
+                        var exitingEntryName = ScenePlacedObjects[globalObjectIdHash][sceneHandle] != null ? ScenePlacedObjects[globalObjectIdHash][sceneHandle].name : "Null Entry";
                         throw new Exception($"{networkObjectInstance.name} tried to registered with {nameof(ScenePlacedObjects)} which already contains " +
-                            $"the same {nameof(NetworkObject.GlobalObjectIdHash)} value {networkObjectInstance.GlobalObjectIdHash} for {exitingEntryName}!");
+                            $"the same {nameof(NetworkObject.GlobalObjectIdHash)} value {globalObjectIdHash} for {exitingEntryName}!");
                     }
                 }
             }

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

@@ -0,0 +1,78 @@
+using System.Collections;
+using System.Collections.Generic;
+using UnityEngine;
+using UnityEngine.SceneManagement;
+using UnityEngine.TestTools;
+using NUnit.Framework;
+using Unity.Netcode;
+using Unity.Netcode.TestHelpers.Runtime;
+using Object = UnityEngine.Object;
+
+namespace TestProject.RuntimeTests
+{
+    public class NetworkSceneManagerPopulateInSceneTests : NetcodeIntegrationTest
+    {
+        protected override int NumberOfClients => 0;
+
+
+        protected Dictionary<uint, GameObject> m_InSceneObjectList = new Dictionary<uint, GameObject>();
+
+        protected override IEnumerator OnSetup()
+        {
+            m_InSceneObjectList.Clear();
+            return base.OnSetup();
+        }
+
+        protected override void OnServerAndClientsCreated()
+        {
+            // Create one that simulates when an in-scene placed NetworkObject is first instantiated when
+            // the scene is loaded (i.e. IsSceneObject is null)
+            var inScenePrefab = CreateNetworkObjectPrefab("NewSceneObject");
+            var networkObject = inScenePrefab.GetComponent<NetworkObject>();
+            networkObject.IsSceneObject = null;
+            networkObject.NetworkManagerOwner = m_ServerNetworkManager;
+            m_InSceneObjectList.Add(networkObject.GlobalObjectIdHash, inScenePrefab);
+
+            // Create one that simulates when an in-scene placed NetworkObject has already been instantiated
+            // (i.e. IsSceneObject is true) which can happen if a client disconnects and then reconnects without
+            // unloading/reloading any scenes.
+            inScenePrefab = CreateNetworkObjectPrefab("SetInSceneObject");
+            networkObject = inScenePrefab.GetComponent<NetworkObject>();
+            networkObject.IsSceneObject = true;
+            networkObject.NetworkManagerOwner = m_ServerNetworkManager;
+            m_InSceneObjectList.Add(networkObject.GlobalObjectIdHash, inScenePrefab);
+        }
+
+        [UnityTest]
+        public IEnumerator PopulateScenePlacedObjectsTest()
+        {
+            var activeScene = SceneManager.GetActiveScene();
+
+            m_ServerNetworkManager.SceneManager.PopulateScenePlacedObjects(activeScene, true);
+            var scenePlacedNetworkObjects = m_ServerNetworkManager.SceneManager.ScenePlacedObjects;
+            foreach (var entry in m_InSceneObjectList)
+            {
+                // Verify the GlobalObjectIdHash for this object has an entry
+                Assert.IsTrue(scenePlacedNetworkObjects.ContainsKey(entry.Key), $"Failed to find {nameof(NetworkObject.GlobalObjectIdHash)}({entry.Key}) for {entry.Value.name} in the {nameof(NetworkSceneManager.ScenePlacedObjects)}!");
+
+                // Verify the active scene for this object has an entry
+                Assert.IsTrue(scenePlacedNetworkObjects[entry.Key].ContainsKey(activeScene.handle), $"Failed to find the scene handle {activeScene.handle} ({activeScene.name}) entry for {entry.Value.name} in the {nameof(NetworkSceneManager.ScenePlacedObjects)}!");
+
+                // Verify the GameObject is the same one
+                var inSceneGameObject = scenePlacedNetworkObjects[entry.Key][activeScene.handle].gameObject;
+                Assert.IsTrue(inSceneGameObject == entry.Value, $"{nameof(GameObject)} {entry.Value.name} is not the same as {inSceneGameObject.name}!");
+            }
+
+            yield break;
+        }
+
+        protected override IEnumerator OnTearDown()
+        {
+            foreach (var spawnedInstance in m_InSceneObjectList)
+            {
+                Object.Destroy(spawnedInstance.Value);
+            }
+            return base.OnTearDown();
+        }
+    }
+}