Skip to content

Commit 34c377d

Browse files
authored
fix: [Backport] Ensure all loaded scenes are registered on session owner promotion (#4217)
* fix: Ensure all loaded scenes are registered on session owner promotion * Update CHANGELOG.md
1 parent e34dba5 commit 34c377d

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
@@ -22,6 +22,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
2222

2323
### Fixed
2424

25+
- Issue where a client promoted to session owner could throw an exception when synchronizing a newly joining client. (#4217)
2526
- Issue where a growable `FastBufferWriter` created with a size of 0 never returned from its first write or `TryBeginWrite` call that needed the buffer to grow. (#4223)
2627
- Issue where a `FastBufferReader` created with an offset and the default length of -1 read past the end of its source. The `ArraySegment<byte>` overload that takes an offset now applies it from the start of the segment. (#4221)
2728
- Issue where a client synchronized while a `NetworkList<T>` still had unsent changes applied those changes twice, resulting in duplicate entries and index-based events landing on the wrong elements on that client. (#4215)

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

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

10921091
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
@@ -233,6 +233,11 @@ internal void SetSessionOwner(ulong sessionOwner)
233233
var isSessionOwner = LocalClientId == sessionOwner;
234234
LocalClient.IsSessionOwner = isSessionOwner;
235235

236+
if (isSessionOwner && NetworkConfig.EnableSceneManagement)
237+
{
238+
SceneManager.InitializeScenesLoaded();
239+
}
240+
236241
foreach (var networkObject in SpawnManager.SpawnedObjects.Values)
237242
{
238243
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
@@ -280,10 +280,6 @@ public void Handle(ref NetworkContext context)
280280
networkManager.SetSessionOwner(GetSessionOwner());
281281
if (networkManager.LocalClient.IsSessionOwner)
282282
{
283-
if (networkManager.NetworkConfig.EnableSceneManagement)
284-
{
285-
networkManager.SceneManager.InitializeScenesLoaded();
286-
}
287283
if (networkManager.NetworkConfig.ConnectionApproval && networkManager.LogLevel <= LogLevel.Developer)
288284
{
289285
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)