fix: network transform interpolation perf improvements (#1272)

* AddMeasurement doesn't sort anymore and relies on consumption to keep track of last item to consume.
It'll also give up interpolating beyond a certain amount of bursted incoming netvar changes.
Fixing endtime vs start time comparison, to make sure we don't have floating point approximation errors

* adding doc
updating burst test to adapt to new behaviour
fixing some algo issue

* Better comments

* making sure buffer will always be the same capacity

Co-authored-by: Matt Walsh <[email protected]>

Author: SamuelBellomo

com.unity.netcode.gameobjects/Components/Interpolator/BufferedLinearInterpolator.cs

@@ -37,8 +37,15 @@ private struct BufferedItem
         {
             public T Item;
             public NetworkTime TimeSent;
+
+            public BufferedItem(T item, NetworkTime timeSent)
+            {
+                Item = item;
+                TimeSent = timeSent;
+            }
         }
 
+        private const double k_SmallValue = 9.999999439624929E-11; // copied from Vector3's equal operator
 
         /// <summary>
         /// Override this if you want configurable buffering, right now using ServerTick's own global buffering
@@ -54,8 +61,36 @@ private struct BufferedItem
         private NetworkTime m_EndTimeConsumed;
         private NetworkTime m_StartTimeConsumed;
 
-        private readonly List<BufferedItem> m_Buffer = new List<BufferedItem>();
-        private const int k_BufferSizeLimit = 100;
+        private readonly List<BufferedItem> m_Buffer = new List<BufferedItem>(k_BufferCountLimit);
+
+        // Buffer consumption scenarios
+        // Perfect case consumption
+        // | 1 | 2 | 3 |
+        // | 2 | 3 | 4 | consume 1
+        // | 3 | 4 | 5 | consume 2
+        // | 4 | 5 | 6 | consume 3
+        // | 5 | 6 | 7 | consume 4
+        // jittered case
+        // | 1 | 2 | 3 |
+        // | 2 | 3 |   | consume 1
+        // | 3 |   |   | consume 2
+        // | 4 | 5 | 6 | consume 3
+        // | 5 | 6 | 7 | consume 4
+        // bursted case (assuming max count is 5)
+        // | 1 | 2 | 3 |
+        // | 2 | 3 |   | consume 1
+        // | 3 |   |   | consume 2
+        // |   |   |   | consume 3
+        // |   |   |   |
+        // | 4 | 5 | 6 | 7 | 8 | --> consume all and teleport to last value <8> --> this is the nuclear option, ideally this example would consume 4 and 5
+        // instead of jumping to 8, but since in OnValueChange we don't yet have an updated server time (updated in pre-update) to know which value
+        // we should keep and which we should drop, we don't have enough information to do this. Another thing would be to not have the burst in the first place.
+
+        // Constant absolute value for max buffer count instead of dynamic time based value. This is in case we have very low tick rates, so
+        // that we don't have a very small buffer because of this.
+        private const int k_BufferCountLimit = 100;
+        private BufferedItem m_LastBufferedItemReceived;
+        private int m_NbItemsReceivedThisFrame;
 
         private int m_LifetimeConsumedCount;
 
@@ -66,6 +101,11 @@ internal BufferedLinearInterpolator(NetworkManager manager)
             InterpolatorTimeProxy = new InterpolatorTime(manager);
         }
 
+        internal BufferedLinearInterpolator(IInterpolatorTime proxy)
+        {
+            InterpolatorTimeProxy = proxy;
+        }
+
         public void ResetTo(T targetValue)
         {
             m_LifetimeConsumedCount = 1;
@@ -87,26 +127,38 @@ private void TryConsumeFromBuffer()
             // only consume if we're ready
             if (RenderTime >= m_EndTimeConsumed.Time)
             {
-                // buffer is sorted so older (smaller) time values are at the end.
-                for (int i = m_Buffer.Count - 1; i >= 0; i--) // todo stretch: consume ahead if we see we're missing values
+                BufferedItem? itemToInterpolateTo = null;
+                // assumes we're using sequenced messages for netvar syncing
+                // buffer contains oldest values first, iterating from end to start to remove elements from list while iterating
+                for (int i = m_Buffer.Count - 1; i >= 0; i--) // todo stretch: consume ahead if we see we're missing values due to packet loss
                 {
                     var bufferedValue = m_Buffer[i];
-                    // Consume when ready. This can consume multiple times
+                    // Consume when ready and interpolate to last value we can consume. This can consume multiple values from the buffer
                     if (bufferedValue.TimeSent.Time <= ServerTimeBeingHandledForBuffering)
                     {
-                        if (m_LifetimeConsumedCount == 0)
-                        {
-                            m_StartTimeConsumed = bufferedValue.TimeSent;
-                            m_InterpStartValue = bufferedValue.Item;
-                        }
-                        else if (consumedCount == 0)
+                        if (!itemToInterpolateTo.HasValue || bufferedValue.TimeSent.Time > itemToInterpolateTo.Value.TimeSent.Time)
                         {
-                            m_StartTimeConsumed = m_EndTimeConsumed;
-                            m_InterpStartValue = m_InterpEndValue;
+                            if (m_LifetimeConsumedCount == 0)
+                            {
+                                // if interpolator not initialized, teleport to first value when available
+                                m_StartTimeConsumed = bufferedValue.TimeSent;
+                                m_InterpStartValue = bufferedValue.Item;
+                            }
+                            else if (consumedCount == 0)
+                            {
+                                // Interpolating to new value, end becomes start. We then look in our buffer for a new end.
+                                m_StartTimeConsumed = m_EndTimeConsumed;
+                                m_InterpStartValue = m_InterpEndValue;
+                            }
+
+                            if (bufferedValue.TimeSent.Time > m_EndTimeConsumed.Time)
+                            {
+                                itemToInterpolateTo = bufferedValue;
+                                m_EndTimeConsumed = bufferedValue.TimeSent;
+                                m_InterpEndValue = bufferedValue.Item;
+                            }
                         }
 
-                        m_EndTimeConsumed = bufferedValue.TimeSent;
-                        m_InterpEndValue = bufferedValue.Item;
                         m_Buffer.RemoveAt(i);
                         consumedCount++;
                         m_LifetimeConsumedCount++;
@@ -132,9 +184,9 @@ public T Update(float deltaTime)
             if (m_LifetimeConsumedCount >= 1) // shouldn't interpolate between default values, let's wait to receive data first, should only interpolate between real measurements
             {
                 float t = 1.0f;
-                if (m_EndTimeConsumed.Time != m_StartTimeConsumed.Time)
+                double range = m_EndTimeConsumed.Time - m_StartTimeConsumed.Time;
+                if (range > k_SmallValue)
                 {
-                    double range = m_EndTimeConsumed.Time - m_StartTimeConsumed.Time;
                     t = (float)((RenderTime - m_StartTimeConsumed.Time) / range);
 
                     if (t < 0.0f)
@@ -155,21 +207,31 @@ public T Update(float deltaTime)
                 m_CurrentInterpValue = Interpolate(m_CurrentInterpValue, target, deltaTime / maxInterpTime); // second interpolate to smooth out extrapolation jumps
             }
 
+            m_NbItemsReceivedThisFrame = 0;
             return m_CurrentInterpValue;
         }
 
         public void AddMeasurement(T newMeasurement, NetworkTime sentTime)
         {
-            if (m_Buffer.Count >= k_BufferSizeLimit)
+            m_NbItemsReceivedThisFrame++;
+
+            // This situation can happen after a game is paused. When starting to receive again, the server will have sent a bunch of messages in the meantime
+            // instead of going through thousands of value updates just to get a big teleport, we're giving up on interpolation and teleporting to the latest value
+            if (m_NbItemsReceivedThisFrame > k_BufferCountLimit)
             {
-                Debug.LogWarning("Going over buffer size limit while adding new interpolation values, interpolation buffering isn't consuming fast enough, removing oldest value now.");
-                m_Buffer.RemoveAt(m_Buffer.Count - 1);
+                if (m_LastBufferedItemReceived.TimeSent.Time < sentTime.Time)
+                {
+                    m_LastBufferedItemReceived = new BufferedItem(newMeasurement, sentTime);
+                    ResetTo(newMeasurement);
+                }
+
+                return;
             }
 
             if (sentTime.Time > m_EndTimeConsumed.Time || m_LifetimeConsumedCount == 0) // treat only if value is newer than the one being interpolated to right now
             {
-                m_Buffer.Add(new BufferedItem { Item = newMeasurement, TimeSent = sentTime });
-                m_Buffer.Sort((item1, item2) => item2.TimeSent.Time.CompareTo(item1.TimeSent.Time));
+                m_LastBufferedItemReceived = new BufferedItem(newMeasurement, sentTime);
+                m_Buffer.Add(m_LastBufferedItemReceived);
             }
         }
 
@@ -197,6 +259,10 @@ protected override float Interpolate(float start, float end, float time)
         public BufferedLinearInterpolatorFloat(NetworkManager manager) : base(manager)
         {
         }
+
+        public BufferedLinearInterpolatorFloat(IInterpolatorTime proxy) : base(proxy)
+        {
+        }
     }
 
     internal class BufferedLinearInterpolatorQuaternion : BufferedLinearInterpolator<Quaternion>

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

@@ -52,6 +52,7 @@ public struct NetworkTime
 
         /// <summary>
         /// Gets the tickrate of the system of this <see cref="NetworkTime"/>.
+        /// Ticks per second.
         /// </summary>
         public uint TickRate => m_TickRate;
 

com.unity.netcode.gameobjects/Tests/Editor/InterpolatorTests.cs

@@ -30,9 +30,9 @@ private NetworkTime T(float time, uint tickRate = k_MockTickRate)
         [Test]
         public void TestReset()
         {
-            var interpolator = new BufferedLinearInterpolatorFloat(null);
             var timeMock = new MockInterpolatorTime(0, k_MockTickRate);
-            interpolator.InterpolatorTimeProxy = timeMock;
+            var interpolator = new BufferedLinearInterpolatorFloat(timeMock);
+
             timeMock.BufferedServerTime = 100f;
 
             interpolator.AddMeasurement(5, T(1.0f));
@@ -51,9 +51,8 @@ public void NormalUsage()
         {
             // Testing float instead of Vector3. The only difference with Vector3 is the lerp method used.
 
-            var interpolator = new BufferedLinearInterpolatorFloat(null);
             var mockBufferedTime = new MockInterpolatorTime(0, k_MockTickRate);
-            interpolator.InterpolatorTimeProxy = mockBufferedTime;
+            var interpolator = new BufferedLinearInterpolatorFloat(mockBufferedTime);
 
             Assert.That(interpolator.GetInterpolatedValue(), Is.EqualTo(0f));
 
@@ -97,9 +96,8 @@ public void NormalUsage()
         [Test]
         public void OutOfOrderShouldStillWork()
         {
-            var interpolator = new BufferedLinearInterpolatorFloat(null);
             var mockBufferedTime = new MockInterpolatorTime(0, k_MockTickRate);
-            interpolator.InterpolatorTimeProxy = mockBufferedTime;
+            var interpolator = new BufferedLinearInterpolatorFloat(mockBufferedTime);
 
             interpolator.AddMeasurement(0, T(0f));
             interpolator.AddMeasurement(2, T(2f));
@@ -127,9 +125,8 @@ public void OutOfOrderShouldStillWork()
         [Test]
         public void MessageLoss()
         {
-            var interpolator = new BufferedLinearInterpolatorFloat(null);
             var mockBufferedTime = new MockInterpolatorTime(0, k_MockTickRate);
-            interpolator.InterpolatorTimeProxy = mockBufferedTime;
+            var interpolator = new BufferedLinearInterpolatorFloat(mockBufferedTime);
 
             interpolator.AddMeasurement(1f, T(1f));
             interpolator.AddMeasurement(2f, T(2f));
@@ -139,10 +136,22 @@ public void MessageLoss()
             // message time=6 was lost
             interpolator.AddMeasurement(100f, T(7f)); // high value to produce a misprediction
 
+            // first value teleports interpolator
+            mockBufferedTime.BufferedServerTime = 1f;
+            interpolator.Update(1f);
+            Assert.That(interpolator.GetInterpolatedValue(), Is.EqualTo(1f));
+
+            // nothing happens, not ready to consume second value yet
+            mockBufferedTime.BufferedServerTime = 1.5f;
+            interpolator.Update(0.5f);
+            Assert.That(interpolator.GetInterpolatedValue(), Is.EqualTo(1f));
+
+            // beginning of interpolation, second value consumed, currently at start
             mockBufferedTime.BufferedServerTime = 2f;
-            interpolator.Update(2f);
+            interpolator.Update(0.5f);
             Assert.That(interpolator.GetInterpolatedValue(), Is.EqualTo(1f));
 
+            // interpolation starts
             mockBufferedTime.BufferedServerTime = 2.5f;
             interpolator.Update(0.5f);
             Assert.That(interpolator.GetInterpolatedValue(), Is.EqualTo(1.5f));
@@ -194,9 +203,8 @@ public void MessageLoss()
         [Test]
         public void AddFirstMeasurement()
         {
-            var interpolator = new BufferedLinearInterpolatorFloat(null);
             var mockBufferedTime = new MockInterpolatorTime(0, k_MockTickRate);
-            interpolator.InterpolatorTimeProxy = mockBufferedTime;
+            var interpolator = new BufferedLinearInterpolatorFloat(mockBufferedTime);
 
             interpolator.AddMeasurement(2f, T(1f));
             interpolator.AddMeasurement(3f, T(2f));
@@ -222,9 +230,8 @@ public void AddFirstMeasurement()
         [Test]
         public void JumpToEachValueIfDeltaTimeTooBig()
         {
-            var interpolator = new BufferedLinearInterpolatorFloat(null);
             var mockBufferedTime = new MockInterpolatorTime(0, k_MockTickRate);
-            interpolator.InterpolatorTimeProxy = mockBufferedTime;
+            var interpolator = new BufferedLinearInterpolatorFloat(mockBufferedTime);
 
             interpolator.AddMeasurement(2f, T(1f));
             interpolator.AddMeasurement(3f, T(2f));
@@ -241,14 +248,14 @@ public void JumpToEachValueIfDeltaTimeTooBig()
         [Test]
         public void JumpToLastValueFromStart()
         {
-            var interpolator = new BufferedLinearInterpolatorFloat(null);
             var mockBufferedTime = new MockInterpolatorTime(0, k_MockTickRate);
-            interpolator.InterpolatorTimeProxy = mockBufferedTime;
+            var interpolator = new BufferedLinearInterpolatorFloat(mockBufferedTime);
 
             interpolator.AddMeasurement(1f, T(1f));
             interpolator.AddMeasurement(2f, T(2f));
             interpolator.AddMeasurement(3f, T(3f));
 
+            // big time jump
             mockBufferedTime.BufferedServerTime = 10f;
             var interpolatedValue = interpolator.Update(10f);
             Assert.That(interpolatedValue, Is.EqualTo(3f));
@@ -272,42 +279,32 @@ public void JumpToLastValueFromStart()
         [Test]
         public void TestBufferSizeLimit()
         {
-            var interpolator = new BufferedLinearInterpolatorFloat(null);
             var mockBufferedTime = new MockInterpolatorTime(0, k_MockTickRate);
-            interpolator.InterpolatorTimeProxy = mockBufferedTime;
+            var interpolator = new BufferedLinearInterpolatorFloat(mockBufferedTime);
 
             // set first value
             interpolator.AddMeasurement(-1f, T(1f));
             mockBufferedTime.BufferedServerTime = 1f;
             interpolator.Update(1f);
 
             // max + 1
-            interpolator.AddMeasurement(2, T(2)); // this value should disappear
-            for (int i = 3; i < 103; i++)
+            interpolator.AddMeasurement(2, T(2)); // +1, this should trigger a burst and teleport to last value
+            for (int i = 0; i < 100; i++)
             {
-                interpolator.AddMeasurement(i, T(i));
+                interpolator.AddMeasurement(i + 3, T(i + 3));
             }
 
-            // make sure the first value isn't there anymore and that we're already using the second value
-            // the following shouldn't happen in real life, since the server time should catchup to the real time and not stay behind like this
-            mockBufferedTime.BufferedServerTime = 2f;
-            // if there was no buffer limit, we'd still be consuming the first "1" value
-            var interpolatedValue = interpolator.Update(1f);
-            // but now, we're still at the initial -1f
-            Assert.That(interpolatedValue, Is.EqualTo(-1f));
-
-            // interpolation continues as expected, interpolating between -1 and 3
-            mockBufferedTime.BufferedServerTime = 3f;
-            interpolatedValue = interpolator.Update(1f);
-            Assert.That(interpolatedValue, Is.EqualTo(1f));
+            // client was paused for a while, some time has past, we just got a burst of values from the server that teleported us to the last value received
+            mockBufferedTime.BufferedServerTime = 102;
+            var interpolatedValue = interpolator.Update(101f);
+            Assert.That(interpolatedValue, Is.EqualTo(102));
         }
 
         [Test]
         public void TestUpdatingInterpolatorWithNoData()
         {
-            var interpolator = new BufferedLinearInterpolatorFloat(null);
             var mockBufferedTime = new MockInterpolatorTime(0, k_MockTickRate);
-            interpolator.InterpolatorTimeProxy = mockBufferedTime;
+            var interpolator = new BufferedLinearInterpolatorFloat(mockBufferedTime);
 
             // invalid case, this is undefined behaviour
             Assert.Throws<InvalidOperationException>(() => interpolator.Update(1f));
@@ -316,20 +313,27 @@ public void TestUpdatingInterpolatorWithNoData()
         [Test]
         public void TestDuplicatedValues()
         {
-            var interpolator = new BufferedLinearInterpolatorFloat(null);
             var mockBufferedTime = new MockInterpolatorTime(0, k_MockTickRate);
-            interpolator.InterpolatorTimeProxy = mockBufferedTime;
+            var interpolator = new BufferedLinearInterpolatorFloat(mockBufferedTime);
 
             interpolator.AddMeasurement(1f, T(1f));
             interpolator.AddMeasurement(2f, T(2f));
             interpolator.AddMeasurement(2f, T(2f));
 
-            mockBufferedTime.BufferedServerTime = 2f;
+            // empty interpolator teleports to initial value
+            mockBufferedTime.BufferedServerTime = 1f;
             var interp = interpolator.Update(1f);
             Assert.That(interp, Is.EqualTo(1f));
+
+            // consume value, start interp, currently at start value
+            mockBufferedTime.BufferedServerTime = 2f;
+            interp = interpolator.Update(1f);
+            Assert.That(interp, Is.EqualTo(1f));
+            // interp
             mockBufferedTime.BufferedServerTime = 2.5f;
             interp = interpolator.Update(0.5f);
             Assert.That(interp, Is.EqualTo(1.5f));
+            // reach end
             mockBufferedTime.BufferedServerTime = 3f;
             interp = interpolator.Update(0.5f);
             Assert.That(interp, Is.EqualTo(2f));