Skip to content

Commit

Permalink
fix: hidden objects from newly promoted session owner still synchroni…
Browse files Browse the repository at this point in the history
…ze with newly joining clients (#3051)

* fix

This fixes the issue where a NetworkObject hidden from a client that is promoted to session owner will still be synchronized with newly joining clients.

* test

The test to validate the fix

* update

adding changelog entry

* style

Minor typo
  • Loading branch information
NoelStephensUnity authored Sep 7, 2024
1 parent 8e62d9e commit 91bb80e
Show file tree
Hide file tree
Showing 8 changed files with 228 additions and 18 deletions.
1 change: 1 addition & 0 deletions com.unity.netcode.gameobjects/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Additional documentation and release notes are available at [Multiplayer Documen

### Fixed

- Fixed issue where a NetworkObject hidden from a client that is then promoted to be session owner was not being synchronized with newly joining clients.(#3051)
- Fixed issue where setting a prefab hash value during connection approval but not having a player prefab assigned could cause an exception when spawning a player. (#3042)
- Fixed issue where the `NetworkSpawnManager.HandleNetworkObjectShow` could throw an exception if one of the `NetworkObject` components to show was destroyed during the same frame. (#3030)
- Fixed issue where the `NetworkManagerHelper` was continuing to check for hierarchy changes when in play mode. (#3026)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -985,10 +985,18 @@ internal NetworkClient AddClient(ulong clientId)
ConnectedClientIds.Add(clientId);
}

var distributedAuthority = NetworkManager.DistributedAuthorityMode;
var sessionOwnerId = NetworkManager.CurrentSessionOwner;
var isSessionOwner = NetworkManager.LocalClient.IsSessionOwner;
foreach (var networkObject in NetworkManager.SpawnManager.SpawnedObjectsList)
{
if (networkObject.SpawnWithObservers)
{
// Don't add the client to the observers if hidden from the session owner
if (networkObject.IsOwner && distributedAuthority && !isSessionOwner && !networkObject.Observers.Contains(sessionOwnerId))
{
continue;
}
networkObject.Observers.Add(clientId);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,6 @@ internal void SetSessionOwner(ulong sessionOwner)
OnSessionOwnerPromoted?.Invoke(sessionOwner);
}

// TODO: Make this internal after testing
internal void PromoteSessionOwner(ulong clientId)
{
if (!DistributedAuthorityMode)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ public void Handle(ref NetworkContext context)
// Don't redistribute for the local instance
if (ClientId != networkManager.LocalClientId)
{
// Show any NetworkObjects that are:
// - Hidden from the session owner
// - Owned by this client
// - Has NetworkObject.SpawnWithObservers set to true (the default)
networkManager.SpawnManager.ShowHiddenObjectsToNewlyJoinedClient(ClientId);

// We defer redistribution to the end of the NetworkUpdateStage.PostLateUpdate
networkManager.RedistributeToClient = true;
networkManager.ClientToRedistribute = ClientId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2550,17 +2550,6 @@ private void HandleSessionOwnerEvent(uint sceneEventId, ulong clientId)
// At this point the client is considered fully "connected"
if ((NetworkManager.DistributedAuthorityMode && NetworkManager.LocalClient.IsSessionOwner) || !NetworkManager.DistributedAuthorityMode)
{
if (NetworkManager.DistributedAuthorityMode && !NetworkManager.DAHost)
{
// DANGO-EXP TODO: Remove this once service is sending the synchronization message to all clients
if (NetworkManager.ConnectedClients.ContainsKey(clientId) && NetworkManager.ConnectionManager.ConnectedClientIds.Contains(clientId) && NetworkManager.ConnectedClientsList.Contains(NetworkManager.ConnectedClients[clientId]))
{
EndSceneEvent(sceneEventId);
return;
}
NetworkManager.ConnectionManager.AddClient(clientId);
}

// Notify the local server that a client has finished synchronizing
OnSceneEvent?.Invoke(new SceneEvent()
{
Expand All @@ -2575,6 +2564,20 @@ private void HandleSessionOwnerEvent(uint sceneEventId, ulong clientId)
}
else
{
// Notify the local server that a client has finished synchronizing
OnSceneEvent?.Invoke(new SceneEvent()
{
SceneEventType = sceneEventData.SceneEventType,
SceneName = string.Empty,
ClientId = clientId
});

// Show any NetworkObjects that are:
// - Hidden from the session owner
// - Owned by this client
// - Has NetworkObject.SpawnWithObservers set to true (the default)
NetworkManager.SpawnManager.ShowHiddenObjectsToNewlyJoinedClient(clientId);

// DANGO-EXP TODO: Remove this once service distributes objects
// Non-session owners receive this notification from newly connected clients and upon receiving
// the event they will redistribute their NetworkObjects
Expand All @@ -2589,9 +2592,6 @@ private void HandleSessionOwnerEvent(uint sceneEventId, ulong clientId)
// At this time the client is fully synchronized with all loaded scenes and
// NetworkObjects and should be considered "fully connected". Send the
// notification that the client is connected.
// TODO 2023: We should have a better name for this or have multiple states the
// client progresses through (the name and associated legacy behavior/expected state
// of the client was persisted since MLAPI)
NetworkManager.ConnectionManager.InvokeOnClientConnectedCallback(clientId);

if (NetworkManager.IsHost)
Expand Down Expand Up @@ -2664,9 +2664,14 @@ internal void HandleSceneEvent(ulong clientId, FastBufferReader reader)
EventData = sceneEventData,
};
// Forward synchronization to client then exit early because DAHost is not the current session owner
NetworkManager.MessageManager.SendMessage(ref message, NetworkDelivery.ReliableFragmentedSequenced, NetworkManager.CurrentSessionOwner);
EndSceneEvent(sceneEventData.SceneEventId);
return;
foreach (var client in NetworkManager.ConnectedClientsIds)
{
if (client == NetworkManager.LocalClientId)
{
continue;
}
NetworkManager.MessageManager.SendMessage(ref message, NetworkDelivery.ReliableFragmentedSequenced, client);
}
}
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1909,5 +1909,55 @@ internal void NotifyNetworkObjectsSynchronized()
networkObject.InternalNetworkSessionSynchronized();
}
}

/// <summary>
/// Distributed Authority Only
/// Should be invoked on non-session owner clients when a newly joined client is finished
/// synchronizing in order to "show" (spawn) anything that might be currently hidden from
/// the session owner.
/// </summary>
internal void ShowHiddenObjectsToNewlyJoinedClient(ulong newClientId)
{
if (!NetworkManager.DistributedAuthorityMode)
{
if (NetworkManager == null || !NetworkManager.ShutdownInProgress && NetworkManager.LogLevel <= LogLevel.Developer)
{
Debug.LogWarning($"[Internal Error] {nameof(ShowHiddenObjectsToNewlyJoinedClient)} invoked while !");
}
return;
}

if (!NetworkManager.DistributedAuthorityMode)
{
Debug.LogError($"[Internal Error] {nameof(ShowHiddenObjectsToNewlyJoinedClient)} should only be invoked when using a distributed authority network topology!");
return;
}

if (NetworkManager.LocalClient.IsSessionOwner)
{
Debug.LogError($"[Internal Error] {nameof(ShowHiddenObjectsToNewlyJoinedClient)} should only be invoked on a non-session owner client!");
return;
}
var localClientId = NetworkManager.LocalClient.ClientId;
var sessionOwnerId = NetworkManager.CurrentSessionOwner;
foreach (var networkObject in SpawnedObjectsList)
{
if (networkObject.SpawnWithObservers && networkObject.OwnerClientId == localClientId && !networkObject.Observers.Contains(sessionOwnerId))
{
if (networkObject.Observers.Contains(newClientId))
{
if (NetworkManager.LogLevel <= LogLevel.Developer)
{
// Track if there is some other location where the client is being added to the observers list when the object is hidden from the session owner
Debug.LogWarning($"[{networkObject.name}] Has new client as an observer but it is hidden from the session owner!");
}
// For now, remove the client (impossible for the new client to have an instance since the session owner doesn't) to make sure newly added
// code to handle this edge case works.
networkObject.Observers.Remove(newClientId);
}
networkObject.NetworkShow(newClientId);
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
using System.Collections;
using NUnit.Framework;
using Unity.Netcode.TestHelpers.Runtime;
using UnityEngine;
using UnityEngine.TestTools;

namespace Unity.Netcode.RuntimeTests
{
[TestFixture(HostOrServer.DAHost)]
public class ExtendedNetworkShowAndHideTests : NetcodeIntegrationTest
{
protected override int NumberOfClients => 3;

private GameObject m_ObjectToSpawn;
private NetworkObject m_SpawnedObject;
private NetworkManager m_ClientToHideFrom;
private NetworkManager m_LateJoinClient;
private NetworkManager m_SpawnOwner;

public ExtendedNetworkShowAndHideTests(HostOrServer hostOrServer) : base(hostOrServer) { }

protected override void OnServerAndClientsCreated()
{
m_ObjectToSpawn = CreateNetworkObjectPrefab("TestObject");
m_ObjectToSpawn.SetActive(false);
base.OnServerAndClientsCreated();
}

private bool AllClientsSpawnedObject()
{
if (!UseCMBService())
{
if (!s_GlobalNetworkObjects.ContainsKey(m_ServerNetworkManager.LocalClientId))
{
return false;
}
if (!s_GlobalNetworkObjects[m_ServerNetworkManager.LocalClientId].ContainsKey(m_SpawnedObject.NetworkObjectId))
{
return false;
}
}

foreach (var client in m_ClientNetworkManagers)
{
if (!s_GlobalNetworkObjects.ContainsKey(client.LocalClientId))
{
return false;
}
if (!s_GlobalNetworkObjects[client.LocalClientId].ContainsKey(m_SpawnedObject.NetworkObjectId))
{
return false;
}
}
return true;
}

private bool IsClientPromotedToSessionOwner()
{
if (!UseCMBService())
{
if (m_ServerNetworkManager.CurrentSessionOwner != m_ClientToHideFrom.LocalClientId)
{
return false;
}
}

foreach (var client in m_ClientNetworkManagers)
{
if (!client.IsConnectedClient)
{
continue;
}
if (client.CurrentSessionOwner != m_ClientToHideFrom.LocalClientId)
{
return false;
}
}
return true;
}

protected override void OnNewClientCreated(NetworkManager networkManager)
{
m_LateJoinClient = networkManager;

networkManager.NetworkConfig.Prefabs = m_SpawnOwner.NetworkConfig.Prefabs;
base.OnNewClientCreated(networkManager);
}

/// <summary>
/// This test validates the following NetworkShow - NetworkHide issue:
/// - During a session, a spawned object is hidden from a client.
/// - The current session owner disconnects and the client the object is hidden from is prommoted to the session owner.
/// - A new client joins and the newly promoted session owner synchronizes the newly joined client with only objects visible to it.
/// - Any already connected non-session owner client should "NetworkShow" the object to the newly connected client
/// (but only if the hidden object has SpawnWithObservers enabled)
/// </summary>
[UnityTest]
public IEnumerator HiddenObjectPromotedSessionOwnerNewClientSynchronizes()
{
// Get the test relative session owner
var sessionOwner = UseCMBService() ? m_ClientNetworkManagers[0] : m_ServerNetworkManager;
m_SpawnOwner = UseCMBService() ? m_ClientNetworkManagers[1] : m_ClientNetworkManagers[0];
m_ClientToHideFrom = UseCMBService() ? m_ClientNetworkManagers[NumberOfClients - 1] : m_ClientNetworkManagers[1];
m_ObjectToSpawn.SetActive(true);

// Spawn the object with a non-session owner client
m_SpawnedObject = SpawnObject(m_ObjectToSpawn, m_SpawnOwner).GetComponent<NetworkObject>();
yield return WaitForConditionOrTimeOut(AllClientsSpawnedObject);
AssertOnTimeout($"Not all clients spawned and instance of {m_SpawnedObject.name}");

// Hide the spawned object from the to be promoted session owner
m_SpawnedObject.NetworkHide(m_ClientToHideFrom.LocalClientId);

yield return WaitForConditionOrTimeOut(() => !m_ClientToHideFrom.SpawnManager.SpawnedObjects.ContainsKey(m_SpawnedObject.NetworkObjectId));
AssertOnTimeout($"{m_SpawnedObject.name} was not hidden from Client-{m_ClientToHideFrom.LocalClientId}!");

// Promoted a new session owner (DAHost promotes while CMB Session we disconnect the current session owner)
if (!UseCMBService())
{
m_ServerNetworkManager.PromoteSessionOwner(m_ClientToHideFrom.LocalClientId);
}
else
{
sessionOwner.Shutdown();
}

// Wait for the new session owner to be promoted and for all clients to acknowledge the promotion
yield return WaitForConditionOrTimeOut(IsClientPromotedToSessionOwner);
AssertOnTimeout($"Client-{m_ClientToHideFrom.LocalClientId} was not promoted as session owner on all client instances!");

// Connect a new client instance
yield return CreateAndStartNewClient();

// Assure the newly connected client is synchronized with the NetworkObject hidden from the newly promoted session owner
yield return WaitForConditionOrTimeOut(() => m_LateJoinClient.SpawnManager.SpawnedObjects.ContainsKey(m_SpawnedObject.NetworkObjectId));
AssertOnTimeout($"Client-{m_LateJoinClient.LocalClientId} never spawned {nameof(NetworkObject)} {m_SpawnedObject.name}!");
}
}
}

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

0 comments on commit 91bb80e

Please sign in to comment.