Merge branch 'develop' into chore/add_auto_labeler

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -12,11 +12,20 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+### Changed
+
+
+## [1.12.2] - 2025-01-17
+
+### Fixed
+
+- Fixed issue where `NetworkVariableBase` derived classes were not being re-initialized if the associated `NetworkObject` instance was not destroyed and re-spawned. (#3217)
 - 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. (#3202)
 - Fixed issue where `NetworkRigidBody2D` was still using the deprecated `isKinematic` property in Unity versions 2022.3 and newer. (#3199)
 - Fixed issue where an exception was thrown when calling `NetworkManager.Shutdown` after calling `UnityTransport.Shutdown`. (#3118)
 
-### Changed
+
+- Changed the `NetworkTimeSystem.Sync` method to use half RTT to calculate the desired local time offset as opposed to the full RTT. (#3206)
 
 ## [1.12.0] - 2024-11-19
 

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

@@ -755,6 +755,13 @@ internal void InternalOnNetworkDespawn()
             {
                 Debug.LogException(e);
             }
+
+            // Deinitialize all NetworkVariables in the event the associated
+            // NetworkObject is recylced (in-scene placed or pooled).
+            for (int i = 0; i < NetworkVariableFields.Count; i++)
+            {
+                NetworkVariableFields[i].Deinitialize();
+            }
         }
 
         /// <summary>
@@ -858,6 +865,22 @@ internal void InitializeVariables()
         {
             if (m_VarInit)
             {
+                // If the primary initialization has already been done, then go ahead
+                // and re-initialize each NetworkVariable in the event it is an in-scene
+                // placed NetworkObject in an already loaded scene that has already been
+                // used within a network session =or= if this is a pooled NetworkObject
+                // that is being repurposed.
+                for (int i = 0; i < NetworkVariableFields.Count; i++)
+                {
+                    // If already initialized, then skip
+                    if (NetworkVariableFields[i].HasBeenInitialized)
+                    {
+                        continue;
+                    }
+                    NetworkVariableFields[i].Initialize(this);
+                }
+                // Exit early as we don't need to run through the rest of this initialization
+                // process
                 return;
             }
 

com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableBase.cs

@@ -34,6 +34,12 @@ public abstract class NetworkVariableBase : IDisposable
         private protected NetworkBehaviour m_NetworkBehaviour;
         private NetworkManager m_InternalNetworkManager;
 
+        // Determines if this NetworkVariable has been "initialized" to prevent initializing more than once which can happen when first
+        // instantiated and spawned. If this NetworkVariable instance is on an in-scene placed NetworkObject =or= a pooled NetworkObject
+        // that can persist between sessions and/or be recycled we need to reset the LastUpdateSent value prior to spawning otherwise
+        // this NetworkVariableBase property instance will not update until the last session time used.
+        internal bool HasBeenInitialized { get; private set; }
+
         public NetworkBehaviour GetBehaviour()
         {
             return m_NetworkBehaviour;
@@ -49,37 +55,82 @@ internal void LogWritePermissionError()
             Debug.LogError(GetWritePermissionError());
         }
 
-        private protected NetworkManager m_NetworkManager
-        {
-            get
-            {
-                if (m_InternalNetworkManager == null && m_NetworkBehaviour && m_NetworkBehaviour.NetworkObject?.NetworkManager)
-                {
-                    m_InternalNetworkManager = m_NetworkBehaviour.NetworkObject?.NetworkManager;
-                }
-                return m_InternalNetworkManager;
-            }
-        }
+        private protected NetworkManager m_NetworkManager => m_InternalNetworkManager;
 
         /// <summary>
         /// Initializes the NetworkVariable
         /// </summary>
         /// <param name="networkBehaviour">The NetworkBehaviour the NetworkVariable belongs to</param>
         public void Initialize(NetworkBehaviour networkBehaviour)
         {
-            m_InternalNetworkManager = null;
+            // If we have already been initialized, then exit early.
+            // This can happen on the very first instantiation and spawning of the associated NetworkObject
+            if (HasBeenInitialized)
+            {
+                return;
+            }
+
+            // Throw an exception if there is an invalid NetworkBehaviour parameter
+            if (!networkBehaviour)
+            {
+                throw new Exception($"[{GetType().Name}][Initialize] {nameof(NetworkBehaviour)} parameter passed in is null!");
+            }
             m_NetworkBehaviour = networkBehaviour;
-            if (m_NetworkBehaviour && m_NetworkBehaviour.NetworkObject?.NetworkManager)
+
+            // Throw an exception if there is no NetworkManager available
+            if (!m_NetworkBehaviour.NetworkManager)
             {
-                m_InternalNetworkManager = m_NetworkBehaviour.NetworkObject?.NetworkManager;
+                // Exit early if there has yet to be a NetworkManager assigned.
+                // This is ok because Initialize is invoked multiple times until
+                // it is considered "initialized".
+                return;
+            }
 
-                if (m_NetworkBehaviour.NetworkManager.NetworkTimeSystem != null)
-                {
-                    UpdateLastSentTime();
-                }
+            if (!m_NetworkBehaviour.NetworkObject)
+            {
+                // Exit early if there has yet to be a NetworkObject assigned.
+                // This is ok because Initialize is invoked multiple times until
+                // it is considered "initialized".
+                return;
             }
 
+            if (!m_NetworkBehaviour.NetworkObject.NetworkManagerOwner)
+            {
+                // Exit early if there has yet to be a NetworkManagerOwner assigned
+                // to the NetworkObject. This is ok because Initialize is invoked 
+                // multiple times until it is considered "initialized".
+                return;
+            }
+            m_InternalNetworkManager = m_NetworkBehaviour.NetworkObject.NetworkManagerOwner;
+
             OnInitialize();
+
+            // Some unit tests don't operate with a running NetworkManager.
+            // Only update the last time if there is a NetworkTimeSystem.
+            if (m_InternalNetworkManager.NetworkTimeSystem != null)
+            {
+                // Update our last sent time relative to when this was initialized
+                UpdateLastSentTime();
+
+                // At this point, this instance is considered initialized
+                HasBeenInitialized = true;
+            }
+            else if (m_InternalNetworkManager.LogLevel == LogLevel.Developer)
+            {
+                Debug.LogWarning($"[{m_NetworkBehaviour.name}][{m_NetworkBehaviour.GetType().Name}][{GetType().Name}][Initialize] {nameof(NetworkManager)} has no {nameof(NetworkTimeSystem)} assigned!");
+            }
+        }
+
+        /// <summary>
+        /// Deinitialize is invoked when a NetworkObject is despawned.
+        /// This allows for a recyled NetworkObject (in-scene or pooled)
+        /// to be properly initialized upon the next use/spawn.
+        /// </summary>
+        internal void Deinitialize()
+        {
+            // When despawned, reset the HasBeenInitialized so if the associated NetworkObject instance
+            // is recylced (i.e. in-scene placed or pooled) it will re-initialize the LastUpdateSent time.
+            HasBeenInitialized = false;
         }
 
         /// <summary>

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/Runtime/Transports/UTP/UnityTransport.cs

@@ -1167,7 +1167,13 @@ public override void DisconnectLocalClient()
         /// <param name="clientId">The client to disconnect</param>
         public override void DisconnectRemoteClient(ulong clientId)
         {
-            Debug.Assert(m_State == State.Listening, "DisconnectRemoteClient should be called on a listening server");
+#if DEBUG
+            if (m_State != State.Listening)
+            {
+                Debug.LogWarning($"{nameof(DisconnectRemoteClient)} should only be called on a listening server!");
+                return;
+            }
+#endif
 
             if (m_State == State.Listening)
             {
@@ -1239,7 +1245,13 @@ public NetworkEndpoint GetEndpoint(ulong clientId)
         /// <param name="networkManager">The NetworkManager that initialized and owns the transport</param>
         public override void Initialize(NetworkManager networkManager = null)
         {
-            Debug.Assert(sizeof(ulong) == UnsafeUtility.SizeOf<NetworkConnection>(), "Netcode connection id size does not match UTP connection id size");
+#if DEBUG
+            if (sizeof(ulong) != UnsafeUtility.SizeOf<NetworkConnection>())
+            {
+                Debug.LogWarning($"Netcode connection id size {sizeof(ulong)} does not match UTP connection id size {UnsafeUtility.SizeOf<NetworkConnection>()}!");
+                return;
+            }
+#endif
 
             NetworkManager = networkManager;
 

com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs

@@ -1550,8 +1550,12 @@ private void UnloadRemainingScenes()
 
         private void LogWaitForMessages()
         {
-            VerboseDebug(m_WaitForLog.ToString());
-            m_WaitForLog.Clear();
+            // If there is nothing to log, then don't log anything
+            if (m_WaitForLog.Length > 0)
+            {
+                VerboseDebug(m_WaitForLog.ToString());
+                m_WaitForLog.Clear();
+            }
         }
 
         private IEnumerator WaitForTickAndFrames(NetworkManager networkManager, int tickCount, float targetFrames)

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

@@ -18,15 +18,16 @@ public 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);
 
             });