From c7ca276874b65d6d357745a5cf53a17fcfe3c609 Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Tue, 4 Nov 2025 17:47:00 -0600 Subject: [PATCH 1/8] fix This includes a fix for the forum reported issue where spawning with ownership in a distributed authority topology where the owner is not the spawn authority, the owner/authority sets a NetworkVariable during OnNetworkSpawn, then the changes made would not be synchronized. --- .../Runtime/Core/NetworkBehaviour.cs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs index a76066c237..fdba45b677 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs @@ -797,17 +797,6 @@ internal void InternalOnNetworkSpawn() { Debug.LogException(e); } - - // Initialize again in case the user's OnNetworkSpawn changed something - InitializeVariables(); - - if (m_NetworkObject.HasAuthority) - { - // Since we just spawned the object and since user code might have modified their NetworkVariable, esp. - // NetworkList, we need to mark the object as free of updates. - // This should happen for all objects on the machine triggering the spawn. - PostNetworkVariableWrite(true); - } } internal void NetworkPostSpawn() From a997d6ea45eef35119ad5ebe198386607c886047 Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Tue, 4 Nov 2025 17:50:54 -0600 Subject: [PATCH 2/8] test This adds an integration test to validate the fix for this PR. --- .../NetworkVariable/OwnerModifiedTests.cs | 104 ++++++++++++++++-- .../TestHelpers/NetcodeIntegrationTest.cs | 43 +++++--- 2 files changed, 118 insertions(+), 29 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/OwnerModifiedTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/OwnerModifiedTests.cs index 83947888b4..dfc6de950f 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/OwnerModifiedTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/OwnerModifiedTests.cs @@ -1,5 +1,6 @@ using System.Collections; using System.Collections.Generic; +using System.Text; using NUnit.Framework; using Unity.Netcode.TestHelpers.Runtime; using UnityEngine; @@ -7,10 +8,6 @@ namespace Unity.Netcode.RuntimeTests { - // This is a bit of a quirky test. - // Addresses MTT-4386 #2109 - // Where the NetworkVariable updates would be repeated on some clients. - // The twist comes fom the updates needing to happens very specifically for the issue to repro in tests internal class OwnerModifiedObject : NetworkBehaviour, INetworkUpdateSystem { @@ -74,17 +71,36 @@ public void InitializeLastCient() } } + internal class ChangeValueOnAuthority : NetworkBehaviour + { + public NetworkVariable SomeIntValue = new NetworkVariable(); + + public override void OnNetworkSpawn() + { + if (HasAuthority) + { + SomeIntValue.Value++; + } + base.OnNetworkSpawn(); + } + + protected override void OnNetworkPostSpawn() + { + if (HasAuthority) + { + SomeIntValue.Value++; + } + base.OnNetworkPostSpawn(); + } + } + [TestFixture(HostOrServer.DAHost)] [TestFixture(HostOrServer.Host)] internal class OwnerModifiedTests : NetcodeIntegrationTest { protected override int NumberOfClients => 2; - // TODO: [CmbServiceTests] Adapt to run with the service - protected override bool UseCMBService() - { - return false; - } + private GameObject m_SpawnObject; public OwnerModifiedTests(HostOrServer hostOrServer) : base(hostOrServer) { } @@ -93,13 +109,33 @@ protected override void OnCreatePlayerPrefab() m_PlayerPrefab.AddComponent(); } + protected override void OnServerAndClientsCreated() + { + m_SpawnObject = CreateNetworkObjectPrefab("SpawnObj"); + m_SpawnObject.AddComponent(); + base.OnServerAndClientsCreated(); + } + + private NetworkManager m_LastClient; + + protected override void OnNewClientStartedAndConnected(NetworkManager networkManager) + { + m_LastClient = networkManager; + base.OnNewClientStartedAndConnected(networkManager); + } + + /// + /// Addresses MTT-4386 #2109 + /// Verify NetworkVariable updates are not repeated on some clients. + /// TODO: This test needs to be re-written/overhauled. + /// [UnityTest] - public IEnumerator OwnerModifiedTest() + public IEnumerator VerifyDoesNotRepeatOnSomeClients() { OwnerModifiedObject.EnableVerbose = m_EnableVerboseDebug; // We use this to assure we are the "last client" connected. yield return CreateAndStartNewClient(); - var ownerModLastClient = m_ClientNetworkManagers[2].LocalClient.PlayerObject.GetComponent(); + var ownerModLastClient = m_LastClient.LocalClient.PlayerObject.GetComponent(); ownerModLastClient.InitializeLastCient(); // Run through all update loops setting the value once every 5 frames @@ -116,5 +152,51 @@ public IEnumerator OwnerModifiedTest() // We'll have at least one update per stage per client, if all goes well. Assert.True(OwnerModifiedObject.Updates > 20); } + + + private ChangeValueOnAuthority m_SessionAuthorityInstance; + private ChangeValueOnAuthority m_InstanceAuthority; + + private bool NetworkVariablesMatch(StringBuilder errorLog) + { + foreach (var networkManager in m_NetworkManagers) + { + if (networkManager == m_InstanceAuthority.NetworkManager) + { + continue; + } + + var changeValue = networkManager.SpawnManager.SpawnedObjects[m_InstanceAuthority.NetworkObjectId].GetComponent(); + if (changeValue.SomeIntValue.Value != m_InstanceAuthority.SomeIntValue.Value) + { + errorLog.AppendLine($"[Client-{networkManager.LocalClientId}] {changeValue.name} value is {changeValue.SomeIntValue.Value} but was expecting {m_InstanceAuthority.SomeIntValue.Value}!"); + } + } + + return errorLog.Length == 0; + } + + /// + /// Verifies that when running a distributed authority network topology + /// + [UnityTest] + public IEnumerator OwnershipSpawnedAndUpdatedDuringSpawn() + { + var authority = GetAuthorityNetworkManager(); + var nonAuthority = GetNonAuthorityNetworkManager(); + m_SessionAuthorityInstance = Object.Instantiate(m_SpawnObject).GetComponent(); + + SpawnInstanceWithOwnership(m_SessionAuthorityInstance.GetComponent(), authority, nonAuthority.LocalClientId); + yield return WaitForSpawnedOnAllOrTimeOut(m_SessionAuthorityInstance.NetworkObjectId); + AssertOnTimeout($"Failed to spawn {m_SessionAuthorityInstance.name} on all clients!"); + + m_InstanceAuthority = nonAuthority.SpawnManager.SpawnedObjects[m_SessionAuthorityInstance.NetworkObjectId].GetComponent(); + + yield return WaitForConditionOrTimeOut(NetworkVariablesMatch); + AssertOnTimeout($"The {nameof(ChangeValueOnAuthority.SomeIntValue)} failed to synchronize on all clients!"); + + Assert.IsTrue(m_SessionAuthorityInstance.SomeIntValue.Value == 2, "No values were updated on the spawn authority instance!"); + Assert.IsTrue(m_InstanceAuthority.SomeIntValue.Value == 2, "No values were updated on the owner's instance!"); + } } } diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs index b13db62523..a145197d7c 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs @@ -11,6 +11,7 @@ using UnityEngine; using UnityEngine.SceneManagement; using UnityEngine.TestTools; +using static UnityEngine.UI.GridLayoutGroup; using Object = UnityEngine.Object; namespace Unity.Netcode.TestHelpers.Runtime @@ -2171,42 +2172,35 @@ protected GameObject SpawnPlayerObject(GameObject prefabGameObject, NetworkManag return SpawnObject(prefabNetworkObject, owner, destroyWithScene, true); } - /// - /// Spawn an already instantiated instance of a network prefab. - /// Note: If you pass in the NetworkPrefab itself this method will not create an instance but will spawn the pefab itself. (don't do this) - /// - /// the instance of a prefab to spawn - /// the owner of the instance - /// default is false - /// when , the object will be spawned as the owned player. - protected void SpawnObjectInstance(NetworkObject networkObjectToSpawn, NetworkManager owner, bool destroyWithScene = false, bool isPlayerObject = false) + + internal void SpawnInstanceWithOwnership(NetworkObject networkObjectToSpawn, NetworkManager spawnAuthority, ulong clientId, bool destroyWithScene = false, bool isPlayerObject = false) { - if (owner.NetworkConfig.NetworkTopology == NetworkTopologyTypes.DistributedAuthority) + if (spawnAuthority.NetworkConfig.NetworkTopology == NetworkTopologyTypes.DistributedAuthority) { - networkObjectToSpawn.NetworkManagerOwner = owner; // Required to assure the client does the spawning + networkObjectToSpawn.NetworkManagerOwner = spawnAuthority; // Required to assure the client does the spawning if (isPlayerObject) { - networkObjectToSpawn.SpawnAsPlayerObject(owner.LocalClientId, destroyWithScene); + networkObjectToSpawn.SpawnAsPlayerObject(clientId, destroyWithScene); } else { - networkObjectToSpawn.SpawnWithOwnership(owner.LocalClientId, destroyWithScene); + networkObjectToSpawn.SpawnWithOwnership(clientId, destroyWithScene); } } else { networkObjectToSpawn.NetworkManagerOwner = m_ServerNetworkManager; // Required to assure the server does the spawning - if (owner == m_ServerNetworkManager) + if (spawnAuthority == m_ServerNetworkManager) { if (m_UseHost) { if (isPlayerObject) { - networkObjectToSpawn.SpawnAsPlayerObject(owner.LocalClientId, destroyWithScene); + networkObjectToSpawn.SpawnAsPlayerObject(clientId, destroyWithScene); } else { - networkObjectToSpawn.SpawnWithOwnership(owner.LocalClientId, destroyWithScene); + networkObjectToSpawn.SpawnWithOwnership(clientId, destroyWithScene); } } else @@ -2218,16 +2212,29 @@ protected void SpawnObjectInstance(NetworkObject networkObjectToSpawn, NetworkMa { if (isPlayerObject) { - networkObjectToSpawn.SpawnAsPlayerObject(owner.LocalClientId, destroyWithScene); + networkObjectToSpawn.SpawnAsPlayerObject(clientId, destroyWithScene); } else { - networkObjectToSpawn.SpawnWithOwnership(owner.LocalClientId, destroyWithScene); + networkObjectToSpawn.SpawnWithOwnership(clientId, destroyWithScene); } } } } + /// + /// Spawn an already instantiated instance of a network prefab. + /// Note: If you pass in the NetworkPrefab itself this method will not create an instance but will spawn the pefab itself. (don't do this) + /// + /// the instance of a prefab to spawn + /// the owner of the instance + /// default is false + /// when , the object will be spawned as the owned player. + protected void SpawnObjectInstance(NetworkObject networkObjectToSpawn, NetworkManager owner, bool destroyWithScene = false, bool isPlayerObject = false) + { + SpawnInstanceWithOwnership(networkObjectToSpawn, owner, owner.LocalClientId, destroyWithScene, isPlayerObject); + } + /// /// Spawn a NetworkObject prefab instance /// From f3f4eefed13c16ac6848eb34c31788a0c6dd9a6c Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Wed, 5 Nov 2025 08:55:26 -0600 Subject: [PATCH 3/8] update removing an IDE injected namespace that is never usedd. --- .../Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs index a145197d7c..9f6ab62ddf 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs @@ -11,7 +11,6 @@ using UnityEngine; using UnityEngine.SceneManagement; using UnityEngine.TestTools; -using static UnityEngine.UI.GridLayoutGroup; using Object = UnityEngine.Object; namespace Unity.Netcode.TestHelpers.Runtime From 95a020e6000ac67a65da7c41833940f684df5318 Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Wed, 5 Nov 2025 15:05:07 -0600 Subject: [PATCH 4/8] fix: second part This migrates the NetworkList specific script from NetworkBehaviour.InternalOnNetworkSpawn into NetworkList itself. It also provides (currently) two internal virtual methods, NetworkVariableBase.OnSpawned and NetworkVariableBase.OnPreDespawn, to provide a means for NetworkList to handle cleaning its dirty state up after the instance with write permissions has finished running through the spawn stages (pre-spawn, spawning, post-spawn, gets invoked). --- .../Runtime/Core/NetworkBehaviour.cs | 14 ++++++++++++ .../Collections/NetworkList.cs | 14 ++++++++++++ .../NetworkVariable/NetworkVariableBase.cs | 22 +++++++++++++++++++ 3 files changed, 50 insertions(+) diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs index fdba45b677..60b92ed4f4 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs @@ -810,6 +810,13 @@ internal void NetworkPostSpawn() { Debug.LogException(e); } + + // Let each NetworkVariableBase derived instance know that + // all spawn related methods have been invoked. + for (int i = 0; i < NetworkVariableFields.Count; i++) + { + NetworkVariableFields[i].OnSpawned(); + } } internal void NetworkSessionSynchronized() @@ -847,6 +854,13 @@ internal void InternalOnNetworkPreDespawn() { Debug.LogException(e); } + + // Let each NetworkVariableBase derived instance know that + // all spawn related methods have been invoked. + for (int i = 0; i < NetworkVariableFields.Count; i++) + { + NetworkVariableFields[i].OnPreDespawn(); + } } internal void InternalOnNetworkDespawn() diff --git a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs index 18322ee332..f77c0a15c2 100644 --- a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs +++ b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs @@ -58,6 +58,20 @@ public NetworkList(IEnumerable values = default, Dispose(); } + internal override void OnSpawned() + { + // If we are dirty and have write permissions by the time the NetworkObject + // is finished spawning (same frame), then go ahead and reset the dirty related + // properties for NetworkList. + if (IsDirty() && CanSend()) + { + UpdateLastSentTime(); + ResetDirty(); + SetDirty(false); + } + base.OnSpawned(); + } + /// public override void ResetDirty() { diff --git a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs index 2c7276365e..5186c7a13f 100644 --- a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs +++ b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs @@ -140,6 +140,28 @@ public void Initialize(NetworkBehaviour networkBehaviour) } } + /// TODO-API: After further vetting and alignment on these, we might make them part of the public API. + /// Could actually be like an interface that gets automatically registered for these kinds of notifications + /// without having to be a NetworkBehaviour. + #region OnSpawn and OnPreDespawn (ETC) + + /// + /// Invoked after the associated has been invoked. + /// + internal virtual void OnSpawned() + { + + } + + /// + /// Invoked after the associated has been invoked. + /// + internal virtual void OnPreDespawn() + { + + } + #endregion + /// /// Deinitialize is invoked when a NetworkObject is despawned. /// This allows for a recyled NetworkObject (in-scene or pooled) From 66fdf9629e08535f4f4f2c4061681e1820f7a397 Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Wed, 5 Nov 2025 15:08:13 -0600 Subject: [PATCH 5/8] test Adding some debug information to NetworkShowHideTests. Replacing m_ServerNetworkManager in OwnerModifiedTests with GetAuthorityNetworkManager() to become CMB service testing compatible. --- .../Tests/Runtime/NetworkShowHideTests.cs | 53 ++++++++++++------- .../NetworkVariable/OwnerModifiedTests.cs | 5 +- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkShowHideTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkShowHideTests.cs index a12985579c..f75f6e256b 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkShowHideTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkShowHideTests.cs @@ -31,6 +31,8 @@ public static NetworkObject GetNetworkObjectById(ulong networkObjectId) public override void OnNetworkSpawn() { + MyNetworkVariable.OnValueChanged += Changed; + MyOwnerReadNetworkVariable.OnValueChanged += OwnerReadChanged; if (NetworkManager.LocalClientId == ClientIdToTarget) { ClientTargetedNetworkObjects.Add(this); @@ -42,7 +44,7 @@ public override void OnNetworkSpawn() } else { - Debug.Assert(MyListSetOnSpawn.Count == 1); + Debug.Assert(MyListSetOnSpawn.Count == 1, $"[Session Authority][Client-{NetworkManager.LocalClientId}][{name}] Count = {MyListSetOnSpawn.Count} when expecting only 1!"); Debug.Assert(MyListSetOnSpawn[0] == 45); } @@ -53,6 +55,8 @@ public override void OnNetworkSpawn() public override void OnNetworkDespawn() { + MyNetworkVariable.OnValueChanged -= Changed; + MyOwnerReadNetworkVariable.OnValueChanged -= OwnerReadChanged; if (ClientTargetedNetworkObjects.Contains(this)) { ClientTargetedNetworkObjects.Remove(this); @@ -60,27 +64,14 @@ public override void OnNetworkDespawn() base.OnNetworkDespawn(); } - public NetworkVariable MyNetworkVariable; - public NetworkList MyListSetOnSpawn; - public NetworkVariable MyOwnerReadNetworkVariable; - public NetworkList MyList; + public NetworkVariable MyNetworkVariable = new NetworkVariable(); + public NetworkList MyListSetOnSpawn = new NetworkList(); + public NetworkVariable MyOwnerReadNetworkVariable = new NetworkVariable(readPerm: NetworkVariableReadPermission.Owner); + public NetworkList MyList = new NetworkList(); public static NetworkManager NetworkManagerOfInterest; internal static int GainOwnershipCount = 0; - private void Awake() - { - // Debug.Log($"Awake {NetworkManager.LocalClientId}"); - MyNetworkVariable = new NetworkVariable(); - MyNetworkVariable.OnValueChanged += Changed; - - MyListSetOnSpawn = new NetworkList(); - MyList = new NetworkList(); - - MyOwnerReadNetworkVariable = new NetworkVariable(readPerm: NetworkVariableReadPermission.Owner); - MyOwnerReadNetworkVariable.OnValueChanged += OwnerReadChanged; - } - public override void OnGainedOwnership() { GainOwnershipCount++; @@ -532,6 +523,7 @@ public IEnumerator NetworkHideChangeOwnership() AssertOnTimeout($"NetworkObject is still visible to Client-{m_ClientWithoutVisibility} or other clients think it is still visible to Client-{m_ClientWithoutVisibility}:\n {m_ErrorLog}"); yield return WaitForConditionOrTimeOut(() => ShowHideObject.ClientTargetedNetworkObjects.Count == 0); + AssertOnTimeout($"Timed out waiting for ShowHideObject.ClientTargetedNetworkObjects to have a count of 0 but was {ShowHideObject.ClientTargetedNetworkObjects.Count}!"); foreach (var client in m_ClientNetworkManagers) { @@ -564,8 +556,31 @@ public IEnumerator NetworkHideChangeOwnership() } yield return WaitForConditionOrTimeOut(() => ShowHideObject.ClientTargetedNetworkObjects.Count == 1); + AssertOnTimeout($"Timed out waiting for ShowHideObject.ClientTargetedNetworkObjects to have a count of 1 but was {ShowHideObject.ClientTargetedNetworkObjects.Count}!"); + + m_ClientIdToCheck = firstClient.LocalClientId; + yield return WaitForConditionOrTimeOut(CheckIsClientOwner); + AssertOnTimeout($"Timed out waiting for client owner check!"); + } - Assert.True(ShowHideObject.ClientTargetedNetworkObjects[0].OwnerClientId == firstClient.LocalClientId); + private ulong m_ClientIdToCheck; + private bool CheckIsClientOwner(StringBuilder errorLog) + { + if (ShowHideObject.ClientTargetedNetworkObjects[0].OwnerClientId != m_ClientIdToCheck) + { + errorLog.AppendLine($"[CheckIsClientOwner][Index: 0][{ShowHideObject.ClientTargetedNetworkObjects[0].name}] OwnerClientId is {ShowHideObject.ClientTargetedNetworkObjects[0].OwnerClientId} when it was expected to be {m_ClientIdToCheck}!"); + if (ShowHideObject.ClientTargetedNetworkObjects.Count > 1) + { + for (int i = 1; i < ShowHideObject.ClientTargetedNetworkObjects.Count; i++) + { + var target = ShowHideObject.ClientTargetedNetworkObjects[i]; + errorLog.AppendLine($"[CheckIsClientOwner][Index: {i}][{target.name}] OwnerClientId is {target.OwnerClientId}."); + } + } + return false; + } + + return true; } private bool AllClientsSpawnedObject1() diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/OwnerModifiedTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/OwnerModifiedTests.cs index dfc6de950f..ecd4d58fa7 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/OwnerModifiedTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/OwnerModifiedTests.cs @@ -132,6 +132,7 @@ protected override void OnNewClientStartedAndConnected(NetworkManager networkMan [UnityTest] public IEnumerator VerifyDoesNotRepeatOnSomeClients() { + var authority = GetAuthorityNetworkManager(); OwnerModifiedObject.EnableVerbose = m_EnableVerboseDebug; // We use this to assure we are the "last client" connected. yield return CreateAndStartNewClient(); @@ -144,10 +145,10 @@ public IEnumerator VerifyDoesNotRepeatOnSomeClients() ownerModLastClient.NetworkUpdateStageToCheck = (NetworkUpdateStage)updateLoopType; VerboseDebug($"Testing Update Stage: {ownerModLastClient.NetworkUpdateStageToCheck}"); ownerModLastClient.AddValues = true; - yield return WaitForTicks(m_ServerNetworkManager, 5); + yield return WaitForTicks(authority, 5); } - yield return WaitForTicks(m_ServerNetworkManager, 5); + yield return WaitForTicks(authority, 5); // We'll have at least one update per stage per client, if all goes well. Assert.True(OwnerModifiedObject.Updates > 20); From 8ece71de2c9615c78b934f1723ec68c8909af1f6 Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Wed, 5 Nov 2025 16:17:46 -0600 Subject: [PATCH 6/8] test - fix Fixing the race condition test instability when running OwnershipPermissionsTests against CMB service until the order in which a message is received will reflect the order in which it is sent relative to messages received prior to and after the given message. --- .../OwnershipPermissionsTests.cs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs index a414148027..9da821c930 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs @@ -67,7 +67,7 @@ private bool WaitForOneClientToBeApproved(OwnershipPermissionsTestHelper[] clien { approvedClients++; } - else if (helper.OwnershipRequestResponseStatus == NetworkObject.OwnershipRequestResponseStatus.RequestInProgress) + else if (helper.OwnershipRequestResponseStatus == NetworkObject.OwnershipRequestResponseStatus.RequestInProgress || helper.OwnershipRequestResponseStatus == NetworkObject.OwnershipRequestResponseStatus.Denied) { requestInProgressClients++; } @@ -279,8 +279,16 @@ public IEnumerator ValidateOwnershipPermissionsTest() var fourthInstance = fourthClient.SpawnManager.SpawnedObjects[networkObjectId]; var fourthInstanceHelper = fourthInstance.GetComponent(); + // Mock race condition scenario where the second instance request arrives before the rest of the requests. + // This could be on the same frame all requests are received or where they stagger over several frames. + // Until we resolve the CMB service issue where the order in which messages are received does not always + // reflect the order in which they are forwarded to their destination (relative to other messages received from + // other clients). + firstInstanceHelper.OnlyAllowTargetClientId = true; + firstInstanceHelper.ClientToAllowOwnership = secondClient.LocalClientId; + // Send out a request from three clients at the same time - // The first one sent (and received for this test) gets ownership + // The first one received gets ownership requestStatus = secondInstance.RequestOwnership(); Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{secondClient.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); requestStatus = thirdInstance.RequestOwnership(); @@ -291,7 +299,7 @@ public IEnumerator ValidateOwnershipPermissionsTest() // The 2nd and 3rd client should be denied and the 4th client should be approved yield return WaitForConditionOrTimeOut(() => WaitForOneClientToBeApproved(new[] { secondInstanceHelper, thirdInstanceHelper, fourthInstanceHelper })); AssertOnTimeout("[Targeted Owner] A client received an incorrect response. " + - $"Expected one client to have {NetworkObject.OwnershipRequestResponseStatus.Approved} and the others to have {NetworkObject.OwnershipRequestResponseStatus.RequestInProgress}!." + $"Expected one client to have {NetworkObject.OwnershipRequestResponseStatus.Approved} and the others to have {NetworkObject.OwnershipRequestResponseStatus.RequestInProgress} or {NetworkObject.OwnershipRequestResponseStatus.Denied}!." + $"\n Client-{fourthClient.LocalClientId}: has {fourthInstanceHelper.OwnershipRequestResponseStatus}!" + $"\n Client-{thirdClient.LocalClientId}: has {thirdInstanceHelper.OwnershipRequestResponseStatus}!" + $"\n Client-{secondClient.LocalClientId}: has {secondInstanceHelper.OwnershipRequestResponseStatus}!"); @@ -305,6 +313,9 @@ public IEnumerator ValidateOwnershipPermissionsTest() yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); AssertOnTimeout($"[Multiple request race condition][Permissions Mismatch] {secondInstance.name}"); + // Reset this value once this part of the test is complete + firstInstanceHelper.OnlyAllowTargetClientId = false; + /////////////////////////////////////////////// // Test for targeted ownership request: /////////////////////////////////////////////// From a3db8b2a3028769a7232845026f4e012deed47cd Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Wed, 5 Nov 2025 16:20:57 -0600 Subject: [PATCH 7/8] style further clarifying the change made here. --- .../Runtime/NetworkVariable/Collections/NetworkList.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs index f77c0a15c2..42bc1b22d0 100644 --- a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs +++ b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/Collections/NetworkList.cs @@ -62,7 +62,8 @@ internal override void OnSpawned() { // If we are dirty and have write permissions by the time the NetworkObject // is finished spawning (same frame), then go ahead and reset the dirty related - // properties for NetworkList. + // properties for NetworkList in the event user script has made changes when + // spawning to prevent duplicate entries. if (IsDirty() && CanSend()) { UpdateLastSentTime(); From c62c10f99dc72353a17cd50ece79dd4729bef3e9 Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Wed, 5 Nov 2025 18:05:38 -0600 Subject: [PATCH 8/8] test Some more instability related fixes. --- .../Tests/Runtime/AttachableBehaviourTests.cs | 6 +++--- .../Runtime/NetworkVariable/OwnerPermissionTests.cs | 12 +++--------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/AttachableBehaviourTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/AttachableBehaviourTests.cs index 4616525896..383651d446 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/AttachableBehaviourTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/AttachableBehaviourTests.cs @@ -118,7 +118,7 @@ private bool ResetAllStates() else { var attachable = networkManager.SpawnManager.SpawnedObjects[currentAttachableRoot.NetworkObjectId].GetComponentInChildren(); - attachable.ResetStates(); + attachable?.ResetStates(); } // Target @@ -129,7 +129,7 @@ private bool ResetAllStates() else { var node = networkManager.SpawnManager.SpawnedObjects[m_TargetInstance.NetworkObjectId].GetComponentInChildren(); - node.ResetStates(); + node?.ResetStates(); } // Target B @@ -140,7 +140,7 @@ private bool ResetAllStates() else { var node = networkManager.SpawnManager.SpawnedObjects[m_TargetInstanceB.NetworkObjectId].GetComponentInChildren(); - node.ResetStates(); + node?.ResetStates(); } } return m_ErrorLog.Length == 0; diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/OwnerPermissionTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/OwnerPermissionTests.cs index 01878e11f4..cee5a131c6 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/OwnerPermissionTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/OwnerPermissionTests.cs @@ -110,16 +110,10 @@ public IEnumerator OwnerPermissionTest() { ownerManager = m_ClientNetworkManagers[objectIndex - 1]; } - SpawnObject(m_PrefabToSpawn, ownerManager); + var spawnedInstance = SpawnObject(m_PrefabToSpawn, ownerManager); - // wait for each object to spawn on each client - for (var clientIndex = 0; clientIndex < 3; clientIndex++) - { - while (OwnerPermissionObject.Objects[objectIndex, clientIndex] == null) - { - yield return new WaitForSeconds(0.0f); - } - } + yield return WaitForSpawnedOnAllOrTimeOut(spawnedInstance); + AssertOnTimeout($"Timed out waiting for all clients to spawn {spawnedInstance.name}!"); } var nextValueToWrite = 1;