Skip to content

fix: MTTB-1273 Inconsistent SceneName in SceneEvent #3458

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: develop-2.0.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions com.unity.netcode.gameobjects/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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<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 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<ulong>()
}
});

//////////////////////////////////////////
// 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>(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}");
}
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.