Merge develop-3.x.x into chore/unified-hybrid-tests-enabled-environment-var

Author: netcode-ci-service

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -10,7 +10,6 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Added
 
-
 ### Changed
 
 - All editor assembly definitions are renamed with `Unity.Netcode.GameObjects.x` variants
@@ -21,21 +20,19 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Deprecated
 
-
 ### Removed
 
-
 ### Fixed
 
 - Fixed issue where scenes additively loaded before a session started were tracked as loaded on the server but had no scene handle entries, which caused `NetworkSceneManager.UnloadScene` to log an error and leave the scene registered as loaded even though it unloaded on all peers. (#4146)
+- Issue where `NetworkTransform` interpolated towards a point in time taken from the local clock rather than the server clock that state updates are stamped on, which starved the interpolator on clients and reduced interpolation to snapping between state updates. (#4135)
+- Issue where `NetworkTransform.GetTickLatencyInSeconds` returned an absolute network timestamp that grew for as long as the session ran, rather than the tick latency as a duration in seconds that it is documented to return. (#4135)
 - Issue with not being able to spawn initially disabled in-scene placed objects. (#4093)
 - Issue with pre-instantiated network prefab instances being marked as in-scene placed. Now pre-instantiated network prefabs are dynamically spawned. (#4093)
 - Issue where a user could spawn runtime created `NetworkObject` that has a GlobalObjectIdHash of zero. These are not valid instances and will no longer be allowed to spawn. (#4093)
 
-
 ### Security
 
-
 ### Obsolete
 
 

com.unity.netcode.gameobjects/Runtime/Components/NetworkTransform.cs

@@ -4259,14 +4259,13 @@ internal BufferedLinearInterpolatorQuaternion GetRotationInterpolator()
         // Non-Authority
         private void UpdateInterpolation()
         {
-            // Use the local time because:
-            // Client-Server:
-            // Local time is server time on a host or server.
-            // Local time on clients takes latency into consideration.
-            // Distributed authority:
-            // Local time is used by the authority.
-            // Local time on non-authority takes latency into consid]eration.
-            var timeSystem = m_CachedNetworkManager.LocalTime;
+            // Use the server time, since that is the clock the states being interpolated between are stamped on
+            // (a state's SentTime is derived from its NetworkTick). Deriving the render time from LocalTime
+            // subtracts the tick latency from a clock that already leads ServerTime by roughly that much, which
+            // leaves the render time at or ahead of the newest state that can exist and starves the interpolator.
+            // Measuring from ServerTime is also self correcting, as the tick latency grows with the round trip
+            // time. This is a no-op on a host or server, where both clocks are the same.
+            var timeSystem = m_CachedNetworkManager.ServerTime;
             var currentTime = timeSystem.Time;
 #if COM_UNITY_MODULES_PHYSICS || COM_UNITY_MODULES_PHYSICS2D
             var cachedDeltaTime = m_UseRigidbodyForMotion ? m_CachedNetworkManager.RealTimeProvider.FixedDeltaTime : m_CachedNetworkManager.RealTimeProvider.DeltaTime;
@@ -4730,7 +4729,10 @@ internal static float GetTickLatencyInSeconds(NetworkManager networkManager)
         {
             if (networkManager.IsListening)
             {
-                return (float)networkManager.LocalTime.TimeTicksAgo(networkManager.NetworkTimeSystem.TickLatency + InterpolationBufferTickOffset).Time;
+                // The number of ticks the interpolators run behind, as a duration. This is not a point in time:
+                // it does not grow as the session runs.
+                var ticksBehind = networkManager.NetworkTimeSystem.TickLatency + InterpolationBufferTickOffset;
+                return (float)(ticksBehind * networkManager.ServerTime.FixedDeltaTimeAsDouble);
             }
             return 0f;
         }

com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformInterpolationRenderTimeTests.cs

@@ -0,0 +1,147 @@
+using System.Collections;
+using NUnit.Framework;
+using Unity.Netcode.Components;
+using Unity.Netcode.TestHelpers.Runtime;
+using UnityEngine;
+using UnityEngine.TestTools;
+
+namespace Unity.Netcode.RuntimeTests
+{
+    /// <summary>
+    /// Validates that the render time a non-authority instance interpolates towards is derived from the same
+    /// clock that the state updates it is interpolating between are stamped on.
+    /// </summary>
+    /// <remarks>
+    /// Measures how far behind ServerTime the state being interpolated towards was sent. The render time is
+    /// ServerTime minus the tick latency and only states sent at or before it are eligible, so that measurement
+    /// can never be less than the tick latency. Deriving the render time from LocalTime eats into that margin by
+    /// however far the two clocks are apart, and can push the target past ServerTime entirely.
+    /// </remarks>
+    [TestFixture(NetworkTransform.InterpolationTypes.Lerp)]
+    [TestFixture(NetworkTransform.InterpolationTypes.SmoothDampening)]
+    internal class NetworkTransformInterpolationRenderTimeTests : IntegrationTestWithApproximation
+    {
+        protected override int NumberOfClients => 1;
+
+        // How far LocalTime is pushed ahead of ServerTime, in ticks. An in-process test has no round trip time
+        // to separate the two clocks, and this is large enough to exceed NetworkTimeSystem's hard reset
+        // threshold so the offset snaps instead of converging at its default adjustment ratio.
+        private const int k_LocalBufferTicks = 12;
+
+        // The separation the clocks must actually reach before any measurement is taken.
+        private const double k_RequiredLeadTicks = 8.0d;
+
+        // Ticks of authority motion after the clocks have separated, so the interpolator reaches steady state.
+        private const int k_WarmUpTicks = 20;
+
+        private const int k_SampledFrames = 90;
+
+        // Far enough each tick that every tick produces a state update rather than being filtered out by the
+        // position threshold.
+        private const float k_DistancePerTick = 1.37f;
+
+        private readonly NetworkTransform.InterpolationTypes m_InterpolationType;
+
+        private GameObject m_TestPrefab;
+        private NetworkManager m_AuthorityNetworkManager;
+        private NetworkTransform m_AuthorityInstance;
+        private Vector3 m_Direction;
+
+        public NetworkTransformInterpolationRenderTimeTests(NetworkTransform.InterpolationTypes interpolationType)
+        {
+            m_InterpolationType = interpolationType;
+        }
+
+        protected override void OnServerAndClientsCreated()
+        {
+            m_TestPrefab = CreateNetworkObjectPrefab("RenderTimeTestObj");
+            var networkTransform = m_TestPrefab.AddComponent<NetworkTransform>();
+            networkTransform.PositionInterpolationType = m_InterpolationType;
+            base.OnServerAndClientsCreated();
+        }
+
+        private static double GetTickInterval(NetworkManager networkManager)
+        {
+            return 1.0d / networkManager.NetworkTickSystem.TickRate;
+        }
+
+        /// <summary>
+        /// How far LocalTime currently leads ServerTime, expressed in ticks.
+        /// </summary>
+        private static double GetClockLeadInTicks(NetworkManager networkManager)
+        {
+            return (networkManager.LocalTime.Time - networkManager.ServerTime.Time) / GetTickInterval(networkManager);
+        }
+
+        /// <summary>
+        /// Moves the authority instance once per tick so that a state update is generated every tick.
+        /// </summary>
+        private void OnNetworkTick()
+        {
+            m_AuthorityInstance.transform.position += m_Direction * k_DistancePerTick;
+        }
+
+        [UnityTest]
+        public IEnumerator RenderTimeTrailsTheServerClock()
+        {
+            m_AuthorityNetworkManager = GetAuthorityNetworkManager();
+            m_AuthorityInstance = SpawnObject(m_TestPrefab, m_AuthorityNetworkManager).GetComponent<NetworkTransform>();
+
+            yield return WaitForSpawnedOnAllOrTimeOut(m_AuthorityInstance.NetworkObject);
+            AssertOnTimeout($"Not all clients spawned {m_AuthorityInstance.name}!");
+
+            var nonAuthority = GetNonAuthorityNetworkManager();
+            var nonAuthorityInstance = nonAuthority.SpawnManager.SpawnedObjects[m_AuthorityInstance.NetworkObject.NetworkObjectId].GetComponent<NetworkTransform>();
+
+            // Separate the two clocks by a known amount so that which one the render time is derived from is
+            // actually distinguishable.
+            nonAuthority.NetworkTimeSystem.LocalBufferSec = k_LocalBufferTicks * GetTickInterval(nonAuthority);
+
+            // Start continuous motion on the authority.
+            m_Direction = GetRandomVector3(-10, 10).normalized;
+            m_AuthorityNetworkManager.NetworkTickSystem.Tick += OnNetworkTick;
+
+            // The offset only moves when the client next receives a time sync, so wait for the separation to
+            // actually take hold rather than assuming it has.
+            yield return WaitForConditionOrTimeOut(() => GetClockLeadInTicks(nonAuthority) >= k_RequiredLeadTicks);
+            AssertOnTimeout($"The nonAuthority clock never fell {k_RequiredLeadTicks} ticks behind, so this test " +
+                "cannot tell the two clocks apart and would pass regardless of which one is used.");
+
+            // Let the interpolator settle at the new separation before measuring.
+            yield return WaitForTicks(m_AuthorityNetworkManager, k_WarmUpTicks);
+
+            // Sample how far behind ServerTime the state being interpolated towards was sent.
+            var interpolator = nonAuthorityInstance.GetPositionInterpolator();
+            var totalTargetLagTicks = 0.0d;
+            var totalBuffered = 0;
+            var samples = 0;
+            for (int frame = 0; frame < k_SampledFrames; frame++)
+            {
+                if (interpolator.InterpolateState.Target.HasValue)
+                {
+                    var targetLag = nonAuthority.ServerTime.Time - interpolator.InterpolateState.Target.Value.TimeSent;
+                    totalTargetLagTicks += targetLag / GetTickInterval(nonAuthority);
+                    totalBuffered += interpolator.m_BufferQueue.Count;
+                    samples++;
+                }
+                yield return null;
+            }
+
+            m_AuthorityNetworkManager.NetworkTickSystem.Tick -= OnNetworkTick;
+
+            Assert.Greater(samples, 0, $"{nonAuthorityInstance.name} never had a state to interpolate towards!");
+
+            var meanTargetLagTicks = totalTargetLagTicks / samples;
+            var meanBuffered = totalBuffered / (float)samples;
+            var tickLatency = nonAuthority.NetworkTimeSystem.TickLatency;
+
+            // Anything less than the tick latency means the render time came from a clock that leads the one
+            // the states are stamped on.
+            Assert.GreaterOrEqual(meanTargetLagTicks, tickLatency,
+                $"[{m_InterpolationType}] {nonAuthorityInstance.name} was interpolating towards a state sent " +
+                $"{meanTargetLagTicks:F3} ticks behind the server clock, but the render time is the server " +
+                $"clock minus a tick latency of {tickLatency}, so it should never be less than that. " +
+                $"(clock lead {GetClockLeadInTicks(nonAuthority):F3} ticks, mean buffered {meanBuffered:F3})");
+        }
+    }
+}

com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformInterpolationRenderTimeTests.cs.meta

@@ -0,0 +1,93 @@
+using System.Collections;
+using NUnit.Framework;
+using Unity.Netcode.Components;
+using Unity.Netcode.TestHelpers.Runtime;
+using UnityEngine.TestTools;
+
+namespace Unity.Netcode.RuntimeTests
+{
+    /// <summary>
+    /// Validates that <see cref="NetworkTransform.GetTickLatencyInSeconds()"/> returns what it is documented to
+    /// return: the tick latency as a duration in seconds.
+    /// </summary>
+    /// <remarks>
+    /// It previously returned <c>TimeTicksAgo(...).Time</c>, which is an absolute network timestamp rather than a
+    /// duration, so the value grew for as long as the session ran.
+    /// </remarks>
+    internal class NetworkTransformTickLatencyTests : NetcodeIntegrationTest
+    {
+        protected override int NumberOfClients => 1;
+
+        // Ticks of additional buffering applied part way through the test to confirm the returned duration
+        // tracks the tick latency it is derived from.
+        private const int k_AddedBufferTicks = 3;
+
+        // Seconds of tolerance when comparing against the expected duration.
+        private const float k_Tolerance = 0.0005f;
+
+        // The number of samples taken while the session runs, to confirm the value does not drift with time.
+        private const int k_Samples = 30;
+
+        private int m_OriginalBufferTickOffset;
+
+        protected override IEnumerator OnSetup()
+        {
+            m_OriginalBufferTickOffset = NetworkTransform.InterpolationBufferTickOffset;
+            return base.OnSetup();
+        }
+
+        protected override IEnumerator OnTearDown()
+        {
+            // This is static, so leaving it modified would leak into every test that runs afterwards.
+            NetworkTransform.InterpolationBufferTickOffset = m_OriginalBufferTickOffset;
+            return base.OnTearDown();
+        }
+
+        [UnityTest]
+        public IEnumerator GetTickLatencyInSecondsReturnsADurationNotATimestamp()
+        {
+            var client = GetNonAuthorityNetworkManager();
+            var tickInterval = (float)client.ServerTime.FixedDeltaTimeAsDouble;
+
+            // A timestamp climbs by roughly a second per second, so sample while the session clock advances
+            // and hold the value to only moving when the tick latency it is derived from moves. That latency
+            // is adaptive and can legitimately change mid-run.
+            var previousTicksBehind = -1;
+            var previousValue = 0f;
+            for (int i = 0; i < k_Samples; i++)
+            {
+                var ticksBehind = client.NetworkTimeSystem.TickLatency + NetworkTransform.InterpolationBufferTickOffset;
+                var latency = NetworkTransform.GetTickLatencyInSeconds(client);
+
+                Assert.Greater(latency, 0f, "A latency of zero or less is not a duration this can be measured against.");
+                if (ticksBehind == previousTicksBehind)
+                {
+                    Assert.AreEqual(previousValue, latency, k_Tolerance,
+                        $"The reported latency moved from {previousValue}s to {latency}s while the tick latency " +
+                        $"stayed at {ticksBehind} ticks, so it is tracking elapsed time rather than latency.");
+                }
+
+                previousTicksBehind = ticksBehind;
+                previousValue = latency;
+                yield return null;
+            }
+
+            // Buffering more ticks has to lengthen the reported duration by exactly those ticks.
+            var latencyBefore = client.NetworkTimeSystem.TickLatency;
+            var before = NetworkTransform.GetTickLatencyInSeconds(client);
+            NetworkTransform.InterpolationBufferTickOffset = m_OriginalBufferTickOffset + k_AddedBufferTicks;
+            yield return null;
+
+            var latencyAfter = client.NetworkTimeSystem.TickLatency;
+            var after = NetworkTransform.GetTickLatencyInSeconds(client);
+
+            // The adaptive tick latency may also have moved in between, so only the buffering is held to an
+            // exact figure.
+            var expectedIncrease = (k_AddedBufferTicks + (latencyAfter - latencyBefore)) * tickInterval;
+            Assert.AreEqual(expectedIncrease, after - before, k_Tolerance,
+                $"Adding {k_AddedBufferTicks} ticks of buffering changed the reported latency by " +
+                $"{after - before}s when a tick is {tickInterval}s, so it should have changed by " +
+                $"{expectedIncrease}s (tick latency went from {latencyBefore} to {latencyAfter}).");
+        }
+    }
+}