Skip to content

Commit 93fff92

Browse files
fix
Fixing issue where we were sending scene migration notifications when migrating into the DDOL. Fixing issue where if a client detected a changed scene but had no scene change to send (later) it would throw and exception and cause a memory leak.
1 parent 8388a66 commit 93fff92

3 files changed

Lines changed: 40 additions & 6 deletions

File tree

com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3655,11 +3655,18 @@ internal void SceneChangedUpdate(Scene scene, bool notify = false)
36553655
return;
36563656
}
36573657

3658+
// Don't create notification if there is a scene event in progress.
36583659
if (NetworkManagerOwner.SceneManager.IsSceneEventInProgress())
36593660
{
36603661
return;
36613662
}
36623663

3664+
// Don't create notification if the scene is the DDOL.
3665+
if (scene == NetworkManager.SceneManager.DontDestroyOnLoadScene)
3666+
{
3667+
return;
3668+
}
3669+
36633670
SceneOriginHandle = scene.handle;
36643671

36653672
// non-authority needs to update the NetworkSceneHandle

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2890,7 +2890,6 @@ internal bool IsSceneUnloading(NetworkObject networkObject)
28902890
/// </summary>
28912891
internal void NotifyNetworkObjectSceneChanged(NetworkObject networkObject)
28922892
{
2893-
Debug.Log($"{networkObject.name} changed scene to {networkObject.gameObject.scene.name}!");
28942893
if (networkObject.NetworkManagerOwner != NetworkManager)
28952894
{
28962895
Debug.Log($"!!!!!!!!!!!!! Integration test is registering for scene migration for instances outside of the bounds of this NetworkManager context !!!!!!!!!!!!!");
@@ -3060,7 +3059,14 @@ internal void CheckForAndSendNetworkObjectSceneChanged()
30603059
// Some NetworkObjects still exist, send the message
30613060
var sceneEvent = BeginSceneEvent();
30623061
sceneEvent.SceneEventType = SceneEventType.ObjectSceneChanged;
3063-
SendSceneEventData(sceneEvent.SceneEventId, NetworkManager.ConnectedClientsIds.Where(c => c != NetworkManager.LocalClientId).ToArray());
3062+
try
3063+
{
3064+
SendSceneEventData(sceneEvent.SceneEventId, NetworkManager.ConnectedClientsIds.Where(c => c != NetworkManager.LocalClientId).ToArray());
3065+
}
3066+
catch (Exception ex)
3067+
{
3068+
Debug.LogException(ex);
3069+
}
30643070
ObjectsMigratedIntoNewScene.Clear();
30653071
EndSceneEvent(sceneEvent.SceneEventId);
30663072
}

testproject/Assets/Tests/Runtime/NetworkSceneManager/NetworkObjectSceneMigrationTests.cs

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,12 @@ protected override void OnNewClientCreated(NetworkManager networkManager)
286286
base.OnNewClientCreated(networkManager);
287287
}
288288

289+
private void SetActiveScene(Scene scene)
290+
{
291+
Debug.Log($"[Previous = {SceneManager.GetActiveScene().name}][New = {scene.name}] Changing the active scene!");
292+
SceneManager.SetActiveScene(scene);
293+
}
294+
289295
/// <summary>
290296
/// Integration test to verify changing the currently active scene
291297
/// will migrate NetworkObjects with ActiveSceneSynchronization set
@@ -311,7 +317,7 @@ public IEnumerator ActiveSceneSynchronizationTest()
311317
authority.SceneManager.OnSceneEvent -= SceneManager_OnSceneEvent;
312318

313319
// Set the active scene to be the 1st scene loaded so we don't instantiate within the test runner scene.
314-
SceneManager.SetActiveScene(m_ScenesLoaded[0]);
320+
SetActiveScene(m_ScenesLoaded[0]);
315321

316322
var autoSyncActive = new List<NetworkObject>();
317323
// Spawn 3 NetworkObject instances that auto synchronize to active scene changes
@@ -385,7 +391,8 @@ public IEnumerator ActiveSceneSynchronizationTest()
385391

386392
// Now change the active scene
387393
var newActiveScene = m_ScenesLoaded[1];
388-
SceneManager.SetActiveScene(newActiveScene);
394+
SetActiveScene(newActiveScene);
395+
389396
// We have to do this
390397
//Object.DontDestroyOnLoad(m_TestPrefabAutoSynchActiveScene);
391398

@@ -451,11 +458,18 @@ public IEnumerator ActiveSceneSynchronizationTest()
451458

452459
// Now unload the active scene to verify all remaining NetworkObjects are migrated into the SceneManager
453460
// assigned active scene
454-
461+
m_UnloadEventCompleted = false;
462+
authority.SceneManager.OnUnloadEventCompleted += OnUnloadEventCompleted;
455463
authority.SceneManager.UnloadScene(newActiveScene);
456-
yield return WaitForConditionOrTimeOut(log => ValidateSceneOnAllClients(log, sceneToMigrateTo.name, ExpectedLoadType.Unloaded));
464+
465+
// Always first: make sure the scene event has completed.
466+
yield return WaitForConditionOrTimeOut(() => m_UnloadEventCompleted);
457467
AssertOnTimeout($"Timed out waiting for all clients to unload scene {newActiveScene.name}!");
458468

469+
// Always second: make sure all spawned objects are in the correct scene
470+
yield return WaitForConditionOrTimeOut(log => ValidateSceneOnAllClients(log, sceneToMigrateTo.name, ExpectedLoadType.Unloaded));
471+
AssertOnTimeout($"Timed out waiting for all clients to validate the correct scenes for spawned objects!");
472+
459473
// Clean up any destroyed NetworkObjects
460474
for (int i = authoritySpawnedInstances.Count - 1; i >= 0; i--)
461475
{
@@ -482,6 +496,13 @@ public IEnumerator ActiveSceneSynchronizationTest()
482496
Assert.AreEqual(3, autoSyncInactive.Count(obj => obj != null), $"All the NetworkObjects with {nameof(NetworkObject.ActiveSceneSynchronization)}=false should have survived the active scene change!");
483497
}
484498

499+
500+
private bool m_UnloadEventCompleted;
501+
private void OnUnloadEventCompleted(string sceneName, LoadSceneMode loadSceneMode, List<ulong> clientsCompleted, List<ulong> clientsTimedOut)
502+
{
503+
m_UnloadEventCompleted = true;
504+
}
505+
485506
/// <summary>
486507
/// Callback invoked when a test prefab, with the <see cref="DestroyWithSceneInstancesTestHelper"/>
487508
/// component attached, is destroyed.

0 commit comments

Comments
 (0)