fix: SceneEventProgress was triggering twice when running a host and no clients were connected

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -14,6 +14,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed issue where the host would receive more than one event completed notification when loading or unloading a scene only when no clients were connected. (#2292)
 - Fixed issue where in-scene placed `NetworkObjects` were not honoring the `AutoObjectParentSync` property. (#2281)
 - Fixed the issue where `NetworkManager.OnClientConnectedCallback` was being invoked before in-scene placed `NetworkObject`s had been spawned when starting `NetworkManager` as a host. (#2277)
 - Creating a `FastBufferReader` with `Allocator.None` will not result in extra memory being allocated for the buffer (since it's owned externally in that scenario). (#2265)

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

@@ -108,19 +108,37 @@ internal bool HasTimedOut()
         internal List<ulong> GetClientsWithStatus(bool completedSceneEvent)
         {
             var clients = new List<ulong>();
-            foreach (var clientStatus in ClientsProcessingSceneEvent)
+            if (completedSceneEvent)
             {
-                if (clientStatus.Value == completedSceneEvent)
+                // If we are the host, then add the host-client to the list
+                // of clients that completed if the AsyncOperation is done.
+                if (m_NetworkManager.IsHost && m_AsyncOperation.isDone)
                 {
-                    clients.Add(clientStatus.Key);
+                    clients.Add(m_NetworkManager.LocalClientId);
                 }
-            }
 
-            // 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)
+                // Add all clients that completed the scene event
+                foreach (var clientStatus in ClientsProcessingSceneEvent)
+                {
+                    if (clientStatus.Value == completedSceneEvent)
+                    {
+                        clients.Add(clientStatus.Key);
+                    }
+                }
+            }
+            else
             {
+                // If we are the host, then add the host-client to the list
+                // of clients that did not complete if the AsyncOperation is
+                // not done.
+                if (m_NetworkManager.IsHost && !m_AsyncOperation.isDone)
+                {
+                    clients.Add(m_NetworkManager.LocalClientId);
+                }
+
+                // 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.
                 clients.AddRange(ClientsThatDisconnected);
             }
             return clients;
@@ -138,6 +156,11 @@ internal SceneEventProgress(NetworkManager networkManager, SceneEventProgressSta
                     // Track the clients that were connected when we started this event
                     foreach (var connectedClientId in networkManager.ConnectedClientsIds)
                     {
+                        // Ignore the host client
+                        if (NetworkManager.ServerClientId == connectedClientId)
+                        {
+                            continue;
+                        }
                         ClientsProcessingSceneEvent.Add(connectedClientId, false);
                     }
 
@@ -218,7 +241,10 @@ private bool HasFinished()
             }
 
             // Return the local scene event's AsyncOperation status
-            return m_AsyncOperation.isDone;
+            // Note: Integration tests process scene loading through a queue
+            // and the AsyncOperation could not be assigned for several
+            // network tick periods. Return false if that is the case.
+            return m_AsyncOperation == null ? false : m_AsyncOperation.isDone;
         }
 
         /// <summary>

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

@@ -274,16 +274,6 @@ protected void CreateServerAndClients()
             CreateServerAndClients(NumberOfClients);
         }
 
-        protected virtual void OnNewClientCreated(NetworkManager networkManager)
-        {
-
-        }
-
-        protected virtual void OnNewClientStartedAndConnected(NetworkManager networkManager)
-        {
-
-        }
-
         private void AddRemoveNetworkManager(NetworkManager networkManager, bool addNetworkManager)
         {
             var clientNetworkManagersList = new List<NetworkManager>(m_ClientNetworkManagers);
@@ -299,6 +289,37 @@ private void AddRemoveNetworkManager(NetworkManager networkManager, bool addNetw
             m_NumberOfClients = clientNetworkManagersList.Count;
         }
 
+        /// <summary>
+        /// CreateAndStartNewClient Only
+        /// Invoked when the newly created client has been created
+        /// </summary>
+        protected virtual void OnNewClientCreated(NetworkManager networkManager)
+        {
+
+        }
+
+        /// <summary>
+        /// CreateAndStartNewClient Only
+        /// Invoked when the newly created client has been created and started
+        /// </summary>
+        protected virtual void OnNewClientStarted(NetworkManager networkManager)
+        {
+        }
+
+        /// <summary>
+        /// CreateAndStartNewClient Only
+        /// Invoked when the newly created client has been created, started, and connected
+        /// to the server-host.
+        /// </summary>
+        protected virtual void OnNewClientStartedAndConnected(NetworkManager networkManager)
+        {
+
+        }
+
+        /// <summary>
+        /// This will create, start, and connect a new client while in the middle of an
+        /// integration test.
+        /// </summary>
         protected IEnumerator CreateAndStartNewClient()
         {
             var networkManager = NetcodeIntegrationTestHelpers.CreateNewClient(m_ClientNetworkManagers.Length);
@@ -309,9 +330,15 @@ protected IEnumerator CreateAndStartNewClient()
             OnNewClientCreated(networkManager);
 
             NetcodeIntegrationTestHelpers.StartOneClient(networkManager);
+
             AddRemoveNetworkManager(networkManager, true);
+
+            OnNewClientStarted(networkManager);
+
             // Wait for the new client to connect
             yield return WaitForClientsConnectedOrTimeOut();
+
+            OnNewClientStartedAndConnected(networkManager);
             if (s_GlobalTimeoutHelper.TimedOut)
             {
                 AddRemoveNetworkManager(networkManager, false);
@@ -322,6 +349,9 @@ protected IEnumerator CreateAndStartNewClient()
             VerboseDebug($"[{networkManager.name}] Created and connected!");
         }
 
+        /// <summary>
+        /// This will stop a client while in the middle of an integration test
+        /// </summary>
         protected IEnumerator StopOneClient(NetworkManager networkManager, bool destroy = false)
         {
             NetcodeIntegrationTestHelpers.StopOneClient(networkManager, destroy);

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

@@ -1,6 +1,5 @@
 using System.Collections;
 using System.Collections.Generic;
-using System.Linq;
 using NUnit.Framework;
 using UnityEngine;
 using UnityEngine.SceneManagement;
@@ -12,48 +11,23 @@ namespace TestProject.RuntimeTests
 {
     public class ClientSynchronizationValidationTest : NetcodeIntegrationTest
     {
-        protected override int NumberOfClients => 1;
+        protected override int NumberOfClients => 0;
         private const string k_FirstSceneToLoad = "UnitTestBaseScene";
-        private const string k_SecondSceneToSkip = "InSceneNetworkObject";
-        private const string k_ThirdSceneToLoad = "EmptyScene";
-        private bool m_CanStartServerAndClients;
-        private List<ClientSceneVerificationHandler> m_ClientSceneVerifiers = new List<ClientSceneVerificationHandler>();
+        private const string k_SecondSceneToLoad = "InSceneNetworkObject";
+        private const string k_ThirdSceneToSkip = "EmptyScene";
 
-        protected override bool CanStartServerAndClients()
-        {
-            return m_CanStartServerAndClients;
-        }
+        private List<ClientSceneVerificationHandler> m_ClientSceneVerifiers = new List<ClientSceneVerificationHandler>();
 
-        protected override IEnumerator OnStartedServerAndClients()
+        protected override void OnNewClientStarted(NetworkManager networkManager)
         {
-            // Create ClientSceneVerificationHandlers for each client
-            foreach (var client in m_ClientNetworkManagers)
-            {
-                m_ClientSceneVerifiers.Add(new ClientSceneVerificationHandler(client));
-            }
-            return base.OnStartedServerAndClients();
+            m_ClientSceneVerifiers.Add(new ClientSceneVerificationHandler(networkManager));
+            base.OnNewClientStarted(networkManager);
         }
 
         [UnityTest]
         public IEnumerator ClientVerifySceneBeforeLoading()
         {
-            // Because despawning a client will cause it to shutdown and clean everything in the
-            // scene hierarchy, we have to prevent one of the clients from spawning initially before
-            // we test synchronizing late joining clients.
-            // So, we prevent the automatic starting of the server and clients, remove the client we
-            // will be targeting to join late from the m_ClientNetworkManagers array, start the server
-            // and the remaining client, despawn the in-scene NetworkObject, and then start and synchronize
-            // the clientToTest.
-            var clientToTest = m_ClientNetworkManagers[0];
-            var clients = m_ClientNetworkManagers.ToList();
-            clients.Remove(clientToTest);
-            m_ClientNetworkManagers = clients.ToArray();
-            m_CanStartServerAndClients = true;
-            yield return StartServerAndClients();
-            clients.Add(clientToTest);
-            m_ClientNetworkManagers = clients.ToArray();
-
-            var scenesToLoad = new List<string>() { k_FirstSceneToLoad, k_SecondSceneToSkip, k_ThirdSceneToLoad };
+            var scenesToLoad = new List<string>() { k_FirstSceneToLoad, k_SecondSceneToLoad, k_ThirdSceneToSkip };
             m_ServerNetworkManager.SceneManager.OnLoadComplete += OnLoadComplete;
             foreach (var sceneToLoad in scenesToLoad)
             {
@@ -65,15 +39,10 @@ public IEnumerator ClientVerifySceneBeforeLoading()
                 AssertOnTimeout($"Timed out waiting for scene {m_SceneBeingLoaded} to finish loading!");
             }
 
-            // Now late join a client to make sure the client synchronizes to 2 of the 3 scenes loaded
-            NetcodeIntegrationTestHelpers.StartOneClient(clientToTest);
-            yield return WaitForConditionOrTimeOut(() => (clientToTest.IsConnectedClient && clientToTest.IsListening));
-            AssertOnTimeout($"Timed out waiting for {clientToTest.name} to reconnect!");
-
-            yield return s_DefaultWaitForTick;
+            yield return CreateAndStartNewClient();
 
-            // Update the newly joined client information
-            ClientNetworkManagerPostStartInit();
+            yield return WaitForConditionOrTimeOut(m_ClientSceneVerifiers[0].HasLoadedExpectedScenes);
+            AssertOnTimeout($"Timed out waiting for the client to have loaded the expected scenes");
 
             // Check to make sure only the two scenes were loaded and one
             // completely skipped.
@@ -110,15 +79,20 @@ public ClientSceneVerificationHandler(NetworkManager networkManager)
                 m_NetworkManager.SceneManager.OnLoad += ClientSceneManager_OnLoad;
                 m_NetworkManager.SceneManager.OnLoadComplete += ClientSceneManager_OnLoadComplete;
                 m_ValidSceneEventCount.Add(k_FirstSceneToLoad, 0);
-                m_ValidSceneEventCount.Add(k_SecondSceneToSkip, 0);
-                m_ValidSceneEventCount.Add(k_ThirdSceneToLoad, 0);
+                m_ValidSceneEventCount.Add(k_SecondSceneToLoad, 0);
+                m_ValidSceneEventCount.Add(k_ThirdSceneToSkip, 0);
+            }
+
+            public bool HasLoadedExpectedScenes()
+            {
+                return m_ValidSceneEventCount[k_FirstSceneToLoad] == 2 && m_ValidSceneEventCount[k_SecondSceneToLoad] == 2;
             }
 
             public void ValidateScenesLoaded()
             {
-                Assert.IsFalse(m_ValidSceneEventCount[k_SecondSceneToSkip] > 0, $"Client still loaded the invalidated scene {k_SecondSceneToSkip}!");
-                Assert.IsTrue(m_ValidSceneEventCount[k_FirstSceneToLoad] == 1, $"Client did not load and process the validated scene {k_FirstSceneToLoad}! Expected (1) but was ({m_ValidSceneEventCount[k_FirstSceneToLoad]})");
-                Assert.IsTrue(m_ValidSceneEventCount[k_ThirdSceneToLoad] == 1, $"Client did not load and process the validated scene {k_ThirdSceneToLoad}! Expected (1) but was ({m_ValidSceneEventCount[k_ThirdSceneToLoad]})");
+                Assert.IsTrue(m_ValidSceneEventCount[k_ThirdSceneToSkip] == 0, $"Client still loaded the invalidated scene {k_ThirdSceneToSkip}!");
+                Assert.IsTrue(m_ValidSceneEventCount[k_FirstSceneToLoad] == 2, $"Client did not load and process the validated scene {k_FirstSceneToLoad}! Expected (1) but was ({m_ValidSceneEventCount[k_FirstSceneToLoad]})");
+                Assert.IsTrue(m_ValidSceneEventCount[k_SecondSceneToLoad] == 2, $"Client did not load and process the validated scene {k_SecondSceneToLoad}! Expected (1) but was ({m_ValidSceneEventCount[k_SecondSceneToLoad]})");
             }
 
             private void ClientSceneManager_OnLoadComplete(ulong clientId, string sceneName, LoadSceneMode loadSceneMode)
@@ -139,7 +113,7 @@ private void ClientSceneManager_OnLoad(ulong clientId, string sceneName, LoadSce
 
             private bool VerifySceneBeforeLoading(int sceneIndex, string sceneName, LoadSceneMode loadSceneMode)
             {
-                if (sceneName == k_SecondSceneToSkip)
+                if (sceneName == k_ThirdSceneToSkip)
                 {
                     return false;
                 }

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

@@ -134,5 +134,41 @@ private void ClientSceneManager_OnLoadComplete(ulong clientId, string sceneName,
                 m_ClientLoadedScene = true;
             }
         }
+
+        private int m_LoadEventCompletedInvocationCount;
+
+        /// <summary>
+        /// This test validates that a host only receives one OnLoadEventCompleted callback per scene loading event when
+        /// no clients are connected.
+        /// Note: the fix was within SceneEventProgress but is associated with NetworkSceneManager
+        /// </summary>
+        [UnityTest]
+        public IEnumerator HostReceivesOneLoadEventCompletedNotification()
+        {
+            // Host only test
+            if (!m_UseHost)
+            {
+                yield break;
+            }
+
+            yield return StopOneClient(m_ClientNetworkManagers[0]);
+            m_LoadEventCompletedInvocationCount = 0;
+            m_ServerNetworkManager.SceneManager.OnLoadEventCompleted += SceneManager_OnLoadEventCompleted;
+
+            var retStatus = m_ServerNetworkManager.SceneManager.LoadScene(k_AdditiveScene1, LoadSceneMode.Additive);
+            Assert.AreEqual(retStatus, SceneEventProgressStatus.Started);
+            yield return WaitForConditionOrTimeOut(() => m_LoadEventCompletedInvocationCount > 0);
+            AssertOnTimeout($"Host timed out loading scene {k_AdditiveScene1} additively!");
+
+            // Wait one tick to make sure any other notifications are not triggered.
+            yield return s_DefaultWaitForTick;
+
+            Assert.IsTrue(m_LoadEventCompletedInvocationCount == 1, $"Expected OnLoadEventCompleted to be triggered once but was triggered {m_LoadEventCompletedInvocationCount} times!");
+        }
+
+        private void SceneManager_OnLoadEventCompleted(string sceneName, LoadSceneMode loadSceneMode, System.Collections.Generic.List<ulong> clientsCompleted, System.Collections.Generic.List<ulong> clientsTimedOut)
+        {
+            m_LoadEventCompletedInvocationCount++;
+        }
     }
 }