fix: clientchangedscene event called too early (#776)

* bug: invoke happens to early. player not ready * fix: invoke last * fix: call event after the additive scene list is populated * fix test * fix test * fix and simplify test
MirageNet · Apr 14, 2021 · 82dda04 · 82dda04
1 parent 5fbdaaf
commit 82dda04
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 12 deletions.
diff --git a/Assets/Mirage/Runtime/NetworkSceneManager.cs b/Assets/Mirage/Runtime/NetworkSceneManager.cs
@@ -140,9 +140,6 @@ internal void ClientSceneMessage(INetworkPlayer player, SceneMessage msg)
 
             if (logger.LogEnabled()) logger.Log($"ClientSceneMessage: changing scenes from: {ActiveScenePath} to: {msg.scenePath}");
 
-            // Let client prepare for scene change
-            OnClientChangeScene(msg.scenePath, msg.sceneOperation);
-
             //Additive are scenes loaded on server and this client is not a host client
             if (msg.additiveScenes != null && msg.additiveScenes.Length > 0 && Client && !Client.IsLocalClient)
             {
@@ -152,6 +149,9 @@ internal void ClientSceneMessage(INetworkPlayer player, SceneMessage msg)
                 }
             }
 
+            // Let client prepare for scene change
+            OnClientChangeScene(msg.scenePath, msg.sceneOperation);
+
             ApplyOperationAsync(msg.scenePath, msg.sceneOperation).Forget();
         }
 
@@ -190,8 +190,6 @@ internal void OnClientChangeScene(string scenePath, SceneOperation sceneOperatio
         /// <param name="sceneOperation">Scene operation that was just  happen</param>
         internal void OnClientSceneChanged(string scenePath, SceneOperation sceneOperation)
         {
-            ClientSceneChanged?.Invoke(scenePath, sceneOperation);
-
             if (pendingAdditiveSceneList.Count > 0 && Client && !Client.IsLocalClient)
             {
                 ApplyOperationAsync(pendingAdditiveSceneList[0], SceneOperation.LoadAdditive).Forget();
@@ -202,6 +200,9 @@ internal void OnClientSceneChanged(string scenePath, SceneOperation sceneOperati
             //set ready after scene change has completed
             if (!Client.Player.IsReady)
                 SetClientReady();
+
+            //Call event once all scene related actions (subscenes and ready) are done.
+            ClientSceneChanged?.Invoke(scenePath, sceneOperation);
         }
 
         /// <summary>

diff --git a/Assets/Tests/Runtime/ClientServer/NetworkSceneManagerTests.cs b/Assets/Tests/Runtime/ClientServer/NetworkSceneManagerTests.cs
@@ -136,21 +136,16 @@ public void ServerSceneChangedTest()
         [Test]
         public void OnClientSceneChangedAdditiveListTest()
         {
-            UnityAction<string, SceneOperation> func1 = Substitute.For<UnityAction<string, SceneOperation>>();
-            clientSceneManager.ClientSceneChanged.AddListener(func1);
             clientSceneManager.pendingAdditiveSceneList.Add("Assets/Mirror/Tests/Runtime/testScene.unity");
-
             clientSceneManager.OnClientSceneChanged(null, SceneOperation.Normal);
-
-            func1.Received(1).Invoke(Arg.Any<string>(), Arg.Any<SceneOperation>());
             Assert.That(clientSceneManager.pendingAdditiveSceneList.Count == 0);
         }
 
         [Test]
         public void ClientSceneMessagePendingAdditiveSceneListTest()
         {
-            //Check for the additive scene in the pending list at the time of ClientSceneChanged before its removed as part of it being loaded.
-            clientSceneManager.ClientSceneChanged.AddListener(CheckForAdditiveScene);
+            //Check for the additive scene in the pending list at the time of ClientChangeScene before its removed as part of it being loaded.
+            clientSceneManager.ClientChangeScene.AddListener(CheckForAdditiveScene);
             clientSceneManager.ClientSceneMessage(client.Player, new SceneMessage { scenePath = "Assets/Mirror/Tests/Runtime/testScene.unity", additiveScenes = new[] { "Assets/Mirror/Tests/Runtime/testScene.unity" } });
 
             Assert.That(additiveSceneWasFound);

diff --git a/Assets/Tests/Runtime/Host/NetworkSceneManagerTests.cs b/Assets/Tests/Runtime/Host/NetworkSceneManagerTests.cs
@@ -135,6 +135,16 @@ public void ClientSceneChangedTest()
             func1.Received(1).Invoke(Arg.Any<string>(), Arg.Any<SceneOperation>());
         }
 
+        [Test]
+        public void ClientSceneReadyAfterChangedTest()
+        {
+            bool _readyAfterSceneChanged = false;
+            sceneManager.ClientSceneChanged.AddListener((string name, SceneOperation operation) => _readyAfterSceneChanged = client.Player.IsReady);
+            sceneManager.OnClientSceneChanged("test", SceneOperation.Normal);
+
+            Assert.That(_readyAfterSceneChanged, Is.True);
+        }
+
         [UnityTest]
         public IEnumerator ChangeSceneAdditiveLoadTest() => UniTask.ToCoroutine(async () =>
         {