fix: NetworkTransform rotation delta check false positives (#1890)

* fix

MTT-3370
This fixes the issue with false positive rotation deltas while also assuring the threshold values cannot be set below the minimum threshold value.

* test

this tests the fix for the rotation delta check and roll over from 0 to 360 degrees.

* style

* Update CHANGELOG.md

* Update CHANGELOG.md

* fix

Minor adjustment

* style

fixing merge issue

* test fix

Making the StopStartRuntimeTests derive from the NetcodeIntegrationTest to avoid the scenario where a specific platform/desktop is running slow for some reason and it take longer than the default NetcodeIntegrationTestHelpers.WaitForClientsConnected's 1 second.  Since it now derives from NetcodeIntegrationTest it will also handle cleaning up everything properly in the event the clients truly cannot connect. and not leave behind artifacts in the test runner scene and/or DDOL that would cause all tests that proceeded StopStartRuntimeTests to fail as well.

* update

removing ThresholdMinimum

Author: NoelStephensUnity

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);
         }
     }
 }