diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 12accde27a..f63cda27e3 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -14,12 +14,13 @@ Additional documentation and release notes are available at [Multiplayer Documen - Fixed exception being thrown when a `GameObject` with an associated `NetworkTransform` is disabled. (#3243) - Fixed `NetworkObject.DeferDespawn` to respect the `DestroyGameObject` parameter. (#3219) -- Changed the `NetworkTimeSystem.Sync` method to use half RTT to calculate the desired local time offset as opposed to the full RTT. (#3212) +- Fixed issue with distributing parented children that have the distributable and/or transferrable permissions set and have the same owner as the root parent, that has the distributable permission set, were not being distributed to the same client upon the owning client disconnecting when using a distributed authority network topology. (#3203) - Fixed issue where a spawned `NetworkObject` that was registered with a prefab handler and owned by a client would invoke destroy more than once on the host-server side if the client disconnected while the `NetworkObject` was still spawned. (#3200) - Fixed issue where `NetworkVariableBase` derived classes were not being re-initialized if the associated `NetworkObject` instance was not destroyed and re-spawned. (#3181) ### Changed +- Changed the `NetworkTimeSystem.Sync` method to use half RTT to calculate the desired local time offset as opposed to the full RTT. (#3212) ## [2.2.0] - 2024-12-12 diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs index 06a07acfc4..fd418d34c2 100644 --- a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs @@ -1225,7 +1225,7 @@ internal void OnClientDisconnectFromServer(ulong clientId) } NetworkManager.SpawnManager.ChangeOwnership(ownedObject, targetOwner, true); // DANGO-TODO: Should we try handling inactive NetworkObjects? - // Ownership gets passed down to all children + // Ownership gets passed down to all children that have the same owner. var childNetworkObjects = ownedObject.GetComponentsInChildren(); foreach (var childObject in childNetworkObjects) { @@ -1245,6 +1245,14 @@ internal void OnClientDisconnectFromServer(ulong clientId) { continue; } + + // If the child's owner is not the client disconnected and the objects are marked with either distributable or transferable, then + // do not change ownership. + if (childObject.OwnerClientId != clientId && (childObject.IsOwnershipDistributable || childObject.IsOwnershipTransferable)) + { + continue; + } + NetworkManager.SpawnManager.ChangeOwnership(childObject, targetOwner, true); if (EnableDistributeLogging) { diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs index c449c9aadb..cf8f1b2bb5 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs @@ -1659,7 +1659,20 @@ internal void SpawnInternal(bool destroyWithScene, ulong ownerClientId, bool pla } if (NetworkManager.NetworkConfig.EnableSceneManagement) { - NetworkSceneHandle = NetworkManager.SceneManager.ClientSceneHandleToServerSceneHandle[gameObject.scene.handle]; + if (!NetworkManager.SceneManager.ClientSceneHandleToServerSceneHandle.ContainsKey(gameObject.scene.handle)) + { + // Most likely this issue is due to an integration test + if (NetworkManager.LogLevel <= LogLevel.Developer) + { + NetworkLog.LogWarning($"Failed to find scene handle {gameObject.scene.handle} for {gameObject.name}!"); + } + // Just use the existing handle + NetworkSceneHandle = gameObject.scene.handle; + } + else + { + NetworkSceneHandle = NetworkManager.SceneManager.ClientSceneHandleToServerSceneHandle[gameObject.scene.handle]; + } } if (DontDestroyWithOwner && !IsOwnershipDistributable) { diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs index beba9c7055..21ac399dd5 100644 --- a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs @@ -1755,16 +1755,24 @@ internal void GetObjectDistribution(ref Dictionary(); + if (parentNetworkObject != null && parentNetworkObject.OwnerClientId == networkObject.OwnerClientId + && (networkObject.IsOwnershipDistributable || networkObject.IsOwnershipTransferable)) { - var parentNetworkObject = networkObject.transform.parent.GetComponent(); - if (parentNetworkObject != null && parentNetworkObject.OwnerClientId == networkObject.OwnerClientId) - { - continue; - } + continue; } + } + + // At this point we only allow things marked with the distributable permission and is not locked to be distributed + if (networkObject.IsOwnershipDistributable && !networkObject.IsOwnershipLocked) + { + // We have to check if it is an in-scene placed NetworkObject and if it is get the source prefab asset GlobalObjectIdHash value of the in-scene placed instance // since all in-scene placed instances use unique GlobalObjectIdHash values. var globalOjectIdHash = networkObject.IsSceneObject.HasValue && networkObject.IsSceneObject.Value ? networkObject.InScenePlacedSourceGlobalObjectIdHash : networkObject.GlobalObjectIdHash; @@ -1879,8 +1887,29 @@ internal void DistributeNetworkObjects(ulong clientId) { if ((i % offsetCount) == 0) { + var children = ownerList.Value[i].GetComponentsInChildren(); + // Since the ownerList.Value[i] has to be distributable, then transfer all child NetworkObjects + // with the same owner clientId and are marked as distributable also to the same client to keep + // the owned distributable parent with the owned distributable children + foreach (var child in children) + { + // Ignore the parent and any child that does not have the same owner or that is already owned by the currently targeted client + if (child == ownerList.Value[i] || child.OwnerClientId != ownerList.Value[i].OwnerClientId || child.OwnerClientId == clientId) + { + continue; + } + if ((!child.IsOwnershipDistributable || !child.IsOwnershipTransferable) && NetworkManager.LogLevel == LogLevel.Developer) + { + 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)}!"); + continue; + } + // Transfer ownership of all distributable =or= transferrable children with the same owner to the same client to preserve the sibling ownership tree. + ChangeOwnership(child, clientId, true); + // Note: We don't increment the distributed count for these children as they are skipped when getting the object distribution + } + // Finally, transfer ownership of the root parent ChangeOwnership(ownerList.Value[i], clientId, true); - //if (EnableDistributeLogging) + if (EnableDistributeLogging) { Debug.Log($"[Client-{ownerList.Key}][NetworkObjectId-{ownerList.Value[i].NetworkObjectId} Distributed to Client-{clientId}"); } diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/ParentChildDistibutionTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/ParentChildDistibutionTests.cs new file mode 100644 index 0000000000..bd74d710a2 --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/ParentChildDistibutionTests.cs @@ -0,0 +1,291 @@ +using System.Collections; +using System.Collections.Generic; +using System.Text; +using NUnit.Framework; +using Unity.Netcode.TestHelpers.Runtime; +using UnityEngine; +using UnityEngine.TestTools; + +namespace Unity.Netcode.RuntimeTests +{ + internal class ParentChildDistibutionTests : IntegrationTestWithApproximation + { + protected override int NumberOfClients => 4; + + private GameObject m_GenericPrefab; + private ulong m_OriginalOwnerId; + private List m_TargetSpawnedObjects = new List(); + private List m_AltTargetSpawnedObjects = new List(); + private Dictionary> m_AncillarySpawnedObjects = new Dictionary>(); + private List m_NetworkManagers = new List(); + private StringBuilder m_ErrorMsg = new StringBuilder(); + + public ParentChildDistibutionTests() : base(HostOrServer.DAHost) + { + } + + protected override IEnumerator OnTearDown() + { + m_NetworkManagers.Clear(); + m_TargetSpawnedObjects.Clear(); + m_AncillarySpawnedObjects.Clear(); + m_AltTargetSpawnedObjects.Clear(); + return base.OnTearDown(); + } + + protected override void OnServerAndClientsCreated() + { + m_GenericPrefab = CreateNetworkObjectPrefab("GenPrefab"); + var networkObject = m_GenericPrefab.GetComponent(); + networkObject.DontDestroyWithOwner = true; + base.OnServerAndClientsCreated(); + } + + private bool AllTargetedInstancesSpawned() + { + m_ErrorMsg.Clear(); + foreach (var client in m_NetworkManagers) + { + foreach (var spawnedObject in m_TargetSpawnedObjects) + { + if (!client.SpawnManager.SpawnedObjects.ContainsKey(spawnedObject.NetworkObjectId)) + { + m_ErrorMsg.AppendLine($"{client.name} has not spawned {spawnedObject.name}!"); + } + } + } + + return m_ErrorMsg.Length == 0; + } + + private bool AllAncillaryInstancesSpawned() + { + m_ErrorMsg.Clear(); + foreach (var client in m_NetworkManagers) + { + foreach (var clientObjects in m_AncillarySpawnedObjects) + { + foreach (var spawnedObject in clientObjects.Value) + { + if (!client.SpawnManager.SpawnedObjects.ContainsKey(spawnedObject.NetworkObjectId)) + { + m_ErrorMsg.AppendLine($"{client.name} has not spawned {spawnedObject.name}!"); + } + } + } + } + + return m_ErrorMsg.Length == 0; + } + + /// + /// Validates that a new owner is assigned to all of the targeted objects + /// + private bool TargetedObjectsChangedOwnership() + { + m_ErrorMsg.Clear(); + var newOwnerId = m_OriginalOwnerId; + foreach (var spawnedObject in m_AltTargetSpawnedObjects) + { + var isParentLocked = spawnedObject.transform.parent != null ? spawnedObject.transform.parent.GetComponent().IsOwnershipLocked : false; + if (!isParentLocked && !spawnedObject.IsOwnershipLocked && spawnedObject.OwnerClientId == m_OriginalOwnerId) + { + m_ErrorMsg.AppendLine($"{spawnedObject.name} still is owned by Client-{m_OriginalOwnerId}!"); + } + else if (!isParentLocked && !spawnedObject.IsOwnershipLocked && m_OriginalOwnerId == newOwnerId) + { + newOwnerId = spawnedObject.OwnerClientId; + } + else if ((isParentLocked || spawnedObject.IsOwnershipLocked) && spawnedObject.OwnerClientId != m_OriginalOwnerId) + { + if (isParentLocked) + { + m_ErrorMsg.AppendLine($"{spawnedObject.name}'s parent was locked but its owner changed to Client-{m_OriginalOwnerId}!"); + } + else + { + m_ErrorMsg.AppendLine($"{spawnedObject.name} was locked but its owner changed to Client-{m_OriginalOwnerId}!"); + } + } + + if (spawnedObject.OwnerClientId != newOwnerId) + { + m_ErrorMsg.AppendLine($"{spawnedObject.name} is not owned by Client-{newOwnerId}!"); + } + } + return m_ErrorMsg.Length == 0; + } + + private bool AncillaryObjectsKeptOwnership() + { + m_ErrorMsg.Clear(); + foreach (var clientObjects in m_AncillarySpawnedObjects) + { + foreach (var spawnedObject in clientObjects.Value) + { + if (spawnedObject.OwnerClientId != clientObjects.Key) + { + m_ErrorMsg.AppendLine($"{spawnedObject.name} changed ownership to Client-{spawnedObject.OwnerClientId}!"); + } + } + } + return m_ErrorMsg.Length == 0; + } + + public enum DistributionTypes + { + UponConnect, + UponDisconnect + } + + public enum OwnershipLocking + { + NoLocking, + LockRootParent, + LockTargetChild, + } + + + [UnityTest] + public IEnumerator DistributeOwnerHierarchy([Values] DistributionTypes distributionType, [Values] OwnershipLocking ownershipLock) + { + m_NetworkManagers.Clear(); + m_TargetSpawnedObjects.Clear(); + m_AncillarySpawnedObjects.Clear(); + m_AltTargetSpawnedObjects.Clear(); + + if (distributionType == DistributionTypes.UponConnect) + { + m_ClientNetworkManagers[3].Shutdown(); + } + + if (!UseCMBService()) + { + m_NetworkManagers.Add(m_ServerNetworkManager); + } + m_NetworkManagers.AddRange(m_ClientNetworkManagers); + if (distributionType == DistributionTypes.UponConnect) + { + m_NetworkManagers.Remove(m_ClientNetworkManagers[3]); + } + + // When testing connect redistribution, + var instances = distributionType == DistributionTypes.UponDisconnect ? 1 : 2; + var rootObject = (GameObject)null; + var childOne = (GameObject)null; + var childTwo = (GameObject)null; + var networkObject = (NetworkObject)null; + + for (int i = 0; i < instances; i++) + { + rootObject = SpawnObject(m_GenericPrefab, m_ClientNetworkManagers[0]); + networkObject = rootObject.GetComponent(); + networkObject.SetOwnershipStatus(NetworkObject.OwnershipStatus.Distributable); + if (ownershipLock == OwnershipLocking.LockRootParent && distributionType == DistributionTypes.UponConnect) + { + networkObject.SetOwnershipLock(true); + } + m_TargetSpawnedObjects.Add(networkObject); + + // Used to validate nested transferable transfers to the same owner + childOne = SpawnObject(m_GenericPrefab, m_ClientNetworkManagers[0]); + networkObject = childOne.GetComponent(); + networkObject.SetOwnershipStatus(NetworkObject.OwnershipStatus.Transferable); + m_TargetSpawnedObjects.Add(networkObject); + + // Used to validate nested distributable transfers to the same owner + childTwo = SpawnObject(m_GenericPrefab, m_ClientNetworkManagers[0]); + networkObject = childTwo.GetComponent(); + networkObject.SetOwnershipStatus(NetworkObject.OwnershipStatus.Distributable); + if (ownershipLock == OwnershipLocking.LockTargetChild && distributionType == DistributionTypes.UponConnect) + { + networkObject.SetOwnershipLock(true); + } + m_TargetSpawnedObjects.Add(childTwo.GetComponent()); + + childOne.transform.parent = rootObject.transform; + childTwo.transform.parent = rootObject.transform; + } + yield return WaitForConditionOrTimeOut(AllTargetedInstancesSpawned); + AssertOnTimeout($"Timed out waiting for all targeted cloned instances to spawn!\n {m_ErrorMsg}"); + + // Used to validate that other children do not transfer ownership when redistributing. + + var altAchildOne = SpawnObject(m_GenericPrefab, m_ClientNetworkManagers[1]); + var altAchildTwo = SpawnObject(m_GenericPrefab, m_ClientNetworkManagers[1]); + m_AncillarySpawnedObjects.Add(m_ClientNetworkManagers[1].LocalClientId, new List()); + networkObject = altAchildOne.GetComponent(); + networkObject.SetOwnershipStatus(NetworkObject.OwnershipStatus.Distributable); + m_AncillarySpawnedObjects[m_ClientNetworkManagers[1].LocalClientId].Add(networkObject); + altAchildOne.transform.parent = rootObject.transform; + + networkObject = altAchildTwo.GetComponent(); + networkObject.SetOwnershipStatus(NetworkObject.OwnershipStatus.Transferable); + m_AncillarySpawnedObjects[m_ClientNetworkManagers[1].LocalClientId].Add(networkObject); + altAchildTwo.transform.parent = rootObject.transform; + + var altBchildOne = SpawnObject(m_GenericPrefab, m_ClientNetworkManagers[2]); + var altBchildTwo = SpawnObject(m_GenericPrefab, m_ClientNetworkManagers[2]); + m_AncillarySpawnedObjects.Add(m_ClientNetworkManagers[2].LocalClientId, new List()); + networkObject = altBchildOne.GetComponent(); + networkObject.SetOwnershipStatus(NetworkObject.OwnershipStatus.Distributable); + m_AncillarySpawnedObjects[m_ClientNetworkManagers[2].LocalClientId].Add(networkObject); + altBchildOne.transform.parent = rootObject.transform; + + networkObject = altBchildTwo.GetComponent(); + networkObject.SetOwnershipStatus(NetworkObject.OwnershipStatus.Transferable); + m_AncillarySpawnedObjects[m_ClientNetworkManagers[2].LocalClientId].Add(networkObject); + altBchildTwo.transform.parent = rootObject.transform; + + yield return WaitForConditionOrTimeOut(AllAncillaryInstancesSpawned); + AssertOnTimeout($"Timed out waiting for all ancillary cloned instances to spawn!\n {m_ErrorMsg}"); + + // Now disconnect the client that owns the rootObject + // Get the original clientId + m_OriginalOwnerId = m_ClientNetworkManagers[0].LocalClientId; + + if (distributionType == DistributionTypes.UponDisconnect) + { + // Swap out the original owner's NetworkObject with one of the other client's since those instances will + // be destroyed when the client disconnects. + foreach (var entry in m_TargetSpawnedObjects) + { + m_AltTargetSpawnedObjects.Add(m_ClientNetworkManagers[1].SpawnManager.SpawnedObjects[entry.NetworkObjectId]); + } + // Disconnect the client to trigger object redistribution + m_ClientNetworkManagers[0].Shutdown(); + } + else + { + m_ClientNetworkManagers[3].StartClient(); + yield return WaitForConditionOrTimeOut(() => m_ClientNetworkManagers[3].IsConnectedClient); + AssertOnTimeout($"{m_ClientNetworkManagers[3].name} failed to reconnect!"); + } + + // Verify all of the targeted objects changed ownership to the same client + yield return WaitForConditionOrTimeOut(TargetedObjectsChangedOwnership); + AssertOnTimeout($"All targeted objects did not get distributed to the same owner!\n {m_ErrorMsg}"); + + // When enabled, you should see one of the two root instances that have children get distributed to + // the reconnected client. + if (m_EnableVerboseDebug && distributionType == DistributionTypes.UponConnect) + { + m_ErrorMsg.Clear(); + m_ErrorMsg.AppendLine($"Original targeted objects owner: {m_OriginalOwnerId}"); + foreach (var spawnedObject in m_TargetSpawnedObjects) + { + m_ErrorMsg.AppendLine($"{spawnedObject.name} new owner: {spawnedObject.OwnerClientId}"); + } + Debug.Log($"{m_ErrorMsg}"); + } + + // We only want to make sure no other children owned by still connected clients change ownership + if (distributionType == DistributionTypes.UponDisconnect) + { + // Verify the ancillary objects kept the same ownership + yield return WaitForConditionOrTimeOut(AncillaryObjectsKeptOwnership); + AssertOnTimeout($"All ancillary objects did not get distributed to the same owner!\n {m_ErrorMsg}"); + } + } + } +} diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/ParentChildDistibutionTests.cs.meta b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/ParentChildDistibutionTests.cs.meta new file mode 100644 index 0000000000..1cf2f2310b --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/ParentChildDistibutionTests.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 45d9d6508546cda4dba613ef82b6e51e \ No newline at end of file