diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 8f2dee46b1..514b6e994b 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -14,6 +14,7 @@ Additional documentation and release notes are available at [Multiplayer Documen - Changed the `NetworkTimeSystem.Sync` method to use half RTT to calculate the desired local time offset as opposed to the full RTT. (#3212) - Fixed issue where a spawned `NetworkObject` that was registered with a prefab handler and owned by a client would invoke destroy more than once on the host-server side if the client disconnected while the `NetworkObject` was still spawned. (#3200) +- Fixed issue where `NetworkVariableBase` derived classes were not being re-initialized if the associated `NetworkObject` instance was not destroyed and re-spawned. (#3181) ### Changed diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs index adc7ec0fa4..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(); + } } /// @@ -918,10 +925,30 @@ 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++) + { + // 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 + // process return; } diff --git a/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs b/com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs index 537d3eba38..66a5b8f955 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,77 @@ 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; + + // When in distributed authority mode, there is no such thing as server write permissions + InternalWritePerm = m_InternalNetworkManager.DistributedAuthorityMode ? NetworkVariableWritePermission.Owner : InternalWritePerm; OnInitialize(); + + // 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!"); + } + } + + /// + /// 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; } /// 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..545b30456e --- /dev/null +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableBaseInitializesWhenPersisted.cs @@ -0,0 +1,412 @@ +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. + // 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; + } + } + } + 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