fix: SceneEventProgress OnComplete not invoked on timeouts [MTT-4772] (#2222)

* fix
Need to track any clients that disconnected to add them to the list of clients that did not complete the scene event.
Reducing the logic used to determine when the SceneEventProgress is finished.
Renamed a few properties and methods for code readability purposes.
Fixed the logic behind determining when the SceneEventProgress should complete.
Renaming the SetSceneAsyncOperation to just SetAsyncOperation.

* refactor
Completely removed the entire SceneEventAction.
Replaced it with an even more simplified version of SceneEventProgress.
We will no longer allocate memory for SceneEventAction.

* style
Added additional comments for clarity
fixing grammar in a comment
removing two CR/LFs
moving a comment above a few lines of code (otherwise it doesn't make sense).
Last comment adjustment

* test
Added several tests to verify that SceneEventProgress will complete if all clients finished the scene event, if clients disconnect while it is still in progress, will time out if one or more clients never finish, and finally will still complete if one or more clients late join while it is still in progress.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -23,6 +23,9 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 - Fixed issue where clients were not rebuilding the `NetworkConfig` hash value for each unique connection request. (#2226)
 - Fixed the issue where player objects were not taking the `DontDestroyWithOwner` property into consideration when a client disconnected. (#2225)
+- Fixed issue where `SceneEventProgress` would not complete if a client late joins while it is still in progress. (#2222)
+- Fixed issue where `SceneEventProgress` would not complete if a client disconnects. (#2222)
+- Fixed issues with detecting if a `SceneEventProgress` has timed out. (#2222)
 - Fixed issue #1924 where `UnityTransport` would fail to restart after a first failure (even if what caused the initial failure was addressed). (#2220)
 - Fixed issue where `NetworkTransform.SetStateServerRpc` and `NetworkTransform.SetStateClientRpc` were not honoring local vs world space settings when applying the position and rotation. (#2203)
 - Fixed ILPP `TypeLoadException` on WebGL on MacOS Editor and potentially other platforms. (#2199)

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

@@ -1,4 +1,3 @@
-using System;
 using UnityEngine;
 using UnityEngine.SceneManagement;
 
@@ -10,25 +9,8 @@ namespace Unity.Netcode
     /// </summary>
     internal interface ISceneManagerHandler
     {
-        // Generic action to call when a scene is finished loading/unloading
-        struct SceneEventAction
-        {
-            internal uint SceneEventId;
-            internal Action<uint> EventAction;
-            /// <summary>
-            /// Used server-side for integration testing in order to
-            /// invoke the SceneEventProgress once done loading
-            /// </summary>
-            internal Action Completed;
-            internal void Invoke()
-            {
-                Completed?.Invoke();
-                EventAction.Invoke(SceneEventId);
-            }
-        }
+        AsyncOperation LoadSceneAsync(string sceneName, LoadSceneMode loadSceneMode, SceneEventProgress sceneEventProgress);
 
-        AsyncOperation LoadSceneAsync(string sceneName, LoadSceneMode loadSceneMode, SceneEventAction sceneEventAction);
-
-        AsyncOperation UnloadSceneAsync(Scene scene, SceneEventAction sceneEventAction);
+        AsyncOperation UnloadSceneAsync(Scene scene, SceneEventProgress sceneEventProgress);
     }
 }

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

@@ -338,17 +338,17 @@ public class NetworkSceneManager : IDisposable
         /// </summary>
         private class DefaultSceneManagerHandler : ISceneManagerHandler
         {
-            public AsyncOperation LoadSceneAsync(string sceneName, LoadSceneMode loadSceneMode, ISceneManagerHandler.SceneEventAction sceneEventAction)
+            public AsyncOperation LoadSceneAsync(string sceneName, LoadSceneMode loadSceneMode, SceneEventProgress sceneEventProgress)
             {
                 var operation = SceneManager.LoadSceneAsync(sceneName, loadSceneMode);
-                operation.completed += new Action<AsyncOperation>(asyncOp2 => { sceneEventAction.Invoke(); });
+                sceneEventProgress.SetAsyncOperation(operation);
                 return operation;
             }
 
-            public AsyncOperation UnloadSceneAsync(Scene scene, ISceneManagerHandler.SceneEventAction sceneEventAction)
+            public AsyncOperation UnloadSceneAsync(Scene scene, SceneEventProgress sceneEventProgress)
             {
                 var operation = SceneManager.UnloadSceneAsync(scene);
-                operation.completed += new Action<AsyncOperation>(asyncOp2 => { sceneEventAction.Invoke(); });
+                sceneEventProgress.SetAsyncOperation(operation);
                 return operation;
             }
         }
@@ -887,12 +887,14 @@ private SceneEventProgress ValidateSceneEvent(string sceneName, bool isUnloading
         private bool OnSceneEventProgressCompleted(SceneEventProgress sceneEventProgress)
         {
             var sceneEventData = BeginSceneEvent();
+            var clientsThatCompleted = sceneEventProgress.GetClientsWithStatus(true);
+            var clientsThatTimedOut = sceneEventProgress.GetClientsWithStatus(false);
             sceneEventData.SceneEventProgressId = sceneEventProgress.Guid;
             sceneEventData.SceneHash = sceneEventProgress.SceneHash;
             sceneEventData.SceneEventType = sceneEventProgress.SceneEventType;
-            sceneEventData.ClientsCompleted = sceneEventProgress.DoneClients;
+            sceneEventData.ClientsCompleted = clientsThatCompleted;
             sceneEventData.LoadSceneMode = sceneEventProgress.LoadSceneMode;
-            sceneEventData.ClientsTimedOut = sceneEventProgress.ClientsThatStartedSceneEvent.Except(sceneEventProgress.DoneClients).ToList();
+            sceneEventData.ClientsTimedOut = clientsThatTimedOut;
 
             var message = new SceneEventMessage
             {
@@ -913,8 +915,8 @@ private bool OnSceneEventProgressCompleted(SceneEventProgress sceneEventProgress
                 SceneName = SceneNameFromHash(sceneEventProgress.SceneHash),
                 ClientId = NetworkManager.ServerClientId,
                 LoadSceneMode = sceneEventProgress.LoadSceneMode,
-                ClientsThatCompleted = sceneEventProgress.DoneClients,
-                ClientsThatTimedOut = m_NetworkManager.ConnectedClients.Keys.Except(sceneEventProgress.DoneClients).ToList(),
+                ClientsThatCompleted = clientsThatCompleted,
+                ClientsThatTimedOut = clientsThatTimedOut,
             });
 
             if (sceneEventData.SceneEventType == SceneEventType.LoadEventCompleted)
@@ -969,18 +971,9 @@ public SceneEventProgressStatus UnloadScene(Scene scene)
             sceneEventProgress.SceneEventType = SceneEventType.UnloadEventCompleted;
 
             ScenesLoaded.Remove(scene.handle);
-            var sceneEventAction = new ISceneManagerHandler.SceneEventAction() { SceneEventId = sceneEventData.SceneEventId, EventAction = OnSceneUnloaded };
-            var sceneUnload = SceneManagerHandler.UnloadSceneAsync(scene, sceneEventAction);
-
-            // If integration testing, IntegrationTestSceneHandler returns null
-            if (sceneUnload == null)
-            {
-                sceneEventProgress.SetSceneLoadOperation(sceneEventAction);
-            }
-            else
-            {
-                sceneEventProgress.SetSceneLoadOperation(sceneUnload);
-            }
+            sceneEventProgress.SceneEventId = sceneEventData.SceneEventId;
+            sceneEventProgress.OnSceneEventCompleted = OnSceneUnloaded;
+            var sceneUnload = SceneManagerHandler.UnloadSceneAsync(scene, sceneEventProgress);
 
             // Notify local server that a scene is going to be unloaded
             OnSceneEvent?.Invoke(new SceneEvent()
@@ -1024,9 +1017,10 @@ private void OnClientUnloadScene(uint sceneEventId)
                     $"because the client scene handle {sceneHandle} was not found in ScenesLoaded!");
             }
             m_IsSceneEventActive = true;
-
-            var sceneUnload = SceneManagerHandler.UnloadSceneAsync(ScenesLoaded[sceneHandle],
-                new ISceneManagerHandler.SceneEventAction() { SceneEventId = sceneEventData.SceneEventId, EventAction = OnSceneUnloaded });
+            var sceneEventProgress = new SceneEventProgress(m_NetworkManager);
+            sceneEventProgress.SceneEventId = sceneEventData.SceneEventId;
+            sceneEventProgress.OnSceneEventCompleted = OnSceneUnloaded;
+            var sceneUnload = SceneManagerHandler.UnloadSceneAsync(ScenesLoaded[sceneHandle], sceneEventProgress);
 
             ScenesLoaded.Remove(sceneHandle);
 
@@ -1070,7 +1064,7 @@ private void OnSceneUnloaded(uint sceneEventId)
                 //Only if we are a host do we want register having loaded for the associated SceneEventProgress
                 if (SceneEventProgressTracking.ContainsKey(sceneEventData.SceneEventProgressId) && m_NetworkManager.IsHost)
                 {
-                    SceneEventProgressTracking[sceneEventData.SceneEventProgressId].AddClientAsDone(NetworkManager.ServerClientId);
+                    SceneEventProgressTracking[sceneEventData.SceneEventProgressId].ClientFinishedSceneEvent(NetworkManager.ServerClientId);
                 }
             }
 
@@ -1119,8 +1113,10 @@ internal void UnloadAdditivelyLoadedScenes(uint sceneEventId)
                 // Validate the scene as well as ignore the DDOL (which will have a negative buildIndex)
                 if (currentActiveScene.name != keyHandleEntry.Value.name && keyHandleEntry.Value.buildIndex >= 0)
                 {
-                    var sceneUnload = SceneManagerHandler.UnloadSceneAsync(keyHandleEntry.Value,
-                        new ISceneManagerHandler.SceneEventAction() { SceneEventId = sceneEventId, EventAction = EmptySceneUnloadedOperation });
+                    var sceneEventProgress = new SceneEventProgress(m_NetworkManager);
+                    sceneEventProgress.SceneEventId = sceneEventId;
+                    sceneEventProgress.OnSceneEventCompleted = EmptySceneUnloadedOperation;
+                    var sceneUnload = SceneManagerHandler.UnloadSceneAsync(keyHandleEntry.Value, sceneEventProgress);
                     SceneUnloadEventHandler.RegisterScene(this, keyHandleEntry.Value, LoadSceneMode.Additive, sceneUnload);
                 }
             }
@@ -1180,18 +1176,9 @@ public SceneEventProgressStatus LoadScene(string sceneName, LoadSceneMode loadSc
             }
 
             // 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)
-            {
-                sceneEventProgress.SetSceneLoadOperation(sceneEventAction);
-            }
-            else
-            {
-                sceneEventProgress.SetSceneLoadOperation(sceneLoad);
-            }
-
+            sceneEventProgress.SceneEventId = sceneEventId;
+            sceneEventProgress.OnSceneEventCompleted = OnSceneLoaded;
+            var sceneLoad = SceneManagerHandler.LoadSceneAsync(sceneName, loadSceneMode, sceneEventProgress);
             // Notify the local server that a scene loading event has begun
             OnSceneEvent?.Invoke(new SceneEvent()
             {
@@ -1355,9 +1342,10 @@ private void OnClientSceneLoadingEvent(uint sceneEventId)
                 SceneUnloadEventHandler.RegisterScene(this, SceneManager.GetActiveScene(), LoadSceneMode.Single);
 
             }
-
-            var sceneLoad = SceneManagerHandler.LoadSceneAsync(sceneName, sceneEventData.LoadSceneMode,
-                new ISceneManagerHandler.SceneEventAction() { SceneEventId = sceneEventId, EventAction = OnSceneLoaded });
+            var sceneEventProgress = new SceneEventProgress(m_NetworkManager);
+            sceneEventProgress.SceneEventId = sceneEventId;
+            sceneEventProgress.OnSceneEventCompleted = OnSceneLoaded;
+            var sceneLoad = SceneManagerHandler.LoadSceneAsync(sceneName, sceneEventData.LoadSceneMode, sceneEventProgress);
 
             OnSceneEvent?.Invoke(new SceneEvent()
             {
@@ -1488,7 +1476,7 @@ private void OnServerLoadedScene(uint sceneEventId, Scene scene)
             //Second, only if we are a host do we want register having loaded for the associated SceneEventProgress
             if (SceneEventProgressTracking.ContainsKey(sceneEventData.SceneEventProgressId) && m_NetworkManager.IsHost)
             {
-                SceneEventProgressTracking[sceneEventData.SceneEventProgressId].AddClientAsDone(NetworkManager.ServerClientId);
+                SceneEventProgressTracking[sceneEventData.SceneEventProgressId].ClientFinishedSceneEvent(NetworkManager.ServerClientId);
             }
             EndSceneEvent(sceneEventId);
         }
@@ -1662,8 +1650,10 @@ private void OnClientBeginSync(uint sceneEventId)
             if (!shouldPassThrough)
             {
                 // If not, then load the scene
-                sceneLoad = SceneManagerHandler.LoadSceneAsync(sceneName, loadSceneMode,
-                new ISceneManagerHandler.SceneEventAction() { SceneEventId = sceneEventId, EventAction = ClientLoadedSynchronization });
+                var sceneEventProgress = new SceneEventProgress(m_NetworkManager);
+                sceneEventProgress.SceneEventId = sceneEventId;
+                sceneEventProgress.OnSceneEventCompleted = ClientLoadedSynchronization;
+                sceneLoad = SceneManagerHandler.LoadSceneAsync(sceneName, loadSceneMode, sceneEventProgress);
 
                 // Notify local client that a scene load has begun
                 OnSceneEvent?.Invoke(new SceneEvent()
@@ -1880,7 +1870,7 @@ private void HandleServerSceneEvent(uint sceneEventId, ulong clientId)
 
                         if (SceneEventProgressTracking.ContainsKey(sceneEventData.SceneEventProgressId))
                         {
-                            SceneEventProgressTracking[sceneEventData.SceneEventProgressId].AddClientAsDone(clientId);
+                            SceneEventProgressTracking[sceneEventData.SceneEventProgressId].ClientFinishedSceneEvent(clientId);
                         }
                         EndSceneEvent(sceneEventId);
                         break;
@@ -1889,7 +1879,7 @@ private void HandleServerSceneEvent(uint sceneEventId, ulong clientId)
                     {
                         if (SceneEventProgressTracking.ContainsKey(sceneEventData.SceneEventProgressId))
                         {
-                            SceneEventProgressTracking[sceneEventData.SceneEventProgressId].AddClientAsDone(clientId);
+                            SceneEventProgressTracking[sceneEventData.SceneEventProgressId].ClientFinishedSceneEvent(clientId);
                         }
                         // Notify the local server that the client has finished unloading a scene
                         OnSceneEvent?.Invoke(new SceneEvent()