From 571d565eb6e073056f5703da949c4ca91af4ee72 Mon Sep 17 00:00:00 2001 From: Emma Date: Thu, 22 May 2025 14:35:54 -0400 Subject: [PATCH 1/9] fix: MTTB-1273 Inconsistent SceneName in SceneEvent --- .../SceneManagement/NetworkSceneManager.cs | 273 ++++++------------ .../SceneManagement/SceneEventProgress.cs | 2 +- .../Runtime/NetcodeIntegrationTest.cs | 2 +- .../OnSceneEventCallbackTests.cs | 264 +++++++++++++++++ .../OnSceneEventCallbackTests.cs.meta | 3 + 5 files changed, 361 insertions(+), 183 deletions(-) create mode 100644 testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs create mode 100644 testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs.meta diff --git a/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs b/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs index b904056c8a..36da672101 100644 --- a/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Runtime.CompilerServices; +using System.Text; using Unity.Collections; using UnityEngine; using UnityEngine.SceneManagement; @@ -62,6 +64,20 @@ public class SceneEvent /// public string SceneName; + /// + /// This will be set to the path to the scene that the event pertains to.
+ /// This is set for the following s: + /// + /// + /// + /// + /// + /// + /// + /// + /// + public string ScenePath; + /// /// When a scene is loaded, the Scene structure is returned.
/// This is set for the following s: @@ -1056,43 +1072,18 @@ private void SendSceneEventData(uint sceneEventId, ulong[] targetClientIds) var sceneEvent = SceneEventDataStore[sceneEventId]; sceneEvent.SenderClientId = NetworkManager.LocalClientId; - if (NetworkManager.DistributedAuthorityMode && !NetworkManager.DAHost) - { - if (NetworkManager.DistributedAuthorityMode && HasSceneAuthority()) - { - sceneEvent.TargetClientId = NetworkManager.ServerClientId; - var message = new SceneEventMessage - { - EventData = sceneEvent, - }; - var size = NetworkManager.ConnectionManager.SendMessage(ref message, k_DeliveryType, NetworkManager.ServerClientId); - NetworkManager.NetworkMetrics.TrackSceneEventSent(NetworkManager.ServerClientId, (uint)sceneEvent.SceneEventType, SceneNameFromHash(sceneEvent.SceneHash), size); - } - foreach (var clientId in targetClientIds) - { - sceneEvent.TargetClientId = clientId; - var message = new SceneEventMessage - { - EventData = sceneEvent, - }; - var size = NetworkManager.ConnectionManager.SendMessage(ref message, k_DeliveryType, NetworkManager.ServerClientId); - NetworkManager.NetworkMetrics.TrackSceneEventSent(clientId, (uint)sceneEvent.SceneEventType, SceneNameFromHash(sceneEvent.SceneHash), size); - } - } - else + // Send to each individual client to assure only the in-scene placed NetworkObjects being observed by the client + // is serialized + foreach (var clientId in targetClientIds) { - // Send to each individual client to assure only the in-scene placed NetworkObjects being observed by the client - // is serialized - foreach (var clientId in targetClientIds) + sceneEvent.TargetClientId = clientId; + var message = new SceneEventMessage { - sceneEvent.TargetClientId = clientId; - var message = new SceneEventMessage - { - EventData = sceneEvent, - }; - var size = NetworkManager.ConnectionManager.SendMessage(ref message, k_DeliveryType, clientId); - NetworkManager.NetworkMetrics.TrackSceneEventSent(clientId, (uint)sceneEvent.SceneEventType, SceneNameFromHash(sceneEvent.SceneHash), size); - } + EventData = sceneEvent, + }; + var sendTarget = NetworkManager.CMBServiceConnection ? NetworkManager.ServerClientId : clientId; + var size = NetworkManager.ConnectionManager.SendMessage(ref message, k_DeliveryType, sendTarget); + NetworkManager.NetworkMetrics.TrackSceneEventSent(clientId, (uint)sceneEvent.SceneEventType, SceneNameFromHash(sceneEvent.SceneHash), size); } } @@ -1233,24 +1224,7 @@ private bool OnSceneEventProgressCompleted(SceneEventProgress sceneEventProgress } // Send a local notification to the session owner that all clients are done loading or unloading - OnSceneEvent?.Invoke(new SceneEvent() - { - SceneEventType = sceneEventProgress.SceneEventType, - SceneName = SceneNameFromHash(sceneEventProgress.SceneHash), - ClientId = NetworkManager.CurrentSessionOwner, - LoadSceneMode = sceneEventProgress.LoadSceneMode, - ClientsThatCompleted = clientsThatCompleted, - ClientsThatTimedOut = clientsThatTimedOut, - }); - - if (sceneEventData.SceneEventType == SceneEventType.LoadEventCompleted) - { - OnLoadEventCompleted?.Invoke(SceneNameFromHash(sceneEventProgress.SceneHash), sceneEventProgress.LoadSceneMode, sceneEventData.ClientsCompleted, sceneEventData.ClientsTimedOut); - } - else - { - OnUnloadEventCompleted?.Invoke(SceneNameFromHash(sceneEventProgress.SceneHash), sceneEventProgress.LoadSceneMode, sceneEventData.ClientsCompleted, sceneEventData.ClientsTimedOut); - } + InvokeSceneEvents(NetworkManager.CurrentSessionOwner, sceneEventData); EndSceneEvent(sceneEventData.SceneEventId); return true; @@ -1268,7 +1242,6 @@ public SceneEventProgressStatus UnloadScene(Scene scene) var sceneName = scene.name; var sceneHandle = scene.handle; - if (!scene.isLoaded) { Debug.LogWarning($"{nameof(UnloadScene)} was called, but the scene {scene.name} is not currently loaded!"); @@ -1295,6 +1268,8 @@ public SceneEventProgressStatus UnloadScene(Scene scene) } } + sceneEventProgress.LoadSceneMode = LoadSceneMode.Additive; + // Any NetworkObjects marked to not be destroyed with a scene and reside within the scene about to be unloaded // should be migrated temporarily into the DDOL, once the scene is unloaded they will be migrated into the // currently active scene. @@ -1322,16 +1297,7 @@ public SceneEventProgressStatus UnloadScene(Scene scene) var sceneUnload = SceneManagerHandler.UnloadSceneAsync(scene, sceneEventProgress); // Notify local server that a scene is going to be unloaded - OnSceneEvent?.Invoke(new SceneEvent() - { - AsyncOperation = sceneUnload, - SceneEventType = sceneEventData.SceneEventType, - LoadSceneMode = sceneEventData.LoadSceneMode, - SceneName = sceneName, - ClientId = NetworkManager.LocalClientId // Session owner can only invoke this - }); - - OnUnload?.Invoke(NetworkManager.LocalClientId, sceneName, sceneUnload); + InvokeSceneEvents(NetworkManager.LocalClientId, sceneEventData, sceneUnload); //Return the status return sceneEventProgress.Status; @@ -1393,16 +1359,12 @@ private void OnClientUnloadScene(uint sceneEventId) } // Notify the local client that a scene is going to be unloaded - OnSceneEvent?.Invoke(new SceneEvent() - { - AsyncOperation = sceneUnload, - SceneEventType = sceneEventData.SceneEventType, - LoadSceneMode = LoadSceneMode.Additive, // The only scenes unloaded are scenes that were additively loaded - SceneName = sceneName, - ClientId = NetworkManager.LocalClientId // Server sent this message to the client, but client is executing it - }); - OnUnload?.Invoke(NetworkManager.LocalClientId, sceneName, sceneUnload); + // The only scenes unloaded are scenes that were additively loaded + sceneEventData.LoadSceneMode = LoadSceneMode.Additive; + + // Server sent this message to the client, but client is executing it + InvokeSceneEvents(NetworkManager.LocalClientId, sceneEventData, sceneUnload); } /// @@ -1447,15 +1409,7 @@ private void OnSceneUnloaded(uint sceneEventId) sceneEventData.SceneEventType = SceneEventType.UnloadComplete; //Notify the client or server that a scene was unloaded - OnSceneEvent?.Invoke(new SceneEvent() - { - SceneEventType = sceneEventData.SceneEventType, - LoadSceneMode = sceneEventData.LoadSceneMode, - SceneName = SceneNameFromHash(sceneEventData.SceneHash), - ClientId = NetworkManager.LocalClientId, - }); - - OnUnloadComplete?.Invoke(NetworkManager.LocalClientId, SceneNameFromHash(sceneEventData.SceneHash)); + InvokeSceneEvents(NetworkManager.LocalClientId, sceneEventData); if (!HasSceneAuthority()) { @@ -1547,6 +1501,8 @@ public SceneEventProgressStatus LoadScene(string sceneName, LoadSceneMode loadSc sceneEventData.SceneHash = SceneHashFromNameOrPath(sceneName); sceneEventData.LoadSceneMode = loadSceneMode; var sceneEventId = sceneEventData.SceneEventId; + // LoadScene can be called with either a sceneName or a scenePath. Ensure that sceneName is correct at this point + sceneName = SceneNameFromHash(sceneEventData.SceneHash); // This both checks to make sure the scene is valid and if not resets the active scene event m_IsSceneEventActive = ValidateSceneBeforeLoading(sceneEventData.SceneHash, loadSceneMode); if (!m_IsSceneEventActive) @@ -1583,15 +1539,7 @@ public SceneEventProgressStatus LoadScene(string sceneName, LoadSceneMode loadSc var sceneLoad = SceneManagerHandler.LoadSceneAsync(sceneName, loadSceneMode, sceneEventProgress); // Notify the local server that a scene loading event has begun - OnSceneEvent?.Invoke(new SceneEvent() - { - AsyncOperation = sceneLoad, - SceneEventType = sceneEventData.SceneEventType, - LoadSceneMode = sceneEventData.LoadSceneMode, - SceneName = sceneName, - ClientId = NetworkManager.LocalClientId - }); - OnLoad?.Invoke(NetworkManager.LocalClientId, sceneName, sceneEventData.LoadSceneMode, sceneLoad); + InvokeSceneEvents(NetworkManager.LocalClientId, sceneEventData, sceneLoad); //Return our scene progress instance return sceneEventProgress.Status; @@ -1673,6 +1621,7 @@ private void SceneUnloaded(Scene scene) AsyncOperation = m_AsyncOperation, SceneEventType = SceneEventType.UnloadComplete, SceneName = m_Scene.name, + ScenePath = m_Scene.path, LoadSceneMode = m_LoadSceneMode, ClientId = m_ClientId }); @@ -1697,6 +1646,7 @@ private SceneUnloadEventHandler(NetworkSceneManager networkSceneManager, Scene s AsyncOperation = m_AsyncOperation, SceneEventType = SceneEventType.Unload, SceneName = m_Scene.name, + ScenePath = m_Scene.path, LoadSceneMode = m_LoadSceneMode, ClientId = clientId }); @@ -1758,16 +1708,7 @@ private void OnClientSceneLoadingEvent(uint sceneEventId) } var sceneLoad = SceneManagerHandler.LoadSceneAsync(sceneName, sceneEventData.LoadSceneMode, sceneEventProgress); - OnSceneEvent?.Invoke(new SceneEvent() - { - AsyncOperation = sceneLoad, - SceneEventType = sceneEventData.SceneEventType, - LoadSceneMode = sceneEventData.LoadSceneMode, - SceneName = sceneName, - ClientId = NetworkManager.LocalClientId - }); - - OnLoad?.Invoke(NetworkManager.LocalClientId, sceneName, sceneEventData.LoadSceneMode, sceneLoad); + InvokeSceneEvents(NetworkManager.LocalClientId, sceneEventData, sceneLoad); } /// @@ -1905,17 +1846,8 @@ private void OnSessionOwnerLoadedScene(uint sceneEventId, Scene scene) m_IsSceneEventActive = false; sceneEventData.SceneEventType = SceneEventType.LoadComplete; - //First, notify local server that the scene was loaded - OnSceneEvent?.Invoke(new SceneEvent() - { - SceneEventType = sceneEventData.SceneEventType, - LoadSceneMode = sceneEventData.LoadSceneMode, - SceneName = SceneNameFromHash(sceneEventData.SceneHash), - ClientId = NetworkManager.LocalClientId, - Scene = scene, - }); - - OnLoadComplete?.Invoke(NetworkManager.LocalClientId, SceneNameFromHash(sceneEventData.SceneHash), sceneEventData.LoadSceneMode); + // First, notify local server that the scene was loaded + InvokeSceneEvents(NetworkManager.LocalClientId, sceneEventData, scene: scene); //Second, only if we are a host do we want register having loaded for the associated SceneEventProgress if (SceneEventProgressTracking.ContainsKey(sceneEventData.SceneEventProgressId) && NetworkManager.IsClient) @@ -1965,16 +1897,7 @@ private void OnClientLoadedScene(uint sceneEventId, Scene scene) } // Notify local client that the scene was loaded - OnSceneEvent?.Invoke(new SceneEvent() - { - SceneEventType = SceneEventType.LoadComplete, - LoadSceneMode = sceneEventData.LoadSceneMode, - SceneName = SceneNameFromHash(sceneEventData.SceneHash), - ClientId = NetworkManager.LocalClientId, - Scene = scene, - }); - - OnLoadComplete?.Invoke(NetworkManager.LocalClientId, SceneNameFromHash(sceneEventData.SceneHash), sceneEventData.LoadSceneMode); + InvokeSceneEvents(NetworkManager.LocalClientId, sceneEventData, scene: scene); EndSceneEvent(sceneEventId); } @@ -2204,6 +2127,7 @@ private void OnClientBeginSync(uint sceneEventId) SceneEventType = SceneEventType.Load, LoadSceneMode = loadSceneMode, SceneName = sceneName, + ScenePath = ScenePathFromHash(sceneHash), ClientId = NetworkManager.LocalClientId, }); @@ -2278,16 +2202,7 @@ private void ClientLoadedSynchronization(uint sceneEventId) EndSceneEvent(responseSceneEventData.SceneEventId); // Send notification to local client that the scene has finished loading - OnSceneEvent?.Invoke(new SceneEvent() - { - SceneEventType = SceneEventType.LoadComplete, - LoadSceneMode = loadSceneMode, - SceneName = sceneName, - Scene = nextScene, - ClientId = NetworkManager.LocalClientId, - }); - - OnLoadComplete?.Invoke(NetworkManager.LocalClientId, sceneName, loadSceneMode); + InvokeSceneEvents(NetworkManager.LocalClientId, responseSceneEventData, scene: nextScene); // Check to see if we still have scenes to load and synchronize with HandleClientSceneEvent(sceneEventId); @@ -2512,27 +2427,10 @@ private void HandleClientSceneEvent(uint sceneEventId) case SceneEventType.UnloadEventCompleted: { // Notify the local client that all clients have finished loading or unloading - OnSceneEvent?.Invoke(new SceneEvent() - { - SceneEventType = sceneEventData.SceneEventType, - LoadSceneMode = sceneEventData.LoadSceneMode, - SceneName = SceneNameFromHash(sceneEventData.SceneHash), - ClientId = NetworkManager.ServerClientId, - ClientsThatCompleted = sceneEventData.ClientsCompleted, - ClientsThatTimedOut = sceneEventData.ClientsTimedOut, - }); - - if (sceneEventData.SceneEventType == SceneEventType.LoadEventCompleted) - { - OnLoadEventCompleted?.Invoke(SceneNameFromHash(sceneEventData.SceneHash), sceneEventData.LoadSceneMode, sceneEventData.ClientsCompleted, sceneEventData.ClientsTimedOut); - } - else - { - OnUnloadEventCompleted?.Invoke(SceneNameFromHash(sceneEventData.SceneHash), sceneEventData.LoadSceneMode, sceneEventData.ClientsCompleted, sceneEventData.ClientsTimedOut); - } + var clientId = NetworkManager.CMBServiceConnection ? NetworkManager.CurrentSessionOwner : NetworkManager.ServerClientId; + InvokeSceneEvents(clientId, sceneEventData); EndSceneEvent(sceneEventId); - break; } default: @@ -2553,42 +2451,15 @@ private void HandleSessionOwnerEvent(uint sceneEventId, ulong clientId) switch (sceneEventData.SceneEventType) { case SceneEventType.LoadComplete: - { - // Notify the local server that the client has finished loading a scene - OnSceneEvent?.Invoke(new SceneEvent() - { - SceneEventType = sceneEventData.SceneEventType, - LoadSceneMode = sceneEventData.LoadSceneMode, - SceneName = SceneNameFromHash(sceneEventData.SceneHash), - ClientId = clientId - }); - - OnLoadComplete?.Invoke(clientId, SceneNameFromHash(sceneEventData.SceneHash), sceneEventData.LoadSceneMode); - - if (SceneEventProgressTracking.ContainsKey(sceneEventData.SceneEventProgressId)) - { - SceneEventProgressTracking[sceneEventData.SceneEventProgressId].ClientFinishedSceneEvent(clientId); - } - EndSceneEvent(sceneEventId); - break; - } case SceneEventType.UnloadComplete: { + // Notify the local server that the client has finished unloading a scene + InvokeSceneEvents(clientId, sceneEventData); + if (SceneEventProgressTracking.ContainsKey(sceneEventData.SceneEventProgressId)) { SceneEventProgressTracking[sceneEventData.SceneEventProgressId].ClientFinishedSceneEvent(clientId); } - // Notify the local server that the client has finished unloading a scene - OnSceneEvent?.Invoke(new SceneEvent() - { - SceneEventType = sceneEventData.SceneEventType, - LoadSceneMode = sceneEventData.LoadSceneMode, - SceneName = SceneNameFromHash(sceneEventData.SceneHash), - ClientId = clientId - }); - - OnUnloadComplete?.Invoke(clientId, SceneNameFromHash(sceneEventData.SceneHash)); - EndSceneEvent(sceneEventId); break; } @@ -3324,5 +3195,45 @@ public List GetSceneMapping(MapTypes mapType) return mapping; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private void InvokeSceneEvents(ulong clientId, SceneEventData eventData, AsyncOperation asyncOperation = null, Scene scene = default) + { + var sceneName = SceneNameFromHash(eventData.SceneHash); + OnSceneEvent?.Invoke(new SceneEvent() + { + AsyncOperation = asyncOperation, + SceneEventType = eventData.SceneEventType, + SceneName = sceneName, + ScenePath = ScenePathFromHash(eventData.SceneHash), + ClientId = clientId, + LoadSceneMode = eventData.LoadSceneMode, + ClientsThatCompleted = eventData.ClientsCompleted, + ClientsThatTimedOut = eventData.ClientsTimedOut, + Scene = scene, + }); + + switch (eventData.SceneEventType) + { + case SceneEventType.Load: + OnLoad?.Invoke(clientId, sceneName, eventData.LoadSceneMode, asyncOperation); + break; + case SceneEventType.Unload: + OnUnload?.Invoke(clientId, sceneName, asyncOperation); + break; + case SceneEventType.LoadComplete: + OnLoadComplete?.Invoke(NetworkManager.LocalClientId, sceneName, eventData.LoadSceneMode); + break; + case SceneEventType.UnloadComplete: + OnUnloadComplete?.Invoke(clientId, sceneName); + break; + case SceneEventType.LoadEventCompleted: + OnLoadEventCompleted?.Invoke(SceneNameFromHash(eventData.SceneHash), eventData.LoadSceneMode, eventData.ClientsCompleted, eventData.ClientsTimedOut); + break; + case SceneEventType.UnloadEventCompleted: + OnUnloadEventCompleted?.Invoke(SceneNameFromHash(eventData.SceneHash), eventData.LoadSceneMode, eventData.ClientsCompleted, eventData.ClientsTimedOut); + break; + } + } + } } diff --git a/com.unity.netcode.gameobjects/Runtime/SceneManagement/SceneEventProgress.cs b/com.unity.netcode.gameobjects/Runtime/SceneManagement/SceneEventProgress.cs index e9b52a218e..ebbb7b1bb9 100644 --- a/com.unity.netcode.gameobjects/Runtime/SceneManagement/SceneEventProgress.cs +++ b/com.unity.netcode.gameobjects/Runtime/SceneManagement/SceneEventProgress.cs @@ -124,7 +124,7 @@ internal List 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); } diff --git a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs index ea3d919228..df262dea0c 100644 --- a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs +++ b/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) { diff --git a/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs b/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs new file mode 100644 index 0000000000..761407651a --- /dev/null +++ b/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs @@ -0,0 +1,264 @@ +using System.Collections; +using System.Collections.Generic; +using System.Linq; +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 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{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 WaitForConditionOrTimeOut(ValidateSceneIsLoaded); + AssertOnTimeout($"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() + } + }); + + ////////////////////////////////////////// + // Testing event notifications + managerToTest.SceneManager.OnSceneEvent += OnSceneEvent; + + if (action == Action.Load) + { + Assert.That(authority.SceneManager.LoadScene(loadCall, LoadSceneMode.Additive) == SceneEventProgressStatus.Started); + + yield return WaitForConditionOrTimeOut(ValidateSceneIsLoaded); + AssertOnTimeout($"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 WaitForConditionOrTimeOut(ValidateSceneIsUnloaded); + AssertOnTimeout($"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() + { + 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) + { + return false; + } + + if (manager.SceneManager.SceneEventProgressTracking.Count > 0) + { + 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 expected, T actual, string fieldName, SceneEventType type) + { + Assert.AreEqual(expected, actual, $"Failed on event {s_NumEventsProcessed} - {type}: Expected {fieldName} to be {expected}. Found {actual}"); + } + } +} diff --git a/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs.meta b/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs.meta new file mode 100644 index 0000000000..c4c2a7de06 --- /dev/null +++ b/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: 85ef6a4707d04010a84eeea61fdeb9a8 +timeCreated: 1747776388 \ No newline at end of file From b78c6a6b76661e11113eb9576e9b098e12b83c7e Mon Sep 17 00:00:00 2001 From: Emma Date: Thu, 22 May 2025 14:40:27 -0400 Subject: [PATCH 2/9] update CHANGELOG.md --- com.unity.netcode.gameobjects/CHANGELOG.md | 1 + .../Runtime/SceneManagement/NetworkSceneManager.cs | 1 - .../Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 3dd4001177..d9f395baa2 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/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) diff --git a/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs b/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs index 36da672101..b27c1ddb71 100644 --- a/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs @@ -2,7 +2,6 @@ using System.Collections.Generic; using System.Linq; using System.Runtime.CompilerServices; -using System.Text; using Unity.Collections; using UnityEngine; using UnityEngine.SceneManagement; diff --git a/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs b/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs index 761407651a..f6cfea5aa1 100644 --- a/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs +++ b/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs @@ -92,7 +92,7 @@ private IEnumerator RunSceneEventCallbackTest(ClientType clientType, Action acti var managerToTest = clientType == ClientType.Authority ? authority : nonAuthority; - var expectedCompletedClients = new List{nonAuthority.LocalClientId}; + var expectedCompletedClients = new List { nonAuthority.LocalClientId }; // the authority ID is not inside ClientsThatCompleted when running as a server if (m_UseHost) { From 730e160d8c9685c8808f98a59781105ea7e72b6c Mon Sep 17 00:00:00 2001 From: Emma Date: Mon, 26 May 2025 16:21:04 -0400 Subject: [PATCH 3/9] Get more information on test failures --- .../Runtime/NetcodeIntegrationTest.cs | 19 +++++++++++++------ .../OnSceneEventCallbackTests.cs | 14 +++++++------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs index df262dea0c..da2c8166a2 100644 --- a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs +++ b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs @@ -1635,12 +1635,6 @@ public static IEnumerator WaitForConditionOrTimeOut(IConditionalPredicate condit throw new ArgumentNullException($"checkForCondition cannot be null!"); } - // If none is provided we use the default global time out helper - if (timeOutHelper == null) - { - timeOutHelper = s_GlobalTimeoutHelper; - } - conditionalPredicate.Started(); yield return WaitForConditionOrTimeOut(conditionalPredicate.HasConditionBeenReached, timeOutHelper); conditionalPredicate.Finished(timeOutHelper.TimedOut); @@ -1669,6 +1663,19 @@ public bool WaitForConditionOrTimeOutWithTimeTravel(IConditionalPredicate condit return success; } + public IEnumerator WaitForConditionOrAssert(Func checkForCondition, string timeoutErrorMessage, TimeoutHelper timeOutHelper = null) + { + yield return WaitForConditionOrTimeOut(checkForCondition, timeOutHelper); + AssertOnTimeout(timeoutErrorMessage, timeOutHelper); + } + + public IEnumerator WaitForConditionOrAssert(Func checkForCondition, string timeoutErrorMessage, TimeoutHelper timeOutHelper = null) + { + var errorBuilder = new StringBuilder(); + yield return WaitForConditionOrTimeOut(() => checkForCondition(errorBuilder), timeOutHelper); + AssertOnTimeout($"{timeoutErrorMessage}\n{errorBuilder.ToString()}", timeOutHelper); + } + /// /// 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 diff --git a/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs b/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs index f6cfea5aa1..72f21f8da4 100644 --- a/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs +++ b/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs @@ -1,6 +1,7 @@ using System.Collections; using System.Collections.Generic; using System.Linq; +using System.Text; using NUnit.Framework; using Unity.Netcode; using Unity.Netcode.TestHelpers.Runtime; @@ -105,8 +106,7 @@ private IEnumerator RunSceneEventCallbackTest(ClientType clientType, Action acti // Load the scene initially authority.SceneManager.LoadScene(k_SceneToLoad, LoadSceneMode.Additive); - yield return WaitForConditionOrTimeOut(ValidateSceneIsLoaded); - AssertOnTimeout($"Timed out waiting for client to load the scene {k_SceneToLoad}!"); + 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; @@ -179,16 +179,14 @@ private IEnumerator RunSceneEventCallbackTest(ClientType clientType, Action acti { Assert.That(authority.SceneManager.LoadScene(loadCall, LoadSceneMode.Additive) == SceneEventProgressStatus.Started); - yield return WaitForConditionOrTimeOut(ValidateSceneIsLoaded); - AssertOnTimeout($"Timed out waiting for client to load the scene {k_SceneToLoad}!"); + 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 WaitForConditionOrTimeOut(ValidateSceneIsUnloaded); - AssertOnTimeout($"Timed out waiting for client to unload the scene {k_SceneToLoad}!"); + yield return WaitForConditionOrAssert(ValidateSceneIsUnloaded, $"[Test] Timed out waiting for client to unload the scene {k_SceneToLoad}!"); } // Wait for all messages to process @@ -202,7 +200,7 @@ private IEnumerator RunSceneEventCallbackTest(ClientType clientType, Action acti managerToTest.SceneManager.OnSceneEvent -= OnSceneEvent; } - private bool ValidateSceneIsLoaded() + private bool ValidateSceneIsLoaded(StringBuilder errorBuilder) { foreach (var manager in m_NetworkManagers) { @@ -210,11 +208,13 @@ private bool ValidateSceneIsLoaded() 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; } } From 0895f55065d3dcbafba87626a4135f729313f1e8 Mon Sep 17 00:00:00 2001 From: Emma Date: Mon, 26 May 2025 16:27:08 -0400 Subject: [PATCH 4/9] Add code docs --- .../TestHelpers/Runtime/NetcodeIntegrationTest.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs index da2c8166a2..8ba7460b68 100644 --- a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs +++ b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs @@ -1663,12 +1663,27 @@ public bool WaitForConditionOrTimeOutWithTimeTravel(IConditionalPredicate condit return success; } + /// + /// Waits until the specified condition returns true or a timeout occurs, then asserts if the timeout was reached. + /// + /// A delegate that returns true when the desired condition is met. + /// The error message to include in the assertion if the timeout is reached. + /// An optional to control the timeout period. If null, the default timeout is used. + /// An for use in Unity coroutines. public IEnumerator WaitForConditionOrAssert(Func checkForCondition, string timeoutErrorMessage, TimeoutHelper timeOutHelper = null) { yield return WaitForConditionOrTimeOut(checkForCondition, timeOutHelper); AssertOnTimeout(timeoutErrorMessage, timeOutHelper); } + /// + /// 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 . + /// + /// A delegate that takes a for error details and returns true when the desired condition is met. + /// 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 + /// An optional to control the timeout period. If null, the default timeout is used. + /// An for use in Unity coroutines. public IEnumerator WaitForConditionOrAssert(Func checkForCondition, string timeoutErrorMessage, TimeoutHelper timeOutHelper = null) { var errorBuilder = new StringBuilder(); From 68c3f6c93d9021eaa2e5306c4d23e8991723944a Mon Sep 17 00:00:00 2001 From: Emma Date: Mon, 26 May 2025 16:52:17 -0400 Subject: [PATCH 5/9] Don't break conditional tests --- .../TestHelpers/Runtime/NetcodeIntegrationTest.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs index 8ba7460b68..d031ec4596 100644 --- a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs +++ b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs @@ -1635,6 +1635,12 @@ public static IEnumerator WaitForConditionOrTimeOut(IConditionalPredicate condit throw new ArgumentNullException($"checkForCondition cannot be null!"); } + // If none is provided we use the default global time out helper + if (timeOutHelper == null) + { + timeOutHelper = s_GlobalTimeoutHelper; + } + conditionalPredicate.Started(); yield return WaitForConditionOrTimeOut(conditionalPredicate.HasConditionBeenReached, timeOutHelper); conditionalPredicate.Finished(timeOutHelper.TimedOut); @@ -1670,7 +1676,7 @@ public bool WaitForConditionOrTimeOutWithTimeTravel(IConditionalPredicate condit /// The error message to include in the assertion if the timeout is reached. /// An optional to control the timeout period. If null, the default timeout is used. /// An for use in Unity coroutines. - public IEnumerator WaitForConditionOrAssert(Func checkForCondition, string timeoutErrorMessage, TimeoutHelper timeOutHelper = null) + protected IEnumerator WaitForConditionOrAssert(Func checkForCondition, string timeoutErrorMessage, TimeoutHelper timeOutHelper = null) { yield return WaitForConditionOrTimeOut(checkForCondition, timeOutHelper); AssertOnTimeout(timeoutErrorMessage, timeOutHelper); @@ -1684,11 +1690,11 @@ public IEnumerator WaitForConditionOrAssert(Func checkForCondition, string /// 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 /// An optional to control the timeout period. If null, the default timeout is used. /// An for use in Unity coroutines. - public IEnumerator WaitForConditionOrAssert(Func checkForCondition, string timeoutErrorMessage, TimeoutHelper timeOutHelper = null) + protected IEnumerator WaitForConditionOrAssert(Func checkForCondition, string timeoutErrorMessage, TimeoutHelper timeOutHelper = null) { var errorBuilder = new StringBuilder(); yield return WaitForConditionOrTimeOut(() => checkForCondition(errorBuilder), timeOutHelper); - AssertOnTimeout($"{timeoutErrorMessage}\n{errorBuilder.ToString()}", timeOutHelper); + AssertOnTimeout($"{timeoutErrorMessage}\n{errorBuilder}", timeOutHelper); } /// From b42f324b68ca8b28668086e9efb9fc3178170a2f Mon Sep 17 00:00:00 2001 From: Emma Date: Mon, 26 May 2025 18:28:56 -0400 Subject: [PATCH 6/9] Clear the error builder --- .../TestHelpers/Runtime/NetcodeIntegrationTest.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs index d031ec4596..1dd2c18463 100644 --- a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs +++ b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs @@ -1693,7 +1693,12 @@ protected IEnumerator WaitForConditionOrAssert(Func checkForCondition, str protected IEnumerator WaitForConditionOrAssert(Func checkForCondition, string timeoutErrorMessage, TimeoutHelper timeOutHelper = null) { var errorBuilder = new StringBuilder(); - yield return WaitForConditionOrTimeOut(() => checkForCondition(errorBuilder), timeOutHelper); + 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); } From f0414600cfc3ffcecb37d0cf3cff5160eee295ce Mon Sep 17 00:00:00 2001 From: Emma Date: Mon, 26 May 2025 18:36:18 -0400 Subject: [PATCH 7/9] Fix SceneEventCallbackNotifications --- .../Runtime/SceneManagement/NetworkSceneManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs b/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs index b27c1ddb71..e62c4fb239 100644 --- a/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs @@ -3220,7 +3220,7 @@ private void InvokeSceneEvents(ulong clientId, SceneEventData eventData, AsyncOp OnUnload?.Invoke(clientId, sceneName, asyncOperation); break; case SceneEventType.LoadComplete: - OnLoadComplete?.Invoke(NetworkManager.LocalClientId, sceneName, eventData.LoadSceneMode); + OnLoadComplete?.Invoke(clientId, sceneName, eventData.LoadSceneMode); break; case SceneEventType.UnloadComplete: OnUnloadComplete?.Invoke(clientId, sceneName); From a30d9be8781f985f0ce78f5de9ea890f92cd1fd1 Mon Sep 17 00:00:00 2001 From: Emma Date: Wed, 28 May 2025 14:34:43 -0400 Subject: [PATCH 8/9] Fix integration test helpers --- .../Runtime/NetcodeIntegrationTest.cs | 31 +++++++------------ .../OnSceneEventCallbackTests.cs | 11 ++++--- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs index 1dd2c18463..b8abe399c4 100644 --- a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs +++ b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs @@ -1669,37 +1669,21 @@ public bool WaitForConditionOrTimeOutWithTimeTravel(IConditionalPredicate condit return success; } - /// - /// Waits until the specified condition returns true or a timeout occurs, then asserts if the timeout was reached. - /// - /// A delegate that returns true when the desired condition is met. - /// The error message to include in the assertion if the timeout is reached. - /// An optional to control the timeout period. If null, the default timeout is used. - /// An for use in Unity coroutines. - protected IEnumerator WaitForConditionOrAssert(Func checkForCondition, string timeoutErrorMessage, TimeoutHelper timeOutHelper = null) - { - yield return WaitForConditionOrTimeOut(checkForCondition, timeOutHelper); - AssertOnTimeout(timeoutErrorMessage, timeOutHelper); - } - /// /// 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 . /// /// A delegate that takes a for error details and returns true when the desired condition is met. - /// 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 /// An optional to control the timeout period. If null, the default timeout is used. /// An for use in Unity coroutines. - protected IEnumerator WaitForConditionOrAssert(Func checkForCondition, string timeoutErrorMessage, TimeoutHelper timeOutHelper = null) + protected IEnumerator WaitForConditionOrTimeOut(Func checkForCondition, 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); + m_InternalErrorLog.Clear(); + return checkForCondition(m_InternalErrorLog); }, timeOutHelper); - AssertOnTimeout($"{timeoutErrorMessage}\n{errorBuilder}", timeOutHelper); } /// @@ -2057,7 +2041,14 @@ private void InitializeTestConfiguration(NetworkTopologyTypes networkTopologyTyp protected void AssertOnTimeout(string timeOutErrorMessage, TimeoutHelper assignedTimeoutHelper = null) { var timeoutHelper = assignedTimeoutHelper ?? s_GlobalTimeoutHelper; - Assert.False(timeoutHelper.TimedOut, timeOutErrorMessage); + + if (m_InternalErrorLog.Length > 0) + { + Assert.False(timeoutHelper.TimedOut, $"{timeOutErrorMessage}\n{m_InternalErrorLog}"); + m_InternalErrorLog.Clear(); + } + + Assert.False(timeoutHelper.TimedOut, $"{timeOutErrorMessage}"); } diff --git a/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs b/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs index 72f21f8da4..92a3778df7 100644 --- a/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs +++ b/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs @@ -106,7 +106,8 @@ private IEnumerator RunSceneEventCallbackTest(ClientType clientType, Action acti // 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}!"); + 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 null; @@ -179,14 +180,16 @@ private IEnumerator RunSceneEventCallbackTest(ClientType clientType, Action acti { 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}!"); + 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(authority.SceneManager.UnloadScene(loadedScene) == SceneEventProgressStatus.Started); - yield return WaitForConditionOrAssert(ValidateSceneIsUnloaded, $"[Test] Timed out waiting for client to unload the scene {k_SceneToLoad}!"); + yield return WaitForConditionOrTimeOut(ValidateSceneIsUnloaded); + AssertOnTimeout($"[Test] Timed out waiting for client to unload the scene {k_SceneToLoad}!"); } // Wait for all messages to process @@ -219,7 +222,7 @@ private bool ValidateSceneIsLoaded(StringBuilder errorBuilder) } } - return true; + return false; } private bool ValidateSceneIsUnloaded() From f1b1c81f05c090925ce9561924d3ea1fc8cefea8 Mon Sep 17 00:00:00 2001 From: Emma Date: Wed, 28 May 2025 17:01:53 -0400 Subject: [PATCH 9/9] Fix the tests --- .../TestHelpers/Runtime/NetcodeIntegrationTest.cs | 1 + .../Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs index b8abe399c4..ab4a164e82 100644 --- a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs +++ b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs @@ -2046,6 +2046,7 @@ protected void AssertOnTimeout(string timeOutErrorMessage, TimeoutHelper assigne { Assert.False(timeoutHelper.TimedOut, $"{timeOutErrorMessage}\n{m_InternalErrorLog}"); m_InternalErrorLog.Clear(); + return; } Assert.False(timeoutHelper.TimedOut, $"{timeOutErrorMessage}"); diff --git a/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs b/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs index 92a3778df7..d3189dd081 100644 --- a/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs +++ b/testproject/Assets/Tests/Runtime/NetworkSceneManager/OnSceneEventCallbackTests.cs @@ -222,7 +222,7 @@ private bool ValidateSceneIsLoaded(StringBuilder errorBuilder) } } - return false; + return true; } private bool ValidateSceneIsUnloaded()