fix: Active scene not being added to server side NetworkSceneManager scenes loaded [MTT-7537] (#2723)

* fix

This fixes the issue where the currently active scene, on the server side, is not taken into consideration as being a valid "loaded scene" which can cause issues as outlined in GitHub issue #2722.

* fix: TestHelpers

This resolves an issue with integration testing where:
- the scene handler registration was being registered multiple times
- the server registration was not passing in true to NetcodeIntegrationTestHelper.RegisterHandlers
- the IntegrationTestSceneHandler.CoroutineRunner could get destroyed if the active scene it was instantiated within was unloaded (now it is migrated to the DDOL)
- Registration of the currently active scene during the scene handler registration was adjusted to no longer use NetworkSceneManager.GetAndAddNewlyLoadedSceneByName (but still registers the scene).

* test

Added an integration test: `NetworkSceneManagerFixValidationTests.InitialActiveSceneUnload`
This validates the fix for #2722.

* update

Adding change log entry for this fix.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -18,6 +18,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
 - Added `NetworkVariableBase.MarkNetworkBehaviourDirty` so that user-created network variable types can mark their containing `NetworkBehaviour` to be processed by the update loop. (#2694)
 
 ### Fixed
+
+- Fixed issue where the server side `NetworkSceneManager` instance was not adding the currently active scene to its list of scenes loaded. (#2723)
 - Generic NetworkBehaviour types no longer result in compile errors or runtime errors (#2720)
 - Rpcs within Generic NetworkBehaviour types can now serialize parameters of the class's generic types (but may not have generic types of their own) (#2720)
 - Errors are no longer thrown when entering play mode with domain reload disabled (#2720)

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

@@ -740,6 +740,14 @@ internal NetworkSceneManager(NetworkManager networkManager)
             // Since NetworkManager is now always migrated to the DDOL we will use this to get the DDOL scene
             DontDestroyOnLoadScene = networkManager.gameObject.scene;
 
+            // Since the server tracks loaded scenes, we need to add the currently active scene
+            // to the list of scenes that can be unloaded.
+            if (networkManager.IsServer)
+            {
+                var activeScene = SceneManager.GetActiveScene();
+                ScenesLoaded.Add(activeScene.handle, activeScene);
+            }
+
             // Add to the server to client scene handle table
             UpdateServerClientSceneHandle(DontDestroyOnLoadScene.handle, DontDestroyOnLoadScene.handle, DontDestroyOnLoadScene);
         }

com.unity.netcode.gameobjects/TestHelpers/Runtime/IntegrationTestSceneHandler.cs

@@ -913,6 +913,9 @@ public IntegrationTestSceneHandler(NetworkManager networkManager)
             if (CoroutineRunner == null)
             {
                 CoroutineRunner = new GameObject("UnitTestSceneHandlerCoroutine").AddComponent<CoroutineRunner>();
+                // Move the CoroutineRunner into the DDOL in case we unload the scene it was instantiated in.
+                // (which if that gets destroyed then it basically stops all integration test queue processing)
+                Object.DontDestroyOnLoad(CoroutineRunner);
             }
         }
 

com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs

@@ -886,7 +886,6 @@ protected void RegisterSceneManagerHandler()
         {
             IntegrationTestSceneHandler.CanClientsLoad += ClientSceneHandler_CanClientsLoad;
             IntegrationTestSceneHandler.CanClientsUnload += ClientSceneHandler_CanClientsUnload;
-            NetcodeIntegrationTestHelpers.RegisterSceneManagerHandler(m_ServerNetworkManager, true);
         }
 
         private bool ClientSceneHandler_CanClientsUnload()

com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTestHelpers.cs

@@ -165,7 +165,8 @@ public static void RegisterHandlers(NetworkManager networkManager, bool serverSi
 
             if (!networkManager.IsServer || networkManager.IsServer && serverSideSceneManager)
             {
-                RegisterSceneManagerHandler(networkManager);
+                // Pass along the serverSideSceneManager property (otherwise the server won't register properly)
+                RegisterSceneManagerHandler(networkManager, serverSideSceneManager);
             }
         }
 
@@ -405,7 +406,10 @@ private static void SceneManagerValidationAndTestRunnerInitialization(NetworkMan
                 // scene to synchronize NetworkObjects.  Next, add the currently active test runner scene to the scenes
                 // loaded and register the server to client scene handle since host-server shares the test runner scene
                 // with the clients.
-                networkManager.SceneManager.GetAndAddNewlyLoadedSceneByName(scene.name);
+                if (!networkManager.SceneManager.ScenesLoaded.ContainsKey(scene.handle))
+                {
+                    networkManager.SceneManager.ScenesLoaded.Add(scene.handle, scene);
+                }
                 networkManager.SceneManager.ServerSceneHandleToClientSceneHandle.Add(scene.handle, scene.handle);
             }
         }
@@ -443,8 +447,8 @@ public static bool Start(bool host, NetworkManager server, NetworkManager[] clie
             server.ConnectionManager.MessageManager.Hook(hooks);
             s_Hooks[server] = hooks;
 
-            // if set, then invoke this for the server
-            RegisterHandlers(server);
+            // Register the server side handler (always pass true for server)
+            RegisterHandlers(server, true);
 
             callback?.Invoke();
 

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

@@ -16,9 +16,23 @@ namespace TestProject.RuntimeTests
     /// </summary>
     public class NetworkSceneManagerFixValidationTests : NetcodeIntegrationTest
     {
+
+        private const string k_SceneToLoad = "UnitTestBaseScene";
+        private const string k_AdditiveScene1 = "InSceneNetworkObject";
+        private const string k_AdditiveScene2 = "AdditiveSceneMultiInstance";
+
         protected override int NumberOfClients => 2;
 
         private bool m_CanStart;
+        private bool m_NoLatency;
+
+        private Scene m_OriginalActiveScene;
+
+        protected override IEnumerator OnSetup()
+        {
+            m_OriginalActiveScene = SceneManager.GetActiveScene();
+            return base.OnSetup();
+        }
 
         protected override bool CanStartServerAndClients()
         {
@@ -76,15 +90,13 @@ public void DDOLPopulateWithNullNetworkObjectsValidation([Values] bool useHost)
             // As long as there are no exceptions this test passes
         }
 
-        private const string k_SceneToLoad = "UnitTestBaseScene";
-
-        protected override void OnCreatePlayerPrefab()
-        {
-            base.OnCreatePlayerPrefab();
-        }
-
         protected override void OnServerAndClientsCreated()
         {
+            if (m_NoLatency)
+            {
+                return;
+            }
+
             // Apply a 500ms latency on packets (primarily for ClientDisconnectsDuringSeneLoadingValidation)
             var serverTransport = m_ServerNetworkManager.GetComponent<UnityTransport>();
             serverTransport.SetDebugSimulatorParameters(500, 0, 0);
@@ -94,8 +106,6 @@ protected override void OnServerAndClientsCreated()
                 var clientTransport = m_ServerNetworkManager.GetComponent<UnityTransport>();
                 clientTransport.SetDebugSimulatorParameters(500, 0, 0);
             }
-
-            base.OnServerAndClientsCreated();
         }
 
         protected override IEnumerator OnServerAndClientsConnected()
@@ -178,9 +188,134 @@ private bool VerifySceneBeforeLoading(int sceneIndex, string sceneName, LoadScen
             return true;
         }
 
+
+        private Scene m_FirstScene;
+        private Scene m_SecondScene;
+        private Scene m_ThirdScene;
+
+        private bool m_SceneUnloadedEventCompleted;
+
+
+        [UnityTest]
+        public IEnumerator InitialActiveSceneUnload()
+        {
+            SceneManager.sceneLoaded += SceneManager_sceneLoaded;
+            SceneManager.LoadScene(k_SceneToLoad, LoadSceneMode.Additive);
+
+            yield return WaitForConditionOrTimeOut(FirstSceneIsLoaded);
+            AssertOnTimeout($"Failed to load scene {k_SceneToLoad}!");
+
+            // Now set the "first scene" as the active scene prior to starting the server and clients
+            SceneManager.SetActiveScene(m_FirstScene);
+            m_NoLatency = true;
+            m_CanStart = true;
+
+            yield return StartServerAndClients();
+
+            var serverSceneManager = m_ServerNetworkManager.SceneManager;
+            serverSceneManager.OnSceneEvent += ServerSceneManager_OnSceneEvent;
+            m_SceneLoadEventCompleted = false;
+            serverSceneManager.LoadScene(k_AdditiveScene1, LoadSceneMode.Additive);
+
+            yield return WaitForConditionOrTimeOut(SecondSceneIsLoaded);
+            AssertOnTimeout($"[Load Event] Failure in loading scene {k_AdditiveScene1} (locally or on client side)!");
+
+            // Since we have to keep the test running scene active, we mimic the "auto assignment" of
+            // the active scene prior to unloading the first scene in order to validate this test scenario.
+            SceneManager.SetActiveScene(m_SecondScene);
+
+            yield return s_DefaultWaitForTick;
+
+            // Now unload the "first" scene which, if this was the only scene loaded prior to loading the second scene,
+            // would automatically make the second scene the currently active scene
+            m_SceneUnloadedEventCompleted = false;
+            serverSceneManager.UnloadScene(m_FirstScene);
+            yield return WaitForConditionOrTimeOut(SceneUnloadEventCompleted);
+            AssertOnTimeout($"[Unload Event] Failure in unloading scene {m_FirstScene} (locally or on client side)!");
+
+            // Now load the third scene, and if no time out occurs then we have validated this test!
+            m_SceneLoadEventCompleted = false;
+            serverSceneManager.LoadScene(k_AdditiveScene2, LoadSceneMode.Additive);
+            yield return WaitForConditionOrTimeOut(ThirdSceneIsLoaded);
+            AssertOnTimeout($"[Load Event] Failure in loading scene {k_AdditiveScene2} (locally or on client side)!");
+            serverSceneManager.OnSceneEvent -= ServerSceneManager_OnSceneEvent;
+        }
+
+        private void ServerSceneManager_OnSceneEvent(SceneEvent sceneEvent)
+        {
+            if (sceneEvent.ClientId != m_ServerNetworkManager.LocalClientId)
+            {
+                return;
+            }
+
+            if (sceneEvent.SceneEventType == SceneEventType.LoadComplete)
+            {
+                if (sceneEvent.Scene.name == k_AdditiveScene1)
+                {
+                    m_SecondScene = sceneEvent.Scene;
+                }
+                else if (sceneEvent.Scene.name == k_AdditiveScene2)
+                {
+                    m_ThirdScene = sceneEvent.Scene;
+                }
+            }
+
+            if (sceneEvent.SceneEventType == SceneEventType.LoadEventCompleted)
+            {
+                if (sceneEvent.SceneName == k_AdditiveScene1 || sceneEvent.SceneName == k_AdditiveScene2)
+                {
+                    m_SceneLoadEventCompleted = true;
+                }
+            }
+
+            if (sceneEvent.SceneEventType == SceneEventType.UnloadEventCompleted)
+            {
+                if (sceneEvent.SceneName == k_SceneToLoad || sceneEvent.SceneName == k_AdditiveScene1 || sceneEvent.SceneName == k_AdditiveScene2)
+                {
+                    m_SceneUnloadedEventCompleted = true;
+                }
+            }
+        }
+
+        private bool SceneUnloadEventCompleted()
+        {
+            return m_SceneUnloadedEventCompleted;
+        }
+
+        private bool FirstSceneIsLoaded()
+        {
+            return m_FirstScene.IsValid() && m_FirstScene.isLoaded;
+        }
+
+        private bool SecondSceneIsLoaded()
+        {
+            return m_SecondScene.IsValid() && m_SecondScene.isLoaded && m_SceneLoadEventCompleted;
+        }
+
+        private bool ThirdSceneIsLoaded()
+        {
+            return m_ThirdScene.IsValid() && m_ThirdScene.isLoaded && m_SceneLoadEventCompleted;
+        }
+
+        private void SceneManager_sceneLoaded(Scene scene, LoadSceneMode mode)
+        {
+            if (scene.name == k_SceneToLoad && mode == LoadSceneMode.Additive)
+            {
+                m_FirstScene = scene;
+                SceneManager.sceneLoaded -= SceneManager_sceneLoaded;
+            }
+        }
+
         protected override IEnumerator OnTearDown()
         {
             m_CanStart = false;
+            m_NoLatency = false;
+
+            if (m_OriginalActiveScene != SceneManager.GetActiveScene())
+            {
+                SceneManager.SetActiveScene(m_OriginalActiveScene);
+            }
+
             return base.OnTearDown();
         }
     }