fix: SceneEventProgress OnComplete not invoked on timeouts [MTT-4772]

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -21,6 +21,9 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- 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

@@ -11,7 +11,7 @@ namespace Unity.Netcode
     internal interface ISceneManagerHandler
     {
         // Generic action to call when a scene is finished loading/unloading
-        struct SceneEventAction
+        class SceneEventAction
         {
             internal uint SceneEventId;
             internal Action<uint> EventAction;

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

@@ -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)
@@ -1070,7 +1072,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);
                 }
             }
 
@@ -1488,7 +1490,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);
         }
@@ -1880,7 +1882,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 +1891,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()

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

@@ -58,12 +58,13 @@ internal class SceneEventProgress
         /// <summary>
         /// List of clientIds of those clients that is done loading the scene.
         /// </summary>
-        internal List<ulong> DoneClients { get; } = new List<ulong>();
+        internal Dictionary<ulong, bool> ClientsProcessingSceneEvent { get; } = new Dictionary<ulong, bool>();
+        internal List<ulong> ClientsThatDisconnected = new List<ulong>();
 
         /// <summary>
-        /// The local time when the scene event was "roughly started"
+        /// This is the time when the current scene event will have timed out
         /// </summary>
-        internal float TimeAtInitiation { get; }
+        internal float WhenSceneEventHasTimedOut;
 
         /// <summary>
         /// Delegate type for when the switch scene progress is completed. Either by all clients done loading the scene or by time out.
@@ -76,16 +77,17 @@ internal class SceneEventProgress
         internal OnCompletedDelegate OnComplete;
 
         /// <summary>
-        /// Is this scene switch progresses completed, all clients are done loading the scene or a timeout has occurred.
+        /// When set the scene event is considered completed locally
         /// </summary>
-        internal bool IsCompleted { get; private set; }
-
-        internal bool TimedOut { get; private set; }
+        internal bool IsSceneEventCompleted { get; private set; }
 
         /// <summary>
-        /// If all clients are done loading the scene, at the moment of completed.
+        /// This will make sure that we only have timed out if we never completed
         /// </summary>
-        internal bool AreAllClientsDoneLoading { get; private set; }
+        internal bool HasTimedOut()
+        {
+            return WhenSceneEventHasTimedOut <= Time.realtimeSinceStartup;
+        }
 
         /// <summary>
         /// The hash value generated from the full scene path
@@ -105,21 +107,57 @@ internal class SceneEventProgress
 
         internal LoadSceneMode LoadSceneMode;
 
-        internal List<ulong> ClientsThatStartedSceneEvent;
+        internal List<ulong> GetClientsWithStatus(bool completedSceneEvent)
+        {
+            var clients = new List<ulong>();
+            foreach (var clientStatus in ClientsProcessingSceneEvent)
+            {
+                if (clientStatus.Value == completedSceneEvent)
+                {
+                    clients.Add(clientStatus.Key);
+                }
+            }
+
+            // If we are getting the list of clients that have not completed the
+            // scene event, then add any clients that disconnected during this
+            // scene event.
+            if (!completedSceneEvent)
+            {
+                clients.AddRange(ClientsThatDisconnected);
+            }
+            return clients;
+        }
 
         internal SceneEventProgress(NetworkManager networkManager, SceneEventProgressStatus status = SceneEventProgressStatus.Started)
         {
             if (status == SceneEventProgressStatus.Started)
             {
                 // Track the clients that were connected when we started this event
-                ClientsThatStartedSceneEvent = new List<ulong>(networkManager.ConnectedClientsIds);
+                foreach (var connectedClientId in networkManager.ConnectedClientsIds)
+                {
+                    ClientsProcessingSceneEvent.Add(connectedClientId, false);
+                }
                 m_NetworkManager = networkManager;
+                m_NetworkManager.OnClientDisconnectCallback += OnClientDisconnectCallback;
+                WhenSceneEventHasTimedOut = Time.realtimeSinceStartup + networkManager.NetworkConfig.LoadSceneTimeOut;
                 m_TimeOutCoroutine = m_NetworkManager.StartCoroutine(TimeOutSceneEventProgress());
-                TimeAtInitiation = Time.realtimeSinceStartup;
             }
             Status = status;
         }
 
+        /// <summary>
+        /// Remove the client from the clients processing the current scene event
+        /// Add this client to the clients that disconnected list
+        /// </summary>
+        private void OnClientDisconnectCallback(ulong clientId)
+        {
+            if (ClientsProcessingSceneEvent.ContainsKey(clientId))
+            {
+                ClientsThatDisconnected.Add(clientId);
+                ClientsProcessingSceneEvent.Remove(clientId);
+            }
+        }
+
         /// <summary>
         /// Coroutine that checks to see if the scene event is complete every network tick period.
         /// This will handle completing the scene event when one or more client(s) disconnect(s)
@@ -129,34 +167,46 @@ internal SceneEventProgress(NetworkManager networkManager, SceneEventProgressSta
         internal IEnumerator TimeOutSceneEventProgress()
         {
             var waitForNetworkTick = new WaitForSeconds(1.0f / m_NetworkManager.NetworkConfig.TickRate);
-            while (!TimedOut && !IsCompleted)
+            while (!HasTimedOut())
             {
                 yield return waitForNetworkTick;
 
-                CheckCompletion();
-                if (!IsCompleted)
-                {
-                    TimedOut = TimeAtInitiation - Time.realtimeSinceStartup >= m_NetworkManager.NetworkConfig.LoadSceneTimeOut;
-                }
+                TryFinishingSceneEventProgress();
             }
         }
 
-        internal void AddClientAsDone(ulong clientId)
+        /// <summary>
+        /// Sets the client's scene event progress to finished/true
+        /// </summary>
+        internal void ClientFinishedSceneEvent(ulong clientId)
         {
-            DoneClients.Add(clientId);
-            CheckCompletion();
+            if (ClientsProcessingSceneEvent.ContainsKey(clientId))
+            {
+                ClientsProcessingSceneEvent[clientId] = true;
+                TryFinishingSceneEventProgress();
+            }
         }
 
-        internal void RemoveClientAsDone(ulong clientId)
+        /// <summary>
+        /// Checks if all known clients processing this scene event
+        /// are finished.
+        /// </summary>
+        private bool AreAllClientsFinished()
         {
-            DoneClients.Remove(clientId);
-            CheckCompletion();
+            foreach (var clientStatus in ClientsProcessingSceneEvent)
+            {
+                if (!clientStatus.Value)
+                {
+                    return false;
+                }
+            }
+            return true;
         }
 
         internal void SetSceneLoadOperation(AsyncOperation sceneLoadOperation)
         {
             m_SceneLoadOperation = sceneLoadOperation;
-            m_SceneLoadOperation.completed += operation => CheckCompletion();
+            m_SceneLoadOperation.completed += operation => TryFinishingSceneEventProgress();
         }
 
         /// <summary>
@@ -171,37 +221,40 @@ internal void SetSceneLoadOperation(AsyncOperation sceneLoadOperation)
         /// </summary>
         internal void SetSceneLoadOperation(ISceneManagerHandler.SceneEventAction sceneEventAction)
         {
-            sceneEventAction.Completed = SetComplete;
+            sceneEventAction.Completed = SetLoadCompleted;
         }
 
         /// <summary>
-        /// Finalizes the SceneEventProgress
+        /// Server is the only one who invokes this during integration tests
+        /// This is triggered once the server-side is done loading its scene.
         /// </summary>
-        internal void SetComplete()
+        internal void SetLoadCompleted()
         {
-            IsCompleted = true;
-            AreAllClientsDoneLoading = true;
-
-            // If OnComplete is not registered or it is and returns true then remove this from the progress tracking
-            if (OnComplete == null || (OnComplete != null && OnComplete.Invoke(this)))
-            {
-                m_NetworkManager.SceneManager.SceneEventProgressTracking.Remove(Guid);
-            }
-            m_NetworkManager.StopCoroutine(m_TimeOutCoroutine);
+            // If this method is invoked, then we know the server already loaded the scene.
+            IsSceneEventCompleted = true;
+            TryFinishingSceneEventProgress();
         }
 
-        internal void CheckCompletion()
+        /// <summary>
+        /// Will try to finish the current scene event in progress as long as all conditions are met.
+        /// </summary>
+        internal void TryFinishingSceneEventProgress()
         {
-            try
+            // m_SceneLoadOperation is set on the server side and so
+            // we always make sure the server has completed the scene
+            // event before we check to see if the clients have finished
+            // or we have timed out.
+            if (m_SceneLoadOperation != null && !IsSceneEventCompleted)
             {
-                if ((!IsCompleted && DoneClients.Count == m_NetworkManager.ConnectedClientsList.Count && (m_SceneLoadOperation == null || m_SceneLoadOperation.isDone)) || (!IsCompleted && TimedOut))
-                {
-                    SetComplete();
-                }
+                IsSceneEventCompleted = m_SceneLoadOperation.isDone;
             }
-            catch (Exception ex)
+
+            if (IsSceneEventCompleted && (AreAllClientsFinished() || HasTimedOut()))
             {
-                Debug.LogException(ex);
+                OnComplete?.Invoke(this);
+                m_NetworkManager.SceneManager.SceneEventProgressTracking.Remove(Guid);
+                m_NetworkManager.OnClientDisconnectCallback -= OnClientDisconnectCallback;
+                m_NetworkManager.StopCoroutine(m_TimeOutCoroutine);
             }
         }
     }