fix: In-scene NetworkObjects disabled but never re-enabled after scene transition (#1354)

* fix

This is the fix for the in-scene NetworkObjects moved into DDOL and the after a scene transition they become disabled but never re-enabled.

* test

Had to make some private methods internal for testing purposes.
Adding test to validate the fix.

* style

white space fixes.

* style

Adding comment above the primary line of code that is the fix for this PR

* style

removing unused namespace.

* style

removing several LFs

* fix and test

This now tracks the state of NetworkObjects when a full scene transition takes place.

This includes a refactored test for NetworkObjects (dynamically spawned and in-scene placed) that persist a full scene transition.

* test update

This makes sure that DestroyWithScene is set to false for all InSceneNetworkObjectState tests.

* style

Removing whitespace.

* updated changelog

Added entries for the fixes applied in this PR.

* refactor

After peer review during PR Triage we all decided to not change or track a NetworkObject's parent GameObject's state and to not disable during full scene transition.

Removed all active state tracking dictionaries and references as well as removed disabling DDOL migrated GameObjects from the runtime NGO code base.

Adjusted the DDOL test to check the state is maintained throughout the scene transition.

The DDOL test only required a minor

* updated changelog

Updating the changelog to reflect most recent updates.

Author: NoelStephensUnity

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

@@ -1830,21 +1830,18 @@ internal void HandleSceneEvent(ulong clientId, FastBufferReader reader)
         /// Moves all NetworkObjects that don't have the <see cref="NetworkObject.DestroyWithScene"/> set to
         /// the "Do not destroy on load" scene.
         /// </summary>
-        private void MoveObjectsToDontDestroyOnLoad()
+        internal void MoveObjectsToDontDestroyOnLoad()
         {
-            // Move ALL NetworkObjects to the temp scene
+            // Move ALL NetworkObjects marked to persist scene transitions into the DDOL scene
             var objectsToKeep = new HashSet<NetworkObject>(m_NetworkManager.SpawnManager.SpawnedObjectsList);
-
             foreach (var sobj in objectsToKeep)
             {
-                if (!sobj.DestroyWithScene || (sobj.IsSceneObject != null && sobj.IsSceneObject.Value && sobj.gameObject.scene == DontDestroyOnLoadScene))
+                if (!sobj.DestroyWithScene || sobj.gameObject.scene == DontDestroyOnLoadScene)
                 {
-                    // Only move objects with no parent as child objects will follow
-                    if (sobj.gameObject.transform.parent == null)
+                    // Only move dynamically spawned network objects with no parent as child objects will follow
+                    if (sobj.gameObject.transform.parent == null && sobj.IsSceneObject != null && !sobj.IsSceneObject.Value)
                     {
                         UnityEngine.Object.DontDestroyOnLoad(sobj.gameObject);
-                        // Since we are doing a scene transition, disable the GameObject until the next scene is loaded
-                        sobj.gameObject.SetActive(false);
                     }
                 }
                 else if (m_NetworkManager.IsServer)
@@ -1907,24 +1904,22 @@ private void PopulateScenePlacedObjects(Scene sceneToFilterBy, bool clearScenePl
         /// Moves all spawned NetworkObjects (from do not destroy on load) to the scene specified
         /// </summary>
         /// <param name="scene">scene to move the NetworkObjects to</param>
-        private void MoveObjectsFromDontDestroyOnLoadToScene(Scene scene)
+        internal void MoveObjectsFromDontDestroyOnLoadToScene(Scene scene)
         {
             // Move ALL NetworkObjects to the temp scene
             var objectsToKeep = m_NetworkManager.SpawnManager.SpawnedObjectsList;
 
             foreach (var sobj in objectsToKeep)
             {
-                if (sobj.gameObject.scene == DontDestroyOnLoadScene && (sobj.IsSceneObject == null || sobj.IsSceneObject.Value))
-                {
-                    continue;
-                }
-
-                // Only move objects with no parent as child objects will follow
-                if (sobj.gameObject.transform.parent == null)
+                // If it is in the DDOL then
+                if (sobj.gameObject.scene == DontDestroyOnLoadScene)
                 {
-                    // set it back to active at this point
-                    sobj.gameObject.SetActive(true);
-                    SceneManager.MoveGameObjectToScene(sobj.gameObject, scene);
+                    // only move dynamically spawned network objects, with no parent as child objects will follow,
+                    // back into the currently active scene
+                    if (sobj.gameObject.transform.parent == null && sobj.IsSceneObject != null && !sobj.IsSceneObject.Value)
+                    {
+                        SceneManager.MoveGameObjectToScene(sobj.gameObject, scene);
+                    }
                 }
             }
         }

testproject/Assets/Tests/Runtime/NetworkSceneManagerTests.cs

@@ -8,6 +8,7 @@
 using UnityEngine.TestTools;
 using Unity.Netcode.RuntimeTests;
 using Unity.Netcode;
+using Object = UnityEngine.Object;
 
 namespace TestProject.RuntimeTests
 {
@@ -935,7 +936,129 @@ public IEnumerator FastReaderAllocationTest()
             nativeArray.Dispose();
             fastBufferWriter.Dispose();
             networkManager.Shutdown();
-            UnityEngine.Object.Destroy(networkManagerGameObject);
+            Object.Destroy(networkManagerGameObject);
         }
     }
+
+    public class NetworkSceneManagerDDOLTests
+    {
+        private NetworkManager m_ServerNetworkManager;
+        private GameObject m_NetworkManagerGameObject;
+        private GameObject m_DDOL_ObjectToSpawn;
+
+        protected float m_ConditionMetFrequency = 0.1f;
+
+        [UnitySetUp]
+        protected IEnumerator SetUp()
+        {
+            m_NetworkManagerGameObject = new GameObject("NetworkManager - Host");
+            m_ServerNetworkManager = m_NetworkManagerGameObject.AddComponent<NetworkManager>();
+
+            m_DDOL_ObjectToSpawn = new GameObject();
+            m_DDOL_ObjectToSpawn.AddComponent<NetworkObject>();
+            m_DDOL_ObjectToSpawn.AddComponent<DDOLBehaviour>();
+
+            m_ServerNetworkManager.NetworkConfig = new NetworkConfig()
+            {
+                ConnectionApproval = false,
+                NetworkPrefabs = new List<NetworkPrefab>(),
+                NetworkTransport = m_NetworkManagerGameObject.AddComponent<SIPTransport>(),
+            };
+            m_ServerNetworkManager.StartHost();
+            yield break;
+        }
+
+        [UnityTearDown]
+        protected IEnumerator TearDown()
+        {
+            m_ServerNetworkManager.Shutdown();
+
+            Object.Destroy(m_NetworkManagerGameObject);
+            Object.Destroy(m_DDOL_ObjectToSpawn);
+
+            yield break;
+        }
+
+        public enum DefaultState
+        {
+            IsEnabled,
+            IsDisabled
+        }
+
+        public enum MovedIntoDDOLBy
+        {
+            User,
+            NetworkSceneManager
+        }
+
+        public enum NetworkObjectType
+        {
+            InScenePlaced,
+            DynamicallySpawned
+        }
+
+        /// <summary>
+        /// Tests to make sure NetworkObjects moved into the DDOL will
+        /// restore back to their currently active state when a full
+        /// scene transition is complete.
+        /// This tests both in-scene placed and dynamically spawned NetworkObjects
+        [UnityTest]
+        public IEnumerator InSceneNetworkObjectState([Values(DefaultState.IsEnabled, DefaultState.IsDisabled)] DefaultState activeState,
+            [Values(MovedIntoDDOLBy.User, MovedIntoDDOLBy.NetworkSceneManager)] MovedIntoDDOLBy movedIntoDDOLBy,
+            [Values(NetworkObjectType.InScenePlaced, NetworkObjectType.DynamicallySpawned)] NetworkObjectType networkObjectType)
+        {
+            var isActive = activeState == DefaultState.IsEnabled ? true : false;
+            var isInScene = networkObjectType == NetworkObjectType.InScenePlaced ? true : false;
+            var networkObject = m_DDOL_ObjectToSpawn.GetComponent<NetworkObject>();
+            var ddolBehaviour = m_DDOL_ObjectToSpawn.GetComponent<DDOLBehaviour>();
+
+            // All tests require this to be false
+            networkObject.DestroyWithScene = false;
+
+            if (movedIntoDDOLBy == MovedIntoDDOLBy.User)
+            {
+                ddolBehaviour.MoveToDDOL();
+            }
+
+            // Sets whether we are in-scene or dynamically spawned NetworkObject
+            ddolBehaviour.SetInScene(isInScene);
+
+            Assert.That(networkObject.IsSpawned);
+
+            m_DDOL_ObjectToSpawn.SetActive(isActive);
+
+            m_ServerNetworkManager.SceneManager.MoveObjectsToDontDestroyOnLoad();
+
+            yield return new WaitForSeconds(0.03f);
+
+            // It should be isActive when MoveObjectsToDontDestroyOnLoad is called.
+            Assert.That(networkObject.isActiveAndEnabled == isActive);
+
+            m_ServerNetworkManager.SceneManager.MoveObjectsFromDontDestroyOnLoadToScene(SceneManager.GetActiveScene());
+
+            yield return new WaitForSeconds(0.03f);
+
+            // It should be isActive when MoveObjectsFromDontDestroyOnLoadToScene is called.
+            Assert.That(networkObject.isActiveAndEnabled == isActive);
+
+            //Done
+            networkObject.Despawn(false);
+        }
+
+        public class DDOLBehaviour : NetworkBehaviour
+        {
+            public void MoveToDDOL()
+            {
+                DontDestroyOnLoad(gameObject);
+            }
+
+            public void SetInScene(bool isInScene)
+            {
+                var networkObject = GetComponent<NetworkObject>();
+                networkObject.IsSceneObject = isInScene;
+            }
+        }
+
+    }
+
 }