fix: more accurate local time sync with server time (up-port) (#3212)

* get a more accurate local time by using half the RTT.

* test

Adjusting ClientNetworkTimeSystemTests to account for half RTT adjustment applied in this PR.

* style

Adding a comment about the change

* update

Adding changelog entry

* style

removing white space after comment

---------

Co-authored-by: CTHULHU\Ben <[email protected]>

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -12,6 +12,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- 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)
 
 ### Changed

com.unity.netcode.gameobjects/Runtime/Timing/NetworkTimeSystem.cs

@@ -248,7 +248,9 @@ public void Sync(double serverTimeSec, double rttSec)
             var timeDif = serverTimeSec - m_TimeSec;
 
             m_DesiredServerTimeOffset = timeDif - ServerBufferSec;
-            m_DesiredLocalTimeOffset = timeDif + rttSec + LocalBufferSec;
+            // We adjust our desired local time offset to be half RTT since the delivery of
+            // the TimeSyncMessage should only take half of the RTT time (legacy was using 1 full RTT)
+            m_DesiredLocalTimeOffset = timeDif + (rttSec * 0.5d) + LocalBufferSec;
         }
     }
 }

com.unity.netcode.gameobjects/Tests/Editor/Timing/ClientNetworkTimeSystemTests.cs

@@ -18,15 +18,16 @@ internal class ClientNetworkTimeSystemTests
         public void StableRttTest()
         {
             double receivedServerTime = 2;
-
-            var timeSystem = new NetworkTimeSystem(0.05d, 0.05d, 0.1d);
+            var baseRtt = 0.1f;
+            var halfRtt = 0.05f;
+            var timeSystem = new NetworkTimeSystem(0.05d, 0.05d, baseRtt);
             timeSystem.Reset(receivedServerTime, 0.15);
             var tickSystem = new NetworkTickSystem(60, timeSystem.LocalTime, timeSystem.ServerTime);
 
             Assert.True(timeSystem.LocalTime > 2);
 
-            var steps = TimingTestHelper.GetRandomTimeSteps(100f, 0.01f, 0.1f, 42);
-            var rttSteps = TimingTestHelper.GetRandomTimeSteps(1000f, 0.095f, 0.105f, 42); // 10ms jitter
+            var steps = TimingTestHelper.GetRandomTimeSteps(100f, 0.01f, baseRtt, 42);
+            var rttSteps = TimingTestHelper.GetRandomTimeSteps(1000f, baseRtt - 0.05f, baseRtt + 0.05f, 42); // 10ms jitter
 
             // run for a while so that we reach regular RTT offset
             TimingTestHelper.ApplySteps(timeSystem, tickSystem, steps, delegate (int step)
@@ -37,10 +38,11 @@ public void StableRttTest()
             });
 
             // check how we close we are to target time.
-            var expectedRtt = 0.1d;
-            var offsetToTarget = (timeSystem.LocalTime - timeSystem.ServerTime) - expectedRtt - timeSystem.ServerBufferSec - timeSystem.LocalBufferSec;
+            var offsetToTarget = (timeSystem.LocalTime - timeSystem.ServerTime) - halfRtt - timeSystem.ServerBufferSec - timeSystem.LocalBufferSec;
             Debug.Log($"offset to target time after running for a while: {offsetToTarget}");
-            Assert.IsTrue(Math.Abs(offsetToTarget) < k_AcceptableRttOffset);
+
+            // server speedup/slowdowns should not be affected by RTT
+            Assert.True(Math.Abs(offsetToTarget) < k_AcceptableRttOffset, $"Expected offset time to be less than {k_AcceptableRttOffset}ms but it was {offsetToTarget}!");
 
             // run again, test that we never need to speed up or slow down under stable RTT
             TimingTestHelper.ApplySteps(timeSystem, tickSystem, steps, delegate (int step)
@@ -51,9 +53,10 @@ public void StableRttTest()
             });
 
             // check again to ensure we are still close to the target
-            var newOffsetToTarget = (timeSystem.LocalTime - timeSystem.ServerTime) - expectedRtt - timeSystem.ServerBufferSec - timeSystem.LocalBufferSec;
+            var newOffsetToTarget = (timeSystem.LocalTime - timeSystem.ServerTime) - halfRtt - timeSystem.ServerBufferSec - timeSystem.LocalBufferSec;
             Debug.Log($"offset to target time after running longer: {newOffsetToTarget}");
-            Assert.IsTrue(Math.Abs(newOffsetToTarget) < k_AcceptableRttOffset);
+            // server speedup/slowdowns should not be affected by RTT
+            Assert.True(Math.Abs(offsetToTarget) < k_AcceptableRttOffset, $"Expected offset time to be less than {k_AcceptableRttOffset}ms but it was {offsetToTarget}!");
 
             // difference between first and second offset should be minimal
             var dif = offsetToTarget - newOffsetToTarget;
@@ -67,13 +70,14 @@ public void StableRttTest()
         public void RttCatchupSlowdownTest()
         {
             double receivedServerTime = 2;
-
-            var timeSystem = new NetworkTimeSystem(0.05d, 0.05d, 0.1d);
+            var baseRtt = 0.1f;
+            var halfRtt = 0.05f;
+            var timeSystem = new NetworkTimeSystem(0.05d, 0.05d, baseRtt);
             timeSystem.Reset(receivedServerTime, 0.15);
             var tickSystem = new NetworkTickSystem(60, timeSystem.LocalTime, timeSystem.ServerTime);
 
-            var steps = TimingTestHelper.GetRandomTimeSteps(100f, 0.01f, 0.1f, 42);
-            var rttSteps = TimingTestHelper.GetRandomTimeSteps(1000f, 0.095f, 0.105f, 42); // 10ms jitter
+            var steps = TimingTestHelper.GetRandomTimeSteps(100f, 0.01f, baseRtt, 42);
+            var rttSteps = TimingTestHelper.GetRandomTimeSteps(1000f, baseRtt - 0.05f, baseRtt + 0.05f, 42); // 10ms jitter
 
             // run for a while so that we reach regular RTT offset
             TimingTestHelper.ApplySteps(timeSystem, tickSystem, steps, delegate (int step)
@@ -102,11 +106,14 @@ public void RttCatchupSlowdownTest()
 
             // speed up of 0.1f expected
             Debug.Log($"Total local speed up time catch up: {totalLocalSpeedUpTime}");
-            Assert.True(Math.Abs(totalLocalSpeedUpTime - 0.1) < k_AcceptableRttOffset);
-            Assert.True(Math.Abs(totalServerSpeedUpTime) < k_AcceptableRttOffset); // server speedup/slowdowns should not be affected by RTT
+            var expectedSpeedUpTime = Math.Abs(totalLocalSpeedUpTime - halfRtt);
+            var expectedServerSpeedUpTime = Math.Abs(totalServerSpeedUpTime);
+            Assert.True(expectedSpeedUpTime < k_AcceptableRttOffset, $"Expected local speed up time to be less than {k_AcceptableRttOffset}ms but it was {expectedSpeedUpTime}!");
+            // server speedup/slowdowns should not be affected by RTT
+            Assert.True(Math.Abs(totalServerSpeedUpTime) < k_AcceptableRttOffset, $"Expected server speed up time to be less than {k_AcceptableRttOffset}ms but it was {expectedServerSpeedUpTime}!");
 
 
-            // run again with RTT ~100ms and see whether we slow down by -0.1f
+            // run again with RTT ~100ms and see whether we slow down by -halfRtt
             unscaledLocalTime = timeSystem.LocalTime;
             unscaledServerTime = timeSystem.ServerTime;
 
@@ -121,13 +128,13 @@ public void RttCatchupSlowdownTest()
 
             totalLocalSpeedUpTime = timeSystem.LocalTime - unscaledLocalTime;
             totalServerSpeedUpTime = timeSystem.ServerTime - unscaledServerTime;
-
-            // slow down of 0.1f expected
+            // slow down of half halfRtt expected
             Debug.Log($"Total local speed up time slow down: {totalLocalSpeedUpTime}");
-            Assert.True(Math.Abs(totalLocalSpeedUpTime + 0.1) < k_AcceptableRttOffset);
-            Assert.True(Math.Abs(totalServerSpeedUpTime) < k_AcceptableRttOffset); // server speedup/slowdowns should not be affected by RTT
-
-
+            expectedSpeedUpTime = Math.Abs(totalLocalSpeedUpTime + halfRtt);
+            expectedServerSpeedUpTime = Math.Abs(totalServerSpeedUpTime);
+            Assert.True(expectedSpeedUpTime < k_AcceptableRttOffset, $"Expected local speed up time to be less than {k_AcceptableRttOffset}ms but it was {expectedSpeedUpTime}!");
+            // server speedup/slowdowns should not be affected by RTT
+            Assert.True(Math.Abs(totalServerSpeedUpTime) < k_AcceptableRttOffset, $"Expected server speed up time to be less than {k_AcceptableRttOffset}ms but it was {expectedServerSpeedUpTime}!");
         }
 
         /// <summary>
@@ -172,8 +179,8 @@ public void ResetTest()
                 receivedServerTime += steps[step];
                 timeSystem.Sync(receivedServerTime, rttSteps2[step]);
 
-                // after hard reset time should stay close to rtt
-                var expectedRtt = 0.5d;
+                // after hard reset time should stay close to half rtt
+                var expectedRtt = 0.25d;
                 Assert.IsTrue(Math.Abs((timeSystem.LocalTime - timeSystem.ServerTime) - expectedRtt - timeSystem.ServerBufferSec - timeSystem.LocalBufferSec) < k_AcceptableRttOffset);
 
             });