fix: OnSceneEvent should raise Unload events for all scenes not just additive scenes [MTT-3580] (#1975)

* fix

MTT-3580
This resolves the issue with the active scene and any additively loaded scenes not sending scene event notifications to inform the user that the scenes did, indeed, unload.

* Update CHANGELOG.md

* Update CHANGELOG.md

* Update CHANGELOG.md

* test - NetworkSceneManagerEventNotifications - update

Added some additional tests for switch scene notifications for additively loaded scenes and the currently active scene.

* update

Minor update to NetworkSceneManager.SceneUnloadEventHandler to deal with the scenario where a scene is unloaded but NetworkManager has already shutdown.

* update

Final changelog update.
MTT-3580

* update

Added some shutdown code to make sure SceneUnloadEventHandler cleans up after itself in the event a large scene is in the middle of loading and the player exits the current game-session.

Cleaned up some comments.
Cleaned up some debug output in the NetworkSceneManagerEventNotifications test.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -20,7 +20,9 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ### Removed
 
 ### Fixed
+
 - Fixed issue where `NetworkBehaviourReference` would throw a type cast exception if using `NetworkBehaviourReference.TryGet` and the component type was not found. (#1984)
+- Fixed `NetworkSceneManager` was not sending scene event notifications for the currently active scene and any additively loaded scenes when loading a new scene in `LoadSceneMode.Single` mode. (#1975)
 - Fixed issue where one or more clients disconnecting during a scene event would cause `LoadEventCompleted` or `UnloadEventCompleted` to wait until the `NetworkConfig.LoadSceneTimeOut` period before being triggered. (#1973)
 - Fixed issues when multiple `ConnectionApprovalCallback`s were registered (#1972)
 - `FixedString` types can now be used in NetworkVariables and RPCs again without requiring a `ForceNetworkSerializeByMemcpy<>` wrapper (#1961)

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

@@ -329,8 +329,12 @@ public class NetworkSceneManager : IDisposable
         public VerifySceneBeforeLoadingDelegateHandler VerifySceneBeforeLoading;
 
         /// <summary>
-        ///  Proof of concept: Interface version that allows for the decoupling from
-        ///  the SceneManager's Load and Unload methods for testing purposes (potentially other future usage)
+        /// The SceneManagerHandler implementation
+        /// </summary>
+        internal ISceneManagerHandler SceneManagerHandler = new DefaultSceneManagerHandler();
+
+        /// <summary>
+        ///  The default SceneManagerHandler that interfaces between the SceneManager and NetworkSceneManager
         /// </summary>
         private class DefaultSceneManagerHandler : ISceneManagerHandler
         {
@@ -349,10 +353,6 @@ public AsyncOperation UnloadSceneAsync(Scene scene, ISceneManagerHandler.SceneEv
             }
         }
 
-        internal ISceneManagerHandler SceneManagerHandler = new DefaultSceneManagerHandler();
-        /// End of Proof of Concept
-
-
         internal readonly Dictionary<Guid, SceneEventProgress> SceneEventProgressTracking = new Dictionary<Guid, SceneEventProgress>();
 
         /// <summary>
@@ -435,6 +435,8 @@ public AsyncOperation UnloadSceneAsync(Scene scene, ISceneManagerHandler.SceneEv
         /// </summary>
         public void Dispose()
         {
+            SceneUnloadEventHandler.Shutdown();
+
             foreach (var keypair in SceneEventDataStore)
             {
                 if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
@@ -882,7 +884,6 @@ private SceneEventProgress ValidateSceneEvent(string sceneName, bool isUnloading
         /// Callback for the <see cref="SceneEventProgress.OnComplete"/> <see cref="SceneEventProgress.OnCompletedDelegate"/> handler
         /// </summary>
         /// <param name="sceneEventProgress"></param>
-        /// <returns></returns>
         private bool OnSceneEventProgressCompleted(SceneEventProgress sceneEventProgress)
         {
             var sceneEventData = BeginSceneEvent();
@@ -981,7 +982,6 @@ public SceneEventProgressStatus UnloadScene(Scene scene)
                 sceneEventProgress.SetSceneLoadOperation(sceneUnload);
             }
 
-
             // Notify local server that a scene is going to be unloaded
             OnSceneEvent?.Invoke(new SceneEvent()
             {
@@ -1101,7 +1101,7 @@ private void OnSceneUnloaded(uint sceneEventId)
 
         private void EmptySceneUnloadedOperation(uint sceneEventId)
         {
-            // Do nothing (this is a stub call since it is only used to flush all currently loaded scenes)
+            // Do nothing (this is a stub call since it is only used to flush all additively loaded scenes)
         }
 
         /// <summary>
@@ -1111,6 +1111,7 @@ private void EmptySceneUnloadedOperation(uint sceneEventId)
         /// </summary>
         internal void UnloadAdditivelyLoadedScenes(uint sceneEventId)
         {
+            var sceneEventData = SceneEventDataStore[sceneEventId];
             // Unload all additive scenes while making sure we don't try to unload the base scene ( loaded in single mode ).
             var currentActiveScene = SceneManager.GetActiveScene();
             foreach (var keyHandleEntry in ScenesLoaded)
@@ -1120,15 +1121,7 @@ internal void UnloadAdditivelyLoadedScenes(uint sceneEventId)
                 {
                     var sceneUnload = SceneManagerHandler.UnloadSceneAsync(keyHandleEntry.Value,
                         new ISceneManagerHandler.SceneEventAction() { SceneEventId = sceneEventId, EventAction = EmptySceneUnloadedOperation });
-
-                    OnSceneEvent?.Invoke(new SceneEvent()
-                    {
-                        AsyncOperation = sceneUnload,
-                        SceneEventType = SceneEventType.Unload,
-                        SceneName = keyHandleEntry.Value.name,
-                        LoadSceneMode = LoadSceneMode.Additive, // The only scenes unloaded are scenes that were additively loaded
-                        ClientId = NetworkManager.ServerClientId
-                    });
+                    SceneUnloadEventHandler.RegisterScene(this, keyHandleEntry.Value, LoadSceneMode.Additive, sceneUnload);
                 }
             }
             // clear out our scenes loaded list
@@ -1180,12 +1173,14 @@ public SceneEventProgressStatus LoadScene(string sceneName, LoadSceneMode loadSc
 
                 // Now Unload all currently additively loaded scenes
                 UnloadAdditivelyLoadedScenes(sceneEventId);
+
+                // Register the active scene for unload scene event notifications
+                SceneUnloadEventHandler.RegisterScene(this, SceneManager.GetActiveScene(), LoadSceneMode.Single);
             }
 
             // Now start loading the scene
             var sceneEventAction = new ISceneManagerHandler.SceneEventAction() { SceneEventId = sceneEventId, EventAction = OnSceneLoaded };
             var sceneLoad = SceneManagerHandler.LoadSceneAsync(sceneName, loadSceneMode, sceneEventAction);
-
             // If integration testing, IntegrationTestSceneHandler returns null
             if (sceneLoad == null)
             {
@@ -1212,6 +1207,114 @@ public SceneEventProgressStatus LoadScene(string sceneName, LoadSceneMode loadSc
             return sceneEventProgress.Status;
         }
 
+        /// <summary>
+        /// Helper class used to handle "odd ball" scene unload event notification scenarios
+        /// when scene switching.
+        /// </summary>
+        internal class SceneUnloadEventHandler
+        {
+            private static Dictionary<NetworkManager, List<SceneUnloadEventHandler>> s_Instances = new Dictionary<NetworkManager, List<SceneUnloadEventHandler>>();
+
+            internal static void RegisterScene(NetworkSceneManager networkSceneManager, Scene scene, LoadSceneMode loadSceneMode, AsyncOperation asyncOperation = null)
+            {
+                var networkManager = networkSceneManager.m_NetworkManager;
+                if (!s_Instances.ContainsKey(networkManager))
+                {
+                    s_Instances.Add(networkManager, new List<SceneUnloadEventHandler>());
+                }
+                var clientId = networkManager.IsServer ? NetworkManager.ServerClientId : networkManager.LocalClientId;
+                s_Instances[networkManager].Add(new SceneUnloadEventHandler(networkSceneManager, scene, clientId, loadSceneMode, asyncOperation));
+            }
+
+            private static void SceneUnloadComplete(SceneUnloadEventHandler sceneUnloadEventHandler)
+            {
+                if (sceneUnloadEventHandler == null || sceneUnloadEventHandler.m_NetworkSceneManager == null || sceneUnloadEventHandler.m_NetworkSceneManager.m_NetworkManager == null)
+                {
+                    return;
+                }
+                var networkManager = sceneUnloadEventHandler.m_NetworkSceneManager.m_NetworkManager;
+                if (s_Instances.ContainsKey(networkManager))
+                {
+                    s_Instances[networkManager].Remove(sceneUnloadEventHandler);
+                    if (s_Instances[networkManager].Count == 0)
+                    {
+                        s_Instances.Remove(networkManager);
+                    }
+                }
+            }
+
+            /// <summary>
+            /// Called by NetworkSceneManager when it is disposing
+            /// </summary>
+            internal static void Shutdown()
+            {
+                foreach (var instanceEntry in s_Instances)
+                {
+                    foreach (var instance in instanceEntry.Value)
+                    {
+                        instance.OnShutdown();
+                    }
+                    instanceEntry.Value.Clear();
+                }
+                s_Instances.Clear();
+            }
+
+            private NetworkSceneManager m_NetworkSceneManager;
+            private AsyncOperation m_AsyncOperation;
+            private LoadSceneMode m_LoadSceneMode;
+            private ulong m_ClientId;
+            private Scene m_Scene;
+            private bool m_ShuttingDown;
+
+            private void OnShutdown()
+            {
+                m_ShuttingDown = true;
+                SceneManager.sceneUnloaded -= SceneUnloaded;
+            }
+
+            private void SceneUnloaded(Scene scene)
+            {
+                if (m_Scene.handle == scene.handle && !m_ShuttingDown)
+                {
+                    if (m_NetworkSceneManager != null && m_NetworkSceneManager.m_NetworkManager != null)
+                    {
+                        m_NetworkSceneManager.OnSceneEvent?.Invoke(new SceneEvent()
+                        {
+                            AsyncOperation = m_AsyncOperation,
+                            SceneEventType = SceneEventType.UnloadComplete,
+                            SceneName = m_Scene.name,
+                            LoadSceneMode = m_LoadSceneMode,
+                            ClientId = m_ClientId
+                        });
+                        m_NetworkSceneManager.OnUnloadComplete?.Invoke(m_ClientId, m_Scene.name);
+                    }
+                    SceneManager.sceneUnloaded -= SceneUnloaded;
+                    SceneUnloadComplete(this);
+                }
+            }
+
+            private SceneUnloadEventHandler(NetworkSceneManager networkSceneManager, Scene scene, ulong clientId, LoadSceneMode loadSceneMode, AsyncOperation asyncOperation = null)
+            {
+                m_LoadSceneMode = loadSceneMode;
+                m_AsyncOperation = asyncOperation;
+                m_NetworkSceneManager = networkSceneManager;
+                m_ClientId = clientId;
+                m_Scene = scene;
+                SceneManager.sceneUnloaded += SceneUnloaded;
+                // Send the initial unload event notification
+                m_NetworkSceneManager.OnSceneEvent?.Invoke(new SceneEvent()
+                {
+                    AsyncOperation = m_AsyncOperation,
+                    SceneEventType = SceneEventType.Unload,
+                    SceneName = m_Scene.name,
+                    LoadSceneMode = m_LoadSceneMode,
+                    ClientId = clientId
+                });
+
+                m_NetworkSceneManager.OnUnload?.Invoke(networkSceneManager.m_NetworkManager.LocalClientId, m_Scene.name, null);
+            }
+        }
+
         /// <summary>
         /// Client Side:
         /// Handles both forms of scene loading
@@ -1246,6 +1349,10 @@ private void OnClientSceneLoadingEvent(uint sceneEventId)
             if (sceneEventData.LoadSceneMode == LoadSceneMode.Single)
             {
                 IsSpawnedObjectsPendingInDontDestroyOnLoad = true;
+
+                // Register the active scene for unload scene event notifications
+                SceneUnloadEventHandler.RegisterScene(this, SceneManager.GetActiveScene(), LoadSceneMode.Single);
+
             }
 
             var sceneLoad = SceneManagerHandler.LoadSceneAsync(sceneName, sceneEventData.LoadSceneMode,

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

@@ -174,7 +174,7 @@ static internal IEnumerator ProcessUnloadingSceneJob(QueuedSceneJob queuedSceneJ
             }
 
             SceneManager.sceneUnloaded += SceneManager_sceneUnloaded;
-            if (queuedSceneJob.Scene.IsValid() && queuedSceneJob.Scene.isLoaded)
+            if (queuedSceneJob.Scene.IsValid() && queuedSceneJob.Scene.isLoaded && !queuedSceneJob.Scene.name.Contains(NetcodeIntegrationTestHelpers.FirstPartOfTestRunnerSceneName))
             {
                 SceneManager.UnloadSceneAsync(queuedSceneJob.Scene);
             }