Skip to content

Commit 16a7308

Browse files
authored
fix: Ensure all loaded scenes are registered on session owner promotion (#4214)
1 parent c377807 commit 16a7308

7 files changed

Lines changed: 183 additions & 34 deletions

File tree

‎com.unity.netcode.gameobjects/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
2323

2424
### Fixed
2525

26+
- Issue where a client promoted to session owner could throw an exception when synchronizing a newly joining client. (#4214)
2627
- Issue where a client's clock kept alternating between running slightly fast and slightly slow on every frame, even after it had synchronized with the server. Interpolated objects juddered (a periodic motion artifact caused by mismatched frame rates): they moved in uneven steps instead of smoothly. Barely noticeable at the default `NetworkTimeSystem.AdjustmentRatio`, and clearly visible at higher values.
2728
- Issue where the IL post-processor could not resolve core library types, such as `Guid` and `List<T>`, in assemblies compiled against .NET reference assemblies, for example when targeting .NET 10. Those split the core library across `System.Runtime`, `System.Collections` and other assemblies.
2829
- Fixed issue where a prefab added to a `NetworkPrefabsList` while a session was running was registered more than once and logged a duplicate `GlobalObjectIdHash` error. (#4184)

‎com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1085,7 +1085,6 @@ internal void HandleConnectionApproval(ulong ownerClientId, bool createPlayerObj
10851085
if (NetworkManager.DistributedAuthorityMode && NetworkManager.DAHost)
10861086
{
10871087
NetworkManager.SetSessionOwner(NetworkManager.LocalClientId);
1088-
NetworkManager.SceneManager.InitializeScenesLoaded();
10891088
}
10901089

10911090
if (NetworkManager.DistributedAuthorityMode && NetworkManager.AutoSpawnPlayerPrefabClientSide)

‎com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,11 @@ internal void SetSessionOwner(ulong sessionOwner)
232232
var isSessionOwner = LocalClientId == sessionOwner;
233233
LocalClient.IsSessionOwner = isSessionOwner;
234234

235+
if (isSessionOwner && NetworkConfig.EnableSceneManagement)
236+
{
237+
SceneManager.InitializeScenesLoaded();
238+
}
239+
235240
foreach (var networkObject in SpawnManager.SpawnedObjects.Values)
236241
{
237242
if (isSessionOwner)

‎com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -281,10 +281,6 @@ public void Handle(ref NetworkContext context)
281281
networkManager.SetSessionOwner(GetSessionOwner());
282282
if (networkManager.LocalClient.IsSessionOwner)
283283
{
284-
if (networkManager.NetworkConfig.EnableSceneManagement)
285-
{
286-
networkManager.SceneManager.InitializeScenesLoaded();
287-
}
288284
if (networkManager.NetworkConfig.ConnectionApproval && networkManager.LogLevel <= LogLevel.Developer)
289285
{
290286
NetworkLog.LogWarning($"{nameof(NetworkConfig.ConnectionApproval)} is enabled but is not supported when using a distributed authority topology. The {nameof(NetworkManager.ConnectionApprovalCallback)} will not be invoked.");

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

Lines changed: 36 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System.Linq;
44
using System.Runtime.CompilerServices;
55
using Unity.Collections;
6+
using Unity.Netcode.Logging;
67
using UnityEngine;
78
using UnityEngine.SceneManagement;
89
using Debug = UnityEngine.Debug;
@@ -1951,6 +1952,37 @@ private void OnClientLoadedScene(uint sceneEventId, Scene scene)
19511952
/// </summary>
19521953
internal List<ulong> ClientConnectionQueue = new List<ulong>();
19531954

1955+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
1956+
private void AddActiveSceneToClientSynchronization(ref SceneEventData sceneEventData, ref Scene activeScene, bool shouldRunValidation)
1957+
{
1958+
// It is possible a user might not want to synchronize the active scene, so we will check to see if it is valid before adding it to the synchronization list.
1959+
if (shouldRunValidation && !ValidateSceneBeforeLoading(activeScene.buildIndex, activeScene.name, sceneEventData.LoadSceneMode))
1960+
{
1961+
return;
1962+
}
1963+
1964+
if (NetworkManager.DistributedAuthorityMode)
1965+
{
1966+
if (!ClientSceneHandleToServerSceneHandle.TryGetValue(activeScene.handle, out var localHandle))
1967+
{
1968+
NetworkManager.Log.Warning(new Context(LogLevel.Developer, $"Failed to synchronize active scene. Use {nameof(ValidateSceneBeforeLoading)} to exclude this scene from validation").AddInfo("SceneName", activeScene.name).AddInfo("SceneHandle", activeScene.handle));
1969+
return;
1970+
}
1971+
1972+
// For DA session owners we need to get the scene handle reference that is shared between all clients
1973+
// This ensures any late joiners get the same scene handle references after a session owner migration.
1974+
sceneEventData.SceneHandle = localHandle;
1975+
sceneEventData.SenderClientId = NetworkManager.LocalClientId;
1976+
}
1977+
// If we're a normal server, then always use the server scene handle
1978+
else
1979+
{
1980+
sceneEventData.SceneHandle = activeScene.handle;
1981+
}
1982+
1983+
sceneEventData.SceneHash = SceneHashFromNameOrPath(activeScene.path);
1984+
AddSceneToClientSynchronization(ref sceneEventData, ref activeScene);
1985+
}
19541986

19551987
[MethodImpl(MethodImplOptions.AggressiveInlining)]
19561988
private void AddSceneToClientSynchronization(ref SceneEventData sceneEventData, ref Scene scene)
@@ -2009,37 +2041,17 @@ internal void SynchronizeNetworkObjects(ulong clientId, bool synchronizingServic
20092041
sceneEventData.LoadSceneMode = ClientSynchronizationMode;
20102042
var activeScene = SceneManager.GetActiveScene();
20112043
sceneEventData.SceneEventType = SceneEventType.Synchronize;
2012-
if (BuildIndexToHash.ContainsKey(activeScene.buildIndex))
2044+
if (BuildIndexToHash.TryGetValue(activeScene.buildIndex, out var activeSceneHash))
20132045
{
2014-
sceneEventData.ActiveSceneHash = BuildIndexToHash[activeScene.buildIndex];
2046+
sceneEventData.ActiveSceneHash = activeSceneHash;
20152047
}
20162048

20172049
// Organize how (and when) we serialize our NetworkObjects
2018-
var hasSynchronizedActive = false;
2019-
2020-
// It is possible a user might not want to synchronize the active scene, so we will check to see if it is valid before adding it to the synchronization list.
20212050
// !! Important !!
20222051
// The active scene MUST always be the first scene in the synchronization list.
2023-
if (ValidateSceneBeforeLoading(activeScene.buildIndex, activeScene.name, sceneEventData.LoadSceneMode))
2024-
{
2025-
sceneEventData.SceneHash = SceneHashFromNameOrPath(activeScene.path);
2026-
if (sceneEventData.SceneHash == sceneEventData.ActiveSceneHash)
2027-
{
2028-
hasSynchronizedActive = true;
2029-
}
20302052

2031-
// If we are just a normal client, then always use the server scene handle
2032-
if (NetworkManager.DistributedAuthorityMode)
2033-
{
2034-
sceneEventData.SenderClientId = NetworkManager.LocalClientId;
2035-
sceneEventData.SceneHandle = ClientSceneHandleToServerSceneHandle[activeScene.handle];
2036-
}
2037-
else
2038-
{
2039-
sceneEventData.SceneHandle = activeScene.handle;
2040-
}
2041-
AddSceneToClientSynchronization(ref sceneEventData, ref activeScene);
2042-
}
2053+
// Don't run user-validation when we're synchronizing the CMB service. CMB always needs to know about the active scene
2054+
AddActiveSceneToClientSynchronization(ref sceneEventData, ref activeScene, !synchronizingService);
20432055

20442056
for (int i = 0; i < SceneManager.sceneCount; i++)
20452057
{
@@ -2065,11 +2077,6 @@ internal void SynchronizeNetworkObjects(ulong clientId, bool synchronizingServic
20652077
AddSceneToClientSynchronization(ref sceneEventData, ref scene);
20662078
}
20672079

2068-
if (!hasSynchronizedActive && NetworkManager.CMBServiceConnection && synchronizingService)
2069-
{
2070-
sceneEventData.AddSceneToSynchronize(BuildIndexToHash[activeScene.buildIndex], ClientSceneHandleToServerSceneHandle[activeScene.handle]);
2071-
}
2072-
20732080
sceneEventData.AddSpawnedNetworkObjects();
20742081
sceneEventData.AddDespawnedInSceneNetworkObjects();
20752082
var message = new SceneEventMessage
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
using System.Collections;
2+
using System.Text;
3+
using NUnit.Framework;
4+
using Unity.Netcode;
5+
using Unity.Netcode.TestHelpers.Runtime;
6+
using UnityEngine.SceneManagement;
7+
using UnityEngine.TestTools;
8+
9+
namespace TestProject.RuntimeTests
10+
{
11+
/// <summary>
12+
/// Validates that a client promoted to session owner registers the scenes it loaded outside of
13+
/// <see cref="NetworkSceneManager"/> so it can synchronize newly joining clients.
14+
/// Runs against the DAHost by default and against the live CMB service when USE_CMB_SERVICE is set.
15+
/// </summary>
16+
internal class SessionOwnerPromotionSceneTests : NetcodeIntegrationTest
17+
{
18+
private const string k_LocallyLoadedScene = "EmptyScene";
19+
protected override int NumberOfClients => 2;
20+
21+
private Scene m_OriginalActiveScene;
22+
private Scene m_LocallyLoadedScene;
23+
24+
public SessionOwnerPromotionSceneTests() : base(HostOrServer.DAHost) { }
25+
26+
protected override void OnOneTimeSetup()
27+
{
28+
m_OriginalActiveScene = SceneManager.GetActiveScene();
29+
base.OnOneTimeSetup();
30+
}
31+
32+
protected override IEnumerator OnTearDown()
33+
{
34+
SceneManager.SetActiveScene(m_OriginalActiveScene);
35+
if (m_LocallyLoadedScene.IsValid() && m_LocallyLoadedScene.isLoaded)
36+
{
37+
var scene = m_LocallyLoadedScene;
38+
SceneManager.UnloadSceneAsync(scene);
39+
yield return WaitForConditionOrTimeOut(() => !scene.isLoaded);
40+
}
41+
yield return base.OnTearDown();
42+
}
43+
44+
private NetworkManager m_InitialSessionOwner;
45+
46+
/// <summary>
47+
/// Avoids <see cref="NetcodeIntegrationTest.GetAuthorityNetworkManager"/> since it fails the test when
48+
/// invoked before a new session owner has been promoted.
49+
/// </summary>
50+
private bool IsNewSessionOwnerPromoted(StringBuilder errorLog)
51+
{
52+
// Have to do our own loop looking for a session owner as GetAuthorityNetworkManager will fail the test if no session owner is found.
53+
// It is expected that no session owner will be found for a frame or two during session owner promotion.
54+
NetworkManager sessionOwner = null;
55+
foreach (var networkManager in m_NetworkManagers)
56+
{
57+
if (networkManager.LocalClient.IsSessionOwner)
58+
{
59+
sessionOwner = networkManager;
60+
break;
61+
}
62+
}
63+
64+
if (sessionOwner == null || sessionOwner == m_InitialSessionOwner)
65+
{
66+
errorLog.AppendLine("No new session owner has been promoted.");
67+
return false;
68+
}
69+
70+
var allClientsSynchronized = true;
71+
foreach (var networkManager in m_NetworkManagers)
72+
{
73+
if (networkManager.CurrentSessionOwner != sessionOwner.LocalClientId)
74+
{
75+
errorLog.AppendLine($"[Client-{networkManager.LocalClientId}] Incorrect {nameof(NetworkManager.CurrentSessionOwner)}! Expected: {sessionOwner.LocalClientId}. Has: {networkManager.CurrentSessionOwner}.");
76+
allClientsSynchronized = false;
77+
}
78+
}
79+
return allClientsSynchronized;
80+
}
81+
82+
/// <summary>
83+
/// Loads and activates a scene without using <see cref="NetworkSceneManager"/>, promotes a new session owner,
84+
/// and then verifies the promoted session owner can synchronize late joining clients.
85+
/// </summary>
86+
[UnityTest]
87+
public IEnumerator PromotedSessionOwnerSynchronizesLocallyLoadedActiveScene()
88+
{
89+
// Load and activate a scene outside of NetworkSceneManager after the session has started,
90+
// so no NetworkManager instance has a network scene handle registered for it.
91+
var loadOperation = SceneManager.LoadSceneAsync(k_LocallyLoadedScene, LoadSceneMode.Additive);
92+
Assert.NotNull(loadOperation, "Failed to load test scene!");
93+
94+
yield return WaitForConditionOrTimeOut(() => loadOperation.isDone);
95+
AssertOnTimeout($"Timed out waiting for {k_LocallyLoadedScene} to load!");
96+
97+
// Set the newly loaded scene as the active scene
98+
// Now all clients have this unregistered scene as their active scene.
99+
m_LocallyLoadedScene = SceneManager.GetSceneByName(k_LocallyLoadedScene);
100+
SceneManager.SetActiveScene(m_LocallyLoadedScene);
101+
102+
// Now all clients have this unregistered scene as their active scene.
103+
m_InitialSessionOwner = GetAuthorityNetworkManager();
104+
105+
// Sanity check: Double-check the newly loaded scene is unregistered on all clients
106+
foreach (var networkManager in m_NetworkManagers)
107+
{
108+
var isSceneRegistered = networkManager.SceneManager.ClientSceneHandleToServerSceneHandle.ContainsKey(m_LocallyLoadedScene.handle);
109+
Assert.That(isSceneRegistered, Is.False, $"[Client-{networkManager.LocalClientId}] Scene loaded by the UnityEngine SceneManager should not have been registered!");
110+
}
111+
112+
if (m_UseCmbService)
113+
{
114+
// the CMB service promotes a client when the current session owner disconnects
115+
yield return StopOneClient(m_InitialSessionOwner);
116+
}
117+
else
118+
{
119+
// The DAHost can manually promote another client as the session owner.
120+
m_InitialSessionOwner.PromoteSessionOwner(GetNonAuthorityNetworkManager().LocalClientId);
121+
}
122+
123+
yield return WaitForConditionOrTimeOut(IsNewSessionOwnerPromoted);
124+
AssertOnTimeout("Session owner promotion failed!");
125+
126+
// Connect a late-joining client to verify the new session owner handles the previously unregistered scene.
127+
yield return CreateAndStartNewClient();
128+
}
129+
}
130+
}

‎testproject/Assets/Tests/Runtime/NetworkSceneManager/SessionOwnerPromotionSceneTests.cs.meta‎

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)