diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index a8e276b622..7bce305b1b 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -20,6 +20,13 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Changed +## [2.4.3] - 2025-06-25 + +### Fixed + +- Fixed issue where spawned objects with `NetworkObject.DontDestroyWithOwner` set to `false` would not be destroyed when using a client-server network topology. (#3522) + + ## [2.4.2] - 2025-06-13 ### Fixed diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs index ade21e09ce..f55997b3e7 100644 --- a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs @@ -1177,8 +1177,7 @@ internal void OnClientDisconnectFromServer(ulong clientId) if (ownedObject) { // If destroying with owner, then always despawn and destroy (or defer destroying to prefab handler) - // Handle an object with no observers other than the current disconnecting client as destroying with owner - if (!ownedObject.DontDestroyWithOwner && (ownedObject.Observers.Count == 0 || (ownedObject.Observers.Count == 1 && ownedObject.Observers.Contains(clientId)))) + if (!ownedObject.DontDestroyWithOwner) { if (NetworkManager.PrefabHandler.ContainsHandler(ownedObject.GlobalObjectIdHash)) { @@ -1244,7 +1243,7 @@ internal void OnClientDisconnectFromServer(ulong clientId) } // Skip destroy with owner objects as they will be processed by the outer loop - if (!childObject.DontDestroyWithOwner && (childObject.Observers.Count == 0 || (childObject.Observers.Count == 1 && childObject.Observers.Contains(clientId)))) + if (!childObject.DontDestroyWithOwner) { continue; } diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectDontDestroyWithOwnerTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectDontDestroyWithOwnerTests.cs index 9466148d38..cf4f83d17f 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectDontDestroyWithOwnerTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectDontDestroyWithOwnerTests.cs @@ -1,5 +1,7 @@ using System.Collections; +using System.Collections.Generic; using System.Linq; +using System.Text; using NUnit.Framework; using Unity.Netcode.TestHelpers.Runtime; using UnityEngine; @@ -13,24 +15,32 @@ namespace Unity.Netcode.RuntimeTests [TestFixture(HostOrServer.Server)] internal class NetworkObjectDontDestroyWithOwnerTests : NetcodeIntegrationTest { - private const int k_NumberObjectsToSpawn = 32; - protected override int NumberOfClients => 1; + private const int k_NumberObjectsToSpawn = 16; + protected override int NumberOfClients => 3; - // TODO: [CmbServiceTests] Adapt to run with the service - protected override bool UseCMBService() + public enum ParentedPass { - return false; + NoParent, + HasParent } - protected GameObject m_PrefabToSpawn; + protected GameObject m_DestroyWithOwnerPrefab; + protected GameObject m_DontDestroyWithOwnerPrefab; protected GameObject m_PrefabNoObserversSpawn; + private ulong m_NonAuthorityClientId; + + private List m_DontDestroyObjectIds = new List(); + private List m_DestroyObjectIds = new List(); + public NetworkObjectDontDestroyWithOwnerTests(HostOrServer hostOrServer) : base(hostOrServer) { } protected override void OnServerAndClientsCreated() { - m_PrefabToSpawn = CreateNetworkObjectPrefab("ClientOwnedObject"); - m_PrefabToSpawn.GetComponent().DontDestroyWithOwner = true; + m_DontDestroyWithOwnerPrefab = CreateNetworkObjectPrefab("DestroyWith"); + m_DontDestroyWithOwnerPrefab.GetComponent().DontDestroyWithOwner = true; + + m_DestroyWithOwnerPrefab = CreateNetworkObjectPrefab("DontDestroyWith"); m_PrefabNoObserversSpawn = CreateNetworkObjectPrefab("NoObserversObject"); var prefabNoObserversNetworkObject = m_PrefabNoObserversSpawn.GetComponent(); @@ -38,46 +48,230 @@ protected override void OnServerAndClientsCreated() prefabNoObserversNetworkObject.DontDestroyWithOwner = true; } - [UnityTest] - public IEnumerator DontDestroyWithOwnerTest() + /// + /// Validates all instances of both prefab types have spawned on all clients. + /// + private bool HaveAllObjectInstancesSpawned(StringBuilder errorLog) + { + foreach (var networkManager in m_NetworkManagers) + { + var relativeSpawnedObjects = networkManager.SpawnManager.SpawnedObjects; + for (int i = 0; i < k_NumberObjectsToSpawn; i++) + { + var dontDestroyObjectId = m_DontDestroyObjectIds[i]; + var destroyObjectId = m_DestroyObjectIds[i]; + if (!relativeSpawnedObjects.ContainsKey(dontDestroyObjectId)) + { + errorLog.AppendLine($"[Client-{networkManager.LocalClientId}][DontDestroyWithOwner] Has not spawned {nameof(NetworkObject)}-{dontDestroyObjectId}!"); + } + if (!relativeSpawnedObjects.ContainsKey(destroyObjectId)) + { + errorLog.AppendLine($"[Client-{networkManager.LocalClientId}][DestroyWithOwner] Has not spawned {nameof(NetworkObject)}-{destroyObjectId}!"); + } + } + } + return errorLog.Length == 0; + } + + /// + /// Helper method to spawn the two different sets of objects. + /// Those that will destroy with the owner and those that will not. + /// + /// type of prefab to use for spawning + private void SpawnAllObjects(bool dontDestroyWithOwner) + { + var networkObjectIds = new List(); + var nonAuthority = m_NetworkManagers.Where((c) => c.LocalClientId == m_NonAuthorityClientId).First(); + var objectToSpawn = dontDestroyWithOwner ? m_DontDestroyWithOwnerPrefab : m_DestroyWithOwnerPrefab; + var spawnedObjects = SpawnObjects(objectToSpawn, nonAuthority, k_NumberObjectsToSpawn); + foreach (var spawnedObject in spawnedObjects) + { + networkObjectIds.Add(spawnedObject.GetComponent().NetworkObjectId); + } + + if (dontDestroyWithOwner) + { + m_DontDestroyObjectIds.Clear(); + m_DontDestroyObjectIds.AddRange(networkObjectIds); + } + else + { + m_DestroyObjectIds.Clear(); + m_DestroyObjectIds.AddRange(networkObjectIds); + } + } + + /// + /// Validates that the dont destroy with owner object is parented under + /// the destroy with owner object. + /// + private bool HaveAllObjectInstancesParented(StringBuilder errorLog) + { + foreach (var networkManager in m_NetworkManagers) + { + var relativeSpawnedObjects = networkManager.SpawnManager.SpawnedObjects; + for (int i = 0; i < k_NumberObjectsToSpawn; i++) + { + var dontDestroyObjectId = m_DontDestroyObjectIds[i]; + var destroyObjectId = m_DestroyObjectIds[i]; + var dontDestroyObject = (NetworkObject)null; + var destroyObject = (NetworkObject)null; + if (!relativeSpawnedObjects.ContainsKey(dontDestroyObjectId)) + { + errorLog.AppendLine($"[Client-{networkManager.LocalClientId}][DontDestroyWithOwner] Has not spawned {nameof(NetworkObject)}-{dontDestroyObjectId}!"); + } + else + { + dontDestroyObject = relativeSpawnedObjects[dontDestroyObjectId]; + } + if (!relativeSpawnedObjects.ContainsKey(destroyObjectId)) + { + errorLog.AppendLine($"[Client-{networkManager.LocalClientId}][DestroyWithOwner] Has not spawned {nameof(NetworkObject)}-{destroyObjectId}!"); + } + else + { + destroyObject = relativeSpawnedObjects[destroyObjectId]; + } + + if (dontDestroyObject != null && destroyObject != null && dontDestroyObject.transform.parent != destroyObject.transform) + { + errorLog.AppendLine($"[Client-{networkManager.LocalClientId}][Not Parented] {destroyObject.name} is not parented under {dontDestroyObject.name}!"); + } + } + } + return errorLog.Length == 0; + } + + /// + /// Parents the dont destroy with owner objects under the destroy with owner objects for the parenting portion of the test. + /// + private void ParentObjects() + { + var networkManager = !m_DistributedAuthority ? GetAuthorityNetworkManager() : m_NetworkManagers.Where((c) => c.LocalClientId == m_NonAuthorityClientId).First(); + for (int i = 0; i < k_NumberObjectsToSpawn; i++) + { + var dontDestroyObjectId = m_DontDestroyObjectIds[i]; + var destroyObjectId = m_DestroyObjectIds[i]; + var dontDestroyObject = networkManager.SpawnManager.SpawnedObjects[dontDestroyObjectId]; + var destroyObject = networkManager.SpawnManager.SpawnedObjects[destroyObjectId]; + Assert.IsTrue(dontDestroyObject.TrySetParent(destroyObject), $"[Client-{networkManager.LocalClientId}][Parent Failure] Could not parent {destroyObject.name} under {dontDestroyObject.name}!"); + } + } + + /// + /// Validates that the non-authority owner client disconnection + /// was registered on all clients. + /// + private bool NonAuthorityHasDisconnected(StringBuilder errorLog) { - var client = m_ClientNetworkManagers[0]; - var clientId = client.LocalClientId; - var networkObjects = SpawnObjects(m_PrefabToSpawn, m_ClientNetworkManagers[0], k_NumberObjectsToSpawn); + foreach (var networkManager in m_NetworkManagers) + { + if (!networkManager.IsListening) + { + continue; + } - // wait for object spawn on client to reach k_NumberObjectsToSpawn + 1 (k_NumberObjectsToSpawn and 1 for the player) - yield return WaitForConditionOrTimeOut(() => client.SpawnManager.GetClientOwnedObjects(clientId).Count() == k_NumberObjectsToSpawn + 1); - Assert.False(s_GlobalTimeoutHelper.TimedOut, $"Timed out waiting for client to have 33 NetworkObjects spawned! Only {client.SpawnManager.GetClientOwnedObjects(clientId).Count()} were assigned!"); + if (networkManager.ConnectedClientsIds.Contains(m_NonAuthorityClientId)) + { + errorLog.AppendLine($"[Client-{networkManager.LocalClientId}][ClientDisconnect] Still thinks Client-{m_NonAuthorityClientId} is connected!"); + } + } + return errorLog.Length == 0; + } - // Since clients spawn their objects locally in distributed authority mode, we have to rebuild the list of the client - // owned objects on the (DAHost) server-side because when the client disconnects it will destroy its local instances. - if (m_DistributedAuthority) + /// + /// The primary validation for the . + /// This validates that: + /// - Spawned objects that are set to destroy with the owner gets destroyed/despawned when the owning client disconnects. + /// - Spawned objects that are set to not destroy with the owner are not destroyed/despawned when the owning client disconnects. + /// + private bool ValidateDontDestroyWithOwner(StringBuilder errorLog) + { + foreach (var networkManager in m_NetworkManagers) { - networkObjects.Clear(); - var serversideClientOwnedObjects = m_ServerNetworkManager.SpawnManager.GetClientOwnedObjects(clientId); + var relativeSpawnedObjects = networkManager.SpawnManager.SpawnedObjects; + for (int i = 0; i < k_NumberObjectsToSpawn; i++) + { + var dontDestroyObjectId = m_DontDestroyObjectIds[i]; + var destroyObjectId = m_DestroyObjectIds[i]; + if (!relativeSpawnedObjects.ContainsKey(dontDestroyObjectId)) + { + errorLog.AppendLine($"[Client-{networkManager.LocalClientId}][DontDestroyWithOwner][!Despawned!] {nameof(NetworkObject)}-{dontDestroyObjectId} should not despawn upon the owner disconnecting!"); + } + if (relativeSpawnedObjects.ContainsKey(destroyObjectId)) + { + errorLog.AppendLine($"[Client-{networkManager.LocalClientId}][DestroyWithOwner][!Not Despawned!] {nameof(NetworkObject)}-{destroyObjectId} should have despawned upon the owner disconnecting!"); + } + } + } + return errorLog.Length == 0; + } - foreach (var networkObject in serversideClientOwnedObjects) + /// + /// The primary parented validation for the . + /// This validates that: + /// - Spawned objects that are set to destroy with the owner gets destroyed/despawned when the owning client disconnects. + /// - Spawned objects that are set to not destroy with the owner and parented under a spawned object set to destroy with owner that + /// the objects that are set to not destroy with the owner are not destroyed/despawned when the owning client disconnects. + /// + private bool ValidateParentedDontDestroyWithOwnerId(StringBuilder errorLog) + { + foreach (var networkManager in m_NetworkManagers) + { + var relativeSpawnedObjects = networkManager.SpawnManager.SpawnedObjects; + for (int i = 0; i < k_NumberObjectsToSpawn; i++) { - if (!networkObject.IsPlayerObject) + var dontDestroyObjectId = m_DontDestroyObjectIds[i]; + var dontDestroyObjectOwnerId = relativeSpawnedObjects[dontDestroyObjectId].OwnerClientId; + + if (dontDestroyObjectOwnerId == m_NonAuthorityClientId) { - networkObjects.Add(networkObject.gameObject); + errorLog.AppendLine($"[Client-{networkManager.LocalClientId}][DontDestroyWithOwner][!Owner!] {nameof(NetworkObject)}-{dontDestroyObjectId} should not still belong to Client-{m_NonAuthorityClientId}!"); } } } + return errorLog.Length == 0; + } - // disconnect the client that owns all the clients - NetcodeIntegrationTestHelpers.StopOneClient(client); + /// + /// This validates that: + /// - Spawned objects that are set to destroy with the owner gets destroyed/despawned when the owning client disconnects. + /// - Spawned objects that are set to not destroy with the owner are not destroyed/despawned when the owning client disconnects. + /// + [UnityTest] + public IEnumerator DontDestroyWithOwnerTest([Values] ParentedPass parentedPass) + { + var authority = GetAuthorityNetworkManager(); + var nonAuthority = GetNonAuthorityNetworkManager(); + m_NonAuthorityClientId = nonAuthority.LocalClientId; + SpawnAllObjects(true); + SpawnAllObjects(false); + + // This should never fail. + Assert.IsTrue(m_DontDestroyObjectIds.Count == m_DestroyObjectIds.Count, $"Mismatch in spawn count! ({m_DontDestroyObjectIds.Count}) vs ({m_DestroyObjectIds.Count})"); - var remainingClients = Mathf.Max(0, TotalClients - 1); - // wait for disconnect - yield return WaitForConditionOrTimeOut(() => m_ServerNetworkManager.ConnectedClients.Count == remainingClients); - Assert.False(s_GlobalTimeoutHelper.TimedOut, "Timed out waiting for client to disconnect!"); + yield return WaitForConditionOrTimeOut(HaveAllObjectInstancesSpawned); + AssertOnTimeout($"Timed out waiting for all clients to spawn objects!"); - for (int i = 0; i < networkObjects.Count; i++) + if (parentedPass == ParentedPass.HasParent) { - var networkObject = networkObjects[i].GetComponent(); - // ensure ownership was transferred back - Assert.That(networkObject.OwnerClientId == m_ServerNetworkManager.LocalClientId); + ParentObjects(); + yield return WaitForConditionOrTimeOut(HaveAllObjectInstancesParented); + AssertOnTimeout($"Timed out waiting for all DontDestroy objects to be parented under the Destroy objects!"); + } + + yield return StopOneClient(nonAuthority); + + yield return WaitForConditionOrTimeOut(NonAuthorityHasDisconnected); + AssertOnTimeout($"Timed out waiting for all clients to register that Client-{m_NonAuthorityClientId} has disconnected!"); + + yield return WaitForConditionOrTimeOut(ValidateDontDestroyWithOwner); + AssertOnTimeout($"Timed out while validating the base-line DontDestroyWithOwnerTest results!"); + + if (parentedPass == ParentedPass.HasParent) + { + yield return WaitForConditionOrTimeOut(ValidateParentedDontDestroyWithOwnerId); + AssertOnTimeout($"Timed out while validating the parented don't destroy objects do not still belong to disconnected Client-{m_NonAuthorityClientId}!"); } } @@ -87,22 +281,55 @@ public IEnumerator NetworkShowThenClientDisconnects() var authorityManager = GetAuthorityNetworkManager(); var networkObject = SpawnObject(m_PrefabNoObserversSpawn, authorityManager).GetComponent(); var longWait = new WaitForSeconds(0.25f); + // Wait long enough to assure that no client receives the spawn notification yield return longWait; + + foreach (var networkManager in m_NetworkManagers) + { + // Skip the authority as it will have an instance + if (networkManager == authorityManager) + { + continue; + } + Assert.False(networkManager.SpawnManager.SpawnedObjects.ContainsKey(networkObject.NetworkObjectId), $"[Client-{networkManager.LocalClientId}]" + + $" Spawned an instance of {networkObject.name} when it was spawned with no observers!"); + } + + // Get a non-authority client, show the spawned object to it, and then make that client the owner. var nonAuthorityManager = GetNonAuthorityNetworkManager(); - Assert.False(nonAuthorityManager.SpawnManager.SpawnedObjects.ContainsKey(networkObject.NetworkObjectId), $"[Client-{nonAuthorityManager.LocalClientId}] " + - $"Already has an instance of {networkObject.name} when it should not!"); networkObject.NetworkShow(nonAuthorityManager.LocalClientId); + // This validates that the change in ownership is not sent when making a NetworkObject visible and changing the ownership + // within the same frame/callstack. networkObject.ChangeOwnership(nonAuthorityManager.LocalClientId); + // Verifies the object was spawned on the non-authority client and that the non-authority client is the owner. yield return WaitForConditionOrTimeOut(() => nonAuthorityManager.SpawnManager.SpawnedObjects.ContainsKey(networkObject.NetworkObjectId) && nonAuthorityManager.SpawnManager.SpawnedObjects[networkObject.NetworkObjectId].OwnerClientId == nonAuthorityManager.LocalClientId); AssertOnTimeout($"[Client-{nonAuthorityManager.LocalClientId}] Failed to spawn {networkObject.name} when it was shown!"); + foreach (var networkManager in m_NetworkManagers) + { + // Skip the authority and the non-authority client as they should now both have instances + if (networkManager == authorityManager || networkManager == nonAuthorityManager) + { + continue; + } + + // No other client should have an instance + Assert.False(networkManager.SpawnManager.SpawnedObjects.ContainsKey(networkObject.NetworkObjectId), $"[Client-{networkManager.LocalClientId}]" + + $" Spawned an instance of {networkObject.name} when it was shown to Client-{nonAuthorityManager.LocalClientId}!"); + } + + // Wait a few frames yield return s_DefaultWaitForTick; + // Shutdown the non-authority client to assure this does not cause the spawned object to despawn nonAuthorityManager.Shutdown(); + // Wait long enough to assure all messages generated from the client shutting down have been processed yield return longWait; + + // Validate the object is still spawned Assert.True(networkObject.IsSpawned, $"The spawned test prefab was despawned on the authority side when it shouldn't have been!"); } } diff --git a/com.unity.netcode.gameobjects/package.json b/com.unity.netcode.gameobjects/package.json index e29f475fc6..70eeba81b1 100644 --- a/com.unity.netcode.gameobjects/package.json +++ b/com.unity.netcode.gameobjects/package.json @@ -2,7 +2,7 @@ "name": "com.unity.netcode.gameobjects", "displayName": "Netcode for GameObjects", "description": "Netcode for GameObjects is a high-level netcode SDK that provides networking capabilities to GameObject/MonoBehaviour workflows within Unity and sits on top of underlying transport layer.", - "version": "2.4.2", + "version": "2.4.3", "unity": "6000.0", "dependencies": { "com.unity.nuget.mono-cecil": "1.11.4",