Skip to content

Commit 6ddef4d

Browse files
NoelStephensUnityNoelStephensUnity
authored
fix: provide post synchronization scene unloading and validation [MTT-5743] (#2461)
* fix Providing a way for users to not always unload all scenes when running in client synchronization mode additive. Replaced <br/> with <br /> for docusaurus compatibility. * update adding warning if a client tries to change client synchronization mode and exit early. adding warning if a server or host tries to change client synchronization mode after clients are connected and synchronized. * test Generic testing of the fix with a more specific test coming. tests for the warning messages when setting client synchronization mode.
1 parent fdc58b0 commit 6ddef4d

7 files changed

Lines changed: 249 additions & 104 deletions

File tree

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

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -203,12 +203,11 @@ public void PopulateLoadedScenes(ref Dictionary<int, Scene> scenesLoaded, Networ
203203

204204
/// <summary>
205205
/// Unloads any scenes that have not been assigned.
206-
/// TODO: There needs to be a way to validate if a scene should be unloaded
207-
/// or not (i.e. local client-side UI loaded additively)
208206
/// </summary>
209207
/// <param name="networkManager"></param>
210208
public void UnloadUnassignedScenes(NetworkManager networkManager = null)
211209
{
210+
var sceneManager = networkManager.SceneManager;
212211
SceneManager.sceneUnloaded += SceneManager_SceneUnloaded;
213212
foreach (var sceneEntry in SceneNameToSceneHandles)
214213
{
@@ -217,7 +216,10 @@ public void UnloadUnassignedScenes(NetworkManager networkManager = null)
217216
{
218217
if (!sceneHandleEntry.Value.IsAssigned)
219218
{
220-
m_ScenesToUnload.Add(sceneHandleEntry.Value.Scene);
219+
if (sceneManager.VerifySceneBeforeUnloading == null || sceneManager.VerifySceneBeforeUnloading.Invoke(sceneHandleEntry.Value.Scene))
220+
{
221+
m_ScenesToUnload.Add(sceneHandleEntry.Value.Scene);
222+
}
221223
}
222224
}
223225
}
@@ -325,6 +327,23 @@ public void MoveObjectsFromSceneToDontDestroyOnLoad(ref NetworkManager networkMa
325327
public void SetClientSynchronizationMode(ref NetworkManager networkManager, LoadSceneMode mode)
326328
{
327329
var sceneManager = networkManager.SceneManager;
330+
// Don't let client's set this value
331+
if (!networkManager.IsServer)
332+
{
333+
if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
334+
{
335+
NetworkLog.LogWarning("Clients should not set this value as it is automatically synchronized with the server's setting!");
336+
}
337+
return;
338+
}
339+
else // Warn users if they are changing this after there are clients already connected and synchronized
340+
if (networkManager.ConnectedClientsIds.Count > (networkManager.IsServer ? 0 : 1) && sceneManager.ClientSynchronizationMode != mode)
341+
{
342+
if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
343+
{
344+
NetworkLog.LogWarning("Server is changing client synchronization mode after clients have been synchronized! It is recommended to do this before clients are connected!");
345+
}
346+
}
328347

329348
// For additive client synchronization, we take into consideration scenes
330349
// already loaded.

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

Lines changed: 123 additions & 74 deletions
Large diffs are not rendered by default.

‎com.unity.netcode.gameobjects/TestHelpers/Runtime/IntegrationTestSceneHandler.cs‎

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -625,26 +625,20 @@ public void PopulateLoadedScenes(ref Dictionary<int, Scene> scenesLoaded, Networ
625625

626626
private Dictionary<Scene, NetworkManager> m_ScenesToUnload = new Dictionary<Scene, NetworkManager>();
627627

628-
// When true, any remaining scenes loaded will be unloaded
629-
// TODO: There needs to be a way to validate if a scene should be unloaded
630-
// or not (i.e. local client-side UI loaded additively)
631-
public bool AllowUnassignedScenesToBeUnloaded = false;
632-
633628
/// <summary>
634629
/// Handles unloading any scenes that might remain on a client that
635630
/// need to be unloaded.
636631
/// </summary>
637632
/// <param name="networkManager"></param>
638633
public void UnloadUnassignedScenes(NetworkManager networkManager = null)
639634
{
640-
// Only if we are specifically testing this functionality
641-
if (!AllowUnassignedScenesToBeUnloaded)
635+
if (!SceneNameToSceneHandles.ContainsKey(networkManager))
642636
{
643637
return;
644638
}
645-
646-
SceneManager.sceneUnloaded += SceneManager_SceneUnloaded;
647639
var relativeSceneNameToSceneHandles = SceneNameToSceneHandles[networkManager];
640+
var sceneManager = networkManager.SceneManager;
641+
SceneManager.sceneUnloaded += SceneManager_SceneUnloaded;
648642

649643
foreach (var sceneEntry in relativeSceneNameToSceneHandles)
650644
{
@@ -653,10 +647,14 @@ public void UnloadUnassignedScenes(NetworkManager networkManager = null)
653647
{
654648
if (!sceneHandleEntry.Value.IsAssigned)
655649
{
656-
m_ScenesToUnload.Add(sceneHandleEntry.Value.Scene, networkManager);
650+
if (sceneManager.VerifySceneBeforeUnloading == null || sceneManager.VerifySceneBeforeUnloading.Invoke(sceneHandleEntry.Value.Scene))
651+
{
652+
m_ScenesToUnload.Add(sceneHandleEntry.Value.Scene, networkManager);
653+
}
657654
}
658655
}
659656
}
657+
660658
foreach (var sceneToUnload in m_ScenesToUnload)
661659
{
662660
SceneManager.UnloadSceneAsync(sceneToUnload.Key);
@@ -775,8 +773,27 @@ public void MoveObjectsFromSceneToDontDestroyOnLoad(ref NetworkManager networkMa
775773
/// <param name="mode"><see cref="LoadSceneMode.Single"/> or <see cref="LoadSceneMode.Additive"/></param>
776774
public void SetClientSynchronizationMode(ref NetworkManager networkManager, LoadSceneMode mode)
777775
{
776+
778777
var sceneManager = networkManager.SceneManager;
779778

779+
// Don't let client's set this value
780+
if (!networkManager.IsServer)
781+
{
782+
if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
783+
{
784+
NetworkLog.LogWarning("Clients should not set this value as it is automatically synchronized with the server's setting!");
785+
}
786+
return;
787+
}
788+
else if (networkManager.ConnectedClientsIds.Count > (networkManager.IsHost ? 1 : 0) && sceneManager.ClientSynchronizationMode != mode)
789+
{
790+
if (NetworkLog.CurrentLogLevel <= LogLevel.Normal)
791+
{
792+
NetworkLog.LogWarning("Server is changing client synchronization mode after clients have been synchronized! It is recommended to do this before clients are connected!");
793+
}
794+
}
795+
796+
780797

781798
// For additive client synchronization, we take into consideration scenes
782799
// already loaded.

‎com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTestHelpers.cs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,12 @@ private static bool VerifySceneIsValidForClientsToLoad(int sceneIndex, string sc
339339
return true;
340340
}
341341

342+
private static bool VerifySceneIsValidForClientsToUnload(Scene scene)
343+
{
344+
// Unless specifically set, we always return false
345+
return false;
346+
}
347+
342348
/// <summary>
343349
/// This registers scene validation callback for the server to prevent it from telling connecting
344350
/// clients to synchronize (i.e. load) the test runner scene. This will also register the test runner
@@ -351,10 +357,21 @@ private static void SceneManagerValidationAndTestRunnerInitialization(NetworkMan
351357
if (networkManager.IsServer && networkManager.SceneManager.VerifySceneBeforeLoading == null)
352358
{
353359
networkManager.SceneManager.VerifySceneBeforeLoading = VerifySceneIsValidForClientsToLoad;
360+
354361
// If a unit/integration test does not handle this on their own, then Ignore the validation warning
355362
networkManager.SceneManager.DisableValidationWarnings(true);
356363
}
357364

365+
// For testing purposes, all clients always set the VerifySceneBeforeUnloading callback and enabled
366+
// PostSynchronizationSceneUnloading. Where tests that expect clients to unload scenes should override
367+
// the callback and return true for the scenes the client(s) is/are allowed to unload.
368+
if (!networkManager.IsServer && networkManager.SceneManager.VerifySceneBeforeUnloading == null)
369+
{
370+
networkManager.SceneManager.VerifySceneBeforeUnloading = VerifySceneIsValidForClientsToUnload;
371+
networkManager.SceneManager.PostSynchronizationSceneUnloading = true;
372+
}
373+
374+
358375
// Register the test runner scene so it will be able to synchronize NetworkObjects without logging a
359376
// warning about using the currently active scene
360377
var scene = SceneManager.GetActiveScene();

‎testproject/Assets/Tests/Runtime/NetworkSceneManager/NetworkSceneManagerEventDataPoolTest.cs‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,18 @@
1010

1111
namespace TestProject.RuntimeTests
1212
{
13-
[TestFixture(HostOrServer.Host)]
14-
[TestFixture(HostOrServer.Server)]
13+
[TestFixture(HostOrServer.Host, LoadSceneMode.Single)]
14+
[TestFixture(HostOrServer.Host, LoadSceneMode.Additive)]
15+
[TestFixture(HostOrServer.Server, LoadSceneMode.Single)]
16+
[TestFixture(HostOrServer.Server, LoadSceneMode.Additive)]
1517
public class NetworkSceneManagerEventDataPoolTest : NetcodeIntegrationTest
1618
{
1719
protected override int NumberOfClients => 4;
18-
public NetworkSceneManagerEventDataPoolTest(HostOrServer hostOrServer) : base(hostOrServer) { }
20+
21+
public NetworkSceneManagerEventDataPoolTest(HostOrServer hostOrServer, LoadSceneMode loadSceneMode) : base(hostOrServer)
22+
{
23+
m_LoadSceneMode = loadSceneMode;
24+
}
1925

2026
private const string k_BaseUnitTestSceneName = "UnitTestBaseScene";
2127
private const string k_MultiInstanceTestScenename = "AdditiveSceneMultiInstance";
@@ -56,6 +62,7 @@ protected override IEnumerator OnStartedServerAndClients()
5662
m_ServerVerificationAction = DataPoolVerifySceneServer;
5763
m_ServerNetworkManager.SceneManager.OnSceneEvent += ServerSceneManager_OnSceneEvent;
5864
m_ServerNetworkManager.SceneManager.DisableValidationWarnings(true);
65+
m_ServerNetworkManager.SceneManager.SetClientSynchronizationMode(m_LoadSceneMode);
5966
foreach (var client in m_ClientNetworkManagers)
6067
{
6168
client.SceneManager.ClientSynchronizationMode = m_LoadSceneMode;
@@ -142,17 +149,15 @@ private void ResetWait()
142149
/// <summary>
143150
/// Initializes the m_ShouldWaitList
144151
/// </summary>
145-
private void InitializeSceneTestInfo(LoadSceneMode clientSynchronizationMode, bool enableSceneVerification = false)
152+
private void InitializeSceneTestInfo(bool enableSceneVerification = false)
146153
{
147154
m_ShouldWaitList.Add(new SceneTestInfo() { ClientId = NetworkManager.ServerClientId, ShouldWait = false });
148155
m_ServerNetworkManager.SceneManager.VerifySceneBeforeLoading = m_ServerVerificationAction;
149-
m_ServerNetworkManager.SceneManager.SetClientSynchronizationMode(clientSynchronizationMode);
150156
m_ScenesLoaded.Clear();
151157
foreach (var manager in m_ClientNetworkManagers)
152158
{
153159
m_ShouldWaitList.Add(new SceneTestInfo() { ClientId = manager.LocalClientId, ShouldWait = false });
154160
manager.SceneManager.VerifySceneBeforeLoading = m_ClientVerificationAction;
155-
manager.SceneManager.SetClientSynchronizationMode(clientSynchronizationMode);
156161
}
157162
}
158163

@@ -307,15 +312,14 @@ private bool DataPoolVerifySceneClient(int sceneIndex, string sceneName, LoadSce
307312
/// Will load from 1 to 32 scenes in both single and additive ClientSynchronizationMode
308313
/// </summary>
309314
[UnityTest]
310-
public IEnumerator SceneEventDataPoolSceneLoadingTest([Values(LoadSceneMode.Single, LoadSceneMode.Additive)] LoadSceneMode clientSynchronizationMode, [Values(1, 2, 4, 6)] int numberOfScenesToLoad)
315+
public IEnumerator SceneEventDataPoolSceneLoadingTest([Values(1, 2, 4, 6)] int numberOfScenesToLoad)
311316
{
312-
m_LoadSceneMode = clientSynchronizationMode;
313317
m_CanStartServerOrClients = true;
314318

315319
yield return StartServerAndClients();
316320

317321
// Now prepare for the loading and unloading additive scene testing
318-
InitializeSceneTestInfo(clientSynchronizationMode, true);
322+
InitializeSceneTestInfo(true);
319323

320324
yield return WaitForConditionOrTimeOut(() => m_ClientsReceivedSynchronize.Count == (m_ClientNetworkManagers.Length));
321325
Assert.False(s_GlobalTimeoutHelper.TimedOut, $"Timed out waiting for all clients to receive synchronization event! Received: {m_ClientsReceivedSynchronize.Count} | Expected: {m_ClientNetworkManagers.Length}");

‎testproject/Assets/Tests/Runtime/NetworkSceneManager/NetworkSceneManagerSeneVerification.cs‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,17 @@
99

1010
namespace TestProject.RuntimeTests
1111
{
12-
[TestFixture(HostOrServer.Host)]
13-
[TestFixture(HostOrServer.Server)]
12+
[TestFixture(HostOrServer.Host, LoadSceneMode.Single)]
13+
[TestFixture(HostOrServer.Host, LoadSceneMode.Additive)]
14+
[TestFixture(HostOrServer.Server, LoadSceneMode.Single)]
15+
[TestFixture(HostOrServer.Server, LoadSceneMode.Additive)]
1416
public class NetworkSceneManagerSceneVerification : NetcodeIntegrationTest
1517
{
1618
protected override int NumberOfClients => 4;
17-
public NetworkSceneManagerSceneVerification(HostOrServer hostOrServer) : base(hostOrServer) { }
19+
public NetworkSceneManagerSceneVerification(HostOrServer hostOrServer, LoadSceneMode loadSceneMode) : base(hostOrServer)
20+
{
21+
m_LoadSceneMode = loadSceneMode;
22+
}
1823

1924
private const string k_AdditiveScene1 = "InSceneNetworkObject";
2025
private const string k_AdditiveScene2 = "AdditiveSceneMultiInstance";
@@ -65,9 +70,9 @@ protected override IEnumerator OnStartedServerAndClients()
6570
m_ServerVerificationAction = ServerVerifySceneBeforeLoading;
6671
m_ServerNetworkManager.SceneManager.OnSceneEvent += ServerSceneManager_OnSceneEvent;
6772
m_ServerNetworkManager.SceneManager.DisableValidationWarnings(true);
73+
m_ServerNetworkManager.SceneManager.SetClientSynchronizationMode(m_LoadSceneMode);
6874
foreach (var client in m_ClientNetworkManagers)
6975
{
70-
client.SceneManager.ClientSynchronizationMode = m_LoadSceneMode;
7176
client.SceneManager.DisableValidationWarnings(true);
7277
}
7378

@@ -153,11 +158,10 @@ private void ResetWait()
153158
/// <summary>
154159
/// Initializes the m_ShouldWaitList
155160
/// </summary>
156-
private void InitializeSceneTestInfo(LoadSceneMode clientSynchronizationMode, bool enableSceneVerification = false)
161+
private void InitializeSceneTestInfo(bool enableSceneVerification = false)
157162
{
158163
m_ShouldWaitList.Add(new SceneTestInfo() { ClientId = NetworkManager.ServerClientId, ShouldWait = false });
159164
m_ServerNetworkManager.SceneManager.VerifySceneBeforeLoading = m_ServerVerificationAction;
160-
m_ServerNetworkManager.SceneManager.SetClientSynchronizationMode(clientSynchronizationMode);
161165
m_ScenesLoaded.Clear();
162166
foreach (var manager in m_ClientNetworkManagers)
163167
{
@@ -287,15 +291,15 @@ private bool ClientVerifySceneBeforeLoading(int sceneIndex, string sceneName, Lo
287291
/// </summary>
288292
/// <returns></returns>
289293
[UnityTest]
290-
public IEnumerator SceneVerifyBeforeLoadTest([Values(LoadSceneMode.Single, LoadSceneMode.Additive)] LoadSceneMode clientSynchronizationMode)
294+
public IEnumerator SceneVerifyBeforeLoadTest()
291295
{
292296
m_CurrentSceneName = k_AdditiveScene1;
293297
m_CanStartServerOrClients = true;
294298
m_IsTestingVerifyScene = false;
295299
yield return StartServerAndClients();
296300

297301
// Now prepare for the loading and unloading additive scene testing
298-
InitializeSceneTestInfo(clientSynchronizationMode);
302+
InitializeSceneTestInfo();
299303

300304
// Test VerifySceneBeforeLoading with both server and client set to true
301305
ResetWait();
@@ -311,6 +315,7 @@ public IEnumerator SceneVerifyBeforeLoadTest([Values(LoadSceneMode.Single, LoadS
311315

312316
// Unload the scene
313317
ResetWait();
318+
314319
Assert.AreEqual(m_ServerNetworkManager.SceneManager.UnloadScene(m_CurrentScene), SceneEventProgressStatus.Started);
315320

316321
yield return WaitForConditionOrTimeOut(ConditionPassed);

‎testproject/Assets/Tests/Runtime/NetworkSceneManager/NetworkSceneManagerUsageTests.cs‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,31 @@ public void SceneManagementDisabledException([Values(LoadSceneMode.Single, LoadS
4747
Assert.IsTrue(threwException);
4848
}
4949

50+
/// <summary>
51+
/// Validate warning message generation for setting client synchronization mode on the client side.
52+
/// </summary>
53+
[Test]
54+
public void ClientSetClientSynchronizationMode()
55+
{
56+
LogAssert.Expect(UnityEngine.LogType.Warning, "[Netcode] Clients should not set this value as it is automatically synchronized with the server's setting!");
57+
m_ClientNetworkManagers[0].SceneManager.SetClientSynchronizationMode(LoadSceneMode.Single);
58+
}
59+
60+
/// <summary>
61+
/// Validate warning message generation for setting client synchronization mode on the server side.
62+
/// </summary>
63+
[UnityTest]
64+
public IEnumerator ServerSetClientSynchronizationModeAfterClientsConnected()
65+
{
66+
// Verify that changing this setting when additional clients are connect will generate the warning
67+
LogAssert.Expect(UnityEngine.LogType.Warning, "[Netcode] Server is changing client synchronization mode after clients have been synchronized! It is recommended to do this before clients are connected!");
68+
m_ServerNetworkManager.SceneManager.SetClientSynchronizationMode(LoadSceneMode.Additive);
69+
// Verify that changing this setting when no additional clients are connected will not generate a warning
70+
yield return StopOneClient(m_ClientNetworkManagers[0]);
71+
m_ServerNetworkManager.SceneManager.SetClientSynchronizationMode(LoadSceneMode.Additive);
72+
LogAssert.NoUnexpectedReceived();
73+
}
74+
5075
/// <summary>
5176
/// Checks that a client cannot call LoadScene
5277
/// </summary>
@@ -97,6 +122,10 @@ public IEnumerator ClientCannotUseException([Values(LoadSceneMode.Single, LoadSc
97122
}
98123
Assert.IsTrue(threwException);
99124

125+
foreach (var clientNetworkManager in m_ClientNetworkManagers)
126+
{
127+
clientNetworkManager.SceneManager.VerifySceneBeforeUnloading = OnClientVerifySceneBeforeUnloading;
128+
}
100129
// Loading additive only because we don't want to unload the
101130
// Test Runner's scene using LoadSceneMode.Single
102131
retStatus = m_ServerNetworkManager.SceneManager.UnloadScene(m_CurrentScene);
@@ -105,6 +134,11 @@ public IEnumerator ClientCannotUseException([Values(LoadSceneMode.Single, LoadSc
105134
Assert.IsFalse(s_GlobalTimeoutHelper.TimedOut, $"Timed out waiting for {m_CurrentSceneName} {nameof(SceneEventType.UnloadComplete)} event from client!");
106135
}
107136

137+
private bool OnClientVerifySceneBeforeUnloading(Scene scene)
138+
{
139+
return m_CurrentSceneName == scene.name;
140+
}
141+
108142
private void ServerSceneManager_OnSceneEvent(SceneEvent sceneEvent)
109143
{
110144
if (sceneEvent.ClientId != m_ServerNetworkManager.LocalClientId)

0 commit comments

Comments
 (0)