fix: MTTB-1273 Inconsistent SceneName in SceneEvent

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -14,6 +14,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed inconsistencies in the `OnSceneEvent` callback. (#3458)
 - Fixed issues with the `NetworkBehaviour` and `NetworkVariable` length safety checks. (#3405)
 - Fixed memory leaks when domain reload is disabled. (#3427)
 - Fixed an exception being thrown when unregistering a custom message handler from within the registered callback. (#3417)

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

@@ -124,7 +124,7 @@ internal List<ulong> GetClientsWithStatus(bool 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)
+                if ((m_NetworkManager.IsHost || m_NetworkManager.LocalClient.IsSessionOwner) && m_AsyncOperation.isDone)
                 {
                     clients.Add(m_NetworkManager.LocalClientId);
                 }

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

@@ -1194,7 +1194,7 @@ protected void StartServerAndClientsWithTimeTravel()
                     // Wait for all clients to connect
                     WaitForClientsConnectedOrTimeOutWithTimeTravel();
 
-                    AssertOnTimeout($"{nameof(StartServerAndClients)} timed out waiting for all clients to be connected!");
+                    AssertOnTimeout($"{nameof(StartServerAndClients)} timed out waiting for all clients to be connected!\n {m_InternalErrorLog}");
 
                     if (m_UseHost || authorityManager.IsHost)
                     {
@@ -1669,6 +1669,39 @@ public bool WaitForConditionOrTimeOutWithTimeTravel(IConditionalPredicate condit
             return success;
         }
 
+        /// <summary>
+        /// Waits until the specified condition returns true or a timeout occurs, then asserts if the timeout was reached.
+        /// </summary>
+        /// <param name="checkForCondition">A delegate that returns true when the desired condition is met.</param>
+        /// <param name="timeoutErrorMessage">The error message to include in the assertion if the timeout is reached.</param>
+        /// <param name="timeOutHelper">An optional <see cref="TimeoutHelper"/> to control the timeout period. If null, the default timeout is used.</param>
+        /// <returns>An <see cref="IEnumerator"/> for use in Unity coroutines.</returns>
+        protected IEnumerator WaitForConditionOrAssert(Func<bool> checkForCondition, string timeoutErrorMessage, TimeoutHelper timeOutHelper = null)
+        {
+            yield return WaitForConditionOrTimeOut(checkForCondition, timeOutHelper);
+            AssertOnTimeout(timeoutErrorMessage, timeOutHelper);
+        }
+
+        /// <summary>
+        /// Waits until the specified condition returns true or a timeout occurs, then asserts if the timeout was reached.
+        /// This overload allows the condition to provide additional error details via a <see cref="StringBuilder"/>.
+        /// </summary>
+        /// <param name="checkForCondition">A delegate that takes a <see cref="StringBuilder"/> for error details and returns true when the desired condition is met.</param>
+        /// <param name="timeoutErrorMessage">The error message to include in the assertion if the timeout is reached. The information on the StringBuilder will be appended on a new line</param>
+        /// <param name="timeOutHelper">An optional <see cref="TimeoutHelper"/> to control the timeout period. If null, the default timeout is used.</param>
+        /// <returns>An <see cref="IEnumerator"/> for use in Unity coroutines.</returns>
+        protected IEnumerator WaitForConditionOrAssert(Func<StringBuilder, bool> checkForCondition, string timeoutErrorMessage, TimeoutHelper timeOutHelper = null)
+        {
+            var errorBuilder = new StringBuilder();
+            yield return WaitForConditionOrTimeOut(() =>
+            {
+                // Clear errorBuilder before each check to ensure the errorBuilder only contains information from the lastest run
+                errorBuilder.Clear();
+                return checkForCondition(errorBuilder);
+            }, timeOutHelper);
+            AssertOnTimeout($"{timeoutErrorMessage}\n{errorBuilder}", timeOutHelper);
+        }
+
         /// <summary>
         /// Validates that all remote clients (i.e. non-server) detect they are connected
         /// to the server and that the server reflects the appropriate number of clients

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

@@ -0,0 +1,264 @@
+using System.Collections;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using NUnit.Framework;
+using Unity.Netcode;
+using Unity.Netcode.TestHelpers.Runtime;
+using UnityEngine.SceneManagement;
+using UnityEngine.TestTools;
+
+namespace TestProject.RuntimeTests
+{
+    [TestFixture(HostOrServer.Host)]
+    [TestFixture(HostOrServer.Server)]
+    [TestFixture(HostOrServer.DAHost)]
+    public class OnSceneEventCallbackTests : NetcodeIntegrationTest
+    {
+        protected override int NumberOfClients => 1;
+
+        private const string k_SceneToLoad = "EmptyScene";
+        private const string k_PathToLoad = "Assets/Scenes/EmptyScene.unity";
+
+        public OnSceneEventCallbackTests(HostOrServer hostOrServer) : base(hostOrServer)
+        {
+
+        }
+
+        private struct ExpectedEvent
+        {
+            public SceneEvent SceneEvent;
+            public string SceneName;
+            public string ScenePath;
+        }
+
+        private readonly Queue<ExpectedEvent> m_ExpectedEventQueue = new();
+
+        private static int s_NumEventsProcessed;
+        private void OnSceneEvent(SceneEvent sceneEvent)
+        {
+            VerboseDebug($"OnSceneEvent! Type: {sceneEvent.SceneEventType}.");
+            if (m_ExpectedEventQueue.Count > 0)
+            {
+                var expectedEvent = m_ExpectedEventQueue.Dequeue();
+
+                ValidateEventsAreEqual(expectedEvent.SceneEvent, sceneEvent);
+
+                // Only LoadComplete events have an attached scene
+                if (sceneEvent.SceneEventType == SceneEventType.LoadComplete)
+                {
+                    ValidateReceivedScene(expectedEvent, sceneEvent.Scene);
+                }
+            }
+            else
+            {
+                Assert.Fail($"Received unexpected event at index {s_NumEventsProcessed}: {sceneEvent.SceneEventType}");
+            }
+            s_NumEventsProcessed++;
+        }
+
+        public enum ClientType
+        {
+            Authority,
+            NonAuthority,
+        }
+
+        public enum Action
+        {
+            Load,
+            Unload,
+        }
+
+        [UnityTest]
+        public IEnumerator LoadAndUnloadCallbacks([Values] ClientType clientType, [Values] Action action)
+        {
+            yield return RunSceneEventCallbackTest(clientType, action, k_SceneToLoad);
+        }
+
+        [UnityTest]
+        public IEnumerator LoadSceneFromPath([Values] ClientType clientType)
+        {
+            yield return RunSceneEventCallbackTest(clientType, Action.Load, k_PathToLoad);
+        }
+
+        private IEnumerator RunSceneEventCallbackTest(ClientType clientType, Action action, string loadCall)
+        {
+            if (m_UseCmbService)
+            {
+                yield return s_DefaultWaitForTick;
+            }
+
+            var authority = GetAuthorityNetworkManager();
+            var nonAuthority = GetNonAuthorityNetworkManager();
+            var managerToTest = clientType == ClientType.Authority ? authority : nonAuthority;
+
+
+            var expectedCompletedClients = new List<ulong> { nonAuthority.LocalClientId };
+            // the authority ID is not inside ClientsThatCompleted when running as a server
+            if (m_UseHost)
+            {
+                expectedCompletedClients.Insert(0, authority.LocalClientId);
+            }
+
+            Scene loadedScene = default;
+            if (action == Action.Unload)
+            {
+                // Load the scene initially
+                authority.SceneManager.LoadScene(k_SceneToLoad, LoadSceneMode.Additive);
+
+                yield return WaitForConditionOrAssert(ValidateSceneIsLoaded, $"[Setup] Timed out waiting for client to load the scene {k_SceneToLoad}!");
+
+                // Wait for any pending messages to be processed
+                yield return null;
+
+                // Get a reference to the scene to test
+                loadedScene = SceneManager.GetSceneByName(k_SceneToLoad);
+            }
+
+            s_NumEventsProcessed = 0;
+            m_ExpectedEventQueue.Clear();
+            m_ExpectedEventQueue.Enqueue(new ExpectedEvent()
+            {
+                SceneEvent = new SceneEvent()
+                {
+                    SceneEventType = action == Action.Load ? SceneEventType.Load : SceneEventType.Unload,
+                    LoadSceneMode = LoadSceneMode.Additive,
+                    SceneName = k_SceneToLoad,
+                    ScenePath = k_PathToLoad,
+                    ClientId = managerToTest.LocalClientId,
+                },
+            });
+            m_ExpectedEventQueue.Enqueue(new ExpectedEvent()
+            {
+                SceneEvent = new SceneEvent()
+                {
+                    SceneEventType = action == Action.Load ? SceneEventType.LoadComplete : SceneEventType.UnloadComplete,
+                    LoadSceneMode = LoadSceneMode.Additive,
+                    SceneName = k_SceneToLoad,
+                    ScenePath = k_PathToLoad,
+                    ClientId = managerToTest.LocalClientId,
+                },
+                SceneName = action == Action.Load ? k_SceneToLoad : null,
+                ScenePath = action == Action.Load ? k_PathToLoad : null
+            });
+
+            if (clientType == ClientType.Authority)
+            {
+                m_ExpectedEventQueue.Enqueue(new ExpectedEvent()
+                {
+                    SceneEvent = new SceneEvent()
+                    {
+                        SceneEventType = action == Action.Load ? SceneEventType.LoadComplete : SceneEventType.UnloadComplete,
+                        LoadSceneMode = LoadSceneMode.Additive,
+                        SceneName = k_SceneToLoad,
+                        ScenePath = k_PathToLoad,
+                        ClientId = nonAuthority.LocalClientId,
+                    }
+                });
+            }
+
+            m_ExpectedEventQueue.Enqueue(new ExpectedEvent()
+            {
+                SceneEvent = new SceneEvent()
+                {
+                    SceneEventType = action == Action.Load ? SceneEventType.LoadEventCompleted : SceneEventType.UnloadEventCompleted,
+                    LoadSceneMode = LoadSceneMode.Additive,
+                    SceneName = k_SceneToLoad,
+                    ScenePath = k_PathToLoad,
+                    ClientId = authority.LocalClientId,
+                    ClientsThatCompleted = expectedCompletedClients,
+                    ClientsThatTimedOut = new List<ulong>()
+                }
+            });
+
+            //////////////////////////////////////////
+            // Testing event notifications
+            managerToTest.SceneManager.OnSceneEvent += OnSceneEvent;
+
+            if (action == Action.Load)
+            {
+                Assert.That(authority.SceneManager.LoadScene(loadCall, LoadSceneMode.Additive) == SceneEventProgressStatus.Started);
+
+                yield return WaitForConditionOrAssert(ValidateSceneIsLoaded, $"[Test] Timed out waiting for client to load the scene {k_SceneToLoad}!");
+            }
+            else
+            {
+                Assert.That(loadedScene.name, Is.EqualTo(k_SceneToLoad), "scene was not loaded!");
+                Assert.That(authority.SceneManager.UnloadScene(loadedScene) == SceneEventProgressStatus.Started);
+
+                yield return WaitForConditionOrAssert(ValidateSceneIsUnloaded, $"[Test] Timed out waiting for client to unload the scene {k_SceneToLoad}!");
+            }
+
+            // Wait for all messages to process
+            yield return null;
+
+            if (m_ExpectedEventQueue.Count > 0)
+            {
+                Assert.Fail($"Failed to invoke all expected OnSceneEvent callbacks. {m_ExpectedEventQueue.Count} callbacks missing. First missing event is {m_ExpectedEventQueue.Dequeue().SceneEvent.SceneEventType}");
+            }
+
+            managerToTest.SceneManager.OnSceneEvent -= OnSceneEvent;
+        }
+
+        private bool ValidateSceneIsLoaded(StringBuilder errorBuilder)
+        {
+            foreach (var manager in m_NetworkManagers)
+            {
+                // default will have isLoaded as false so we can get the scene or default and test on isLoaded
+                var loadedScene = manager.SceneManager.ScenesLoaded.Values.FirstOrDefault(scene => scene.name == k_SceneToLoad);
+                if (!loadedScene.isLoaded)
+                {
+                    errorBuilder.AppendLine($"[ValidateIsLoaded] Scene {loadedScene.name} exists but is not loaded!");
+                    return false;
+                }
+
+                if (manager.SceneManager.SceneEventProgressTracking.Count > 0)
+                {
+                    errorBuilder.AppendLine($"[ValidateIsLoaded] NetworkManager {manager.name} still has progress tracking events.");
+                    return false;
+                }
+            }
+
+            return true;
+        }
+
+        private bool ValidateSceneIsUnloaded()
+        {
+            foreach (var manager in m_NetworkManagers)
+            {
+                if (manager.SceneManager.ScenesLoaded.Values.Any(scene => scene.name == k_SceneToLoad))
+                {
+                    return false;
+                }
+
+                if (manager.SceneManager.SceneEventProgressTracking.Count > 0)
+                {
+                    return false;
+                }
+            }
+            return true;
+        }
+
+        private static void ValidateEventsAreEqual(SceneEvent expectedEvent, SceneEvent sceneEvent)
+        {
+            AssertField(expectedEvent.SceneEventType, sceneEvent.SceneEventType, nameof(sceneEvent.SceneEventType), sceneEvent.SceneEventType);
+            AssertField(expectedEvent.LoadSceneMode, sceneEvent.LoadSceneMode, nameof(sceneEvent.LoadSceneMode), sceneEvent.SceneEventType);
+            AssertField(expectedEvent.SceneName, sceneEvent.SceneName, nameof(sceneEvent.SceneName), sceneEvent.SceneEventType);
+            AssertField(expectedEvent.ClientId, sceneEvent.ClientId, nameof(sceneEvent.ClientId), sceneEvent.SceneEventType);
+            AssertField(expectedEvent.ClientsThatCompleted, sceneEvent.ClientsThatCompleted, nameof(sceneEvent.SceneEventType), sceneEvent.SceneEventType);
+            AssertField(expectedEvent.ClientsThatTimedOut, sceneEvent.ClientsThatTimedOut, nameof(sceneEvent.ClientsThatTimedOut), sceneEvent.SceneEventType);
+        }
+
+        // The LoadCompleted event includes the scene being loaded
+        private static void ValidateReceivedScene(ExpectedEvent expectedEvent, Scene scene)
+        {
+            AssertField(expectedEvent.SceneName, scene.name, "Scene.name", SceneEventType.LoadComplete);
+            AssertField(expectedEvent.ScenePath, scene.path, "Scene.path", SceneEventType.LoadComplete);
+        }
+
+        private static void AssertField<T>(T expected, T actual, string fieldName, SceneEventType type)
+        {
+            Assert.AreEqual(expected, actual, $"Failed on event {s_NumEventsProcessed} - {type}: Expected {fieldName} to be {expected}. Found {actual}");
+        }
+    }
+}