Skip to content
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 @@ -23,6 +23,7 @@ Additional documentation and release notes are available at [Multiplayer Documen

### Fixed

- Issue where a client promoted to session owner could throw an exception when synchronizing a newly joining client. (#4214)
- 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.
- 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.
- 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1085,7 +1085,6 @@ internal void HandleConnectionApproval(ulong ownerClientId, bool createPlayerObj
if (NetworkManager.DistributedAuthorityMode && NetworkManager.DAHost)
{
NetworkManager.SetSessionOwner(NetworkManager.LocalClientId);
NetworkManager.SceneManager.InitializeScenesLoaded();
}

if (NetworkManager.DistributedAuthorityMode && NetworkManager.AutoSpawnPlayerPrefabClientSide)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,11 @@ internal void SetSessionOwner(ulong sessionOwner)
var isSessionOwner = LocalClientId == sessionOwner;
LocalClient.IsSessionOwner = isSessionOwner;

if (isSessionOwner && NetworkConfig.EnableSceneManagement)
{
SceneManager.InitializeScenesLoaded();
}

foreach (var networkObject in SpawnManager.SpawnedObjects.Values)
{
if (isSessionOwner)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,10 +281,6 @@ public void Handle(ref NetworkContext context)
networkManager.SetSessionOwner(GetSessionOwner());
if (networkManager.LocalClient.IsSessionOwner)
{
if (networkManager.NetworkConfig.EnableSceneManagement)
{
networkManager.SceneManager.InitializeScenesLoaded();
}
if (networkManager.NetworkConfig.ConnectionApproval && networkManager.LogLevel <= LogLevel.Developer)
{
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.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Linq;
using System.Runtime.CompilerServices;
using Unity.Collections;
using Unity.Netcode.Logging;
using UnityEngine;
using UnityEngine.SceneManagement;
using Debug = UnityEngine.Debug;
Expand Down Expand Up @@ -1951,6 +1952,37 @@
/// </summary>
internal List<ulong> ClientConnectionQueue = new List<ulong>();

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void AddActiveSceneToClientSynchronization(ref SceneEventData sceneEventData, ref Scene activeScene, bool shouldRunValidation)
{
// 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.
if (shouldRunValidation && !ValidateSceneBeforeLoading(activeScene.buildIndex, activeScene.name, sceneEventData.LoadSceneMode))
{
return;
}

if (NetworkManager.DistributedAuthorityMode)
{
if (!ClientSceneHandleToServerSceneHandle.TryGetValue(activeScene.handle, out var localHandle))
{
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));
return;

Check warning on line 1969 in com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

com.unity.netcode.gameobjects/Runtime/SceneManagement/NetworkSceneManager.cs#L1967-L1969

Added lines #L1967 - L1969 were not covered by tests
}

// For DA session owners we need to get the scene handle reference that is shared between all clients
// This ensures any late joiners get the same scene handle references after a session owner migration.
sceneEventData.SceneHandle = localHandle;
sceneEventData.SenderClientId = NetworkManager.LocalClientId;
}
// If we're a normal server, then always use the server scene handle
else
{
sceneEventData.SceneHandle = activeScene.handle;
}

sceneEventData.SceneHash = SceneHashFromNameOrPath(activeScene.path);
AddSceneToClientSynchronization(ref sceneEventData, ref activeScene);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void AddSceneToClientSynchronization(ref SceneEventData sceneEventData, ref Scene scene)
Expand Down Expand Up @@ -2009,37 +2041,17 @@
sceneEventData.LoadSceneMode = ClientSynchronizationMode;
var activeScene = SceneManager.GetActiveScene();
sceneEventData.SceneEventType = SceneEventType.Synchronize;
if (BuildIndexToHash.ContainsKey(activeScene.buildIndex))
if (BuildIndexToHash.TryGetValue(activeScene.buildIndex, out var activeSceneHash))
{
sceneEventData.ActiveSceneHash = BuildIndexToHash[activeScene.buildIndex];
sceneEventData.ActiveSceneHash = activeSceneHash;
}

// Organize how (and when) we serialize our NetworkObjects
var hasSynchronizedActive = false;

// 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.
// !! Important !!
// The active scene MUST always be the first scene in the synchronization list.
if (ValidateSceneBeforeLoading(activeScene.buildIndex, activeScene.name, sceneEventData.LoadSceneMode))
{
sceneEventData.SceneHash = SceneHashFromNameOrPath(activeScene.path);
if (sceneEventData.SceneHash == sceneEventData.ActiveSceneHash)
{
hasSynchronizedActive = true;
}

// If we are just a normal client, then always use the server scene handle
if (NetworkManager.DistributedAuthorityMode)
{
sceneEventData.SenderClientId = NetworkManager.LocalClientId;
sceneEventData.SceneHandle = ClientSceneHandleToServerSceneHandle[activeScene.handle];
}
else
{
sceneEventData.SceneHandle = activeScene.handle;
}
AddSceneToClientSynchronization(ref sceneEventData, ref activeScene);
}
// Don't run user-validation when we're synchronizing the CMB service. CMB always needs to know about the active scene
AddActiveSceneToClientSynchronization(ref sceneEventData, ref activeScene, !synchronizingService);

for (int i = 0; i < SceneManager.sceneCount; i++)
{
Expand All @@ -2065,11 +2077,6 @@
AddSceneToClientSynchronization(ref sceneEventData, ref scene);
}

if (!hasSynchronizedActive && NetworkManager.CMBServiceConnection && synchronizingService)
{
sceneEventData.AddSceneToSynchronize(BuildIndexToHash[activeScene.buildIndex], ClientSceneHandleToServerSceneHandle[activeScene.handle]);
}

sceneEventData.AddSpawnedNetworkObjects();
sceneEventData.AddDespawnedInSceneNetworkObjects();
var message = new SceneEventMessage
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
using System.Collections;
using System.Text;
using NUnit.Framework;
using Unity.Netcode;
using Unity.Netcode.TestHelpers.Runtime;
using UnityEngine.SceneManagement;
using UnityEngine.TestTools;

namespace TestProject.RuntimeTests
{
/// <summary>
/// Validates that a client promoted to session owner registers the scenes it loaded outside of
/// <see cref="NetworkSceneManager"/> so it can synchronize newly joining clients.
/// Runs against the DAHost by default and against the live CMB service when USE_CMB_SERVICE is set.
/// </summary>
internal class SessionOwnerPromotionSceneTests : NetcodeIntegrationTest
{
private const string k_LocallyLoadedScene = "EmptyScene";
protected override int NumberOfClients => 2;

private Scene m_OriginalActiveScene;
private Scene m_LocallyLoadedScene;

public SessionOwnerPromotionSceneTests() : base(HostOrServer.DAHost) { }

protected override void OnOneTimeSetup()
{
m_OriginalActiveScene = SceneManager.GetActiveScene();
base.OnOneTimeSetup();
}

protected override IEnumerator OnTearDown()
{
SceneManager.SetActiveScene(m_OriginalActiveScene);
if (m_LocallyLoadedScene.IsValid() && m_LocallyLoadedScene.isLoaded)
{
var scene = m_LocallyLoadedScene;
SceneManager.UnloadSceneAsync(scene);
yield return WaitForConditionOrTimeOut(() => !scene.isLoaded);
}
yield return base.OnTearDown();
}

private NetworkManager m_InitialSessionOwner;

/// <summary>
/// Avoids <see cref="NetcodeIntegrationTest.GetAuthorityNetworkManager"/> since it fails the test when
/// invoked before a new session owner has been promoted.
/// </summary>
private bool IsNewSessionOwnerPromoted(StringBuilder errorLog)
{
// Have to do our own loop looking for a session owner as GetAuthorityNetworkManager will fail the test if no session owner is found.
// It is expected that no session owner will be found for a frame or two during session owner promotion.
NetworkManager sessionOwner = null;
foreach (var networkManager in m_NetworkManagers)
{
if (networkManager.LocalClient.IsSessionOwner)
{
sessionOwner = networkManager;
break;
}
}

if (sessionOwner == null || sessionOwner == m_InitialSessionOwner)
{
errorLog.AppendLine("No new session owner has been promoted.");
return false;
}

var allClientsSynchronized = true;
foreach (var networkManager in m_NetworkManagers)
{
if (networkManager.CurrentSessionOwner != sessionOwner.LocalClientId)
{
errorLog.AppendLine($"[Client-{networkManager.LocalClientId}] Incorrect {nameof(NetworkManager.CurrentSessionOwner)}! Expected: {sessionOwner.LocalClientId}. Has: {networkManager.CurrentSessionOwner}.");
allClientsSynchronized = false;
}
}
return allClientsSynchronized;
}

/// <summary>
/// Loads and activates a scene without using <see cref="NetworkSceneManager"/>, promotes a new session owner,
/// and then verifies the promoted session owner can synchronize late joining clients.
/// </summary>
[UnityTest]
public IEnumerator PromotedSessionOwnerSynchronizesLocallyLoadedActiveScene()
{
// Load and activate a scene outside of NetworkSceneManager after the session has started,
// so no NetworkManager instance has a network scene handle registered for it.
var loadOperation = SceneManager.LoadSceneAsync(k_LocallyLoadedScene, LoadSceneMode.Additive);
Assert.NotNull(loadOperation, "Failed to load test scene!");

yield return WaitForConditionOrTimeOut(() => loadOperation.isDone);
AssertOnTimeout($"Timed out waiting for {k_LocallyLoadedScene} to load!");

// Set the newly loaded scene as the active scene
// Now all clients have this unregistered scene as their active scene.
m_LocallyLoadedScene = SceneManager.GetSceneByName(k_LocallyLoadedScene);
SceneManager.SetActiveScene(m_LocallyLoadedScene);

// Now all clients have this unregistered scene as their active scene.
m_InitialSessionOwner = GetAuthorityNetworkManager();

// Sanity check: Double-check the newly loaded scene is unregistered on all clients
foreach (var networkManager in m_NetworkManagers)
{
var isSceneRegistered = networkManager.SceneManager.ClientSceneHandleToServerSceneHandle.ContainsKey(m_LocallyLoadedScene.handle);
Assert.That(isSceneRegistered, Is.False, $"[Client-{networkManager.LocalClientId}] Scene loaded by the UnityEngine SceneManager should not have been registered!");
}

if (m_UseCmbService)
{
// the CMB service promotes a client when the current session owner disconnects
yield return StopOneClient(m_InitialSessionOwner);
}
else
{
// The DAHost can manually promote another client as the session owner.
m_InitialSessionOwner.PromoteSessionOwner(GetNonAuthorityNetworkManager().LocalClientId);
}

yield return WaitForConditionOrTimeOut(IsNewSessionOwnerPromoted);
AssertOnTimeout("Session owner promotion failed!");

// Connect a late-joining client to verify the new session owner handles the previously unregistered scene.
yield return CreateAndStartNewClient();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2

This test can pass if a future change removes promotion-time scene registration and the late joiner never synchronizes EmptyScene. AddActiveSceneToClientSynchronization now warns and returns when the active scene has no mapping. EmptyScene contains no NetworkObject, and CreateAndStartNewClient checks connection and player clones rather than scene synchronization. Assert that the promoted owner registers the scene after promotion. Assert that the late joiner has a synchronized EmptyScene mapped to the promoted owner's canonical handle.

🤖 Helpful? 👍/👎

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a regression test for a bug where subsequent joins fail. I don't really care if the scene fails to synchronize, only that subsequent joiners can successfully join.

}
}
}

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

Loading