fix: NetworkTransform Mixed Unreliable & Reliable Order of Operations

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

@@ -73,10 +73,10 @@ public class NetworkTransform : NetworkBehaviour
         /// second (only for the axis marked to synchronize are sent as reliable fragmented sequenced) as long as a delta state
         /// update had been previously sent. When a NetworkObject is at rest, axial frame synchronization updates are not sent.
         /// </remarks>
-        [Tooltip("When set (enabled by default), NetworkTransform will send common state updates using unreliable network delivery " +
+        [Tooltip("When set, NetworkTransform will send common state updates using unreliable network delivery " +
             "to provide a higher tolerance to poor network conditions (especially packet loss). When disabled, all state updates are " +
             "sent using reliable fragmented sequenced network delivery.")]
-        public bool UseUnreliableDeltas = true;
+        public bool UseUnreliableDeltas = false;
 
         /// <summary>
         /// Data structure used to synchronize the <see cref="NetworkTransform"/>
@@ -115,6 +115,9 @@ internal uint BitSet
                 set { m_Bitset = value; }
             }
 
+            // Used to determine if this state was deferred
+            internal bool WasDeferred;
+
             // Used to store the tick calculated sent time
             internal double SentTime;
 
@@ -456,6 +459,26 @@ internal set
                 }
             }
 
+            /// <summary>
+            /// Returns whether this state update was an axial frame synchronization
+            /// (only applies when UseUnreliableDeltas is enabled)
+            /// </summary>            
+            public bool IsAxisFrameSync()
+            {
+                return UnreliableFrameSync;
+            }
+
+            /// <summary>
+            /// Returns whether this state update was sent with unreliable delivery.
+            /// If false, then it was sent with reliable delivery.
+            /// (only applies when UseUnreliableDeltas is enabled)
+            /// 
+            /// </summary>            
+            public bool IsUnreliableStateUpdate()
+            {
+                return !ReliableFragmentedSequenced;
+            }
+
             internal bool IsParented
             {
                 get => GetFlag(k_IsParented);
@@ -1506,12 +1529,17 @@ protected virtual void OnAuthorityPushTransformState(ref NetworkTransformState n
         // to assure the instance doesn't continue to send axial synchs when an object is at rest.
         private bool m_DeltaSynch;
 
+        /// <summary>
+        /// Used to defer state updates to the next network tick
+        /// </summary>
+        internal NetworkTransformState DeferredNetworkTransformState;
+
         /// <summary>
         /// Authoritative side only
         /// If there are any transform delta states, this method will synchronize the
         /// state with all non-authority instances.
         /// </summary>
-        private void TryCommitTransform(ref Transform transformToCommit, bool synchronize = false)
+        private void TryCommitTransform(ref Transform transformToCommit, bool synchronize = false, bool settingState = false)
         {
             // Only the server or the owner is allowed to commit a transform
             if (!IsServer && !IsOwner)
@@ -1520,40 +1548,60 @@ private void TryCommitTransform(ref Transform transformToCommit, bool synchroniz
                 return;
             }
 
-            // If the transform has deltas (returns dirty) then...
-            if (ApplyTransformToNetworkStateWithInfo(ref m_LocalAuthoritativeNetworkState, ref transformToCommit, synchronize))
+            // If we are not synchronizing, not setting state, our last state sent was deferred, and we are still on the same tick, then ignore this tick update check
+            if (!synchronize && !settingState && m_LastTick == m_CachedNetworkManager.ServerTime.Tick && m_OldState.WasDeferred)
             {
-                m_LocalAuthoritativeNetworkState.LastSerializedSize = m_OldState.LastSerializedSize;
+                return;
+            }
+
+            // The following check is most noticable when sending unreliable deferred state updates. If we had just sent an unreliable state update on the same tick that 
+            // we send a reliable teleport state update, then the teleport state (on the receiving side) can be processed before the unreliable delta state update.
+            // We can also set the state explicity using SetState before or after sending a state update which can cause two state updates with the same tick which both  
+            // scenarios can cause an undesirable out of synch period. To avoid this, we just defer the state update to the next tick and upon applying the state update
+            // on the next tick we do not check for deltas but do apply the state update.
+            var applyDeferredState = (DeferredNetworkTransformState.NetworkTick == m_CachedNetworkManager.ServerTime.Tick) && !synchronize;
 
-                // Make sure our network tick is incremented
-                if (m_LastTick == m_LocalAuthoritativeNetworkState.NetworkTick && !m_LocalAuthoritativeNetworkState.IsTeleportingNextFrame)
+            // If the transform has deltas (returns dirty) or we are applying a deferred state update then...
+            if (applyDeferredState || ApplyTransformToNetworkStateWithInfo(ref m_LocalAuthoritativeNetworkState, ref transformToCommit, synchronize))
+            {
+                // If we are setting state or teleporting, not applying the deferred state, and arrived on a tick that we have already sent a delta state update for, then
+                // we defer this state update to next tick.
+                if ((settingState || m_LocalAuthoritativeNetworkState.IsTeleportingNextFrame) && !applyDeferredState && m_LastTick == m_CachedNetworkManager.ServerTime.Tick)
                 {
-                    // When running in authority and a remote client is the owner, the client can hit a perfect window of time where
-                    // it is still on the previous network tick (as a count) but still have had the tick event triggered.
-                    // (This is cheaper than calculating the exact tick each time and only can occur on clients)
-                    if (!IsServer)
-                    {
-                        m_LocalAuthoritativeNetworkState.NetworkTick = m_LocalAuthoritativeNetworkState.NetworkTick + 1;
-                    }
-                    else // If we are sending unreliable deltas this could happen with (axial) frame synch
-                    if (!UseUnreliableDeltas)
-                    {
-                        NetworkLog.LogError($"[NT TICK DUPLICATE] Server already sent an update on tick {m_LastTick} and is attempting to send again on the same network tick!");
-                    }
+                    DeferredNetworkTransformState = m_LocalAuthoritativeNetworkState;
+                    DeferredNetworkTransformState.NetworkTick = m_CachedNetworkManager.ServerTime.Tick + 1;
+                    DeferredNetworkTransformState.WasDeferred = true;
+                    return;
                 }
+
+                // If we are processing a deferred state update, then apply it to the local authoritative state
+                if (applyDeferredState)
+                {
+                    m_LocalAuthoritativeNetworkState = DeferredNetworkTransformState;
+                }
+
+                m_LocalAuthoritativeNetworkState.LastSerializedSize = m_OldState.LastSerializedSize;
+
                 m_LastTick = m_LocalAuthoritativeNetworkState.NetworkTick;
+
                 // Update the state
                 UpdateTransformState();
 
-                // When sending unreliable traffic, we send 1 full (axial) frame synch every nth tick
-                // which is based on tick rate and each instance's (axial) frame synch is distributed 
-                // across the ticks per second span (excluding initial synchronization).
+                // The below is part of assuring we only send a frame synch, when sending unreliable deltas, if 
+                // we have already sent at least one unreliable delta state update. At this point in the callstack,
+                // a delta state update has just been sent in the above UpdateTransformState() call and as long as
+                // we didn't send a frame synch and we are not synchronizing then we know at least one unreliable
+                // delta has been sent. Under this scenario, we should start checking for this instance's alloted 
+                // frame synch "tick slot". Once we send a frame synch, if no other deltas occur after that
+                // (i.e. the object is at rest) then we will stop sending frame synch's until the object begins
+                // moving, rotating, or scaling again.
                 if (UseUnreliableDeltas && !m_LocalAuthoritativeNetworkState.UnreliableFrameSync && !synchronize)
                 {
                     m_DeltaSynch = true;
                 }
 
                 OnAuthorityPushTransformState(ref m_LocalAuthoritativeNetworkState);
+                m_OldState = m_LocalAuthoritativeNetworkState;
                 m_LocalAuthoritativeNetworkState.IsTeleportingNextFrame = false;
             }
         }
@@ -1615,19 +1663,25 @@ internal bool ApplyTransformToNetworkState(ref NetworkTransformState networkStat
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private bool ApplyTransformToNetworkStateWithInfo(ref NetworkTransformState networkState, ref Transform transformToUse, bool isSynchronization = false, ulong targetClientId = 0)
         {
-            // As long as we are not teleporting or doing our first synchronization and we are sending unreliable deltas,
-            // each NetworkTransform will stagger their axial synchronization over a 1 second period based on their
+            // As long as we are not doing our first synchronization and we are sending unreliable deltas, each
+            // NetworkTransform will stagger their axial synchronization over a 1 second period based on their
             // assigned tick synchronization (m_TickSync) value.
+            // More about m_DeltaSynch:
+            // If we have not sent any deltas since our last frame synch, then this will prevent us from sending
+            // frame synch's when the object is at rest. If this is false and a state update is detected and sent,
+            // then it will be set to true and each subsequent tick will do this check to determine if it should
+            // send a frame synch.
             var isAxisSync = false;
-            if (!networkState.IsTeleportingNextFrame && !isSynchronization && m_DeltaSynch && UseUnreliableDeltas)
+            if (UseUnreliableDeltas && !isSynchronization && m_DeltaSynch && m_NextTickSync <= m_CachedNetworkManager.ServerTime.Tick)
             {
-                var modTick = m_CachedNetworkManager.ServerTime.Tick % m_CachedNetworkManager.NetworkConfig.TickRate;
-                isAxisSync = modTick == m_TickSync;
-
-                if (isAxisSync)
-                {
-                    m_DeltaSynch = false;
-                }
+                // Increment to the next frame synch tick position for this instance
+                m_NextTickSync += (int)m_CachedNetworkManager.NetworkConfig.TickRate;
+                // If we are teleporting, we do not need to send a frame synch for this tick slot
+                // as a "frame synch" really is effectively just a teleport.
+                isAxisSync = !networkState.IsTeleportingNextFrame;
+                // Reset our delta synch trigger so we don't send another frame synch until we
+                // send at least 1 unreliable state update after this fame synch or teleport
+                m_DeltaSynch = false;
             }
             // This is used to determine if we need to send the state update reliably (if we are doing an axial sync)
             networkState.UnreliableFrameSync = isAxisSync;
@@ -1647,8 +1701,9 @@ private bool ApplyTransformToNetworkStateWithInfo(ref NetworkTransformState netw
             if (isSynchronization || networkState.IsTeleportingNextFrame)
             {
                 // This all has to do with complex nested hierarchies and how it impacts scale
-                // when set for the first time and depending upon whether the NetworkObject is parented
-                // (or not parented) at the time the scale values are applied.
+                // when set for the first time or teleporting and depends upon whether the
+                // NetworkObject is parented (or "de-parented") at the same time any scale
+                // values are applied.
                 var hasParentNetworkObject = false;
 
                 // If the NetworkObject belonging to this NetworkTransform instance has a parent
@@ -2224,7 +2279,6 @@ private void ApplyTeleportingState(NetworkTransformState newState)
                     {
                         currentPosition.z = newState.PositionZ;
                     }
-                    UpdatePositionInterpolator(currentPosition, sentTime, true);
                 }
                 else
                 {
@@ -2255,12 +2309,6 @@ private void ApplyTeleportingState(NetworkTransformState newState)
                         // set the current position to the state's current position
                         currentPosition = newState.CurrentPosition;
                     }
-
-                    if (Interpolate)
-                    {
-                        UpdatePositionInterpolator(currentPosition, sentTime, true);
-                    }
-
                 }
 
                 m_CurrentPosition = currentPosition;
@@ -2275,6 +2323,11 @@ private void ApplyTeleportingState(NetworkTransformState newState)
                 {
                     transform.position = currentPosition;
                 }
+
+                if (Interpolate)
+                {
+                    UpdatePositionInterpolator(currentPosition, sentTime, true);
+                }
             }
 
             if (newState.HasScaleChange)
@@ -2317,10 +2370,14 @@ private void ApplyTeleportingState(NetworkTransformState newState)
 
                 m_CurrentScale = currentScale;
                 m_TargetScale = currentScale;
-                m_ScaleInterpolator.ResetTo(currentScale, sentTime);
 
                 // Apply the adjusted scale
                 transform.localScale = currentScale;
+
+                if (Interpolate)
+                {
+                    m_ScaleInterpolator.ResetTo(currentScale, sentTime);
+                }
             }
 
             if (newState.HasRotAngleChange)
@@ -2351,7 +2408,6 @@ private void ApplyTeleportingState(NetworkTransformState newState)
 
                 m_CurrentRotation = currentRotation;
                 m_TargetRotation = currentRotation.eulerAngles;
-                m_RotationInterpolator.ResetTo(currentRotation, sentTime);
 
                 if (InLocalSpace)
                 {
@@ -2361,6 +2417,11 @@ private void ApplyTeleportingState(NetworkTransformState newState)
                 {
                     transform.rotation = currentRotation;
                 }
+
+                if (Interpolate)
+                {
+                    m_RotationInterpolator.ResetTo(currentRotation, sentTime);
+                }
             }
 
             // Add log after to applying the update if AddLogEntry is defined
@@ -2548,6 +2609,14 @@ private void OnNetworkStateChanged(NetworkTransformState oldState, NetworkTransf
                 return;
             }
 
+            // If we are using UseUnreliableDeltas and our old state tick is greater than the new state tick,
+            // then just ignore the newstate. This avoids any scenario where the new state is out of order
+            // from the old state (with unreliable traffic and/or mixed unreliable and reliable)
+            if (UseUnreliableDeltas && oldState.NetworkTick > newState.NetworkTick && !newState.IsTeleportingNextFrame)
+            {
+                return;
+            }
+
             // Get the time when this new state was sent
             newState.SentTime = new NetworkTime(m_CachedNetworkManager.NetworkConfig.TickRate, newState.NetworkTick).Time;
 
@@ -2949,7 +3018,8 @@ private void SetStateInternal(Vector3 pos, Quaternion rot, Vector3 scale, bool s
             transform.localScale = scale;
             m_LocalAuthoritativeNetworkState.IsTeleportingNextFrame = shouldTeleport;
             var transformToCommit = transform;
-            TryCommitTransform(ref transformToCommit);
+            // Always set settingState to true so we know an explicit state update is being applied
+            TryCommitTransform(ref transformToCommit, settingState: true);
         }
 
         /// <summary>
@@ -3112,7 +3182,11 @@ private void TransformStateUpdate(ulong senderId, FastBufferReader messagePayloa
             // Forward owner authoritative messages before doing anything else
             if (ownerAuthoritativeServerSide)
             {
-                ForwardStateUpdateMessage(messagePayload, networkDelivery);
+                // Forward the state update if there are any remote clients to foward it to
+                if (m_CachedNetworkManager.ConnectionManager.ConnectedClientsList.Count > (IsHost ? 2 : 1))
+                {
+                    ForwardStateUpdateMessage(messagePayload, networkDelivery);
+                }
             }
 
             // Apply the message
@@ -3262,13 +3336,12 @@ public NetworkTransformTickRegistration(NetworkManager networkManager)
             }
         }
         private static int s_TickSynchPosition;
-        private int m_TickSync;
+        private int m_NextTickSync;
 
         internal void RegisterForTickSynchronization()
         {
             s_TickSynchPosition++;
-            s_TickSynchPosition = s_TickSynchPosition % (int)NetworkManager.NetworkConfig.TickRate;
-            m_TickSync = s_TickSynchPosition;
+            m_NextTickSync = NetworkManager.ServerTime.Tick + (s_TickSynchPosition % (int)NetworkManager.NetworkConfig.TickRate);
         }
 
         /// <summary>