diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 3dd4001177..441f02d681 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 ### Fixed +- Fixed issue where `NetworkVariable`s on a `NetworkBehaviour` could fail to synchronize changes if one has `NetworkVariableUpdateTraits` set and is dirty but is not ready to send. (#3466) - Fixed issues with the `NetworkBehaviour` and `NetworkVariable` length safety checks. (#3405) - Fixed memory leaks when domain reload is disabled. (#3427) - Fixed an exception being thrown when unregistering a custom message handler from within the registered callback. (#3417) diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs index df1648dea4..62e60fb436 100644 --- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs +++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkBehaviour.cs @@ -1094,8 +1094,8 @@ internal void NetworkVariableUpdate(ulong targetClientId, bool forceSend = false if (networkVariable.CanSend()) { shouldSend = true; + break; } - break; } } // All of this is just to prevent the DA Host from re-sending a NetworkVariable update it received from the client owner diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableTraitsTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableTraitsTests.cs index 66c326cb99..fd7c886ab4 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableTraitsTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableTraitsTests.cs @@ -1,138 +1,180 @@ +using System.Collections; +using System.Text; using NUnit.Framework; using Unity.Netcode.TestHelpers.Runtime; using UnityEngine; -using Object = UnityEngine.Object; +using UnityEngine.TestTools; namespace Unity.Netcode.RuntimeTests { internal class NetworkVariableTraitsComponent : NetworkBehaviour { public NetworkVariable TheVariable = new NetworkVariable(); + public NetworkVariable AnotherVariable = new NetworkVariable(); } + [TestFixture(HostOrServer.Host)] + [TestFixture(HostOrServer.DAHost)] internal class NetworkVariableTraitsTests : NetcodeIntegrationTest { - protected override int NumberOfClients => 2; + protected override int NumberOfClients => 3; - protected override bool m_EnableTimeTravel => true; - protected override bool m_SetupIsACoroutine => false; - protected override bool m_TearDownIsACoroutine => false; + private StringBuilder m_ErrorLog = new StringBuilder(); + + public NetworkVariableTraitsTests(HostOrServer hostOrServer) : base(hostOrServer) { } protected override void OnPlayerPrefabGameObjectCreated() { m_PlayerPrefab.AddComponent(); } - public NetworkVariableTraitsComponent GetTestComponent() + public NetworkVariableTraitsComponent GetAuthorityComponent() { - return m_ClientNetworkManagers[0].LocalClient.PlayerObject.GetComponent(); + return GetAuthorityNetworkManager().LocalClient.PlayerObject.GetComponent(); } - public NetworkVariableTraitsComponent GetServerComponent() + private bool AllAuthorityInstanceValuesMatch(float firstValue, float secondValue = 0.0f) { - foreach (var obj in Object.FindObjectsByType(FindObjectsSortMode.None)) + m_ErrorLog.Clear(); + var authorityComponent = GetAuthorityComponent(); + if (authorityComponent.TheVariable.Value != firstValue || authorityComponent.AnotherVariable.Value != secondValue) + { + m_ErrorLog.Append($"[Client-{authorityComponent.OwnerClientId}][{authorityComponent.name}] Authority values did not match ({firstValue} | {secondValue})! " + + $"TheVariable: {authorityComponent.TheVariable.Value} | AnotherVariable: {authorityComponent.AnotherVariable.Value}"); + return false; + } + foreach (var client in m_ClientNetworkManagers) { - if (obj.NetworkManager == m_ServerNetworkManager && obj.OwnerClientId == m_ClientNetworkManagers[0].LocalClientId) + if (client.LocalClient.IsSessionOwner) + { + continue; + } + if (!client.SpawnManager.SpawnedObjects.ContainsKey(authorityComponent.NetworkObjectId)) + { + m_ErrorLog.Append($"Failed to find {authorityComponent.name} instance on Client-{client.LocalClientId}!"); + return false; + } + var testComponent = client.SpawnManager.SpawnedObjects[authorityComponent.NetworkObjectId].GetComponent(); + if (testComponent.TheVariable.Value != firstValue || testComponent.AnotherVariable.Value != secondValue) { - return obj; + m_ErrorLog.Append($"[Client-{client.LocalClientId}][{testComponent.name}] Authority values did not match ({firstValue} | {secondValue})! " + + $"TheVariable: {testComponent.TheVariable.Value} | AnotherVariable: {testComponent.AnotherVariable.Value}"); + return false; } } - - return null; + return true; } - [Test] - public void WhenNewValueIsLessThanThreshold_VariableIsNotSerialized() + [UnityTest] + public IEnumerator WhenNewValueIsLessThanThreshold_VariableIsNotSerialized() { - var serverComponent = GetServerComponent(); - var testComponent = GetTestComponent(); - serverComponent.TheVariable.CheckExceedsDirtinessThreshold = (in float value, in float newValue) => Mathf.Abs(newValue - value) >= 0.1; - - serverComponent.TheVariable.Value = 0.05f; - - TimeTravel(2, 120); - - Assert.AreEqual(0.05f, serverComponent.TheVariable.Value); ; - Assert.AreEqual(0, testComponent.TheVariable.Value); ; + var authorityComponent = GetAuthorityComponent(); + authorityComponent.TheVariable.CheckExceedsDirtinessThreshold = (in float value, in float newValue) => Mathf.Abs(newValue - value) >= 0.1; + + var timeoutHelper = new TimeoutHelper(1.0f); + var newValue = 0.05f; + authorityComponent.TheVariable.Value = newValue; + yield return WaitForConditionOrTimeOut(() => AllAuthorityInstanceValuesMatch(newValue), timeoutHelper); + Assert.True(timeoutHelper.TimedOut, $"Non-authority instances recieved changes when they should not have!"); } - [Test] - public void WhenNewValueIsGreaterThanThreshold_VariableIsSerialized() - { - var serverComponent = GetServerComponent(); - var testComponent = GetTestComponent(); - serverComponent.TheVariable.CheckExceedsDirtinessThreshold = (in float value, in float newValue) => Mathf.Abs(newValue - value) >= 0.1; - serverComponent.TheVariable.Value = 0.15f; - - TimeTravel(2, 120); - - Assert.AreEqual(0.15f, serverComponent.TheVariable.Value); ; - Assert.AreEqual(0.15f, testComponent.TheVariable.Value); ; + [UnityTest] + public IEnumerator WhenNewValueIsGreaterThanThreshold_VariableIsSerialized() + { + var authorityComponent = GetAuthorityComponent(); + authorityComponent.TheVariable.CheckExceedsDirtinessThreshold = (in float value, in float newValue) => Mathf.Abs(newValue - value) >= 0.1; + + var timeoutHelper = new TimeoutHelper(1.0f); + var newValue = 0.15f; + authorityComponent.TheVariable.Value = newValue; + yield return WaitForConditionOrTimeOut(() => AllAuthorityInstanceValuesMatch(newValue), timeoutHelper); + AssertOnTimeout($"{m_ErrorLog}", timeoutHelper); } - [Test] - public void WhenNewValueIsLessThanThresholdButMaxTimeHasPassed_VariableIsSerialized() + [UnityTest] + public IEnumerator WhenNewValueIsLessThanThresholdButMaxTimeHasPassed_VariableIsSerialized() { - var serverComponent = GetServerComponent(); - var testComponent = GetTestComponent(); - serverComponent.TheVariable.CheckExceedsDirtinessThreshold = (in float value, in float newValue) => Mathf.Abs(newValue - value) >= 0.1; - serverComponent.TheVariable.SetUpdateTraits(new NetworkVariableUpdateTraits { MaxSecondsBetweenUpdates = 2 }); - serverComponent.TheVariable.LastUpdateSent = m_ServerNetworkManager.NetworkTimeSystem.LocalTime; - - serverComponent.TheVariable.Value = 0.05f; - - TimeTravel(1 / 60f * 119, 119); - - Assert.AreEqual(0.05f, serverComponent.TheVariable.Value); ; - Assert.AreEqual(0, testComponent.TheVariable.Value); ; - - TimeTravel(1 / 60f * 4, 4); - - Assert.AreEqual(0.05f, serverComponent.TheVariable.Value); ; - Assert.AreEqual(0.05f, testComponent.TheVariable.Value); ; + var authorityComponent = GetAuthorityComponent(); + authorityComponent.TheVariable.CheckExceedsDirtinessThreshold = (in float value, in float newValue) => Mathf.Abs(newValue - value) >= 0.1; + authorityComponent.TheVariable.SetUpdateTraits(new NetworkVariableUpdateTraits { MaxSecondsBetweenUpdates = 1.0f }); + authorityComponent.TheVariable.LastUpdateSent = authorityComponent.NetworkManager.NetworkTimeSystem.LocalTime; + + var timeoutHelper = new TimeoutHelper(0.62f); + var newValue = 0.05f; + authorityComponent.TheVariable.Value = newValue; + // We expect a timeout for this condition + yield return WaitForConditionOrTimeOut(() => AllAuthorityInstanceValuesMatch(newValue), timeoutHelper); + Assert.True(timeoutHelper.TimedOut, $"Non-authority instances recieved changes when they should not have!"); + + // Now we expect this to not timeout + yield return WaitForConditionOrTimeOut(() => AllAuthorityInstanceValuesMatch(newValue), timeoutHelper); + AssertOnTimeout($"{m_ErrorLog}", timeoutHelper); } - [Test] - public void WhenNewValueIsGreaterThanThresholdButMinTimeHasNotPassed_VariableIsNotSerialized() + [UnityTest] + public IEnumerator WhenNewValueIsGreaterThanThresholdButMinTimeHasNotPassed_VariableIsNotSerialized() { - var serverComponent = GetServerComponent(); - var testComponent = GetTestComponent(); - serverComponent.TheVariable.CheckExceedsDirtinessThreshold = (in float value, in float newValue) => Mathf.Abs(newValue - value) >= 0.1; - serverComponent.TheVariable.SetUpdateTraits(new NetworkVariableUpdateTraits { MinSecondsBetweenUpdates = 2 }); - serverComponent.TheVariable.LastUpdateSent = m_ServerNetworkManager.NetworkTimeSystem.LocalTime; - - serverComponent.TheVariable.Value = 0.15f; - - TimeTravel(1 / 60f * 119, 119); - - Assert.AreEqual(0.15f, serverComponent.TheVariable.Value); ; - Assert.AreEqual(0, testComponent.TheVariable.Value); ; - - TimeTravel(1 / 60f * 4, 4); - - Assert.AreEqual(0.15f, serverComponent.TheVariable.Value); ; - Assert.AreEqual(0.15f, testComponent.TheVariable.Value); ; + var authorityComponent = GetAuthorityComponent(); + authorityComponent.TheVariable.CheckExceedsDirtinessThreshold = (in float value, in float newValue) => Mathf.Abs(newValue - value) >= 0.1; + authorityComponent.TheVariable.SetUpdateTraits(new NetworkVariableUpdateTraits { MinSecondsBetweenUpdates = 1 }); + authorityComponent.TheVariable.LastUpdateSent = authorityComponent.NetworkManager.NetworkTimeSystem.LocalTime; + + var timeoutHelper = new TimeoutHelper(0.62f); + var newValue = 0.15f; + authorityComponent.TheVariable.Value = newValue; + // We expect a timeout for this condition + yield return WaitForConditionOrTimeOut(() => AllAuthorityInstanceValuesMatch(newValue), timeoutHelper); + Assert.True(timeoutHelper.TimedOut, $"Non-authority instances recieved changes when they should not have!"); + + // Now we expect this to not timeout + yield return WaitForConditionOrTimeOut(() => AllAuthorityInstanceValuesMatch(newValue), timeoutHelper); + AssertOnTimeout($"{m_ErrorLog}", timeoutHelper); } - [Test] - public void WhenNoThresholdIsSetButMinTimeHasNotPassed_VariableIsNotSerialized() + [UnityTest] + public IEnumerator WhenNoThresholdIsSetButMinTimeHasNotPassed_VariableIsNotSerialized() { - var serverComponent = GetServerComponent(); - var testComponent = GetTestComponent(); - serverComponent.TheVariable.SetUpdateTraits(new NetworkVariableUpdateTraits { MinSecondsBetweenUpdates = 2 }); - serverComponent.TheVariable.LastUpdateSent = m_ServerNetworkManager.NetworkTimeSystem.LocalTime; - - serverComponent.TheVariable.Value = 0.15f; - - TimeTravel(1 / 60f * 119, 119); - - Assert.AreEqual(0.15f, serverComponent.TheVariable.Value); ; - Assert.AreEqual(0, testComponent.TheVariable.Value); ; - - TimeTravel(1 / 60f * 4, 4); + var authorityComponent = GetAuthorityComponent(); + authorityComponent.TheVariable.SetUpdateTraits(new NetworkVariableUpdateTraits { MinSecondsBetweenUpdates = 1 }); + authorityComponent.TheVariable.LastUpdateSent = authorityComponent.NetworkManager.NetworkTimeSystem.LocalTime; + + var timeoutHelper = new TimeoutHelper(0.62f); + var newValue = 0.15f; + authorityComponent.TheVariable.Value = newValue; + // We expect a timeout for this condition + yield return WaitForConditionOrTimeOut(() => AllAuthorityInstanceValuesMatch(newValue), timeoutHelper); + Assert.True(timeoutHelper.TimedOut, $"Non-authority instances recieved changes when they should not have!"); + + // Now we expect this to not timeout + yield return WaitForConditionOrTimeOut(() => AllAuthorityInstanceValuesMatch(newValue), timeoutHelper); + AssertOnTimeout($"{m_ErrorLog}", timeoutHelper); + } - Assert.AreEqual(0.15f, serverComponent.TheVariable.Value); ; - Assert.AreEqual(0.15f, testComponent.TheVariable.Value); ; + /// + /// Integration test to validate that a with + /// does not cause other s to miss an update when they are dirty but the one with + /// traits is not ready to send an update. + /// + [UnityTest] + public IEnumerator WhenNonTraitsIsDirtyButTraitsIsNotReadyToSend() + { + var authorityComponent = GetAuthorityComponent(); + authorityComponent.TheVariable.SetUpdateTraits(new NetworkVariableUpdateTraits { MinSecondsBetweenUpdates = 1 }); + authorityComponent.TheVariable.LastUpdateSent = authorityComponent.NetworkManager.NetworkTimeSystem.LocalTime; + + var timeoutHelper = new TimeoutHelper(0.62f); + var firstValue = 0.15f; + var secondValue = 0.15f; + authorityComponent.TheVariable.Value = firstValue; + // We expect a timeout for this condition + yield return WaitForConditionOrTimeOut(() => AllAuthorityInstanceValuesMatch(firstValue, secondValue), timeoutHelper); + Assert.True(timeoutHelper.TimedOut, $"Non-authority instances recieved changes when they should not have!"); + + secondValue = 1.5f; + authorityComponent.AnotherVariable.Value = secondValue; + // Now we expect this to not timeout + yield return WaitForConditionOrTimeOut(() => AllAuthorityInstanceValuesMatch(firstValue, secondValue), timeoutHelper); + AssertOnTimeout($"{m_ErrorLog}", timeoutHelper); } } }