fix: reset tick system on connection approval (MTT-1346) (#1231)

* fix: reset tick system on connection approval (MTT-1346)

* fix: don't run time system on clients before they are connected

* feat: expose tick system reset as public and add xml docs

* test: implement test which checks intialization of time and tick system

* fix: run ticks only when connected and reset tick system when time resets.

* make initialization test non framerate depended to fix issues when running on mac CI

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

Author: 3 people

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

@@ -1152,8 +1152,18 @@ private void OnNetworkEarlyUpdate()
         // TODO Once we have a way to subscribe to NetworkUpdateLoop with order we can move this out of NetworkManager but for now this needs to be here because we need strict ordering.
         private void OnNetworkPreUpdate()
         {
+            if (IsServer == false && IsConnectedClient == false)
+            {
+                // As a client wait to run the time system until we are connected.
+                return;
+            }
+
             // Only update RTT here, server time is updated by time sync messages
-            NetworkTimeSystem.Advance(Time.deltaTime);
+            var reset = NetworkTimeSystem.Advance(Time.deltaTime);
+            if (reset)
+            {
+                NetworkTickSystem.Reset(NetworkTimeSystem.LocalTime, NetworkTimeSystem.ServerTime);
+            }
             NetworkTickSystem.UpdateTick(NetworkTimeSystem.LocalTime, NetworkTimeSystem.ServerTime);
 
             if (IsServer == false)

com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ConnectionApprovedMessage.cs

@@ -65,6 +65,7 @@ public void Handle(FastBufferReader reader, ulong clientId, NetworkManager netwo
 
             var time = new NetworkTime(networkManager.NetworkTickSystem.TickRate, NetworkTick);
             networkManager.NetworkTimeSystem.Reset(time.Time, 0.15f); // Start with a constant RTT of 150 until we receive values from the transport.
+            networkManager.NetworkTickSystem.Reset(networkManager.NetworkTimeSystem.LocalTime, networkManager.NetworkTimeSystem.ServerTime);
 
             networkManager.LocalClient = new NetworkClient() { ClientId = networkManager.LocalClientId };
 

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

@@ -55,6 +55,17 @@ public NetworkTickSystem(uint tickRate, double localTimeSec, double serverTimeSe
             ServerTime = new NetworkTime(tickRate, serverTimeSec);
         }
 
+        /// <summary>
+        /// Resets the tick system to the given network time.
+        /// </summary>
+        /// <param name="localTimeSec">The local time in seconds.</param>
+        /// <param name="serverTimeSec">The server time in seconds.</param>
+        public void Reset(double localTimeSec, double serverTimeSec)
+        {
+            LocalTime = new NetworkTime(TickRate, localTimeSec);
+            ServerTime = new NetworkTime(TickRate, serverTimeSec);
+        }
+
         /// <summary>
         /// Called after advancing the time system to run ticks based on the difference in time.
         /// </summary>

com.unity.netcode.gameobjects/Tests/Runtime/BaseMultiInstanceTest.cs

@@ -60,7 +60,7 @@ public virtual IEnumerator Teardown()
         /// We want to exclude the TestRunner scene on the host-server side so it won't try to tell clients to
         /// synchronize to this scene when they connect
         /// </summary>
-        private bool VerifySceneIsValidForClientsToLoad(int sceneIndex, string sceneName, LoadSceneMode loadSceneMode)
+        private static bool VerifySceneIsValidForClientsToLoad(int sceneIndex, string sceneName, LoadSceneMode loadSceneMode)
         {
             // exclude test runner scene
             if (sceneName.StartsWith(k_FirstPartOfTestRunnerSceneName))
@@ -75,7 +75,7 @@ private bool VerifySceneIsValidForClientsToLoad(int sceneIndex, string sceneName
         /// clients to synchronize (i.e. load) the test runner scene.  This will also register the test runner
         /// scene and its handle for both client(s) and server-host.
         /// </summary>
-        private void SceneManagerValidationAndTestRunnerInitialization(NetworkManager networkManager)
+        public static void SceneManagerValidationAndTestRunnerInitialization(NetworkManager networkManager)
         {
             // If VerifySceneBeforeLoading is not already set, then go ahead and set it so the host/server
             // will not try to synchronize clients to the TestRunner scene.  We only need to do this for the server.

com.unity.netcode.gameobjects/Tests/Runtime/Timing/TimeInitializationTest.cs

@@ -0,0 +1,102 @@
+using System.Collections;
+using NUnit.Framework;
+using UnityEngine;
+using UnityEngine.TestTools;
+
+namespace Unity.Netcode.RuntimeTests
+{
+    /// <summary>
+    /// Tests that the time and tick system are initialized properly
+    /// </summary>
+    public class TimeInitializationTest
+    {
+        private int m_ClientTickCounter;
+        private int m_ConnectedTick;
+        private NetworkManager m_Client;
+
+        [UnityTest]
+        public IEnumerator TestClientTimeInitializationOnConnect([Values(0, 1f)] float serverStartDelay, [Values(0, 1f)] float clientStartDelay, [Values(true, false)] bool isHost)
+        {
+            // Create multiple NetworkManager instances
+            if (!MultiInstanceHelpers.Create(1, out NetworkManager server, out NetworkManager[] clients, 30))
+            {
+                Debug.LogError("Failed to create instances");
+                Assert.Fail("Failed to create instances");
+            }
+
+            yield return new WaitForSeconds(serverStartDelay);
+            MultiInstanceHelpers.Start(false, server, new NetworkManager[] { }, BaseMultiInstanceTest.SceneManagerValidationAndTestRunnerInitialization); // passing no clients on purpose to start them manually later
+
+            // 0 ticks should have passed
+            var serverTick = server.NetworkTickSystem.ServerTime.Tick;
+            Assert.AreEqual(0, serverTick);
+
+            // server time should be 0
+            Assert.AreEqual(0, server.NetworkTickSystem.ServerTime.Time);
+
+            // wait 2 frames to ensure network tick is run
+            yield return null;
+            yield return null;
+
+            var serverTimePassed = server.NetworkTickSystem.ServerTime.Time;
+            var expectedServerTickCount = Mathf.FloorToInt((float)(serverTimePassed * 30));
+
+            var ticksPassed = server.NetworkTickSystem.ServerTime.Tick - serverTick;
+            Assert.AreEqual(expectedServerTickCount, ticksPassed);
+
+            yield return new WaitForSeconds(clientStartDelay);
+
+            Assert.AreEqual(1, clients.Length);
+            m_Client = clients[0];
+
+            Assert.Null(m_Client.NetworkTickSystem);
+
+            m_Client.OnClientConnectedCallback += ClientOnOnClientConnectedCallback;
+
+            var clientStartRealTime = Time.time;
+
+            m_Client.StartClient();
+            BaseMultiInstanceTest.SceneManagerValidationAndTestRunnerInitialization(clients[0]);
+
+            m_Client.NetworkTickSystem.Tick += NetworkTickSystemOnTick;
+            m_ClientTickCounter = 0;
+
+
+            // don't check for anything here and assume non-async connection.
+
+            // Wait for connection on client side
+            yield return MultiInstanceHelpers.Run(MultiInstanceHelpers.WaitForClientsConnected(clients));
+
+            var clientStartRealTimeDuration = Time.time - clientStartRealTime;
+            var clientStartRealTickDuration = Mathf.FloorToInt(clientStartRealTimeDuration * 30);
+
+            // check tick is initialized with server value
+            Assert.AreNotEqual(0, m_ConnectedTick);
+
+            Assert.True(m_ClientTickCounter <= clientStartRealTickDuration);
+
+            MultiInstanceHelpers.Destroy();
+            yield return null;
+        }
+
+        private void NetworkTickSystemOnTick()
+        {
+            Debug.Log(m_Client.NetworkTickSystem.ServerTime.Tick);
+            m_ClientTickCounter++;
+        }
+
+        private void ClientOnOnClientConnectedCallback(ulong id)
+        {
+            // client connected to server
+            m_ConnectedTick = m_Client.NetworkTickSystem.ServerTime.Tick;
+            Debug.Log($"Connected tick: {m_ConnectedTick}");
+        }
+
+        [UnityTearDown]
+        public virtual IEnumerator Teardown()
+        {
+            MultiInstanceHelpers.Destroy();
+            yield return null;
+        }
+    }
+}