fix: network clock adjustment overshoot and per-frame NetworkManager scene search

.yamato/unified-tests.yml

@@ -35,11 +35,24 @@ unified_test_{{ project.name }}_{{ platform.name }}_{{ editor }}:
 
   commands:
     - unity-downloader-cli --fast --wait -u {{ editor }} -c Editor --arch {{ platform.architecture }}
+    # Import and compile testproject in its own editor session first. The IL post-processor and compiler processes it starts
+    # otherwise stay resident for the whole test pass and, on this agent, leave too little memory for the tests.
+    - mkdir -p warmup
+    # -releaseCodeOptimization pins the Release configuration the test runner compiles. An editor whose code optimization
+    # preference is Debug would otherwise build the other configuration, and the test session would recompile everything.
+    - .Editor/Unity -batchmode -nographics -releaseCodeOptimization -logFile warmup/editor.log -projectPath {{ project.path }} -quit
+    # Stop any build processes that outlive the warm-up editor, then list what is left so the job log shows it.
+    - dotnet build-server shutdown || true
+    - pkill -f Unity.ILPP.Runner || true
+    - pkill -f VBCSCompiler || true
+    - pkill -f bee_backend || true
+    - ps -eo rss,comm --sort=-rss | head -15
     - UnifiedTestRunner --testproject={{ project.path }} --suite=playmode --artifacts-path=test-results --editor-location=.Editor --rerun-strategy=Test --retry={{ num_test_retries }} --clean-library-on-rerun --timeout={{ test_timeout }}
   artifacts:
     logs:
       paths:
         - "test-results/**/*"
+        - "warmup/**/*"
   dependencies:
     - .yamato/_run-all.yml#run_quick_checks
 {% endfor -%}

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -23,6 +23,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Issue where a client's clock kept alternating between running slightly fast and slightly slow on every frame, even after it had synchronized with the server. Interpolated objects juddered (a periodic motion artifact caused by mismatched frame rates): they moved in uneven steps instead of smoothly. Barely noticeable at the default `NetworkTimeSystem.AdjustmentRatio`, and clearly visible at higher values.
 - Issue where the IL post-processor could not resolve core library types, such as `Guid` and `List<T>`, in assemblies compiled against .NET reference assemblies, for example when targeting .NET 10. Those split the core library across `System.Runtime`, `System.Collections` and other assemblies.
 - Fixed issue where a prefab added to a `NetworkPrefabsList` while a session was running was registered more than once and logged a duplicate `GlobalObjectIdHash` error. (#4184)
 - Issue where moving a `NetworkObject` into another scene made the clients that did not observe it log "Trying to synchronize NetworkObjectId but it was not spawned". The scene migration is now only sent to the clients that observe the `NetworkObject`. (#4176)

com.unity.netcode.gameobjects/Runtime/Components/Helpers/UnifiedBootstrap.cs

@@ -71,17 +71,17 @@
                 RegisterCreatedWorlds();
                 Debug.Log($"Starting a world for {(networkManager.IsServer ? "Host" : "Client")}");
                 s_WorldCounter++;
-                LastCreatedWorld = networkManager.IsServer ? CreateSingleWorldHost($"HostSingleWorld-{s_WorldCounter}")
-                    : CreateClientWorld($"ClientWorld-{s_WorldCounter}");
+                var world = CreateWorld(networkManager);
+                LastCreatedWorld = world;
 
-                if (LastCreatedWorld == null)
+                if (world == null)
                 {
                     s_WorldCounter--;
                     Debug.LogError($"[{nameof(UnifiedBootstrap)}] World is null!");
                     return false;
                 }
 
-                if (!LastCreatedWorld.IsCreated)
+                if (!world.IsCreated)
                 {
                     s_WorldCounter--;
                     Debug.LogError($"[{nameof(UnifiedBootstrap)}] World was not created!");
@@ -90,10 +90,10 @@
 
                 if (networkManager.LogLevel <= LogLevel.Developer)
                 {
-                    NetworkLog.LogInfo($"[{nameof(UnifiedBootstrap)}] Created world: {LastCreatedWorld.Name} / {LastCreatedWorld.SequenceNumber}");
+                    NetworkLog.LogInfo($"[{nameof(UnifiedBootstrap)}] Created world: {world.Name} / {world.SequenceNumber}");
                 }
 
-                networkManager.NetcodeWorld = (NetcodeWorld)LastCreatedWorld;
+                networkManager.NetcodeWorld = (NetcodeWorld)world;
                 s_CreatedWorlds.Add(networkManager.NetcodeWorld);
             }
             else
@@ -106,10 +106,33 @@
             return true;
         }
 
-        ~UnifiedBootstrap()
+        /// <summary>
+        /// Creates the world used by a <see cref="NetworkManager"/>'s hybrid session.
+        /// </summary>
+        /// <remarks>
+        /// A server or host uses a single world host. A dedicated server build uses a server-only world instead, because
+        /// Netcode for Entities cannot create a client world there. A build that only targets the client or the server
+        /// cannot start a host.
+        /// </remarks>
+        private static World CreateWorld(NetworkManager networkManager)
         {
-            LastCreatedWorld = null;
-            Instance = null;
+#if UNITY_SERVER && !UNITY_EDITOR
+            if (networkManager.IsClient)
+            {
+                Debug.LogError($"[{nameof(UnifiedBootstrap)}] A dedicated server build cannot start a {(networkManager.IsServer ? "host" : "client")} in hybrid mode.");
+                return null;
+            }
+            return CreateServerWorld($"ServerWorld-{s_WorldCounter}");
+#elif UNITY_CLIENT && !UNITY_EDITOR
+            if (networkManager.IsServer)
+            {
+                Debug.LogError($"[{nameof(UnifiedBootstrap)}] A client only build cannot start a {(networkManager.IsClient ? "host" : "server")} in hybrid mode. Set the Netcode for Entities client build target to ClientAndServer.");
+                return null;
+            }
+            return CreateClientWorld($"ClientWorld-{s_WorldCounter}");
+#else
+            return networkManager.IsServer ? CreateSingleWorldHost($"HostSingleWorld-{s_WorldCounter}") : CreateClientWorld($"ClientWorld-{s_WorldCounter}");
+#endif
         }
     }
 }

com.unity.netcode.gameobjects/Runtime/Components/Helpers/UnifiedUpdateConnections.cs

@@ -2,7 +2,6 @@
 using System.Collections.Generic;
 using Unity.Collections;
 using Unity.Entities;
-using UnityEngine;
 
 namespace Unity.Netcode.Components
 {
@@ -26,37 +25,57 @@
         }
     }
 
+    /// <summary>
+    /// This is used to track the <see cref="NetworkManager"/>, as a singleton, when running a hybrid session.
+    /// </summary>
+    internal struct HybridNetworkManager : IComponentData
+    {
+        public UnityObjectRef<NetworkManager> NetworkManager;
+    }
+
+    // Only exists in client and server worlds, and only updates in a world created for a hybrid session.
+    [WorldSystemFilter(WorldSystemFilterFlags.ServerSimulation | WorldSystemFilterFlags.ClientSimulation)]
     internal partial class UnifiedUpdateConnections : SystemBase
     {
         private List<NetcodeConnection> m_TempConnections = new List<NetcodeConnection>();
 
         private Dictionary<int, NetcodeConnection> m_NewConnections = new Dictionary<int, NetcodeConnection>();
 
+        private EntityQuery m_HybridNetworkManagerQuery;
+
+        protected override void OnCreate()
+        {
+            m_HybridNetworkManagerQuery = GetEntityQuery(ComponentType.ReadOnly<HybridNetworkManager>());
+            RequireForUpdate(m_HybridNetworkManagerQuery);
+        }
+
         protected override void OnUpdate()
         {
-            var isServer = World.IsServer();
             var commandBuffer = new EntityCommandBuffer(Allocator.Temp);
-            foreach (var networkManager in Object.FindObjectsByType<NetworkManager>())
+            var networkManager = m_HybridNetworkManagerQuery.GetSingleton<HybridNetworkManager>().NetworkManager.Value;
+            // A missing NetworkManager counts as shutting down: the world outlives it if ShutdownInternal throws before disposing the world.
+            var isShuttingDown = !networkManager || networkManager.ShutdownInProgress || !networkManager.IsListening;
+            foreach (var (networkId, connectionState, entity) in SystemAPI.Query<NetworkId, ConnectionState>().WithNone<NetworkStreamConnection>().WithEntityAccess())
             {
-                foreach (var (networkId, connectionState, entity) in SystemAPI.Query<NetworkId, ConnectionState>().WithNone<NetworkStreamConnection>().WithEntityAccess())
+                commandBuffer.RemoveComponent<ConnectionState>(entity);
+                m_TempConnections.Add(new NetcodeConnection
                 {
-                    commandBuffer.RemoveComponent<ConnectionState>(entity);
-                    m_TempConnections.Add(new NetcodeConnection
-                    {
-                        World = World,
-                        Entity = entity,
-                        NetworkId = networkId.Value
-                    });
-                }
+                    World = World,
+                    Entity = entity,
+                    NetworkId = networkId.Value
+                });
+            }
 
-                foreach (var con in m_TempConnections)
-                {
-                    NetworkManager.OnNetCodeDisconnect?.Invoke(con);
-                }
+            foreach (var con in m_TempConnections)
+            {
+                NetworkManager.OnNetCodeDisconnect?.Invoke(con);
+            }
 
-                m_TempConnections.Clear();
+            m_TempConnections.Clear();
 
-                // TODO: We should figure out how to associate the N4E NetworkId with the NGO ClientId
+            // TODO: We should figure out how to associate the N4E NetworkId with the NGO ClientId
+            if (!isShuttingDown)
+            {
                 foreach (var (networkId, entity) in SystemAPI.Query<NetworkId>().WithAll<NetworkStreamConnection>()
                              .WithNone<NetworkStreamInGame>().WithEntityAccess())
                 {
@@ -67,43 +86,44 @@
                         m_NewConnections.Add(networkId.Value, newConnection);
                     }
                 }
+            }
 
-                // If we have any pending connections
-                if (m_NewConnections.Count > 0)
+            // If we have any pending connections
+            if (!isShuttingDown && m_NewConnections.Count > 0)
+            {
+                foreach (var entry in m_NewConnections)
                 {
-                    foreach (var entry in m_NewConnections)
+                    // Server: always connect
+                    // Client: wait until we have synchronized before announcing we are ready to receive snapshots
+                    if (networkManager.IsServer || (!networkManager.IsServer && networkManager.IsConnectedClient))
                     {
-                        // Server: always connect
-                        // Client: wait until we have synchronized before announcing we are ready to receive snapshots
-                        if (networkManager.IsServer || (!networkManager.IsServer && networkManager.IsConnectedClient))
-                        {
-                            // Set the connection in-game
-                            commandBuffer.AddComponent<NetworkStreamInGame>(entry.Value.Entity);
-                            commandBuffer.AddComponent(entry.Value.Entity, default(ConnectionState));
-                            NetworkManager.OnNetCodeConnect?.Invoke(entry.Value);
-                            m_TempConnections.Add(entry.Value);
-                        }
+                        // Set the connection in-game
+                        commandBuffer.AddComponent<NetworkStreamInGame>(entry.Value.Entity);
+                        commandBuffer.AddComponent(entry.Value.Entity, default(ConnectionState));
+                        NetworkManager.OnNetCodeConnect?.Invoke(entry.Value);
+                        m_TempConnections.Add(entry.Value);
                     }
+                }
 
-                    // Remove any connections that have "gone in-game".
-                    foreach (var connection in m_TempConnections)
-                    {
-                        m_NewConnections.Remove(connection.NetworkId);
-                    }
+                // Remove any connections that have "gone in-game".
+                foreach (var connection in m_TempConnections)
+                {
+                    m_NewConnections.Remove(connection.NetworkId);
                 }
+            }
 
-                m_TempConnections.Clear();
+            m_TempConnections.Clear();
 
-                // If the local NetworkManager is shutting down or no longer connected, then
-                // make sure we have disconnected all known connections.
-                if (networkManager.ShutdownInProgress || !networkManager.IsListening)
+            // If the local NetworkManager is shutting down or no longer connected, then
+            // make sure we have disconnected all known connections.
+            if (isShuttingDown)
+            {
+                // Only in-game connections have ConnectionState, and removing it here means each one is only reported once.
+                foreach (var (networkId, entity) in SystemAPI.Query<NetworkId>().WithAll<ConnectionState>().WithEntityAccess())
                 {
-                    foreach (var (networkId, entity) in SystemAPI.Query<NetworkId>().WithEntityAccess())
-                    {
-                        commandBuffer.RemoveComponent<ConnectionState>(entity);
-                        NetworkManager.OnNetCodeDisconnect?.Invoke(new NetcodeConnection
-                        { World = World, Entity = entity, NetworkId = networkId.Value });
-                    }
+                    commandBuffer.RemoveComponent<ConnectionState>(entity);
+                    NetworkManager.OnNetCodeDisconnect?.Invoke(new NetcodeConnection
+                    { World = World, Entity = entity, NetworkId = networkId.Value });
                 }
             }
             commandBuffer.Playback(EntityManager);

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

@@ -1395,6 +1395,12 @@
             TypeManager.Initialize();
             UnifiedBootstrap.CurrentNetworkManagerForInitialization = this;
             new UnifiedBootstrap().Initialize("Default World");
+
+            if (NetcodeWorld != null && NetcodeWorld.IsCreated)
+            {
+                var entityManager = NetcodeWorld.EntityManager;
+                entityManager.AddComponentData(entityManager.CreateEntity(), new HybridNetworkManager { NetworkManager = this });
+            }
         }
 
         /// <summary>

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

@@ -260,8 +260,13 @@ public bool Advance(double deltaTimeSec)
                 return true;
             }
 
-            m_CurrentLocalTimeOffset += deltaTimeSec * (m_DesiredLocalTimeOffset > m_CurrentLocalTimeOffset ? AdjustmentRatio : -AdjustmentRatio);
-            m_CurrentServerTimeOffset += deltaTimeSec * (m_DesiredServerTimeOffset > m_CurrentServerTimeOffset ? AdjustmentRatio : -AdjustmentRatio);
+            // Move each offset toward its target by at most AdjustmentRatio of this frame's delta, stopping once it gets there.
+            // Stepping the full amount every frame overshot the target, so a synced clock alternated between running fast and
+            // slow, and anything driven by it juddered.
+            // Math.Clamp throws on a negative step, so a negative delta or AdjustmentRatio applies no adjustment.
+            var maximumStep = Math.Max(0.0, deltaTimeSec * AdjustmentRatio);
+            m_CurrentLocalTimeOffset += Math.Clamp(m_DesiredLocalTimeOffset - m_CurrentLocalTimeOffset, -maximumStep, maximumStep);
+            m_CurrentServerTimeOffset += Math.Clamp(m_DesiredServerTimeOffset - m_CurrentServerTimeOffset, -maximumStep, maximumStep);
 
             return false;
         }

com.unity.netcode.gameobjects/Runtime/Transports/Unified/UnifiedNetcodeTransport.cs

@@ -476,7 +476,8 @@
         public override ulong GetCurrentRtt(ulong clientId)
         {
             var (transportId, _) = m_NetworkManager.ConnectionManager.ClientIdToTransportId(clientId);
-            return (ulong)m_Connections[(int)transportId].Connection.RTT;
+            // Returns 0 when the connection was already removed (for example while shutting down), the same as UnityTransport.
+            return m_Connections.TryGetValue((int)transportId, out ConnectionInfo connectionInfo) ? (ulong)connectionInfo.Connection.RTT : 0;
         }
 
         public override void Initialize(NetworkManager networkManager = null)

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

@@ -138,6 +138,36 @@ public void RttCatchupSlowdownTest()
             Assert.True(Math.Abs(totalServerSpeedUpTime) < k_AcceptableRttOffset, $"Expected server speed up time to be less than {k_AcceptableRttOffset}ms but it was {expectedServerSpeedUpTime}!");
         }
 
+        /// <summary>
+        /// Validates that once the clock has caught up with a time sync, its local and server time both advance by
+        /// exactly the frame's delta time on every frame, instead of alternating between slightly fast and slightly slow.
+        /// </summary>
+        [Test]
+        public void ConvergedClockAdvancesAtRealRate([Values(0.01d, 0.1d)] double adjustmentRatio)
+        {
+            const double deltaTime = 1.0d / 60.0d;
+            double receivedServerTime = 2;
+            var timeSystem = new NetworkTimeSystem(0.05d, 0.05d, 0.1d, adjustmentRatio);
+            timeSystem.Reset(receivedServerTime, 0.1d);
+            // A sync 10 ms ahead, below the hard reset threshold, makes the clock catch up gradually instead of resetting.
+            receivedServerTime += 0.01d;
+            timeSystem.Sync(receivedServerTime, 0.1d);
+
+            for (int i = 0; i < 600; i++)
+            {
+                timeSystem.Advance(deltaTime);
+            }
+
+            for (int i = 0; i < 10; i++)
+            {
+                var localTime = timeSystem.LocalTime;
+                var serverTime = timeSystem.ServerTime;
+                timeSystem.Advance(deltaTime);
+                Assert.AreEqual(deltaTime, timeSystem.LocalTime - localTime, 1e-9d, $"Local time advanced at the wrong rate on frame {i}.");
+                Assert.AreEqual(deltaTime, timeSystem.ServerTime - serverTime, 1e-9d, $"Server time advanced at the wrong rate on frame {i}.");
+            }
+        }
+
         /// <summary>
         /// Tests whether time resets when there is a huge spike in RTT and is able to stabilize again.
         /// </summary>

com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/UnifiedNetworkTransformTest.cs

@@ -100,6 +100,24 @@ public IEnumerator BasicMovementTest()
             AssertOnTimeout($"A client failed to synchronize the changed position: {GetVector3Values(newPos)}!");
             VerboseDebug("Test Passed!");
         }
+
+        /// <summary>
+        /// Validates that each <see cref="NetworkManager"/>'s world only updates that <see cref="NetworkManager"/>'s connections.
+        /// </summary>
+        [Test]
+        public void EachWorldUpdatesConnectionsForItsOwnNetworkManager()
+        {
+            foreach (var networkManager in m_NetworkManagers)
+            {
+                var world = networkManager.NetcodeWorld;
+                Assert.IsNotNull(world, $"[Client-{networkManager.LocalClientId}] Has no world!");
+                Assert.IsNotNull(world.GetExistingSystemManaged<UnifiedUpdateConnections>(), $"[{world.Name}] Has no {nameof(UnifiedUpdateConnections)} system!");
+                using var query = world.EntityManager.CreateEntityQuery(typeof(HybridNetworkManager));
+                Assert.AreEqual(1, query.CalculateEntityCount(), $"[{world.Name}] Does not have exactly one {nameof(HybridNetworkManager)}!");
+                var bound = query.GetSingleton<HybridNetworkManager>().NetworkManager.Value;
+                Assert.AreSame(networkManager, bound, $"[{world.Name}] Is bound to another {nameof(NetworkManager)}!");
+            }
+        }
     }
 }
 #endif