fix: [Backport] MTTB-1273 Inconsistent scene name in scene event (#3487)

<!-- Replace this block with what this PR does and why. Describe what
you'd like reviewers to know, how you applied the engineering
principles, and any interesting tradeoffs made. Delete bullet points
below that don't apply, and update the changelog section as appropriate.
-->

<!-- Add short version of the JIRA ticket to the PR title (e.g. "feat:
new shiny feature [MTT-123]") -->
[MTTB-1278](https://jira.unity3d.com/browse/MTTB-1273)
fixes: #3418 

## Changelog

- Added: A `ScenePath` field on the `SceneEvent` returned by the
`OnSceneEvent` callback.
- Fixed: Various bugs around the `OnSceneEvent` callback.

## Testing and Documentation

- Includes unit tests.

<!-- Uncomment and mark items off with a * if this PR deprecates any
API:
### Deprecated API
- [ ] An `[Obsolete]` attribute was added along with a `(RemovedAfter
yyyy-mm-dd)` entry.
- [ ] An [api updater] was added.
- [ ] Deprecation of the API is explained in the CHANGELOG.
- [ ] The users can understand why this API was removed and what they
should use instead.
-->

## Backport

<!-- If this is a backport:
 - Add the following to the PR title: "\[Backport\] ..." .
 - Link to the original PR.
If this needs a backport - state this here
If a backport is not needed please provide the reason why.
If the "Backports" section is not present it will lead to a CI test
failure.
-->

This is a backport of #3458

---------

Co-authored-by: Noel Stephens <[email protected]>

Author: EmandM

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -15,6 +15,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed inconsistencies in the `OnSceneEvent` callback. (#3487)
 - Fixed issue where `NetworkClient` could persist some settings if re-using the same `NetworkManager` instance. (#3494)
 - Fixed issue where a pooled `NetworkObject` was not resetting the internal latest parent property when despawned. (#3494)
 - Fixed issue where the initial client synchronization pre-serialization process was not excluding spawned `NetworkObjects` that already had pending visibility for the client being synchronized. (#3493)

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

@@ -4,6 +4,7 @@
 using System.Linq;
 using System.Reflection;
 using System.Runtime.CompilerServices;
+using System.Text;
 using NUnit.Framework;
 using Unity.Netcode.RuntimeTests;
 using Unity.Netcode.Transports.UTP;
@@ -36,6 +37,8 @@ public abstract class NetcodeIntegrationTest
         /// </summary>
         protected static WaitForSecondsRealtime s_DefaultWaitForTick = new WaitForSecondsRealtime(1.0f / k_DefaultTickRate);
 
+        private readonly StringBuilder m_InternalErrorLog = new StringBuilder();
+
         /// <summary>
         /// An instance of <see cref="NetcodeLogAssert"/> used to capture and assert log messages during integration tests.
         /// This helps in verifying that expected log messages are produced and unexpected log messages are not.
@@ -1466,6 +1469,27 @@ 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.
+        /// 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="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 WaitForConditionOrTimeOut(Func<StringBuilder, bool> checkForCondition, TimeoutHelper timeOutHelper = null)
+        {
+            if (checkForCondition == null)
+            {
+                throw new ArgumentNullException($"checkForCondition cannot be null!");
+            }
+
+            yield return WaitForConditionOrTimeOut(() =>
+            {
+                m_InternalErrorLog.Clear();
+                return checkForCondition(m_InternalErrorLog);
+            }, timeOutHelper);
+        }
+
         /// <summary>
         /// Validation for clients connected.
         /// </summary>
@@ -1736,6 +1760,13 @@ public NetcodeIntegrationTest(HostOrServer hostOrServer)
         protected void AssertOnTimeout(string timeOutErrorMessage, TimeoutHelper assignedTimeoutHelper = null)
         {
             var timeoutHelper = assignedTimeoutHelper ?? s_GlobalTimeoutHelper;
+            if (m_InternalErrorLog.Length > 0)
+            {
+                Assert.False(timeoutHelper.TimedOut, $"{timeOutErrorMessage}\n{m_InternalErrorLog}");
+                m_InternalErrorLog.Clear();
+                return;
+            }
+
             Assert.False(timeoutHelper.TimedOut, timeOutErrorMessage);
         }
 
@@ -1757,7 +1788,7 @@ private void UnloadRemainingScenes()
             }
         }
 
-        private System.Text.StringBuilder m_WaitForLog = new System.Text.StringBuilder();
+        private StringBuilder m_WaitForLog = new StringBuilder();
 
         private void LogWaitForMessages()
         {

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

@@ -0,0 +1,281 @@
+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)]
+    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}. for client: {sceneEvent.ClientId}");
+            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)
+        {
+            m_EnableVerboseDebug = true;
+            var client = m_ClientNetworkManagers[0];
+            var managerToTest = clientType == ClientType.Authority ? m_ServerNetworkManager : client;
+
+
+            var expectedCompletedClients = new List<ulong> { client.LocalClientId };
+            // the authority ID is not inside ClientsThatCompleted when running as a server
+            if (m_UseHost)
+            {
+                expectedCompletedClients.Insert(0, m_ServerNetworkManager.LocalClientId);
+            }
+
+            Scene loadedScene = default;
+            if (action == Action.Unload)
+            {
+                // Load the scene initially
+                m_ServerNetworkManager.SceneManager.LoadScene(k_SceneToLoad, LoadSceneMode.Additive);
+
+                yield return WaitForConditionOrTimeOut(ValidateSceneIsLoaded);
+                AssertOnTimeout($"[Setup] Timed out waiting for client to load the scene {k_SceneToLoad}!");
+
+                // Wait for any pending messages to be processed
+                yield return s_DefaultWaitForTick;
+
+                // 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 = client.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 = m_ServerNetworkManager.LocalClientId,
+                    ClientsThatCompleted = expectedCompletedClients,
+                    ClientsThatTimedOut = new List<ulong>()
+                }
+            });
+
+            //////////////////////////////////////////
+            // Testing event notifications
+            managerToTest.SceneManager.OnSceneEvent += OnSceneEvent;
+
+            if (action == Action.Load)
+            {
+                Assert.That(m_ServerNetworkManager.SceneManager.LoadScene(loadCall, LoadSceneMode.Additive) == SceneEventProgressStatus.Started);
+
+                yield return WaitForConditionOrTimeOut(ValidateSceneIsLoaded);
+                AssertOnTimeout($"[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(m_ServerNetworkManager.SceneManager.UnloadScene(loadedScene) == SceneEventProgressStatus.Started);
+
+                yield return WaitForConditionOrTimeOut(ValidateSceneIsUnloaded);
+                AssertOnTimeout($"[Test] Timed out waiting for client to unload the scene {k_SceneToLoad}!");
+            }
+
+            // Wait for all messages to process
+            yield return s_DefaultWaitForTick;
+
+            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)
+        {
+            var loadedScene = m_ServerNetworkManager.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 (m_ServerNetworkManager.SceneManager.SceneEventProgressTracking.Count > 0)
+            {
+                errorBuilder.AppendLine($"[ValidateIsLoaded] Server NetworkManager still has progress tracking events.");
+                return false;
+            }
+
+            foreach (var manager in m_ClientNetworkManagers)
+            {
+                // default will have isLoaded as false so we can get the scene or default and test on isLoaded
+                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] Client-{manager.name} still has progress tracking events.");
+                    return false;
+                }
+            }
+
+            return true;
+        }
+
+        private bool ValidateSceneIsUnloaded()
+        {
+            if (m_ServerNetworkManager.SceneManager.ScenesLoaded.Values.Any(scene => scene.name == k_SceneToLoad))
+            {
+                return false;
+            }
+            if (m_ServerNetworkManager.SceneManager.SceneEventProgressTracking.Count > 0)
+            {
+                return false;
+            }
+
+            foreach (var manager in m_ClientNetworkManagers)
+            {
+                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}");
+        }
+    }
+}