Skip to content

fix: add CheckObjectVisibility gating for NetworkShow and NetworkHide #4160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
NoelStephensUnity wants to merge 1 commit into
base: develop-2.0.0
Choose a base branch
from
Draft

fix: add CheckObjectVisibility gating for NetworkShow and NetworkHide #4160

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
111 changes: 64 additions & 47 deletions com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1535,7 +1535,8 @@ internal NetworkSceneHandle GetSceneOriginHandle()
/// <see cref="NetworkHide(ulong)"/> or <see cref="NetworkHide(List{NetworkObject}, ulong)"/><br />
/// </remarks>
/// <param name="clientId">The targeted client</param>
public void NetworkShow(ulong clientId)
/// <param name="checkVisibility">Whether to check object visibility before showing. Requires <see cref="CheckObjectVisibility"/> to be set.</param>
public void NetworkShow(ulong clientId, bool checkVisibility = true)
{
if (!IsSpawned)
{
Expand Down Expand Up @@ -1575,9 +1576,9 @@ public void NetworkShow(ulong clientId)
return;
}

if (CheckObjectVisibility != null && !CheckObjectVisibility(clientId))
if (checkVisibility && CheckObjectVisibility != null && !CheckObjectVisibility(clientId))
{
if (NetworkManagerOwner.LogLevel <= LogLevel.Normal)
if (NetworkManagerOwner.LogLevel <= LogLevel.Developer)
{
NetworkLog.LogWarning($"[NetworkShow] Trying to make {nameof(NetworkObject)} {name} visible to client ({clientId}) but {nameof(CheckObjectVisibility)} returned false!");
}
Expand All @@ -1603,7 +1604,8 @@ public void NetworkShow(ulong clientId)
/// </remarks>
/// <param name="networkObjects">The objects to become "netcode visible" to the targeted client</param>
/// <param name="clientId">The targeted client</param>
public static void NetworkShow(List<NetworkObject> networkObjects, ulong clientId)
/// <param name="checkVisibility">Whether to check object visibility before showing. Requires <see cref="CheckObjectVisibility"/> to be set.</param>
public static void NetworkShow(List<NetworkObject> networkObjects, ulong clientId, bool checkVisibility = true)
{
if (networkObjects == null || networkObjects.Count == 0)
{
Expand All @@ -1612,7 +1614,7 @@ public static void NetworkShow(List<NetworkObject> networkObjects, ulong clientI
}
foreach (var networkObject in networkObjects)
{
networkObject.NetworkShow(clientId);
networkObject.NetworkShow(clientId, checkVisibility);
}
}

Expand All @@ -1630,7 +1632,8 @@ public static void NetworkShow(List<NetworkObject> networkObjects, ulong clientI
/// <see cref="NetworkShow(ulong)"/> or <see cref="NetworkShow(List{NetworkObject}, ulong)"/><br />
/// </remarks>
/// <param name="clientId">The targeted client</param>
public void NetworkHide(ulong clientId)
/// <param name="checkVisibility">Whether to check object visibility before hiding. Requires <see cref="CheckObjectVisibility"/> to be set.</param>
public void NetworkHide(ulong clientId, bool checkVisibility = true)
{
if (!IsSpawned)
{
Expand Down Expand Up @@ -1661,58 +1664,71 @@ public void NetworkHide(ulong clientId)
}
}

if (!NetworkManagerOwner.SpawnManager.RemoveObjectFromShowingTo(this, clientId))
// If an object was marked for showing to a client, but not yet sent, we can just remove it from the list and not send a destroy message
if (NetworkManagerOwner.SpawnManager.RemoveObjectFromShowingTo(this, clientId))
{
return;
}

// If the check visibility is enabled and the CheckObjectVisibility delegate is not null and returns true, log a warning and return
if (checkVisibility && CheckObjectVisibility != null && CheckObjectVisibility(clientId))
{
if (!Observers.Contains(clientId))
if (NetworkManagerOwner.LogLevel <= LogLevel.Developer)
{
if (NetworkManagerOwner.LogLevel <= LogLevel.Developer)
{
NetworkLog.LogWarning($"[{name}] {nameof(NetworkObject)} already hidden from Client-{clientId}! (ignoring)");
}
return;
NetworkLog.LogWarning($"[NetworkShow] Trying to hide {nameof(NetworkObject)} {name} from client ({clientId}) but {nameof(CheckObjectVisibility)} returned true!");
}
Observers.Remove(clientId);
return;
}

var message = new DestroyObjectMessage
if (!Observers.Contains(clientId))
{
if (NetworkManagerOwner.LogLevel <= LogLevel.Developer)
{
NetworkObjectId = NetworkObjectId,
DestroyGameObject = !InScenePlaced,
IsDistributedAuthority = NetworkManagerOwner.DistributedAuthorityMode,
IsTargetedDestroy = NetworkManagerOwner.DistributedAuthorityMode,
TargetClientId = clientId, // Just always populate this value whether we write it or not
DeferredDespawnTick = DeferredDespawnTick,
};
NetworkLog.LogWarning($"[{name}] {nameof(NetworkObject)} already hidden from Client-{clientId}! (ignoring)");
}
return;
}
Observers.Remove(clientId);

var size = 0;
if (NetworkManagerOwner.DistributedAuthorityMode)
var message = new DestroyObjectMessage
{
NetworkObjectId = NetworkObjectId,
DestroyGameObject = !InScenePlaced,
IsDistributedAuthority = NetworkManagerOwner.DistributedAuthorityMode,
IsTargetedDestroy = NetworkManagerOwner.DistributedAuthorityMode,
TargetClientId = clientId, // Just always populate this value whether we write it or not
DeferredDespawnTick = DeferredDespawnTick,
};

var size = 0;
if (NetworkManagerOwner.DistributedAuthorityMode)
{
if (!NetworkManagerOwner.DAHost)
{
if (!NetworkManagerOwner.DAHost)
{
// Send destroy call to service or DAHost
size = NetworkManagerOwner.ConnectionManager.SendMessage(ref message, MessageDeliveryType<DestroyObjectMessage>.DefaultDelivery, NetworkManager.ServerClientId);
}
else // DAHost mocking service
{
// Send destroy call
size = NetworkManagerOwner.ConnectionManager.SendMessage(ref message, MessageDeliveryType<DestroyObjectMessage>.DefaultDelivery, clientId);
// Broadcast the destroy to all clients so they can update their observers list
foreach (var client in NetworkManagerOwner.ConnectionManager.ConnectedClientIds)
{
if (client == clientId || client == NetworkManagerOwner.LocalClientId)
{
continue;
}
size += NetworkManagerOwner.ConnectionManager.SendMessage(ref message, MessageDeliveryType<DestroyObjectMessage>.DefaultDelivery, client);
}
}
// Send destroy call to service or DAHost
size = NetworkManagerOwner.ConnectionManager.SendMessage(ref message, MessageDeliveryType<DestroyObjectMessage>.DefaultDelivery, NetworkManager.ServerClientId);
}
else
else // DAHost mocking service
{
// Send destroy call
size = NetworkManagerOwner.ConnectionManager.SendMessage(ref message, MessageDeliveryType<DestroyObjectMessage>.DefaultDelivery, clientId);
// Broadcast the destroy to all clients so they can update their observers list
foreach (var client in NetworkManagerOwner.ConnectionManager.ConnectedClientIds)
{
if (client == clientId || client == NetworkManagerOwner.LocalClientId)
{
continue;
}
size += NetworkManagerOwner.ConnectionManager.SendMessage(ref message, MessageDeliveryType<DestroyObjectMessage>.DefaultDelivery, client);
}
}
NetworkManagerOwner.NetworkMetrics.TrackObjectDestroySent(clientId, this, size);
}
else
{
// Send destroy call
size = NetworkManagerOwner.ConnectionManager.SendMessage(ref message, MessageDeliveryType<DestroyObjectMessage>.DefaultDelivery, clientId);
}
NetworkManagerOwner.NetworkMetrics.TrackObjectDestroySent(clientId, this, size);
}

/// <summary>
Expand All @@ -1730,7 +1746,8 @@ public void NetworkHide(ulong clientId)
/// </remarks>
/// <param name="networkObjects">The <see cref="NetworkObject"/>s that will become "netcode invisible" to the targeted client</param>
/// <param name="clientId">The targeted client</param>
public static void NetworkHide(List<NetworkObject> networkObjects, ulong clientId)
/// <param name="checkVisibility">Whether to check object visibility before hiding</param>
public static void NetworkHide(List<NetworkObject> networkObjects, ulong clientId, bool checkVisibility = true)
{
if (networkObjects == null || networkObjects.Count == 0)
{
Expand All @@ -1739,7 +1756,7 @@ public static void NetworkHide(List<NetworkObject> networkObjects, ulong clientI
}
foreach (var networkObject in networkObjects)
{
networkObject.NetworkHide(clientId);
networkObject.NetworkHide(clientId, checkVisibility);
}
}

Expand Down
Loading