fix: LoadEventCompleted scene event not invoked when a client disconnects before finishing loading a scene [MTT-3396] (#1973)

* test

This allows me to test this bug to verify it is no longer happening.

* fix

This fixes the issue where if a client disconnects during a scene event it would not get completed.
This will also fix an exception on the client side where if the client was disconnected during a scene event while in the middle of loading or unloading a scene it will not try to continue processing the event.

* test

This includes the integration test that validates the fix.
This also includes some minor adjustments to the integration test scene handler to account for handling a client disconnecting in the middle of a scene loading event.

* Update CHANGELOG.md

* update

MTT-3396
Adding additional code that tracks the initial clients that were connected when the scene event starts in order to provide an accurate list of clients that timed out during scene loading.
Cleaned up some of the code in the integration test as well as added additional comments.

* Update CHANGELOG.md

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

Co-authored-by: LPLafontaineB <[email protected]>

* fix

Fixing an issue where if a virtual mac machine was running slower than expected it would time out on certain deferred message tests.  The primary area impacted was WaitForMessageOfTypeReceived which seemed to not use the DefaultTimeout value but 0.5s as the maximum time it would wait for the message.
This just changes the timeout to the Default timeout for WaitForMessageOfTypeReceived and I noticed WaitForMessageOfTypeHandled used 0.5s for the default time out period and set that to use the DefaultTimeout.

* fix

increasing the timeout period.

Co-authored-by: LPLafontaineB <[email protected]>

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -20,10 +20,10 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ### Removed
 
 ### Fixed
-
+- 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)
-- Fixed endless dialog boxes when adding a NetworkBehaviour to a NetworkManager or vice-versa (#1947)
 - `FixedString` types can now be used in NetworkVariables and RPCs again without requiring a `ForceNetworkSerializeByMemcpy<>` wrapper (#1961)
+- Fixed endless dialog boxes when adding a `NetworkBehaviour` to a `NetworkManager` or vice-versa. (#1947)
 
 ## [1.0.0-pre.9] - 2022-05-10
 

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

@@ -891,7 +891,7 @@ private bool OnSceneEventProgressCompleted(SceneEventProgress sceneEventProgress
             sceneEventData.SceneEventType = sceneEventProgress.SceneEventType;
             sceneEventData.ClientsCompleted = sceneEventProgress.DoneClients;
             sceneEventData.LoadSceneMode = sceneEventProgress.LoadSceneMode;
-            sceneEventData.ClientsTimedOut = m_NetworkManager.ConnectedClients.Keys.Except(sceneEventProgress.DoneClients).ToList();
+            sceneEventData.ClientsTimedOut = sceneEventProgress.ClientsThatStartedSceneEvent.Except(sceneEventProgress.DoneClients).ToList();
 
             var message = new SceneEventMessage
             {
@@ -1052,6 +1052,12 @@ private void OnClientUnloadScene(uint sceneEventId)
         /// </summary>
         private void OnSceneUnloaded(uint sceneEventId)
         {
+            // If we are shutdown or about to shutdown, then ignore this event
+            if (!m_NetworkManager.IsListening || m_NetworkManager.ShutdownInProgress)
+            {
+                return;
+            }
+
             var sceneEventData = SceneEventDataStore[sceneEventId];
             // First thing we do, if we are a server, is to send the unload scene event.
             if (m_NetworkManager.IsServer)
@@ -1257,13 +1263,18 @@ private void OnClientSceneLoadingEvent(uint sceneEventId)
             OnLoad?.Invoke(m_NetworkManager.LocalClientId, sceneName, sceneEventData.LoadSceneMode, sceneLoad);
         }
 
-
         /// <summary>
         /// Client and Server:
         /// Generic on scene loaded callback method to be called upon a scene loading
         /// </summary>
         private void OnSceneLoaded(uint sceneEventId)
         {
+            // If we are shutdown or about to shutdown, then ignore this event
+            if (!m_NetworkManager.IsListening || m_NetworkManager.ShutdownInProgress)
+            {
+                return;
+            }
+
             var sceneEventData = SceneEventDataStore[sceneEventId];
             var nextScene = GetAndAddNewlyLoadedSceneByName(SceneNameFromHash(sceneEventData.SceneHash));
             if (!nextScene.isLoaded || !nextScene.IsValid())

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

@@ -61,9 +61,9 @@ internal class SceneEventProgress
         internal List<ulong> DoneClients { get; } = new List<ulong>();
 
         /// <summary>
-        /// The NetworkTime at the moment the scene switch was initiated by the server.
+        /// The local time when the scene event was "roughly started"
         /// </summary>
-        internal NetworkTime TimeAtInitiation { get; }
+        internal float TimeAtInitiation { get; }
 
         /// <summary>
         /// Delegate type for when the switch scene progress is completed. Either by all clients done loading the scene or by time out.
@@ -105,22 +105,40 @@ internal class SceneEventProgress
 
         internal LoadSceneMode LoadSceneMode;
 
+        internal List<ulong> ClientsThatStartedSceneEvent;
+
         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);
                 m_NetworkManager = networkManager;
                 m_TimeOutCoroutine = m_NetworkManager.StartCoroutine(TimeOutSceneEventProgress());
-                TimeAtInitiation = networkManager.LocalTime;
+                TimeAtInitiation = Time.realtimeSinceStartup;
             }
             Status = status;
         }
 
+        /// <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)
+        /// during a scene event and if it does not complete within the scene loading time out period
+        /// it will time out the scene event.
+        /// </summary>
         internal IEnumerator TimeOutSceneEventProgress()
         {
-            yield return new WaitForSecondsRealtime(m_NetworkManager.NetworkConfig.LoadSceneTimeOut);
-            TimedOut = true;
-            CheckCompletion();
+            var waitForNetworkTick = new WaitForSeconds(1.0f / m_NetworkManager.NetworkConfig.TickRate);
+            while (!TimedOut && !IsCompleted)
+            {
+                yield return waitForNetworkTick;
+
+                CheckCompletion();
+                if (!IsCompleted)
+                {
+                    TimedOut = TimeAtInitiation - Time.realtimeSinceStartup >= m_NetworkManager.NetworkConfig.LoadSceneTimeOut;
+                }
+            }
         }
 
         internal void AddClientAsDone(ulong clientId)

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

@@ -54,6 +54,8 @@ public enum JobTypes
 
         internal NetworkManager NetworkManager;
 
+        internal string NetworkManagerName;
+
         /// <summary>
         /// Used to control when clients should attempt to fake-load a scene
         /// Note: Unit/Integration tests that only use <see cref="NetcodeIntegrationTestHelpers"/>
@@ -211,7 +213,7 @@ static internal IEnumerator JobQueueProcessor()
             while (QueuedSceneJobs.Count != 0)
             {
                 CurrentQueuedSceneJob = QueuedSceneJobs.Dequeue();
-                VerboseDebug($"[ITSH-START] {CurrentQueuedSceneJob.IntegrationTestSceneHandler.NetworkManager.name} processing {CurrentQueuedSceneJob.JobType} for scene {CurrentQueuedSceneJob.SceneName}.");
+                VerboseDebug($"[ITSH-START] {CurrentQueuedSceneJob.IntegrationTestSceneHandler.NetworkManagerName} processing {CurrentQueuedSceneJob.JobType} for scene {CurrentQueuedSceneJob.SceneName}.");
                 if (CurrentQueuedSceneJob.JobType == QueuedSceneJob.JobTypes.Loading)
                 {
                     yield return ProcessLoadingSceneJob(CurrentQueuedSceneJob);
@@ -220,7 +222,7 @@ static internal IEnumerator JobQueueProcessor()
                 {
                     yield return ProcessUnloadingSceneJob(CurrentQueuedSceneJob);
                 }
-                VerboseDebug($"[ITSH-STOP] {CurrentQueuedSceneJob.IntegrationTestSceneHandler.NetworkManager.name} processing {CurrentQueuedSceneJob.JobType} for scene {CurrentQueuedSceneJob.SceneName}.");
+                VerboseDebug($"[ITSH-STOP] {CurrentQueuedSceneJob.IntegrationTestSceneHandler.NetworkManagerName} processing {CurrentQueuedSceneJob.JobType} for scene {CurrentQueuedSceneJob.SceneName}.");
             }
             SceneJobProcessor = null;
             yield break;
@@ -357,6 +359,7 @@ public IntegrationTestSceneHandler(NetworkManager networkManager)
         {
             networkManager.SceneManager.OverrideGetAndAddNewlyLoadedSceneByName = GetAndAddNewlyLoadedSceneByName;
             NetworkManagers.Add(networkManager);
+            NetworkManagerName = networkManager.name;
             if (s_WaitForSeconds == null)
             {
                 s_WaitForSeconds = new WaitForSeconds(1.0f / networkManager.NetworkConfig.TickRate);

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

@@ -16,7 +16,7 @@ namespace Unity.Netcode.TestHelpers.Runtime
     public static class NetcodeIntegrationTestHelpers
     {
         public const int DefaultMinFrames = 1;
-        public const float DefaultTimeout = 2f;
+        public const float DefaultTimeout = 4f;
         private static List<NetworkManager> s_NetworkManagerInstances = new List<NetworkManager>();
         private static Dictionary<NetworkManager, MultiInstanceHooks> s_Hooks = new Dictionary<NetworkManager, MultiInstanceHooks>();
         private static bool s_IsStarted;
@@ -733,7 +733,7 @@ public static IEnumerator WaitForCondition(Func<bool> predicate, ResultWrapper<b
         /// </summary>
         /// <param name="result">The result. If null, it will fail if the predicate is not met</param>
         /// <param name="timeout">The max time in seconds to wait for</param>
-        internal static IEnumerator WaitForMessageOfTypeReceived<T>(NetworkManager toBeReceivedBy, ResultWrapper<bool> result = null, float timeout = 0.5f) where T : INetworkMessage
+        internal static IEnumerator WaitForMessageOfTypeReceived<T>(NetworkManager toBeReceivedBy, ResultWrapper<bool> result = null, float timeout = DefaultTimeout) where T : INetworkMessage
         {
             var hooks = s_Hooks[toBeReceivedBy];
             var check = new MessageReceiveCheckWithResult { CheckType = typeof(T) };
@@ -761,7 +761,7 @@ internal static IEnumerator WaitForMessageOfTypeReceived<T>(NetworkManager toBeR
         /// </summary>
         /// <param name="result">The result. If null, it will fail if the predicate is not met</param>
         /// <param name="timeout">The max time in seconds to wait for</param>
-        internal static IEnumerator WaitForMessageOfTypeHandled<T>(NetworkManager toBeReceivedBy, ResultWrapper<bool> result = null, float timeout = 0.5f) where T : INetworkMessage
+        internal static IEnumerator WaitForMessageOfTypeHandled<T>(NetworkManager toBeReceivedBy, ResultWrapper<bool> result = null, float timeout = DefaultTimeout) where T : INetworkMessage
         {
             var hooks = s_Hooks[toBeReceivedBy];
             if (!hooks.HandleChecks.ContainsKey(typeof(T)))