fix: NetworkTransform rotation delta check false positives

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -14,13 +14,15 @@ Additional documentation and release notes are available at [Multiplayer Documen
 ### Removed
 
 ### Fixed
+
+- Fixed `NetworkTransform` generating false positive rotation delta checks when rolling over between 0 and 360 degrees. (#1890)
 - Fixed client throwing an exception if it has messages in the outbound queue when processing the `NetworkEvent.Disconnect` event and is using UTP. (#1884)
 - Fixed issue during client synchronization if 'ValidateSceneBeforeLoading' returned false it would halt the client synchronization process resulting in a client that was approved but not synchronized or fully connected with the server. (#1883)
 
+
 ## [1.0.0-pre.7] - 2022-04-06
 
 ### Added
-
 - Added editor only check prior to entering into play mode if the currently open and active scene is in the build list and if not displays a dialog box asking the user if they would like to automatically add it prior to entering into play mode. (#1828)
 - Added `UnityTransport` implementation and `com.unity.transport` package dependency (#1823)
 - Added `NetworkVariableWritePermission` to `NetworkVariableBase` and implemented `Owner` client writable netvars. (#1762)

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

@@ -15,9 +15,10 @@ namespace Unity.Netcode.Components
     [DefaultExecutionOrder(100000)] // this is needed to catch the update time after the transform was updated by user scripts
     public class NetworkTransform : NetworkBehaviour
     {
-        public const float PositionThresholdDefault = .001f;
-        public const float RotAngleThresholdDefault = .01f;
-        public const float ScaleThresholdDefault = .01f;
+        public const float PositionThresholdDefault = 0.001f;
+        public const float RotAngleThresholdDefault = 0.01f;
+        public const float ScaleThresholdDefault = 0.01f;
+
         public delegate (Vector3 pos, Quaternion rotOut, Vector3 scale) OnClientRequestChangeDelegate(Vector3 pos, Quaternion rot, Vector3 scale);
         public OnClientRequestChangeDelegate OnClientRequestChange;
 
@@ -249,7 +250,10 @@ public void NetworkSerialize<T>(BufferSerializer<T> serializer) where T : IReade
         public bool SyncScaleX = true, SyncScaleY = true, SyncScaleZ = true;
 
         public float PositionThreshold = PositionThresholdDefault;
+
+        [Range(0.001f, 360.0f)]
         public float RotAngleThreshold = RotAngleThresholdDefault;
+
         public float ScaleThreshold = ScaleThresholdDefault;
 
         /// <summary>
@@ -390,6 +394,16 @@ private void ResetInterpolatedStateToCurrentAuthoritativeState()
             m_ScaleZInterpolator.ResetTo(m_LocalAuthoritativeNetworkState.ScaleZ, serverTime);
         }
 
+        /// <summary>
+        /// Will apply the transform to the LocalAuthoritativeNetworkState and get detailed isDirty information returned.
+        /// </summary>
+        /// <param name="transform">transform to apply</param>
+        /// <returns>bool isDirty, bool isPositionDirty, bool isRotationDirty, bool isScaleDirty</returns>
+        internal (bool isDirty, bool isPositionDirty, bool isRotationDirty, bool isScaleDirty) ApplyLocalNetworkState(Transform transform)
+        {
+            return ApplyTransformToNetworkStateWithInfo(ref m_LocalAuthoritativeNetworkState, m_CachedNetworkManager.LocalTime.Time, transform);
+        }
+
         // updates `NetworkState` properties if they need to and returns a `bool` indicating whether or not there was any changes made
         // returned boolean would be useful to change encapsulating `NetworkVariable<NetworkState>`'s dirty state, e.g. ReplNetworkState.SetDirty(isDirty);
         internal bool ApplyTransformToNetworkState(ref NetworkTransformState networkState, double dirtyTime, Transform transformToUse)
@@ -450,23 +464,23 @@ internal bool ApplyTransformToNetworkState(ref NetworkTransformState networkStat
             }
 
             if (SyncRotAngleX &&
-                Mathf.Abs(networkState.RotAngleX - rotAngles.x) > RotAngleThreshold)
+                Mathf.Abs(Mathf.DeltaAngle(networkState.RotAngleX, rotAngles.x)) > RotAngleThreshold)
             {
                 networkState.RotAngleX = rotAngles.x;
                 networkState.HasRotAngleX = true;
                 isRotationDirty = true;
             }
 
             if (SyncRotAngleY &&
-                Mathf.Abs(networkState.RotAngleY - rotAngles.y) > RotAngleThreshold)
+                Mathf.Abs(Mathf.DeltaAngle(networkState.RotAngleY, rotAngles.y)) > RotAngleThreshold)
             {
                 networkState.RotAngleY = rotAngles.y;
                 networkState.HasRotAngleY = true;
                 isRotationDirty = true;
             }
 
             if (SyncRotAngleZ &&
-                Mathf.Abs(networkState.RotAngleZ - rotAngles.z) > RotAngleThreshold)
+                Mathf.Abs(Mathf.DeltaAngle(networkState.RotAngleZ, rotAngles.z)) > RotAngleThreshold)
             {
                 networkState.RotAngleZ = rotAngles.z;
                 networkState.HasRotAngleZ = true;

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

@@ -22,6 +22,11 @@ public override void OnNetworkSpawn()
 
             ReadyToReceivePositionUpdate = true;
         }
+
+        public (bool isDirty, bool isPositionDirty, bool isRotationDirty, bool isScaleDirty) ApplyState()
+        {
+            return ApplyLocalNetworkState(transform);
+        }
     }
 
     // [TestFixture(true, true)]
@@ -172,6 +177,80 @@ public IEnumerator TestCantChangeTransformFromOtherSideAuthority([Values] bool t
 #endif
         }
 
+
+        /// <summary>
+        /// Validates that rotation checks don't produce false positive
+        /// results when rolling over between 0 and 360 degrees
+        /// </summary>
+        [UnityTest]
+        public IEnumerator TestRotationThresholdDeltaCheck()
+        {
+            // Get the client player's NetworkTransform for both instances
+            var authoritativeNetworkTransform = m_ServerSideClientPlayer.GetComponent<NetworkTransformTestComponent>();
+            var otherSideNetworkTransform = m_ClientSideClientPlayer.GetComponent<NetworkTransformTestComponent>();
+            otherSideNetworkTransform.RotAngleThreshold = authoritativeNetworkTransform.RotAngleThreshold = 5.0f;
+
+            var halfThreshold = authoritativeNetworkTransform.RotAngleThreshold * 0.5001f;
+            var serverRotation = authoritativeNetworkTransform.transform.rotation;
+            var serverEulerRotation = serverRotation.eulerAngles;
+
+            // Verify rotation is not marked dirty when rotated by half of the threshold
+            serverEulerRotation.y += halfThreshold;
+            serverRotation.eulerAngles = serverEulerRotation;
+            authoritativeNetworkTransform.transform.rotation = serverRotation;
+            var results = authoritativeNetworkTransform.ApplyState();
+            Assert.IsFalse(results.isRotationDirty, $"Rotation is dirty when rotation threshold is {authoritativeNetworkTransform.RotAngleThreshold} degrees and only adjusted by {halfThreshold} degrees!");
+            yield return s_DefaultWaitForTick;
+
+            // Verify rotation is marked dirty when rotated by another half threshold value
+            serverEulerRotation.y += halfThreshold;
+            serverRotation.eulerAngles = serverEulerRotation;
+            authoritativeNetworkTransform.transform.rotation = serverRotation;
+            results = authoritativeNetworkTransform.ApplyState();
+            Assert.IsTrue(results.isRotationDirty, $"Rotation was not dirty when rotated by the threshold value: {authoritativeNetworkTransform.RotAngleThreshold} degrees!");
+            yield return s_DefaultWaitForTick;
+
+            //Reset rotation back to zero on all axis
+            serverRotation.eulerAngles = serverEulerRotation = Vector3.zero;
+            authoritativeNetworkTransform.transform.rotation = serverRotation;
+            yield return s_DefaultWaitForTick;
+
+            // Rotate by 360 minus halfThreshold (which is really just negative halfThreshold) and verify rotation is not marked dirty
+            serverEulerRotation.y = 360 - halfThreshold;
+            serverRotation.eulerAngles = serverEulerRotation;
+            authoritativeNetworkTransform.transform.rotation = serverRotation;
+            results = authoritativeNetworkTransform.ApplyState();
+
+            Assert.IsFalse(results.isRotationDirty, $"Rotation is dirty when rotation threshold is {authoritativeNetworkTransform.RotAngleThreshold} degrees and only adjusted by " +
+                $"{Mathf.DeltaAngle(0, serverEulerRotation.y)} degrees!");
+
+            serverEulerRotation.y -= halfThreshold;
+            serverRotation.eulerAngles = serverEulerRotation;
+            authoritativeNetworkTransform.transform.rotation = serverRotation;
+            results = authoritativeNetworkTransform.ApplyState();
+
+            Assert.IsTrue(results.isRotationDirty, $"Rotation was not dirty when rotated by {Mathf.DeltaAngle(0, serverEulerRotation.y)} degrees!");
+
+            //Reset rotation back to zero on all axis
+            serverRotation.eulerAngles = serverEulerRotation = Vector3.zero;
+            authoritativeNetworkTransform.transform.rotation = serverRotation;
+            yield return s_DefaultWaitForTick;
+
+            serverEulerRotation.y -= halfThreshold;
+            serverRotation.eulerAngles = serverEulerRotation;
+            authoritativeNetworkTransform.transform.rotation = serverRotation;
+            results = authoritativeNetworkTransform.ApplyState();
+            Assert.IsFalse(results.isRotationDirty, $"Rotation is dirty when rotation threshold is {authoritativeNetworkTransform.RotAngleThreshold} degrees and only adjusted by " +
+                $"{Mathf.DeltaAngle(0, serverEulerRotation.y)} degrees!");
+
+            serverEulerRotation.y -= halfThreshold;
+            serverRotation.eulerAngles = serverEulerRotation;
+            authoritativeNetworkTransform.transform.rotation = serverRotation;
+            results = authoritativeNetworkTransform.ApplyState();
+
+            Assert.IsTrue(results.isRotationDirty, $"Rotation was not dirty when rotated by {Mathf.DeltaAngle(0, serverEulerRotation.y)} degrees!");
+        }
+
         /*
          * ownership change
          * test teleport with interpolation

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

@@ -6,69 +6,43 @@
 
 namespace Unity.Netcode.RuntimeTests
 {
-    public class StopStartRuntimeTests
+    public class StopStartRuntimeTests : NetcodeIntegrationTest
     {
-        [UnityTest]
-        public IEnumerator WhenShuttingDownAndRestarting_SDKRestartsSuccessfullyAndStaysRunning()
-        {            // create server and client instances
-            NetcodeIntegrationTestHelpers.Create(1, out NetworkManager server, out NetworkManager[] clients);
-
-            try
-            {
-
-                // create prefab
-                var gameObject = new GameObject("PlayerObject");
-                var networkObject = gameObject.AddComponent<NetworkObject>();
-                networkObject.DontDestroyWithOwner = true;
-                NetcodeIntegrationTestHelpers.MakeNetworkObjectTestPrefab(networkObject);
-
-                server.NetworkConfig.PlayerPrefab = gameObject;
-
-                for (int i = 0; i < clients.Length; i++)
-                {
-                    clients[i].NetworkConfig.PlayerPrefab = gameObject;
-                }
-
-                // start server and connect clients
-                NetcodeIntegrationTestHelpers.Start(false, server, clients);
-
-                // wait for connection on client side
-                yield return NetcodeIntegrationTestHelpers.WaitForClientsConnected(clients);
+        protected override int NumberOfClients => 1;
 
-                // wait for connection on server side
-                yield return NetcodeIntegrationTestHelpers.WaitForClientConnectedToServer(server);
-
-                // shutdown the server
-                server.Shutdown();
-
-                // wait 1 frame because shutdowns are delayed
-                var nextFrameNumber = Time.frameCount + 1;
-                yield return new WaitUntil(() => Time.frameCount >= nextFrameNumber);
-
-                // Verify the shutdown occurred
-                Assert.IsFalse(server.IsServer);
-                Assert.IsFalse(server.IsListening);
-                Assert.IsFalse(server.IsHost);
-                Assert.IsFalse(server.IsClient);
-
-                server.StartServer();
-                // Verify the server started
-                Assert.IsTrue(server.IsServer);
-                Assert.IsTrue(server.IsListening);
-
-                // Wait several frames
-                nextFrameNumber = Time.frameCount + 10;
-                yield return new WaitUntil(() => Time.frameCount >= nextFrameNumber);
+        protected override void OnOneTimeSetup()
+        {
+            m_UseHost = false;
+            base.OnOneTimeSetup();
+        }
 
-                // Verify the server is still running
-                Assert.IsTrue(server.IsServer);
-                Assert.IsTrue(server.IsListening);
-            }
-            finally
-            {
-                // cleanup
-                NetcodeIntegrationTestHelpers.Destroy();
-            }
+        [UnityTest]
+        public IEnumerator WhenShuttingDownAndRestarting_SDKRestartsSuccessfullyAndStaysRunning()
+        {
+            // shutdown the server
+            m_ServerNetworkManager.Shutdown();
+
+            // wait 1 frame because shutdowns are delayed
+            var nextFrameNumber = Time.frameCount + 1;
+            yield return new WaitUntil(() => Time.frameCount >= nextFrameNumber);
+
+            // Verify the shutdown occurred
+            Assert.IsFalse(m_ServerNetworkManager.IsServer);
+            Assert.IsFalse(m_ServerNetworkManager.IsListening);
+            Assert.IsFalse(m_ServerNetworkManager.IsHost);
+            Assert.IsFalse(m_ServerNetworkManager.IsClient);
+
+            m_ServerNetworkManager.StartServer();
+            // Verify the server started
+            Assert.IsTrue(m_ServerNetworkManager.IsServer);
+            Assert.IsTrue(m_ServerNetworkManager.IsListening);
+
+            // Wait several frames / one full network tick
+            yield return s_DefaultWaitForTick;
+
+            // Verify the server is still running
+            Assert.IsTrue(m_ServerNetworkManager.IsServer);
+            Assert.IsTrue(m_ServerNetworkManager.IsListening);
         }
     }
 }