Skip to content

Commit ce45556

Browse files
EmandMEmandM
committed
fix: Never distribute an object to a client who is not an observer
1 parent 1ac59f0 commit ce45556

File tree

2 files changed

+96
-73
lines changed

2 files changed

+96
-73
lines changed

‎com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs‎

Lines changed: 71 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,18 +1122,16 @@ internal void OnClientDisconnectFromServer(ulong clientId)
11221122
{
11231123
if (!playerObject.DontDestroyWithOwner)
11241124
{
1125-
// DANGO-TODO: This is something that would be best for CMB Service to handle as it is part of the disconnection process
11261125
// If a player NetworkObject is being despawned, make sure to remove all children if they are marked to not be destroyed
11271126
// with the owner.
1128-
if (NetworkManager.DistributedAuthorityMode && NetworkManager.DAHost)
1127+
if (NetworkManager.DAHost)
11291128
{
11301129
// Remove any children from the player object if they are not going to be destroyed with the owner
11311130
var childNetworkObjects = playerObject.GetComponentsInChildren<NetworkObject>();
11321131
foreach (var child in childNetworkObjects)
11331132
{
1134-
// TODO: We have always just removed all children, but we might think about changing this to preserve the nested child
1135-
// hierarchy.
1136-
if (child.DontDestroyWithOwner && child.transform.transform.parent != null)
1133+
// TODO: We have always just removed all children, but we might think about changing this to preserve the nested child hierarchy.
1134+
if (child.DontDestroyWithOwner && child.transform.transform.parent)
11371135
{
11381136
// If we are here, then we are running in DAHost mode and have the authority to remove the child from its parent
11391137
child.AuthorityAppliedParenting = true;
@@ -1158,10 +1156,7 @@ internal void OnClientDisconnectFromServer(ulong clientId)
11581156
}
11591157
else if (!NetworkManager.ShutdownInProgress)
11601158
{
1161-
if (!NetworkManager.ShutdownInProgress)
1162-
{
1163-
playerObject.RemoveOwnership();
1164-
}
1159+
playerObject.RemoveOwnership();
11651160
}
11661161
}
11671162

@@ -1175,7 +1170,7 @@ internal void OnClientDisconnectFromServer(ulong clientId)
11751170
for (int i = clientOwnedObjects.Count - 1; i >= 0; i--)
11761171
{
11771172
var ownedObject = clientOwnedObjects[i];
1178-
if (ownedObject != null)
1173+
if (ownedObject)
11791174
{
11801175
// If destroying with owner, then always despawn and destroy (or defer destroying to prefab handler)
11811176
if (!ownedObject.DontDestroyWithOwner)
@@ -1196,68 +1191,86 @@ internal void OnClientDisconnectFromServer(ulong clientId)
11961191
}
11971192
else if (!NetworkManager.ShutdownInProgress)
11981193
{
1194+
// DANGO-TODO: We will want to match how the CMB service handles this. For now, we just try to evenly distribute
1195+
// ownership.
11991196
// NOTE: All of the below code only handles ownership transfer.
12001197
// For client-server, we just remove the ownership.
12011198
// For distributed authority, we need to change ownership based on parenting
12021199
if (NetworkManager.DistributedAuthorityMode)
12031200
{
1204-
// Only NetworkObjects that have the OwnershipStatus.Distributable flag set and no parent
1205-
// (ownership is transferred to all children) will have their ownership redistributed.
1206-
if (ownedObject.IsOwnershipDistributable && ownedObject.GetCachedParent() == null && !ownedObject.IsOwnershipSessionOwner)
1201+
// Only NetworkObjects that have the OwnershipStatus.Distributable flag set and are not OwnershipSessionOwner are distributed.
1202+
// If the object has a parent - skip it for now, it will be distributed when its root parent is distributed.
1203+
if (!ownedObject.IsOwnershipDistributable || ownedObject.IsOwnershipSessionOwner || ownedObject.GetCachedParent())
1204+
{
1205+
continue;
1206+
}
1207+
1208+
if (ownedObject.IsOwnershipLocked)
1209+
{
1210+
ownedObject.SetOwnershipLock(false);
1211+
}
1212+
1213+
var targetOwner = NetworkManager.ServerClientId;
1214+
if (predictedClientCount > 1)
12071215
{
1208-
if (ownedObject.IsOwnershipLocked)
1216+
clientCounter++;
1217+
clientCounter %= predictedClientCount;
1218+
targetOwner = remainingClients[clientCounter].ClientId;
1219+
}
1220+
if (EnableDistributeLogging)
1221+
{
1222+
Debug.Log($"[Disconnected][Client-{clientId}][NetworkObjectId-{ownedObject.NetworkObjectId} Distributed to Client-{targetOwner}");
1223+
}
1224+
1225+
if (!ownedObject.Observers.Contains(targetOwner))
1226+
{
1227+
targetOwner = NetworkManager.ServerClientId;
1228+
}
1229+
1230+
if (!ownedObject.Observers.Contains(targetOwner))
1231+
{
1232+
targetOwner = ownedObject.Observers.First();
1233+
}
1234+
1235+
NetworkManager.SpawnManager.ChangeOwnership(ownedObject, targetOwner, true);
1236+
1237+
// Ownership gets passed down to all children that have the same owner.
1238+
var childNetworkObjects = ownedObject.GetComponentsInChildren<NetworkObject>();
1239+
foreach (var childObject in childNetworkObjects)
1240+
{
1241+
// We already changed ownership for this
1242+
if (childObject == ownedObject)
1243+
{
1244+
continue;
1245+
}
1246+
// If the client owner disconnected, it is ok to unlock this at this point in time.
1247+
if (childObject.IsOwnershipLocked)
12091248
{
1210-
ownedObject.SetOwnershipLock(false);
1249+
childObject.SetOwnershipLock(false);
12111250
}
12121251

1213-
// DANGO-TODO: We will want to match how the CMB service handles this. For now, we just try to evenly distribute
1214-
// ownership.
1215-
var targetOwner = NetworkManager.ServerClientId;
1216-
if (predictedClientCount > 1)
1252+
// Ignore session owner marked objects
1253+
if (childObject.IsOwnershipSessionOwner)
12171254
{
1218-
clientCounter++;
1219-
clientCounter = clientCounter % predictedClientCount;
1220-
targetOwner = remainingClients[clientCounter].ClientId;
1255+
continue;
12211256
}
1222-
if (EnableDistributeLogging)
1257+
1258+
// If the child's owner is not the client disconnected and the objects are marked with either distributable or transferable, then
1259+
// do not change ownership.
1260+
if (childObject.OwnerClientId != clientId && (childObject.IsOwnershipDistributable || childObject.IsOwnershipTransferable))
12231261
{
1224-
Debug.Log($"[Disconnected][Client-{clientId}][NetworkObjectId-{ownedObject.NetworkObjectId} Distributed to Client-{targetOwner}");
1262+
continue;
12251263
}
1226-
NetworkManager.SpawnManager.ChangeOwnership(ownedObject, targetOwner, true);
1227-
// DANGO-TODO: Should we try handling inactive NetworkObjects?
1228-
// Ownership gets passed down to all children that have the same owner.
1229-
var childNetworkObjects = ownedObject.GetComponentsInChildren<NetworkObject>();
1230-
foreach (var childObject in childNetworkObjects)
1264+
1265+
if (!ownedObject.Observers.Contains(targetOwner))
1266+
{
1267+
targetOwner = ownedObject.Observers.First();
1268+
}
1269+
1270+
NetworkManager.SpawnManager.ChangeOwnership(childObject, targetOwner, true);
1271+
if (EnableDistributeLogging)
12311272
{
1232-
// We already changed ownership for this
1233-
if (childObject == ownedObject)
1234-
{
1235-
continue;
1236-
}
1237-
// If the client owner disconnected, it is ok to unlock this at this point in time.
1238-
if (childObject.IsOwnershipLocked)
1239-
{
1240-
childObject.SetOwnershipLock(false);
1241-
}
1242-
1243-
// Ignore session owner marked objects
1244-
if (childObject.IsOwnershipSessionOwner)
1245-
{
1246-
continue;
1247-
}
1248-
1249-
// If the child's owner is not the client disconnected and the objects are marked with either distributable or transferable, then
1250-
// do not change ownership.
1251-
if (childObject.OwnerClientId != clientId && (childObject.IsOwnershipDistributable || childObject.IsOwnershipTransferable))
1252-
{
1253-
continue;
1254-
}
1255-
1256-
NetworkManager.SpawnManager.ChangeOwnership(childObject, targetOwner, true);
1257-
if (EnableDistributeLogging)
1258-
{
1259-
Debug.Log($"[Disconnected][Client-{clientId}][Child of {ownedObject.NetworkObjectId}][NetworkObjectId-{ownedObject.NetworkObjectId} Distributed to Client-{targetOwner}");
1260-
}
1273+
Debug.Log($"[Disconnected][Client-{clientId}][Child of {ownedObject.NetworkObjectId}][NetworkObjectId-{ownedObject.NetworkObjectId} Distributed to Client-{targetOwner}");
12611274
}
12621275
}
12631276
}

‎com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -435,21 +435,16 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool
435435
// For client-server:
436436
// If ownership changes faster than the latency between the client-server and there are NetworkVariables being updated during ownership changes,
437437
// then notify the user they could potentially lose state updates if developer logging is enabled.
438-
if (!NetworkManager.DistributedAuthorityMode && m_LastChangeInOwnership.ContainsKey(networkObject.NetworkObjectId) && m_LastChangeInOwnership[networkObject.NetworkObjectId] > Time.realtimeSinceStartup)
438+
if (NetworkManager.LogLevel == LogLevel.Developer && !NetworkManager.DistributedAuthorityMode && m_LastChangeInOwnership.ContainsKey(networkObject.NetworkObjectId) && m_LastChangeInOwnership[networkObject.NetworkObjectId] > Time.realtimeSinceStartup)
439439
{
440-
var hasNetworkVariables = false;
441440
for (int i = 0; i < networkObject.ChildNetworkBehaviours.Count; i++)
442441
{
443-
hasNetworkVariables = networkObject.ChildNetworkBehaviours[i].NetworkVariableFields.Count > 0;
444-
if (hasNetworkVariables)
442+
if (networkObject.ChildNetworkBehaviours[i].NetworkVariableFields.Count > 0)
445443
{
444+
NetworkLog.LogWarningServer($"[Rapid Ownership Change Detected][Potential Loss in State] Detected a rapid change in ownership that exceeds a frequency less than {k_MaximumTickOwnershipChangeMultiplier}x the current network tick rate! Provide at least {k_MaximumTickOwnershipChangeMultiplier}x the current network tick rate between ownership changes to avoid NetworkVariable state loss.");
446445
break;
447446
}
448447
}
449-
if (hasNetworkVariables && NetworkManager.LogLevel == LogLevel.Developer)
450-
{
451-
NetworkLog.LogWarningServer($"[Rapid Ownership Change Detected][Potential Loss in State] Detected a rapid change in ownership that exceeds a frequency less than {k_MaximumTickOwnershipChangeMultiplier}x the current network tick rate! Provide at least {k_MaximumTickOwnershipChangeMultiplier}x the current network tick rate between ownership changes to avoid NetworkVariable state loss.");
452-
}
453448
}
454449

455450
if (NetworkManager.DistributedAuthorityMode)
@@ -517,7 +512,6 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool
517512
throw new SpawnStateException("Object is not spawned");
518513
}
519514

520-
521515
if (networkObject.OwnerClientId == clientId && networkObject.PreviousOwnerId == clientId)
522516
{
523517
if (NetworkManager.LogLevel == LogLevel.Developer)
@@ -527,6 +521,15 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool
527521
return;
528522
}
529523

524+
if (!networkObject.Observers.Contains(clientId))
525+
{
526+
if (NetworkManager.LogLevel == LogLevel.Developer)
527+
{
528+
NetworkLog.LogWarningServer($"[Invalid Owner] Cannot send Ownership change as client-{clientId} cannot see {networkObject.name}! Use {nameof(NetworkObject.NetworkShow)} first.");
529+
}
530+
return;
531+
}
532+
530533
// Used to distinguish whether a new owner should receive any currently dirty NetworkVariable updates
531534
networkObject.PreviousOwnerId = networkObject.OwnerClientId;
532535

@@ -1877,6 +1880,12 @@ internal void GetObjectDistribution(ref Dictionary<uint, Dictionary<ulong, List<
18771880
}
18781881
}
18791882

1883+
/// <summary>
1884+
/// Distributes an even portion of spawned NetworkObjects to the given client.
1885+
/// This is called as part of the client connection process to ensure that all clients have a fair share of the spawned NetworkObjects.
1886+
/// DANGO-TODO: This will be handled by the CMB Service in the future.
1887+
/// </summary>
1888+
/// <param name="clientId">Client to distribute NetworkObjects to</param>
18801889
internal void DistributeNetworkObjects(ulong clientId)
18811890
{
18821891
if (!NetworkManager.DistributedAuthorityMode)
@@ -1889,7 +1898,6 @@ internal void DistributeNetworkObjects(ulong clientId)
18891898
return;
18901899
}
18911900

1892-
18931901
// DA-NGO CMB SERVICE NOTES:
18941902
// The most basic object distribution should be broken up into a table of spawned object types
18951903
// where each type contains a list of each client's owned objects of that type that can be
@@ -1956,6 +1964,11 @@ internal void DistributeNetworkObjects(ulong clientId)
19561964
{
19571965
if ((i % offsetCount) == 0)
19581966
{
1967+
while (!ownerList.Value[i].Observers.Contains(clientId))
1968+
{
1969+
i++;
1970+
}
1971+
19591972
var children = ownerList.Value[i].GetComponentsInChildren<NetworkObject>();
19601973
// Since the ownerList.Value[i] has to be distributable, then transfer all child NetworkObjects
19611974
// with the same owner clientId and are marked as distributable also to the same client to keep
@@ -1969,13 +1982,10 @@ internal void DistributeNetworkObjects(ulong clientId)
19691982
}
19701983
if (!child.IsOwnershipDistributable || !child.IsOwnershipTransferable)
19711984
{
1972-
if (NetworkManager.LogLevel == LogLevel.Developer)
1973-
{
1974-
NetworkLog.LogWarning($"Sibling {child.name} of root parent {ownerList.Value[i].name} is neither transferrable or distributable! Object distribution skipped and could lead to a potentially un-owned or owner-mismatched {nameof(NetworkObject)}!");
1975-
}
1985+
NetworkLog.LogWarning($"Sibling {child.name} of root parent {ownerList.Value[i].name} is neither transferable or distributable! Object distribution skipped and could lead to a potentially un-owned or owner-mismatched {nameof(NetworkObject)}!");
19761986
continue;
19771987
}
1978-
// Transfer ownership of all distributable =or= transferrable children with the same owner to the same client to preserve the sibling ownership tree.
1988+
// Transfer ownership of all distributable =or= transferable children with the same owner to the same client to preserve the sibling ownership tree.
19791989
ChangeOwnership(child, clientId, true);
19801990
// Note: We don't increment the distributed count for these children as they are skipped when getting the object distribution
19811991
}

0 commit comments

Comments
 (0)