From 13abd463c7803865d0ce0d0303365959a331d1b6 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Fri, 13 Jun 2025 07:35:59 -0500 Subject: [PATCH 1/6] update updating change log and package version --- com.unity.netcode.gameobjects/CHANGELOG.md | 7 +------ com.unity.netcode.gameobjects/package.json | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 51fe4a90f6..7d0ec33d6f 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -6,10 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) Additional documentation and release notes are available at [Multiplayer Documentation](https://docs-multiplayer.unity3d.com). -## [Unreleased] - -### Added - +## [2.4.2] - 2025-06-13 ### Fixed @@ -18,8 +15,6 @@ Additional documentation and release notes are available at [Multiplayer Documen - Fixed issue where the initial client synchronization pre-serialization process was not excluding spawned `NetworkObject` instances that already had pending visibility for the client being synchronized. (#3488) - Fixed issue where there was a potential for a small memory leak in the `ConnectionApprovedMessage`. (#3486) -### Changed - ## [2.4.1] - 2025-06-11 diff --git a/com.unity.netcode.gameobjects/package.json b/com.unity.netcode.gameobjects/package.json index c43dc42ec2..e29f475fc6 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.1", + "version": "2.4.2", "unity": "6000.0", "dependencies": { "com.unity.nuget.mono-cecil": "1.11.4", From 7bbc1c3a950ed4c5c10f3be25bd19ae1900d5f52 Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Fri, 13 Jun 2025 19:00:28 -0500 Subject: [PATCH 2/6] fix: Avoid throwing exception when using NetworkList without a NetworkManager [Up-port] (#3504) Up-port of #3502 Fixes #2539 ## Changelog - Added: `LocalClientCannotWrite` function to co-locate the shared functionality - Fixed: Ensure that the `NetworkManager` exists before attempting to access the `LocalClientId` ## Testing and Documentation - No tests have been added. ## Backport Backported in #3502 --------- Co-authored-by: unity-renovate[bot] <120015202+unity-renovate[bot]@users.noreply.github.com> Co-authored-by: Emma --- com.unity.netcode.gameobjects/CHANGELOG.md | 1 + .../Messages/NetworkVariableDeltaMessage.cs | 6 +++--- .../NetworkVariable/AnticipatedNetworkVariable.cs | 2 +- .../NetworkVariable/Collections/NetworkList.cs | 12 ++++++------ .../Runtime/NetworkVariable/NetworkVariable.cs | 10 +++++----- .../Runtime/NetworkVariable/NetworkVariableBase.cs | 10 ++++++++++ 6 files changed, 26 insertions(+), 15 deletions(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 7d0ec33d6f..937a0193b4 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -10,6 +10,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed +- Fixed `NullReferenceException` on `NetworkList` when used without a NetworkManager in scene. (#3503) - Fixed issue where `NetworkClient` could persist some settings if re-using the same `NetworkManager` instance. (#3491) - Fixed issue where a pooled `NetworkObject` was not resetting the internal latest parent property when despawned. (#3491) - Fixed issue where the initial client synchronization pre-serialization process was not excluding spawned `NetworkObject` instances that already had pending visibility for the client being synchronized. (#3488) diff --git a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NetworkVariableDeltaMessage.cs b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NetworkVariableDeltaMessage.cs index 05ae29b32e..af520a0468 100644 --- a/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NetworkVariableDeltaMessage.cs +++ b/com.unity.netcode.gameobjects/Runtime/Messaging/Messages/NetworkVariableDeltaMessage.cs @@ -144,9 +144,9 @@ public void Serialize(FastBufferWriter writer, int targetVersion) var startingSize = writer.Length; var networkVariable = NetworkBehaviour.NetworkVariableFields[i]; var shouldWrite = networkVariable.IsDirty() && - networkVariable.CanClientRead(TargetClientId) && - (networkManager.IsServer || networkVariable.CanClientWrite(networkManager.LocalClientId)) && - networkVariable.CanSend(); + networkVariable.CanClientRead(TargetClientId) + && (networkManager.IsServer || + (networkVariable.CanWrite && networkVariable.CanSend())); // Prevent the server from writing to the client that owns a given NetworkVariable // Allowing the write would send an old value to the client and cause jitter diff --git a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/AnticipatedNetworkVariable.cs b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/AnticipatedNetworkVariable.cs index 5fe5d40818..2e14bccf9c 100644 --- a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/AnticipatedNetworkVariable.cs +++ b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/AnticipatedNetworkVariable.cs @@ -217,7 +217,7 @@ public void Anticipate(T value) m_LastAnticipationCounter = m_NetworkBehaviour.NetworkManager.AnticipationSystem.AnticipationCounter; m_AnticipatedValue = value; NetworkVariableSerialization.Duplicate(m_AnticipatedValue, ref m_PreviousAnticipatedValue); - if (CanClientWrite(m_NetworkBehaviour.NetworkManager.LocalClientId)) + if (CanWrite) { AuthoritativeValue = value; } diff --git a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs index 97fbeabc72..1a78fc7f81 100644 --- a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs +++ b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs @@ -428,7 +428,7 @@ public IEnumerator GetEnumerator() public void Add(T item) { // check write permissions - if (!CanClientWrite(m_NetworkManager.LocalClientId)) + if (CannotWrite) { LogWritePermissionError(); return; @@ -455,7 +455,7 @@ public void Add(T item) public void Clear() { // check write permissions - if (!CanClientWrite(m_NetworkManager.LocalClientId)) + if (CannotWrite) { LogWritePermissionError(); return; @@ -493,7 +493,7 @@ public bool Contains(T item) public bool Remove(T item) { // check write permissions - if (!CanClientWrite(m_NetworkManager.LocalClientId)) + if (CannotWrite) { LogWritePermissionError(); return false; @@ -542,7 +542,7 @@ public int IndexOf(T item) public void Insert(int index, T item) { // check write permissions - if (!CanClientWrite(m_NetworkManager.LocalClientId)) + if (CannotWrite) { LogWritePermissionError(); return; @@ -578,7 +578,7 @@ public void Insert(int index, T item) public void RemoveAt(int index) { // check write permissions - if (!CanClientWrite(m_NetworkManager.LocalClientId)) + if (CannotWrite) { throw new InvalidOperationException("Client is not allowed to write to this NetworkList"); } @@ -610,7 +610,7 @@ public T this[int index] set { // check write permissions - if (!CanClientWrite(m_NetworkManager.LocalClientId)) + if (CannotWrite) { LogWritePermissionError(); return; diff --git a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariable.cs b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariable.cs index 21f92957d2..068a6bfb68 100644 --- a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariable.cs +++ b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariable.cs @@ -128,7 +128,7 @@ public virtual T Value get => m_InternalValue; set { - if (m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId)) + if (CannotWrite) { LogWritePermissionError(); return; @@ -162,7 +162,7 @@ public bool CheckDirtyState(bool forceCheck = false) var isDirty = base.IsDirty(); // A client without permissions invoking this method should only check to assure the current value is equal to the last known current value - if (m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId)) + if (CannotWrite) { // If modifications are detected, then revert back to the last known current value if (!NetworkVariableSerialization.AreEqual(ref m_InternalValue, ref m_InternalOriginalValue)) @@ -245,7 +245,7 @@ public override bool IsDirty() { // If the client does not have write permissions but the internal value is determined to be locally modified and we are applying updates, then we should revert // to the original collection value prior to applying updates (primarily for collections). - if (!NetworkUpdaterCheck && m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId) && !NetworkVariableSerialization.AreEqual(ref m_InternalValue, ref m_InternalOriginalValue)) + if (!NetworkUpdaterCheck && CannotWrite && !NetworkVariableSerialization.AreEqual(ref m_InternalValue, ref m_InternalOriginalValue)) { NetworkVariableSerialization.Duplicate(m_InternalOriginalValue, ref m_InternalValue); return true; @@ -307,7 +307,7 @@ public override void ReadDelta(FastBufferReader reader, bool keepDirtyDelta) { // If the client does not have write permissions but the internal value is determined to be locally modified and we are applying updates, then we should revert // to the original collection value prior to applying updates (primarily for collections). - if (m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId) && !NetworkVariableSerialization.AreEqual(ref m_InternalOriginalValue, ref m_InternalValue)) + if (CannotWrite && !NetworkVariableSerialization.AreEqual(ref m_InternalOriginalValue, ref m_InternalValue)) { NetworkVariableSerialization.Duplicate(m_InternalOriginalValue, ref m_InternalValue); } @@ -349,7 +349,7 @@ public override void ReadField(FastBufferReader reader) { // If the client does not have write permissions but the internal value is determined to be locally modified and we are applying updates, then we should revert // to the original collection value prior to applying updates (primarily for collections). - if (m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId) && !NetworkVariableSerialization.AreEqual(ref m_InternalOriginalValue, ref m_InternalValue)) + if (CannotWrite && !NetworkVariableSerialization.AreEqual(ref m_InternalOriginalValue, ref m_InternalValue)) { NetworkVariableSerialization.Duplicate(m_InternalOriginalValue, ref m_InternalValue); } diff --git a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs index 94871ab329..8761496d2a 100644 --- a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs +++ b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs @@ -381,6 +381,16 @@ public bool CanClientWrite(ulong clientId) } } + /// + /// Returns true if the current can write to this variable; otherwise false. + /// + internal bool CanWrite => m_NetworkManager && CanClientWrite(m_NetworkManager.LocalClientId); + + /// + /// Returns false if the current can write to this variable; otherwise true. + /// + internal bool CannotWrite => m_NetworkManager && !CanClientWrite(m_NetworkManager.LocalClientId); + /// /// Returns the ClientId of the owning client /// From 6afbdc6f294faa6392c855e519b01d0c0eeaa7b5 Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Wed, 25 Jun 2025 20:47:39 -0500 Subject: [PATCH 3/6] fix: DontDestroyWithOwner v2.4.0 - v2.4.2 client-server regression (#3522) This resolves the [Forum-1658887](https://discussions.unity.com/t/network-objects-dont-despawn-on-disconnect-even-with-dont-destroy-with-owner-unchecked/1658887) discovered client-server specific issue in the v2.4.2 release where spawned objects with the default `NetworkObject.DontDestroyWithOwner` setting (disabled/false) would not be despawned and destroyed when the owning client disconnected. ## Changelog - Fixed: Issue where spawned objects with `NetworkObject.DontDestroyWithOwner` set to `false` would not be destroyed when using a client-server network topology. ## Testing and Documentation - Includes updates to the `NetworkObjectDontDestroyWithOwnerTests` integration test. - No documentation changes or additions were necessary. ## Backport This is specific to NGO v2.4.2 release. --- com.unity.netcode.gameobjects/CHANGELOG.md | 7 + .../Connection/NetworkConnectionManager.cs | 2 +- .../NetworkObjectDontDestroyWithOwnerTests.cs | 207 ++++++++++++++---- 3 files changed, 174 insertions(+), 42 deletions(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 937a0193b4..f522ca936a 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -6,6 +6,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) Additional documentation and release notes are available at [Multiplayer Documentation](https://docs-multiplayer.unity3d.com). +## [Unreleased] + +### Fixed + +- Fixed issue where spawned objects with `NetworkObject.DontDestroyWithOwner` is 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..cedb01630e 100644 --- a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs @@ -1178,7 +1178,7 @@ internal void OnClientDisconnectFromServer(ulong clientId) { // 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 && (ownedObject.Observers.Count == 0 || (ownedObject.Observers.Count >= 1 && ownedObject.Observers.Contains(clientId)))) { if (NetworkManager.PrefabHandler.ContainsHandler(ownedObject.GlobalObjectIdHash)) { diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectDontDestroyWithOwnerTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectDontDestroyWithOwnerTests.cs index 9466148d38..749c90b111 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,26 @@ 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() - { - return false; - } - - 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,47 +42,135 @@ 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 client = m_ClientNetworkManagers[0]; - var clientId = client.LocalClientId; - var networkObjects = SpawnObjects(m_PrefabToSpawn, m_ClientNetworkManagers[0], k_NumberObjectsToSpawn); + 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); + } - // 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 (dontDestroyWithOwner) + { + m_DontDestroyObjectIds.Clear(); + m_DontDestroyObjectIds.AddRange(networkObjectIds); + } + else + { + m_DestroyObjectIds.Clear(); + m_DestroyObjectIds.AddRange(networkObjectIds); + } + } - // 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) + /// + /// Validates that the non-authority owner client disconnection + /// was registered on all clients. + /// + private bool NonAuthorityHasDisconnected(StringBuilder errorLog) + { + foreach (var networkManager in m_NetworkManagers) { - networkObjects.Clear(); - var serversideClientOwnedObjects = m_ServerNetworkManager.SpawnManager.GetClientOwnedObjects(clientId); + if (!networkManager.IsListening) + { + continue; + } + + if (networkManager.ConnectedClientsIds.Contains(m_NonAuthorityClientId)) + { + errorLog.AppendLine($"[Client-{networkManager.LocalClientId}][ClientDisconnect] Still thinks Client-{m_NonAuthorityClientId} is connected!"); + } + } + return errorLog.Length == 0; + } - foreach (var networkObject in serversideClientOwnedObjects) + /// + /// 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) + { + var relativeSpawnedObjects = networkManager.SpawnManager.SpawnedObjects; + for (int i = 0; i < k_NumberObjectsToSpawn; i++) { - if (!networkObject.IsPlayerObject) + 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)) { - networkObjects.Add(networkObject.gameObject); + errorLog.AppendLine($"[Client-{networkManager.LocalClientId}][DestroyWithOwner][!Not Despawned!] {nameof(NetworkObject)}-{destroyObjectId} should have despawned upon the owner disconnecting!"); } } } + 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() + { + var authority = GetAuthorityNetworkManager(); + var nonAuthority = GetNonAuthorityNetworkManager(); + m_NonAuthorityClientId = nonAuthority.LocalClientId; + SpawnAllObjects(true); + SpawnAllObjects(false); - 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!"); + // This should never fail. + Assert.IsTrue(m_DontDestroyObjectIds.Count == m_DestroyObjectIds.Count, $"Mismatch in spawn count! ({m_DontDestroyObjectIds.Count}) vs ({m_DestroyObjectIds.Count})"); - for (int i = 0; i < networkObjects.Count; i++) - { - var networkObject = networkObjects[i].GetComponent(); - // ensure ownership was transferred back - Assert.That(networkObject.OwnerClientId == m_ServerNetworkManager.LocalClientId); - } + yield return WaitForConditionOrTimeOut(HaveAllObjectInstancesSpawned); + AssertOnTimeout($"Timed out waiting for all clients to spawn 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 DontDestroyWithOwnerTest results!"); } [UnityTest] @@ -87,22 +179,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!"); } } From fe919640cb672c527409bac9361ad1c92bb5e7ec Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 25 Jun 2025 20:56:37 -0500 Subject: [PATCH 4/6] update Setting version and updating change log version for the unreleased entry. --- com.unity.netcode.gameobjects/CHANGELOG.md | 4 ++-- com.unity.netcode.gameobjects/package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index f522ca936a..a693aa100e 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -6,11 +6,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) Additional documentation and release notes are available at [Multiplayer Documentation](https://docs-multiplayer.unity3d.com). -## [Unreleased] +## [2.4.3] - 2025-06-25 ### Fixed -- Fixed issue where spawned objects with `NetworkObject.DontDestroyWithOwner` is set to `false` would not be destroyed when using a client-server network topology. (#3522) +- 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 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", From 808858b198cb3e9aec3ed2ecb5e26fd398ebf98c Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 25 Jun 2025 22:15:18 -0500 Subject: [PATCH 5/6] test - additional parenting check Adding an additional test to validate that when a don't destroy with owner object is parented under a destroy with owner object that the don't destroy with owner object will not be despawned when the owning client disconnects. --- .../NetworkObjectDontDestroyWithOwnerTests.cs | 106 +++++++++++++++++- 1 file changed, 104 insertions(+), 2 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectDontDestroyWithOwnerTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectDontDestroyWithOwnerTests.cs index 749c90b111..cf4f83d17f 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectDontDestroyWithOwnerTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectDontDestroyWithOwnerTests.cs @@ -18,6 +18,12 @@ internal class NetworkObjectDontDestroyWithOwnerTests : NetcodeIntegrationTest private const int k_NumberObjectsToSpawn = 16; protected override int NumberOfClients => 3; + public enum ParentedPass + { + NoParent, + HasParent + } + protected GameObject m_DestroyWithOwnerPrefab; protected GameObject m_DontDestroyWithOwnerPrefab; protected GameObject m_PrefabNoObserversSpawn; @@ -95,6 +101,63 @@ private void SpawnAllObjects(bool dontDestroyWithOwner) } } + /// + /// 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. @@ -144,13 +207,39 @@ private bool ValidateDontDestroyWithOwner(StringBuilder errorLog) return errorLog.Length == 0; } + /// + /// 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++) + { + var dontDestroyObjectId = m_DontDestroyObjectIds[i]; + var dontDestroyObjectOwnerId = relativeSpawnedObjects[dontDestroyObjectId].OwnerClientId; + + if (dontDestroyObjectOwnerId == m_NonAuthorityClientId) + { + errorLog.AppendLine($"[Client-{networkManager.LocalClientId}][DontDestroyWithOwner][!Owner!] {nameof(NetworkObject)}-{dontDestroyObjectId} should not still belong to Client-{m_NonAuthorityClientId}!"); + } + } + } + return errorLog.Length == 0; + } + /// /// 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() + public IEnumerator DontDestroyWithOwnerTest([Values] ParentedPass parentedPass) { var authority = GetAuthorityNetworkManager(); var nonAuthority = GetNonAuthorityNetworkManager(); @@ -164,13 +253,26 @@ public IEnumerator DontDestroyWithOwnerTest() yield return WaitForConditionOrTimeOut(HaveAllObjectInstancesSpawned); AssertOnTimeout($"Timed out waiting for all clients to spawn objects!"); + if (parentedPass == ParentedPass.HasParent) + { + 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 DontDestroyWithOwnerTest results!"); + 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}!"); + } } [UnityTest] From 31017e2eae5a9ef797303a05ba4eb6f317fa80f0 Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Thu, 26 Jun 2025 18:25:59 -0500 Subject: [PATCH 6/6] fix: remove observers from da host (#3526) Removing observers in disconnected client check. ## Changelog NA ## Testing and Documentation - No tests have been added. - No documentation changes or additions were necessary. ## Backport --- .../Runtime/Connection/NetworkConnectionManager.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs b/com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs index cedb01630e..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; }