fix: new interpolators delaying between stopping and starting (#3413)

This fixes the issue where a server authority instance would lag behind
when using the new interpolators. This was especially visible when the
moving body completely stopped, all state updates were processed, and
then shortly later it began to move again there would be a delay between
the 1st state update and the rest of the pending state updates.

This also fixes an issue where if the frame rate takes longer than the
tick frequency authority instances could invoke the same check for state
changes more than once in the same frame. This is not needed since
changes to the transform are frame driven and the check should only be
performed once per new tick(s).

<!-- Add short version of the JIRA ticket to the PR title (e.g. "feat:
new shiny feature [MTT-123]") -->

## Changelog

- Fixed: Issue where the authority instance of `NetworkTransform` could
check for state updates more than one time in a frame if the frame rate
is slower than the tick frequency.
- Fixed: Issue where the new interpolator types were blocking after the
first consumption of a sequence of buffered state updates.

## Testing and Documentation

- Includes the integration test: `InterpolationStopAndStartMotionTest`.
- No documentation changes or additions were necessary.

<!-- Uncomment and mark items off with a * if this PR deprecates any
API:
### Deprecated API
- [ ] An `[Obsolete]` attribute was added along with a `(RemovedAfter
yyyy-mm-dd)` entry.
- [ ] An [api updater] was added.
- [ ] Deprecation of the API is explained in the CHANGELOG.
- [ ] The users can understand why this API was removed and what they
should use instead.
-->

## Backport

No backport is needed for this update since it pertains to 2.x only
features.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -13,6 +13,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed issue where the authority instance of NetworkTransform could check for state updates more than one time in a frame if the frame rate is greater than the tick frequency. (#3413)
+- Fixed issue where the new interpolator types were blocking after the first consumption of a sequence of buffered state updates. (#3413)
 - Fixed issue where root level in-scene placed `NetworkObject`s would only allow the ownership permission to be no less than distributable or sessionowner. (#3407)
 
 ### Changed

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

@@ -192,6 +192,8 @@ public void Reset(T currentValue)
                 TimeToTargetValue = 0.0f;
                 DeltaTime = 0.0f;
                 m_CurrentDeltaTime = 0.0f;
+                MaxDeltaTime = 0.0f;
+                LastRemainingTime = 0.0f;
             }
         }
 
@@ -292,13 +294,9 @@ private void TryConsumeFromBuffer(double renderTime, double minDeltaTime, double
             var potentialItemNeedsProcessing = false;
 
             // In the event there is nothing left in the queue (i.e. motion/change stopped), we still need to determine if the target has been reached.
-            if (!noStateSet && m_BufferQueue.Count == 0)
+            if (!noStateSet && !InterpolateState.TargetReached)
             {
-                if (!InterpolateState.TargetReached)
-                {
-                    InterpolateState.TargetReached = IsApproximately(InterpolateState.CurrentValue, InterpolateState.Target.Value.Item, GetPrecision());
-                }
-                return;
+                InterpolateState.TargetReached = IsApproximately(InterpolateState.CurrentValue, InterpolateState.Target.Value.Item, GetPrecision());
             }
 
             // Continue to process any remaining state updates in the queue (if any)
@@ -314,14 +312,10 @@ private void TryConsumeFromBuffer(double renderTime, double minDeltaTime, double
                 if (!noStateSet)
                 {
                     potentialItemNeedsProcessing = ((potentialItem.TimeSent <= renderTime) && potentialItem.TimeSent > InterpolateState.Target.Value.TimeSent);
-                    if (!InterpolateState.TargetReached)
-                    {
-                        InterpolateState.TargetReached = IsApproximately(InterpolateState.CurrentValue, InterpolateState.Target.Value.Item, GetPrecision());
-                    }
                 }
 
                 // If we haven't set a target or we have another item that needs processing.
-                if (noStateSet || potentialItemNeedsProcessing)
+                if ((noStateSet && (potentialItem.TimeSent <= renderTime)) || potentialItemNeedsProcessing)
                 {
                     if (m_BufferQueue.TryDequeue(out BufferedItem target))
                     {

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

@@ -935,6 +935,12 @@ public void NetworkSerialize<T>(BufferSerializer<T> serializer) where T : IReade
 
         #region PROPERTIES AND GENERAL METHODS
 
+        /// <summary>
+        /// Used on the authority side only.
+        /// This is the current network tick and is set within <see cref="NetworkManager.NetworkUpdate(NetworkUpdateStage)"/>.
+        /// </summary>
+        internal static int CurrentTick;
+
         /// <summary>
         /// Pertains to Owner Authority and Interpolation<br />
         /// When enabled (default), 1 additional tick is added to the total number of ticks used to calculate the tick latency ("ticks ago") as a time.
@@ -1878,6 +1884,8 @@ private void TryCommitTransform(ref Transform transformToCommit, bool synchroniz
                 // If the state was explicitly set, then update the network tick to match the locally calculate tick
                 if (m_LocalAuthoritativeNetworkState.ExplicitSet)
                 {
+                    // For explicit set, we use the current ServerTime.Tick and not CurrentTick since this is a SetState specific flow
+                    // that is outside of the normal internal tick flow.
                     m_LocalAuthoritativeNetworkState.NetworkTick = m_CachedNetworkManager.NetworkTickSystem.ServerTime.Tick;
                 }
 
@@ -2011,7 +2019,7 @@ private bool CheckForStateChange(ref NetworkTransformState networkState, ref Tra
             // send a full frame synch.
             var isAxisSync = false;
             // We compare against the NetworkTickSystem version since ServerTime is set when updating ticks
-            if (UseUnreliableDeltas && !isSynchronization && m_DeltaSynch && m_NextTickSync <= m_CachedNetworkManager.NetworkTickSystem.ServerTime.Tick)
+            if (UseUnreliableDeltas && !isSynchronization && m_DeltaSynch && m_NextTickSync <= CurrentTick)
             {
                 // Increment to the next frame synch tick position for this instance
                 m_NextTickSync += (int)m_CachedNetworkManager.NetworkConfig.TickRate;
@@ -2179,7 +2187,7 @@ private bool CheckForStateChange(ref NetworkTransformState networkState, ref Tra
             {
                 // If we are teleporting then we can skip the delta threshold check
                 isPositionDirty = networkState.IsTeleportingNextFrame || isAxisSync || forceState;
-                if (m_HalfFloatTargetTickOwnership > m_CachedNetworkManager.ServerTime.Tick)
+                if (m_HalfFloatTargetTickOwnership > CurrentTick)
                 {
                     isPositionDirty = true;
                 }
@@ -2225,7 +2233,7 @@ private bool CheckForStateChange(ref NetworkTransformState networkState, ref Tra
                         networkState.NetworkDeltaPosition = m_HalfPositionState;
 
                         // If ownership offset is greater or we are doing an axial synchronization then synchronize the base position
-                        if ((m_HalfFloatTargetTickOwnership > m_CachedNetworkManager.ServerTime.Tick || isAxisSync) && !networkState.IsTeleportingNextFrame)
+                        if ((m_HalfFloatTargetTickOwnership > CurrentTick || isAxisSync) && !networkState.IsTeleportingNextFrame)
                         {
                             networkState.SynchronizeBaseHalfFloat = true;
                         }
@@ -2409,7 +2417,7 @@ private bool CheckForStateChange(ref NetworkTransformState networkState, ref Tra
                 if (enabled)
                 {
                     // We use the NetworkTickSystem version since ServerTime is set when updating ticks
-                    networkState.NetworkTick = m_CachedNetworkManager.NetworkTickSystem.ServerTime.Tick;
+                    networkState.NetworkTick = CurrentTick;
                 }
             }
 
@@ -2440,7 +2448,7 @@ private void OnNetworkTick(bool isCalledFromParent = false)
                 }
 
                 // If we are nested and have already sent a state update this tick, then exit early (otherwise check for any changes in state)
-                if (IsNested && m_LocalAuthoritativeNetworkState.NetworkTick == m_CachedNetworkManager.ServerTime.Tick)
+                if (IsNested && m_LocalAuthoritativeNetworkState.NetworkTick == CurrentTick)
                 {
                     return;
                 }
@@ -4047,15 +4055,24 @@ public double GetPositionLastRemainingTime()
         {
             return m_PositionInterpolator.InterpolateState.LastRemainingTime;
         }
-#endif
-
+#else
+        internal BufferedLinearInterpolatorVector3 GetPositionInterpolator()
+        {
+            return m_PositionInterpolator;
+        }
 
+        internal BufferedLinearInterpolatorQuaternion GetRotationInterpolator()
+        {
+            return m_RotationInterpolator;
+        }
+#endif
 
         // Non-Authority
         private void UpdateInterpolation()
         {
             AdjustForChangeInTransformSpace();
-            var timeSystem = m_CachedNetworkManager.ServerTime;
+            // Select the time system relative to the type of NetworkManager instance.
+            var timeSystem = m_CachedNetworkManager.IsServer ? m_CachedNetworkManager.ServerTime : m_CachedNetworkManager.LocalTime;
             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;
@@ -4501,6 +4518,15 @@ private void UpdateTransformState()
 
         #region NETWORK TICK REGISTRATOIN AND HANDLING
         private static Dictionary<NetworkManager, NetworkTransformTickRegistration> s_NetworkTickRegistration = new Dictionary<NetworkManager, NetworkTransformTickRegistration>();
+
+        internal static void UpdateNetworkTick(NetworkManager networkManager)
+        {
+            if (s_NetworkTickRegistration.ContainsKey(networkManager))
+            {
+                s_NetworkTickRegistration[networkManager].TickUpdate();
+            }
+        }
+
         /// <summary>
         /// Adjusts the over-all tick offset (i.e. how many ticks ago) and how wide of a maximum delta time will be used for the
         /// various <see cref="InterpolationTypes"/>.
@@ -4562,9 +4588,8 @@ private static void RemoveTickUpdate(NetworkManager networkManager)
         /// Having the tick update once and cycling through registered instances to update is evidently less processor
         /// intensive than having each instance subscribe and update individually.
         /// </summary>
-        private class NetworkTransformTickRegistration
+        internal class NetworkTransformTickRegistration
         {
-            private Action m_NetworkTickUpdate;
             private NetworkManager m_NetworkManager;
             public HashSet<NetworkTransform> NetworkTransforms = new HashSet<NetworkTransform>();
 
@@ -4576,8 +4601,6 @@ private void OnNetworkManagerStopped(bool value)
 
             public void Remove()
             {
-                m_NetworkManager.NetworkTickSystem.Tick -= m_NetworkTickUpdate;
-                m_NetworkTickUpdate = null;
                 NetworkTransforms.Clear();
                 RemoveTickUpdate(m_NetworkManager);
             }
@@ -4586,10 +4609,10 @@ public void Remove()
             /// Invoked once per network tick, this will update any registered
             /// authority instances.
             /// </summary>
-            private void TickUpdate()
+            internal void TickUpdate()
             {
                 // TODO FIX: The local NetworkTickSystem can invoke with the same network tick as before
-                if (m_NetworkManager.ServerTime.Tick <= m_LastTick)
+                if (CurrentTick <= m_LastTick)
                 {
                     return;
                 }
@@ -4600,13 +4623,11 @@ private void TickUpdate()
                         networkTransform.OnNetworkTick();
                     }
                 }
-                m_LastTick = m_NetworkManager.ServerTime.Tick;
+                m_LastTick = CurrentTick;
             }
             public NetworkTransformTickRegistration(NetworkManager networkManager)
             {
                 m_NetworkManager = networkManager;
-                m_NetworkTickUpdate = new Action(TickUpdate);
-                networkManager.NetworkTickSystem.Tick += m_NetworkTickUpdate;
                 if (networkManager.IsServer)
                 {
                     networkManager.OnServerStopped += OnNetworkManagerStopped;

com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs

@@ -9,6 +9,7 @@
 #endif
 using UnityEngine.SceneManagement;
 using Debug = UnityEngine.Debug;
+using Unity.Netcode.Components;
 
 namespace Unity.Netcode
 {
@@ -386,7 +387,19 @@ public void NetworkUpdate(NetworkUpdateStage updateStage)
 #endif
                 case NetworkUpdateStage.PreUpdate:
                     {
+                        var currentTick = ServerTime.Tick;
                         NetworkTimeSystem.UpdateTime();
+                        if (ServerTime.Tick != currentTick)
+                        {
+                            // If we have a lower than expected frame rate and our number of ticks that have passed since the last
+                            // frame is greater than 1, then use the first next tick as opposed to the last tick when checking for
+                            // changes in transform state.
+                            // Note: This is an adjustment from using the NetworkTick event as that can be invoked more than once in
+                            // a single frame under the above condition and since any changes to the transform are frame driven there
+                            // is no need to check for changes to the transform more than once per frame.
+                            NetworkTransform.CurrentTick = (ServerTime.Tick - currentTick) > 1 ? currentTick + 1 : ServerTime.Tick;
+                            NetworkTransform.UpdateNetworkTick(this);
+                        }
                         AnticipationSystem.Update();
                     }
                     break;