From f73e380cba6b9e718b7814c87d960d5e32564563 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 18 Dec 2024 16:59:52 -0600 Subject: [PATCH 01/10] fix This will "re-initialize" NetworkVariableBase derived classes when the instance is not destroyed and repurposed. --- .../Runtime/Core/NetworkBehaviour.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs index adc7ec0fa4..6b4c3585c3 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs @@ -918,10 +918,25 @@ internal void __nameNetworkVariable(NetworkVariableBase variable, string varName variable.Name = varName; } + /// + /// Does a first pass initialization for RPCs and NetworkVariables + /// If already initialized, then it just re-initializes the NetworkVariables. + /// internal void InitializeVariables() { if (m_VarInit) { + // If the primary initialization has already been done, then go ahead + // and re-initialize each NetworkVariable in the event it is an in-scene + // placed NetworkObject in an already loaded scene that has already been + // used within a network session =or= if this is a pooled NetworkObject + // that is being repurposed. + for (int i = 0; i < NetworkVariableFields.Count; i++) + { + NetworkVariableFields[i].Initialize(this); + } + // Exit early as we don't need to run through the rest of this initialization + // process return; } From 1b6e0cb571e8ff69652b2c594379ae3febb5e856 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 18 Dec 2024 17:01:43 -0600 Subject: [PATCH 02/10] update adding changelog entry --- com.unity.netcode.gameobjects/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 423096ac35..98dca3ec08 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -12,6 +12,8 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed +- Fixed issue where `NetworkVariableBase` derived classes were not being re-initialized if the associated `NetworkObject` instance was not destroyed and respawned. + ### Changed From 626acca6627efb7d044d52953076c545ee3ebcd4 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 18 Dec 2024 17:09:16 -0600 Subject: [PATCH 03/10] update Adding PR number to changelog entry --- com.unity.netcode.gameobjects/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 98dca3ec08..43b381c876 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -12,7 +12,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed -- Fixed issue where `NetworkVariableBase` derived classes were not being re-initialized if the associated `NetworkObject` instance was not destroyed and respawned. +- Fixed issue where `NetworkVariableBase` derived classes were not being re-initialized if the associated `NetworkObject` instance was not destroyed and respawned. (#3181) ### Changed From 7c21d8c3ea05e22c934789fe5fdba462c9b09478 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 18 Dec 2024 19:04:06 -0600 Subject: [PATCH 04/10] fix ? This test needs more comments. --- .../NetworkVariableAnticipationTests.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableAnticipationTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableAnticipationTests.cs index c436275fea..f73b42484a 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableAnticipationTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableAnticipationTests.cs @@ -248,25 +248,25 @@ public void WhenServerChangesSmoothValue_ValuesAreLerped() WaitForMessageReceivedWithTimeTravel(m_ClientNetworkManagers.ToList()); - Assert.AreEqual(15 + 1f / 60f * 5, testComponent.SmoothOnAnticipationFailVariable.Value, Mathf.Epsilon); + Assert.AreEqual(15 + 1f / 60f * 15, testComponent.SmoothOnAnticipationFailVariable.Value, Mathf.Epsilon); Assert.AreEqual(20, testComponent.SmoothOnAnticipationFailVariable.AuthoritativeValue, Mathf.Epsilon); - Assert.AreEqual(0 + 1f / 60f * 20, otherClientComponent.SmoothOnAnticipationFailVariable.Value, Mathf.Epsilon); + Assert.AreEqual(1f, otherClientComponent.SmoothOnAnticipationFailVariable.Value, 0.000015f); Assert.AreEqual(20, otherClientComponent.SmoothOnAnticipationFailVariable.AuthoritativeValue, Mathf.Epsilon); - for (var i = 1; i < 60; ++i) + for (var i = 1; i < 15; ++i) { TimeTravel(1f / 60f, 1); - Assert.AreEqual(15 + 1f / 60f * 5 * (i + 1), testComponent.SmoothOnAnticipationFailVariable.Value, 0.00001); + Assert.AreEqual(15 + 1f / 60f * 15 * (i + 1), testComponent.SmoothOnAnticipationFailVariable.Value, 0.00001); Assert.AreEqual(20, testComponent.SmoothOnAnticipationFailVariable.AuthoritativeValue, Mathf.Epsilon); - Assert.AreEqual(0 + 1f / 60f * 20 * (i + 1), otherClientComponent.SmoothOnAnticipationFailVariable.Value, 0.00001); + Assert.AreEqual(1.0f + i, otherClientComponent.SmoothOnAnticipationFailVariable.Value, 0.00001); Assert.AreEqual(20, otherClientComponent.SmoothOnAnticipationFailVariable.AuthoritativeValue, Mathf.Epsilon); } TimeTravel(1f / 60f, 1); - Assert.AreEqual(20, testComponent.SmoothOnAnticipationFailVariable.Value, Mathf.Epsilon); - Assert.AreEqual(20, otherClientComponent.SmoothOnAnticipationFailVariable.Value, Mathf.Epsilon); + Assert.AreEqual(19, testComponent.SmoothOnAnticipationFailVariable.Value, Mathf.Epsilon); + Assert.AreEqual(16, otherClientComponent.SmoothOnAnticipationFailVariable.Value, 0.00001); } [Test] From b4c3d0c93a16af3074176d56183db1867cc8c1ac Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 15 Jan 2025 13:32:57 -0600 Subject: [PATCH 05/10] update Changing the approach to this fix while also cleaning up the initialize process so it can be invoked when the derived class is not yet fully spawned (or just recently instantiated) without potentially assigning the wrong NetworkManager instance. It is being invoked multiple times anyway, so this just gradually initializes all of the required elements until it is finally considered "initialized". This also prevents OnInitialize from being invoked multiple times during this process and OnInitialize is only invoked as a last step to being considered "initialized". Added a Deinitialize step to NetworkVariableBase when despawning a NetworkObject where the initialized status is reset. This allows for the next spawning of the NetworkObject (if in-scene placed or pooled) to run through the initialization process. --- .../Runtime/Core/NetworkBehaviour.cs | 12 +++ .../NetworkVariable/NetworkVariableBase.cs | 99 +++++++++++++++---- 2 files changed, 90 insertions(+), 21 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs index 6b4c3585c3..da6b35e001 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs @@ -815,6 +815,13 @@ internal void InternalOnNetworkDespawn() { Debug.LogException(e); } + + // Deinitialize all NetworkVariables in the event the associated + // NetworkObject is recylced (in-scene placed or pooled). + for (int i = 0; i < NetworkVariableFields.Count; i++) + { + NetworkVariableFields[i].Deinitialize(); + } } /// @@ -933,6 +940,11 @@ internal void InitializeVariables() // that is being repurposed. for (int i = 0; i < NetworkVariableFields.Count; i++) { + // If already initialized, then skip + if (NetworkVariableFields[i].HasBeenInitialized) + { + continue; + } NetworkVariableFields[i].Initialize(this); } // Exit early as we don't need to run through the rest of this initialization diff --git a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs index 537d3eba38..78225f4d54 100644 --- a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs +++ b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs @@ -35,6 +35,12 @@ public abstract class NetworkVariableBase : IDisposable private NetworkManager m_InternalNetworkManager; + // Determines if this NetworkVariable has been "initialized" to prevent initializing more than once which can happen when first + // instantiated and spawned. If this NetworkVariable instance is on an in-scene placed NetworkObject =or= a pooled NetworkObject + // that can persist between sessions and/or be recycled we need to reset the LastUpdateSent value prior to spawning otherwise + // this NetworkVariableBase property instance will not update until the last session time used. + internal bool HasBeenInitialized { get; private set; } + internal string GetWritePermissionError() { return $"|Client-{m_NetworkManager.LocalClientId}|{m_NetworkBehaviour.name}|{Name}| Write permissions ({WritePerm}) for this client instance is not allowed!"; @@ -45,17 +51,7 @@ internal void LogWritePermissionError() Debug.LogError(GetWritePermissionError()); } - private protected NetworkManager m_NetworkManager - { - get - { - if (m_InternalNetworkManager == null && m_NetworkBehaviour && m_NetworkBehaviour.NetworkObject?.NetworkManager) - { - m_InternalNetworkManager = m_NetworkBehaviour.NetworkObject?.NetworkManager; - } - return m_InternalNetworkManager; - } - } + private protected NetworkManager m_NetworkManager => m_InternalNetworkManager; public NetworkBehaviour GetBehaviour() { @@ -68,21 +64,82 @@ public NetworkBehaviour GetBehaviour() /// The NetworkBehaviour the NetworkVariable belongs to public void Initialize(NetworkBehaviour networkBehaviour) { - m_InternalNetworkManager = null; + // If we have already been initialized, then exit early. + // This can happen on the very first instantiation and spawning of the associated NetworkObject + if (HasBeenInitialized) + { + return; + } + + // Throw an exception if there is an invalid NetworkBehaviour parameter + if (!networkBehaviour) + { + throw new Exception($"[{GetType().Name}][Initialize] {nameof(NetworkBehaviour)} parameter passed in is null!"); + } m_NetworkBehaviour = networkBehaviour; - if (m_NetworkBehaviour && m_NetworkBehaviour.NetworkObject?.NetworkManager) + + // Throw an exception if there is no NetworkManager available + if (!m_NetworkBehaviour.NetworkManager) { - m_InternalNetworkManager = m_NetworkBehaviour.NetworkObject?.NetworkManager; - // When in distributed authority mode, there is no such thing as server write permissions - InternalWritePerm = m_InternalNetworkManager.DistributedAuthorityMode ? NetworkVariableWritePermission.Owner : InternalWritePerm; + // Exit early if there has yet to be a NetworkManager assigned. + // This is ok because Initialize is invoked multiple times until + // it is considered "initialized". + return; + } - if (m_NetworkBehaviour.NetworkManager.NetworkTimeSystem != null) - { - UpdateLastSentTime(); - } + if (!m_NetworkBehaviour.NetworkObject) + { + // Exit early if there has yet to be a NetworkObject assigned. + // This is ok because Initialize is invoked multiple times until + // it is considered "initialized". + return; + } + + if (!m_NetworkBehaviour.NetworkObject.NetworkManagerOwner) + { + // Exit early if there has yet to be a NetworkManagerOwner assigned + // to the NetworkObject. This is ok because Initialize is invoked + // multiple times until it is considered "initialized". + return; + } + m_InternalNetworkManager = m_NetworkBehaviour.NetworkObject.NetworkManagerOwner; + + // Throw an exception if there is no valid NetworkTimeSystem + if (m_InternalNetworkManager.NetworkTimeSystem == null) + { + throw new Exception($"[{m_NetworkBehaviour.name}][{m_NetworkBehaviour.GetType().Name}][{GetType().Name}][Initialize] {nameof(NetworkManager)} has no {nameof(NetworkTimeSystem)} assigned!"); + } + + // When in distributed authority mode, there is no such thing as server write permissions + InternalWritePerm = m_InternalNetworkManager.DistributedAuthorityMode ? NetworkVariableWritePermission.Owner : InternalWritePerm; + + // Update our last sent time relative to when this was initialized + UpdateLastSentTime(); + + // Wrap potential user script override to catch and log any exceptions + try + { + OnInitialize(); + } + catch (Exception ex) + { + Debug.LogException(ex); } - OnInitialize(); + // At this point, this instance is considered initialized + HasBeenInitialized = true; + } + + /// + /// Deinitialize is invoked when a NetworkObject is despawned. + /// This allows for a recyled NetworkObject (in-scene or pooled) + /// to be properly initialized upon the next use/spawn. + /// + internal void Deinitialize() + { + // When despawned, reset the HasBeenInitialized so if the associated NetworkObject instance + // is recylced (i.e. in-scene placed or pooled) it will re-initialize the LastUpdateSent time. + HasBeenInitialized = false; } /// From 9fafd5f2183a7aed0f0e7e020d901916199b4cff Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 15 Jan 2025 13:34:49 -0600 Subject: [PATCH 06/10] test reverting the changes to WhenServerChangesSmoothValue_ValuesAreLerped as the recent update resolves the issue from the previous fix. --- .../NetworkVariableAnticipationTests.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableAnticipationTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableAnticipationTests.cs index f73b42484a..c436275fea 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableAnticipationTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableAnticipationTests.cs @@ -248,25 +248,25 @@ public void WhenServerChangesSmoothValue_ValuesAreLerped() WaitForMessageReceivedWithTimeTravel(m_ClientNetworkManagers.ToList()); - Assert.AreEqual(15 + 1f / 60f * 15, testComponent.SmoothOnAnticipationFailVariable.Value, Mathf.Epsilon); + Assert.AreEqual(15 + 1f / 60f * 5, testComponent.SmoothOnAnticipationFailVariable.Value, Mathf.Epsilon); Assert.AreEqual(20, testComponent.SmoothOnAnticipationFailVariable.AuthoritativeValue, Mathf.Epsilon); - Assert.AreEqual(1f, otherClientComponent.SmoothOnAnticipationFailVariable.Value, 0.000015f); + Assert.AreEqual(0 + 1f / 60f * 20, otherClientComponent.SmoothOnAnticipationFailVariable.Value, Mathf.Epsilon); Assert.AreEqual(20, otherClientComponent.SmoothOnAnticipationFailVariable.AuthoritativeValue, Mathf.Epsilon); - for (var i = 1; i < 15; ++i) + for (var i = 1; i < 60; ++i) { TimeTravel(1f / 60f, 1); - Assert.AreEqual(15 + 1f / 60f * 15 * (i + 1), testComponent.SmoothOnAnticipationFailVariable.Value, 0.00001); + Assert.AreEqual(15 + 1f / 60f * 5 * (i + 1), testComponent.SmoothOnAnticipationFailVariable.Value, 0.00001); Assert.AreEqual(20, testComponent.SmoothOnAnticipationFailVariable.AuthoritativeValue, Mathf.Epsilon); - Assert.AreEqual(1.0f + i, otherClientComponent.SmoothOnAnticipationFailVariable.Value, 0.00001); + Assert.AreEqual(0 + 1f / 60f * 20 * (i + 1), otherClientComponent.SmoothOnAnticipationFailVariable.Value, 0.00001); Assert.AreEqual(20, otherClientComponent.SmoothOnAnticipationFailVariable.AuthoritativeValue, Mathf.Epsilon); } TimeTravel(1f / 60f, 1); - Assert.AreEqual(19, testComponent.SmoothOnAnticipationFailVariable.Value, Mathf.Epsilon); - Assert.AreEqual(16, otherClientComponent.SmoothOnAnticipationFailVariable.Value, 0.00001); + Assert.AreEqual(20, testComponent.SmoothOnAnticipationFailVariable.Value, Mathf.Epsilon); + Assert.AreEqual(20, otherClientComponent.SmoothOnAnticipationFailVariable.Value, Mathf.Epsilon); } [Test] From 5b35d26e18029ad5f21d7d04bdd04b3534d4b02a Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 15 Jan 2025 18:10:20 -0600 Subject: [PATCH 07/10] update Removing exception. Adjusting when we update the last time. Logging warning if log level is developer and there is no NetworkTimeSystem. --- .../NetworkVariable/NetworkVariableBase.cs | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs index 78225f4d54..35cdb4e005 100644 --- a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs +++ b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs @@ -104,18 +104,9 @@ public void Initialize(NetworkBehaviour networkBehaviour) } m_InternalNetworkManager = m_NetworkBehaviour.NetworkObject.NetworkManagerOwner; - // Throw an exception if there is no valid NetworkTimeSystem - if (m_InternalNetworkManager.NetworkTimeSystem == null) - { - throw new Exception($"[{m_NetworkBehaviour.name}][{m_NetworkBehaviour.GetType().Name}][{GetType().Name}][Initialize] {nameof(NetworkManager)} has no {nameof(NetworkTimeSystem)} assigned!"); - } - // When in distributed authority mode, there is no such thing as server write permissions InternalWritePerm = m_InternalNetworkManager.DistributedAuthorityMode ? NetworkVariableWritePermission.Owner : InternalWritePerm; - // Update our last sent time relative to when this was initialized - UpdateLastSentTime(); - // Wrap potential user script override to catch and log any exceptions try { @@ -126,8 +117,20 @@ public void Initialize(NetworkBehaviour networkBehaviour) Debug.LogException(ex); } - // At this point, this instance is considered initialized - HasBeenInitialized = true; + // Some unit tests don't operate with a running NetworkManager. + // Only update the last time if there is a NetworkTimeSystem. + if (m_InternalNetworkManager.NetworkTimeSystem != null) + { + // Update our last sent time relative to when this was initialized + UpdateLastSentTime(); + + // At this point, this instance is considered initialized + HasBeenInitialized = true; + } + else if (m_InternalNetworkManager.LogLevel == LogLevel.Developer) + { + Debug.LogWarning($"[{m_NetworkBehaviour.name}][{m_NetworkBehaviour.GetType().Name}][{GetType().Name}][Initialize] {nameof(NetworkManager)} has no {nameof(NetworkTimeSystem)} assigned!"); + } } /// From edf14705d4e5893999d84fd1dc218680263e8e36 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Wed, 15 Jan 2025 19:57:53 -0600 Subject: [PATCH 08/10] update remove the try catch because evidently it is the only way we can catch certain exceptions during unit testing when duplicating the value (i.e. serializing). --- .../Runtime/NetworkVariable/NetworkVariableBase.cs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs index 35cdb4e005..66a5b8f955 100644 --- a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs +++ b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs @@ -107,15 +107,7 @@ public void Initialize(NetworkBehaviour networkBehaviour) // When in distributed authority mode, there is no such thing as server write permissions InternalWritePerm = m_InternalNetworkManager.DistributedAuthorityMode ? NetworkVariableWritePermission.Owner : InternalWritePerm; - // Wrap potential user script override to catch and log any exceptions - try - { - OnInitialize(); - } - catch (Exception ex) - { - Debug.LogException(ex); - } + OnInitialize(); // Some unit tests don't operate with a running NetworkManager. // Only update the last time if there is a NetworkTimeSystem. From 91bf7475d50a25265b65c077c7e6e96a5f93675e Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Tue, 21 Jan 2025 17:57:33 -0600 Subject: [PATCH 09/10] test This validates the fix for resetting the NetworkVariableBase.LastUpdated property when a NetworkObject is persisted (i.e. recycled via in-scene placed NetworkObject or object pools). Also minor update to prevent the integration test from logging a blank entry if there has been no logs added. --- .../Runtime/NetcodeIntegrationTest.cs | 8 +- ...orkVariableBaseInitializesWhenPersisted.cs | 411 ++++++++++++++++++ ...riableBaseInitializesWhenPersisted.cs.meta | 2 + 3 files changed, 419 insertions(+), 2 deletions(-) create mode 100644 com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableBaseInitializesWhenPersisted.cs create mode 100644 com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableBaseInitializesWhenPersisted.cs.meta diff --git a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs index 921d70b928..6222ce7993 100644 --- a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs +++ b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs @@ -1821,8 +1821,12 @@ private void UnloadRemainingScenes() private void LogWaitForMessages() { - VerboseDebug(m_WaitForLog.ToString()); - m_WaitForLog.Clear(); + // If there is nothing to log, then don't log anything + if (m_WaitForLog.Length > 0) + { + VerboseDebug(m_WaitForLog.ToString()); + m_WaitForLog.Clear(); + } } private IEnumerator WaitForTickAndFrames(NetworkManager networkManager, int tickCount, float targetFrames) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableBaseInitializesWhenPersisted.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableBaseInitializesWhenPersisted.cs new file mode 100644 index 0000000000..a6f46bc5a6 --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableBaseInitializesWhenPersisted.cs @@ -0,0 +1,411 @@ +using System.Collections; +using System.Collections.Generic; +using System.Linq; +using NUnit.Framework; +using Unity.Netcode.TestHelpers.Runtime; +using UnityEngine; +using UnityEngine.SceneManagement; +using UnityEngine.TestTools; + +namespace Unity.Netcode.RuntimeTests +{ + [TestFixture(NetworkTopologyTypes.ClientServer, HostOrServer.Server)] + [TestFixture(NetworkTopologyTypes.ClientServer, HostOrServer.Host)] + [TestFixture(NetworkTopologyTypes.DistributedAuthority, HostOrServer.DAHost)] + public class NetworkVariableBaseInitializesWhenPersisted : NetcodeIntegrationTest + { + protected override int NumberOfClients => 1; + + private static GameObject s_NetworkPrefab; + + public NetworkVariableBaseInitializesWhenPersisted(NetworkTopologyTypes networkTopologyTypes, HostOrServer hostOrServer) : base(networkTopologyTypes, hostOrServer) { } + + protected override void OnOneTimeSetup() + { + // Create a prefab to persist for all tests + s_NetworkPrefab = new GameObject("PresistPrefab"); + var networkObject = s_NetworkPrefab.AddComponent(); + networkObject.GlobalObjectIdHash = 8888888; + networkObject.SetSceneObjectStatus(false); + s_NetworkPrefab.AddComponent(); + s_NetworkPrefab.AddComponent(); + // Create enough prefab instance handlers to be re-used for all tests. + PrefabInstanceHandler.OneTimeSetup(NumberOfClients + 1, s_NetworkPrefab); + Object.DontDestroyOnLoad(s_NetworkPrefab); + base.OnOneTimeSetup(); + } + + protected override void OnServerAndClientsCreated() + { + // Assign one of the precreated prefab instance handlers to the server + PrefabInstanceHandler.AssignHandler(m_ServerNetworkManager); + // Add the network prefab to the server networkmanager + m_ServerNetworkManager.AddNetworkPrefab(s_NetworkPrefab); + // Repeat these steps for clients + foreach (var client in m_ClientNetworkManagers) + { + PrefabInstanceHandler.AssignHandler(client); + client.AddNetworkPrefab(s_NetworkPrefab); + } + // !!! IMPORTANT !!! + // Disable the persisted network prefab so it isn't spawned nor destroyed + s_NetworkPrefab.SetActive(false); + base.OnServerAndClientsCreated(); + } + + private List m_NetworkManagers = new List(); + private List m_SpawnedObjects = new List(); + + [UnityTest] + public IEnumerator PrefabSessionIstantiationPass([Values(4, 3, 2, 1)] int iterationsLeft) + { + // Start out waiting for a long duration before updating the NetworkVariable so each + // next iteration we change it earlier than the previous. This validates that the + // NetworkVariable's last update time is being reset each time a persisted NetworkObject + // is being spawned. + var baseWaitTime = 0.35f; + var waitPeriod = baseWaitTime * iterationsLeft; + m_NetworkManagers.Add(m_ServerNetworkManager); + m_NetworkManagers.AddRange(m_ClientNetworkManagers); + + foreach (var networkManager in m_NetworkManagers) + { + // Validate Pooled Objects Persisted Between Tests + // We start with having 4 iterations (including the first of the 4), which means that when we have 4 + // iterations remaining we haven't spawned any instances so it will test that there are no instances. + // When we there are 3 iterations left, every persisted instance of the network prefab should have been + // spawned at least once...when 2 then all should have been spawned twice...when 1 then all should been + // spawned three times. + Assert.True(PrefabInstanceHandler.ValidatePersistedInstances(networkManager, 4 - iterationsLeft), "Failed validation of persisted pooled objects!"); + + // Spawn 1 NetworkObject per NetworkManager with the NetworkManager's client being the owner + var networkManagerToSpawn = m_DistributedAuthority ? networkManager : m_ServerNetworkManager; + var objectToSpawn = PrefabInstanceHandler.GetInstanceToSpawn(networkManagerToSpawn); + objectToSpawn.NetworkManagerOwner = networkManagerToSpawn; + objectToSpawn.SpawnWithOwnership(networkManager.LocalClientId); + m_SpawnedObjects.Add(objectToSpawn); + } + + // Conditional wait for all clients to spawn all objects + bool AllInstancesSpawnedOnAllCLients() + { + foreach (var spawnedObject in m_SpawnedObjects) + { + foreach (var networkManager in m_NetworkManagers) + { + if (!networkManager.SpawnManager.SpawnedObjects.ContainsKey(spawnedObject.NetworkObjectId)) + { + return false; + } + } + } + return true; + } + + // Wait for all clients to locally clone and spawn all spawned NetworkObjects + yield return WaitForConditionOrTimeOut(AllInstancesSpawnedOnAllCLients); + AssertOnTimeout($"Failed to spawn all instances on all clients!"); + + // Wait for the continually decreasing waitPeriod to validate that + // NetworkVariableBase has reset the last update time. + yield return new WaitForSeconds(waitPeriod); + + // Have the owner of each spawned NetworkObject assign a new NetworkVariable + foreach (var spawnedObject in m_SpawnedObjects) + { + var testBehaviour = spawnedObject.GetComponent(); + if (!m_DistributedAuthority) + { + var client = m_NetworkManagers.Where((c) => c.LocalClientId == testBehaviour.OwnerClientId).First(); + testBehaviour = client.SpawnManager.SpawnedObjects[testBehaviour.NetworkObjectId].GetComponent(); + } + testBehaviour.TestNetworkVariable.Value = Random.Range(0, 1000); + } + + // Wait for half of the base time before checking to see if the time delta is within the acceptable range. + // The time delta is the time from spawn to when the value changes. Each iteration of this test decreases + // the total wait period, and so the delta should always be less than the wait period plus the base wait time. + yield return new WaitForSeconds(baseWaitTime * 0.5f); + + // Conditional to verify the time delta between spawn and the NetworkVariable being updated is within the + // expected range + bool AllSpawnedObjectsUpdatedWithinTimeSpan() + { + foreach (var spawnedObject in m_SpawnedObjects) + { + foreach (var networkManager in m_NetworkManagers) + { + if (spawnedObject.OwnerClientId == networkManager.LocalClientId) + { + continue; + } + if (!networkManager.SpawnManager.SpawnedObjects.ContainsKey(spawnedObject.NetworkObjectId)) + { + return false; + } + var instance = networkManager.SpawnManager.SpawnedObjects[spawnedObject.NetworkObjectId].GetComponent(); + // Check to make sure all clients' delta between updates is not greater than the wait period plus the baseWaitTime. + if (instance.LastUpdateDelta >= (waitPeriod + baseWaitTime)) + { + VerboseDebug($"Last Spawn Delta = {instance.LastUpdateDelta} is greater or equal to {waitPeriod + baseWaitTime}"); + return false; + } + } + } + return true; + } + + yield return WaitForConditionOrTimeOut(AllSpawnedObjectsUpdatedWithinTimeSpan); + AssertOnTimeout($"Failed to reset NetworkVariableBase instances!"); + } + + + protected override IEnumerator OnTearDown() + { + m_NetworkManagers.Clear(); + m_SpawnedObjects.Clear(); + PrefabInstanceHandler.ReleaseAll(); + yield return base.OnTearDown(); + } + + protected override void OnOneTimeTearDown() + { + Object.DestroyImmediate(s_NetworkPrefab); + s_NetworkPrefab = null; + PrefabInstanceHandler.ReleaseAll(true); + base.OnOneTimeTearDown(); + } + + /// + /// Test NetworkBehaviour that updates a NetworkVariable and tracks the time between + /// spawn and when the NetworkVariable is updated. + /// + public class TestBehaviour : NetworkBehaviour + { + public NetworkVariable TestNetworkVariable = new NetworkVariable(default, NetworkVariableReadPermission.Everyone, NetworkVariableWritePermission.Owner); + + public float LastUpdateDelta { get; private set; } + private float m_TimeSinceLastUpdate = 0.0f; + + // How many times has this instance been spawned. + public int SpawnedCount { get; private set; } + + public override void OnNetworkSpawn() + { + SpawnedCount++; + if (IsOwner) + { + TestNetworkVariable.Value = 0; + } + base.OnNetworkSpawn(); + } + + protected override void OnNetworkPostSpawn() + { + if (!IsOwner) + { + TestNetworkVariable.OnValueChanged += OnTestValueChanged; + } + m_TimeSinceLastUpdate = Time.realtimeSinceStartup; + base.OnNetworkPostSpawn(); + } + + public override void OnDestroy() + { + base.OnDestroy(); + } + + public override void OnNetworkDespawn() + { + TestNetworkVariable.OnValueChanged -= OnTestValueChanged; + LastUpdateDelta = 0.0f; + base.OnNetworkDespawn(); + } + + private void OnTestValueChanged(int previous, int current) + { + LastUpdateDelta = Time.realtimeSinceStartup - m_TimeSinceLastUpdate; + } + } + + /// + /// Creates a specified number of instances that persist throughout the entire test session + /// and will only be destroyed/released during the OneTimeTeardown + /// + public class PrefabInstanceHandler : INetworkPrefabInstanceHandler + { + private static Dictionary s_AssignedInstances = new Dictionary(); + private static Queue s_PrefabInstanceHandlers = new Queue(); + public Queue PrefabInstances = new Queue(); + private GameObject m_NetworkPrefab; + private NetworkManager m_NetworkManager; + private ulong m_AssignedClientId; + + public static void OneTimeSetup(int numberOfInstances, GameObject prefabInstance) + { + for (int i = 0; i < numberOfInstances; i++) + { + s_PrefabInstanceHandlers.Enqueue(new PrefabInstanceHandler(prefabInstance)); + } + } + + /// + /// Invoke when s are created but not started. + /// + /// + public static void AssignHandler(NetworkManager networkManager) + { + if (s_PrefabInstanceHandlers.Count > 0) + { + var instance = s_PrefabInstanceHandlers.Dequeue(); + instance.Initialize(networkManager); + s_AssignedInstances.Add(networkManager, instance); + } + else + { + Debug.LogError($"[{nameof(PrefabInstanceHandler)}] Exhausted total number of instances available!"); + } + } + + public static NetworkObject GetInstanceToSpawn(NetworkManager networkManager) + { + if (s_AssignedInstances.ContainsKey(networkManager)) + { + return s_AssignedInstances[networkManager].GetInstance(); + } + return null; + } + + + public static bool ValidatePersistedInstances(NetworkManager networkManager, int minCount) + { + if (s_AssignedInstances.ContainsKey(networkManager)) + { + var prefabInstanceHandler = s_AssignedInstances[networkManager]; + return prefabInstanceHandler.ValidateInstanceSpawnCount(minCount); + } + return false; + } + + /// + /// Releases back to the queue and if destroy is true it will completely + /// remove all references so they are cleaned up when + /// + /// + public static void ReleaseAll(bool destroy = false) + { + foreach (var entry in s_AssignedInstances) + { + entry.Value.DeregisterHandler(); + if (!destroy) + { + s_PrefabInstanceHandlers.Enqueue(entry.Value); + } + else if (entry.Value.PrefabInstances.Count > 0) + { + entry.Value.CleanInstances(); + } + } + s_AssignedInstances.Clear(); + + if (destroy) + { + while (s_PrefabInstanceHandlers.Count > 0) + { + s_PrefabInstanceHandlers.Dequeue(); + } + } + } + + public PrefabInstanceHandler(GameObject gameObject) + { + m_NetworkPrefab = gameObject; + } + + + public void Initialize(NetworkManager networkManager) + { + m_NetworkManager = networkManager; + networkManager.PrefabHandler.AddHandler(m_NetworkPrefab, this); + } + + /// + /// This validates that the instances persisted to the next test set and persisted + /// between network sessions + public bool ValidateInstanceSpawnCount(int minCount) + { + // First pass we should have no instances + if (minCount == 0) + { + return PrefabInstances.Count == 0; + } + else + { + foreach (var instance in PrefabInstances) + { + var testBehaviour = instance.GetComponent(); + if (testBehaviour.SpawnedCount < minCount) + { + return false; + } + } + } + return true; + } + + /// + /// When we are done with all tests, we finally destroy the persisted objects + /// + public void CleanInstances() + { + while (PrefabInstances.Count > 0) + { + var instance = PrefabInstances.Dequeue(); + Object.DestroyImmediate(instance); + } + } + + public void DeregisterHandler() + { + if (m_NetworkManager != null && m_NetworkManager.PrefabHandler != null) + { + m_NetworkManager.PrefabHandler.RemoveHandler(m_NetworkPrefab); + } + } + + public NetworkObject GetInstance() + { + var instanceToReturn = (NetworkObject)null; + if (PrefabInstances.Count == 0) + { + instanceToReturn = Object.Instantiate(m_NetworkPrefab).GetComponent(); + instanceToReturn.SetSceneObjectStatus(false); + instanceToReturn.gameObject.SetActive(true); + } + else + { + instanceToReturn = PrefabInstances.Dequeue().GetComponent(); + instanceToReturn.gameObject.SetActive(true); + SceneManager.MoveGameObjectToScene(instanceToReturn.gameObject, SceneManager.GetActiveScene()); + } + return instanceToReturn; + } + + public NetworkObject Instantiate(ulong ownerClientId, Vector3 position, Quaternion rotation) + { + return GetInstance(); + } + + public void Destroy(NetworkObject networkObject) + { + if (m_NetworkPrefab != null && m_NetworkPrefab.gameObject == networkObject.gameObject) + { + return; + } + Object.DontDestroyOnLoad(networkObject.gameObject); + networkObject.gameObject.SetActive(false); + PrefabInstances.Enqueue(networkObject.gameObject); + } + } + } +} diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableBaseInitializesWhenPersisted.cs.meta b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableBaseInitializesWhenPersisted.cs.meta new file mode 100644 index 0000000000..160261ff3a --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableBaseInitializesWhenPersisted.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 3f68143136bcd1643a564952c2d48e2c \ No newline at end of file From be057cedd1073d7732c49ad740a40475f19504c8 Mon Sep 17 00:00:00 2001 From: Noel Stephens Date: Fri, 24 Jan 2025 14:48:30 -0600 Subject: [PATCH 10/10] test The first iteration should ignore the time delta as that could be impacted by the test or VM running the test. --- .../NetworkVariableBaseInitializesWhenPersisted.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableBaseInitializesWhenPersisted.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableBaseInitializesWhenPersisted.cs index a6f46bc5a6..545b30456e 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableBaseInitializesWhenPersisted.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableBaseInitializesWhenPersisted.cs @@ -145,7 +145,8 @@ bool AllSpawnedObjectsUpdatedWithinTimeSpan() } var instance = networkManager.SpawnManager.SpawnedObjects[spawnedObject.NetworkObjectId].GetComponent(); // Check to make sure all clients' delta between updates is not greater than the wait period plus the baseWaitTime. - if (instance.LastUpdateDelta >= (waitPeriod + baseWaitTime)) + // Ignore the first iteration as that becomes our baseline. + if (iterationsLeft < 4 && instance.LastUpdateDelta >= (waitPeriod + baseWaitTime)) { VerboseDebug($"Last Spawn Delta = {instance.LastUpdateDelta} is greater or equal to {waitPeriod + baseWaitTime}"); return false;