fix: Fix a bug where NetworkTime initialized with wrong tick

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -20,6 +20,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 - Fixed NetworkVariables containing more than 1300 bytes of data (such as large NetworkLists) no longer cause an OverflowException (the limit on data size is now whatever limit the chosen transport imposes on fragmented NetworkDelivery mechanisms) (#1481)
 - Fixed error when serializing ConnectionApprovalMessage with scene management disabled when one or more objects is hidden via the CheckObjectVisibility delegate (#1509)
 - Fixed The NetworkConfig's checksum hash includes the NetworkTick so that clients with a different tickrate than the server are identified and not allowed to connect. (#1513)
+- Fixed a bug where when constructing a NetworkTime from a tick the resulting instance sometimes had a different tick value due to floating point math imprecision.(#1561)
 
 ### Changed
 

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

@@ -11,33 +11,32 @@ namespace Unity.Netcode
     /// </summary>
     public struct NetworkTime
     {
-        private double m_TimeSec;
+        private double m_TickOffset;
+        private int m_Tick;
+        private double m_CachedTime;
 
         private uint m_TickRate;
         private double m_TickInterval;
 
-        private int m_CachedTick;
-        private double m_CachedTickOffset;
-
         /// <summary>
         /// Gets the amount of time which has passed since the last network tick.
         /// </summary>
-        public double TickOffset => m_CachedTickOffset;
+        public double TickOffset => m_TickOffset;
 
         /// <summary>
         /// Gets the current time. This is a non fixed time value and similar to <see cref="Time.time"/>
         /// </summary>
-        public double Time => m_TimeSec;
+        public double Time => m_CachedTime;
 
         /// <summary>
         /// Gets the current time as a float.
         /// </summary>
-        public float TimeAsFloat => (float)m_TimeSec;
+        public float TimeAsFloat => (float)m_CachedTime;
 
         /// <summary>
         /// Gets he current fixed network time. This is the time value of the last network tick. Similar to <see cref="Time.fixedTime"/>
         /// </summary>
-        public double FixedTime => m_CachedTick * m_TickInterval;
+        public double FixedTime => m_Tick * m_TickInterval;
 
         /// <summary>
         /// Gets the fixed delta time. This value is based on the <see cref="TickRate"/> and stays constant.
@@ -48,7 +47,7 @@ public struct NetworkTime
         /// <summary>
         /// Gets the amount of network ticks which have passed until reaching the current time value.
         /// </summary>
-        public int Tick => m_CachedTick;
+        public int Tick => m_Tick;
 
         /// <summary>
         /// Gets the tickrate of the system of this <see cref="NetworkTime"/>.
@@ -64,11 +63,12 @@ public NetworkTime(uint tickRate)
         {
             Assert.IsTrue(tickRate > 0, "Tickrate must be a positive value.");
 
+            m_CachedTime = 0;
+            m_Tick = 0;
+            m_TickOffset = 0;
+
             m_TickRate = tickRate;
             m_TickInterval = 1f / m_TickRate; // potential floating point precision issue, could result in different interval on different machines
-            m_CachedTickOffset = 0;
-            m_CachedTick = 0;
-            m_TimeSec = 0;
         }
 
         /// <summary>
@@ -80,8 +80,9 @@ public NetworkTime(uint tickRate)
         public NetworkTime(uint tickRate, int tick, double tickOffset = 0d)
             : this(tickRate)
         {
-            Assert.IsTrue(tickOffset < 1d / tickRate);
-            this += tick * m_TickInterval + tickOffset;
+            m_Tick = tick;
+            m_TickOffset = tickOffset;
+            Recalculate();
         }
 
         /// <summary>
@@ -102,42 +103,53 @@ public NetworkTime(uint tickRate, double timeSec)
         /// <returns>A <see cref="NetworkTime"/> where Time is the FixedTime value of this instance.</returns>
         public NetworkTime ToFixedTime()
         {
-            return new NetworkTime(m_TickRate, m_CachedTick);
+            return new NetworkTime(m_TickRate, m_Tick);
         }
 
         public NetworkTime TimeTicksAgo(int ticks)
         {
             return this - new NetworkTime(TickRate, ticks);
         }
 
-        private void UpdateCache()
+        /// <summary>
+        /// Converts excess tick offset into ticks and recalculates the cached time value.
+        /// </summary>
+        private void Recalculate()
         {
-            double d = m_TimeSec / m_TickInterval;
-            m_CachedTick = (int)d;
-            m_CachedTickOffset = ((d - Math.Truncate(d)) * m_TickInterval);
+            if (m_TickOffset >= m_TickInterval || m_TickOffset <= m_TickInterval)
+            {
+                double d = m_TickOffset / m_TickInterval;
+                m_Tick += (int)d;
+                m_TickOffset = ((d - Math.Truncate(d)) * m_TickInterval);
+            }
 
-            // This handles negative time, decreases tick by 1 and makes offset positive.
-            if (m_CachedTick < 0 && m_CachedTickOffset != 0d)
+            // This handles negative time offset, decreases tick by 1 and makes offset positive.
+            if (m_TickOffset < 0d)
             {
-                m_CachedTick--;
-                m_CachedTickOffset = m_TickInterval + m_CachedTickOffset;
+                m_Tick--;
+                m_TickOffset += m_TickInterval;
+                Assert.IsTrue(m_TickOffset < m_TickInterval);
             }
+
+            m_CachedTime = m_Tick * m_TickInterval + m_TickOffset;
         }
 
         public static NetworkTime operator -(NetworkTime a, NetworkTime b)
         {
-            return new NetworkTime(a.TickRate, a.Time - b.Time);
+            Assert.AreEqual(a.TickRate, b.TickRate, $"Can only subtract two {nameof(NetworkTime)} with equal {nameof(TickRate)}");
+            return new NetworkTime(a.TickRate, a.Tick - b.Tick, a.TickOffset - b.TickOffset);
         }
 
         public static NetworkTime operator +(NetworkTime a, NetworkTime b)
         {
-            return new NetworkTime(a.TickRate, a.Time + b.Time);
+            Assert.AreEqual(a.TickRate, b.TickRate, $"Can only add two {nameof(NetworkTime)} with equal {nameof(TickRate)}");
+            return new NetworkTime(a.TickRate, a.Tick + b.Tick, a.TickOffset + b.TickOffset);
         }
 
         public static NetworkTime operator +(NetworkTime a, double b)
         {
-            a.m_TimeSec += b;
-            a.UpdateCache();
+            a.m_TickOffset += b;
+            a.Recalculate();
             return a;
         }
 

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

@@ -28,9 +28,9 @@ public void TestFailCreateInvalidTime(double time, uint tickrate)
         [TestCase(-4301d, -4301f, 20u)]
         [TestCase(-4301d, -4301f, 30u)]
         [TestCase(-4301d, -4301f, 60u)]
-        [TestCase(float.MaxValue, float.MaxValue, 20u)]
-        [TestCase(float.MaxValue, float.MaxValue, 30u)]
-        [TestCase(float.MaxValue, float.MaxValue, 60u)]
+        [TestCase(int.MaxValue / 21d, int.MaxValue / 21f, 20u)]
+        [TestCase(int.MaxValue / 31d, int.MaxValue / 31f, 30u)]
+        [TestCase(int.MaxValue / 61d, int.MaxValue / 61f, 60u)]
         public void TestTimeAsFloat(double d, float f, uint tickRate)
         {
             var networkTime = new NetworkTime(tickRate, d);
@@ -141,6 +141,16 @@ public void NetworkTimeSubNetworkTimeTest(double time)
             Assert.IsTrue(Approximately(floatResultB, timeB.Time));
         }
 
+        [Test]
+        public void NetworkTimeCorrectTick([Values(30u, 60u, 1u, 100u)] uint tickRate)
+        {
+            for (int i = 0; i < 10000; i++)
+            {
+                var time = new NetworkTime(tickRate, i);
+                Assert.AreEqual(i, time.Tick);
+            }
+        }
+
         [Test]
         public void NetworkTimeAdvanceTest()
         {

com.unity.netcode.gameobjects/Tests/Runtime/Timing/NetworkTimeSystemTests.cs

@@ -51,6 +51,30 @@ public IEnumerator CorrectAmountTicksTest()
                 Assert.AreEqual(Math.Floor((tickSystem.ServerTime.Time / delta)), NetworkManager.Singleton.ServerTime.Tick);
                 Assert.True(Mathf.Approximately((float)NetworkManager.Singleton.LocalTime.Time, (float)NetworkManager.Singleton.ServerTime.Time));
             }
+
+            Assert.AreNotEqual(0, tickSystem.LocalTime.Tick);
+        }
+
+        /// <summary>
+        /// Tests whether the ticks invoked by the time system consistently increments it's tick value by 1 during each invocation of the tick event.
+        /// </summary>
+        /// <returns></returns>
+        [UnityTest]
+        public IEnumerator CorrectTickIncrementationTest()
+        {
+            var tickSystem = NetworkManager.Singleton.NetworkTickSystem;
+
+            var ticks = tickSystem.LocalTime.Tick;
+            tickSystem.Tick += () =>
+            {
+                ticks++;
+                Assert.AreEqual(ticks, tickSystem.LocalTime.Tick);
+            };
+
+            while (tickSystem.LocalTime.Time < 3f)
+            {
+                yield return null;
+            }
         }
 
         [TearDown]