test: Refer to the single non-authority instance directly

There is one connected client, so the per instance dictionaries and the
loops over m_NetworkManagers only ever held one entry. Resolve the
non-authority instance once through GetNonAuthorityNetworkManager and
name the two sampling frame counts that were inline literals.

Author: NoelStephensUnity

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

@@ -1,5 +1,4 @@
 using System.Collections;
-using System.Collections.Generic;
 using NUnit.Framework;
 using Unity.Netcode.Components;
 using Unity.Netcode.TestHelpers.Runtime;
@@ -12,9 +11,9 @@ namespace Unity.Netcode.RuntimeTests
     /// Validates that <see cref="NetworkTransform.UseHalfFloatPrecision"/> does not introduce motion of its own.
     /// </summary>
     /// <remarks>
-    /// Both tests move the authority in one direction only and require non-authority instances to follow without
-    /// ever moving backwards. Interpolation cannot overshoot, so any movement opposite to the authority's has to
-    /// have come from how the position was encoded rather than from the authority.
+    /// Both tests move the authority in one direction only and require the non-authority instance to follow
+    /// without ever moving backwards. Interpolation cannot overshoot, so any movement opposite to the direction
+    /// the authority moved came from how the position was encoded.
     /// <br /><br />
     /// These do not use the time travel harness because the behavior only appears over multiple real state update
     /// and interpolation cycles.
@@ -25,35 +24,37 @@ internal class NetworkTransformHalfFloatPrecisionTests : IntegrationTestWithAppr
     {
         protected override int NumberOfClients => 1;
 
-        /// <summary>
-        /// How far the object travels before the position is checked.
-        /// </summary>
-        /// <remarks>
-        /// Half float resolution gets coarser the further the object is from the base position established when it
-        /// spawned, so the object has to travel away from that base for the resolution to be worth testing.
-        /// </remarks>
+        // Half float resolution gets coarser the further the object is from the base position established when it
+        // spawned, so it has to travel away from that base for the resolution to be worth testing.
         private const float k_TravelDistance = 30.0f;
 
         private const float k_TravelStep = 1.5f;
 
-        // Moves the object off a position that a half float can represent exactly, which is a position that leaves
-        // no rounding loss behind and so cannot show the problem being tested for.
+        // Moves the object off a position that a half float can represent exactly, which leaves no rounding loss
+        // behind and so cannot show the problem being tested for.
         private const float k_UnrepresentableOffset = 0.0007f;
 
         // Small enough per update that the encoding cannot represent the change on its own.
         private const float k_CreepStep = 0.0005f;
 
         private const int k_CreepTicks = 60;
 
+        // Frames sampled after the creep, while the last states sent are still being interpolated.
+        private const int k_SettleFrames = 30;
+
+        // Frames sampled with nothing moving at all.
+        private const int k_StationaryFrames = 120;
+
         // Tolerated backwards movement, which is float noise only. Well below the roughly 1mm resolution.
         private const float k_MonotonicEpsilon = 1e-5f;
 
         private GameObject m_TestPrefab;
         private NetworkManager m_AuthorityNetworkManager;
         private NetworkTransform m_AuthorityInstance;
+        private NetworkTransform m_NonAuthorityInstance;
 
-        private readonly Dictionary<NetworkTransform, float> m_WorstRegression = new Dictionary<NetworkTransform, float>();
-        private readonly Dictionary<NetworkTransform, float> m_LastObserved = new Dictionary<NetworkTransform, float>();
+        private float m_WorstRegression;
+        private float m_LastObserved;
 
         private int m_TicksApplied;
         private float m_StepThisPhase;
@@ -93,22 +94,9 @@ protected override void OnServerAndClientsCreated()
             base.OnServerAndClientsCreated();
         }
 
-        private bool AllInstancesCaughtUp()
+        private bool NonAuthorityCaughtUp()
         {
-            var authority = GetAuthorityNetworkManager();
-            foreach (var networkManager in m_NetworkManagers)
-            {
-                if (networkManager == authority)
-                {
-                    continue;
-                }
-                var nonAuthority = networkManager.SpawnManager.SpawnedObjects[m_AuthorityInstance.NetworkObjectId];
-                if (!Approximately(nonAuthority.transform.position, m_AuthorityInstance.transform.position))
-                {
-                    return false;
-                }
-            }
-            return true;
+            return Approximately(m_NonAuthorityInstance.transform.position, m_AuthorityInstance.transform.position);
         }
 
         /// <summary>
@@ -120,53 +108,23 @@ private bool AllInstancesCaughtUp()
         /// </remarks>
         private void SampleForRegression()
         {
-            var authority = GetAuthorityNetworkManager();
-            foreach (var networkManager in m_NetworkManagers)
-            {
-                if (networkManager == authority)
-                {
-                    continue;
-                }
-                var nonAuthority = networkManager.SpawnManager.SpawnedObjects[m_AuthorityInstance.NetworkObjectId].GetComponent<NetworkTransform>();
-                var current = nonAuthority.transform.position.x;
-                if (m_LastObserved.TryGetValue(nonAuthority, out var previous))
-                {
-                    var regression = previous - current;
-                    if (regression > m_WorstRegression[nonAuthority])
-                    {
-                        m_WorstRegression[nonAuthority] = regression;
-                    }
-                }
-                m_LastObserved[nonAuthority] = current;
-            }
+            var current = m_NonAuthorityInstance.transform.position.x;
+            m_WorstRegression = Mathf.Max(m_WorstRegression, m_LastObserved - current);
+            m_LastObserved = current;
         }
 
         private void BeginSampling()
         {
-            m_WorstRegression.Clear();
-            m_LastObserved.Clear();
-            var authority = GetAuthorityNetworkManager();
-            foreach (var networkManager in m_NetworkManagers)
-            {
-                if (networkManager == authority)
-                {
-                    continue;
-                }
-                var nonAuthority = networkManager.SpawnManager.SpawnedObjects[m_AuthorityInstance.NetworkObjectId].GetComponent<NetworkTransform>();
-                m_WorstRegression.Add(nonAuthority, 0.0f);
-                m_LastObserved.Add(nonAuthority, nonAuthority.transform.position.x);
-            }
+            m_WorstRegression = 0.0f;
+            m_LastObserved = m_NonAuthorityInstance.transform.position.x;
         }
 
         private void AssertNoRegression(string phase)
         {
-            foreach (var entry in m_WorstRegression)
-            {
-                Assert.LessOrEqual(entry.Value, k_MonotonicEpsilon,
-                    $"[{phase}] {entry.Key.NetworkManager.name} moved {entry.Value} backwards along X while the " +
-                    $"authority only ever moved forwards. Interpolation cannot overshoot, so this motion was " +
-                    $"introduced by the half float position encoding rather than reproduced from the authority.");
-            }
+            Assert.LessOrEqual(m_WorstRegression, k_MonotonicEpsilon,
+                $"[{phase}] {m_NonAuthorityInstance.NetworkManager.name} moved {m_WorstRegression} backwards along " +
+                $"X while the authority only ever moved forwards. Interpolation cannot overshoot, so this motion " +
+                $"was introduced by the half float position encoding rather than reproduced from the authority.");
         }
 
         /// <summary>
@@ -194,24 +152,34 @@ private IEnumerator DriveAuthority(float stepPerTick, int ticks)
         }
 
         /// <summary>
-        /// Moves an object away from its base position and then moves it forward in very small steps, requiring
-        /// every non-authority instance to follow without ever moving backwards.
+        /// Spawns the test object and travels it away from the base position established when it spawned.
         /// </summary>
-        /// <returns>An <see cref="IEnumerator"/> for the test coroutine.</returns>
-        [UnityTest]
-        public IEnumerator HalfFloatPrecisionDoesNotInvertMotion()
+        private IEnumerator SpawnAndTravel()
         {
             m_AuthorityNetworkManager = GetAuthorityNetworkManager();
             m_AuthorityInstance = SpawnObject(m_TestPrefab, m_AuthorityNetworkManager).GetComponent<NetworkTransform>();
 
-            yield return WaitForSpawnedOnAllOrTimeOut(m_AuthorityInstance.gameObject);
+            yield return WaitForSpawnedOnAllOrTimeOut(m_AuthorityInstance.NetworkObject);
             AssertOnTimeout($"Not all clients spawned {m_AuthorityInstance.name}!");
 
-            var travelTicks = (int)(k_TravelDistance / k_TravelStep);
-            yield return DriveAuthority(k_TravelStep, travelTicks);
+            var nonAuthority = GetNonAuthorityNetworkManager();
+            m_NonAuthorityInstance = nonAuthority.SpawnManager.SpawnedObjects[m_AuthorityInstance.NetworkObjectId].GetComponent<NetworkTransform>();
+
+            yield return DriveAuthority(k_TravelStep, (int)(k_TravelDistance / k_TravelStep));
+        }
+
+        /// <summary>
+        /// Moves an object away from its base position and then moves it forward in very small steps, requiring
+        /// the non-authority instance to follow without ever moving backwards.
+        /// </summary>
+        /// <returns>An <see cref="IEnumerator"/> for the test coroutine.</returns>
+        [UnityTest]
+        public IEnumerator HalfFloatPrecisionDoesNotInvertMotion()
+        {
+            yield return SpawnAndTravel();
 
-            yield return WaitForConditionOrTimeOut(AllInstancesCaughtUp);
-            AssertOnTimeout("Non-authority instances did not catch up to the authority after the travel phase.");
+            yield return WaitForConditionOrTimeOut(NonAuthorityCaughtUp);
+            AssertOnTimeout("The non-authority instance did not catch up to the authority after the travel phase.");
 
             BeginSampling();
             m_TicksApplied = 0;
@@ -225,7 +193,7 @@ public IEnumerator HalfFloatPrecisionDoesNotInvertMotion()
             m_AuthorityNetworkManager.NetworkTickSystem.Tick -= OnNetworkTick;
 
             // Keep sampling while the last sent states are still being interpolated.
-            for (var i = 0; i < 30; i++)
+            for (var i = 0; i < k_SettleFrames; i++)
             {
                 SampleForRegression();
                 yield return null;
@@ -234,8 +202,8 @@ public IEnumerator HalfFloatPrecisionDoesNotInvertMotion()
             AssertNoRegression("creep");
 
             // Small movements still have to arrive rather than be discarded.
-            yield return WaitForConditionOrTimeOut(AllInstancesCaughtUp);
-            AssertOnTimeout($"Non-authority instances did not converge on the authority position " +
+            yield return WaitForConditionOrTimeOut(NonAuthorityCaughtUp);
+            AssertOnTimeout($"The non-authority instance did not converge on the authority position " +
                 $"{m_AuthorityInstance.transform.position} after creeping, which means slow motion is being " +
                 $"discarded rather than transmitted.");
         }
@@ -247,28 +215,21 @@ public IEnumerator HalfFloatPrecisionDoesNotInvertMotion()
         [UnityTest]
         public IEnumerator HalfFloatPrecisionHoldsStillWhenStationary()
         {
-            m_AuthorityNetworkManager = GetAuthorityNetworkManager();
-            m_AuthorityInstance = SpawnObject(m_TestPrefab, m_AuthorityNetworkManager).GetComponent<NetworkTransform>();
-
-            yield return WaitForSpawnedOnAllOrTimeOut(m_AuthorityInstance.gameObject);
-            AssertOnTimeout($"Not all clients spawned {m_AuthorityInstance.name}!");
-
-            var travelTicks = (int)(k_TravelDistance / k_TravelStep);
-            yield return DriveAuthority(k_TravelStep, travelTicks);
+            yield return SpawnAndTravel();
 
             // A position that a half float happens to represent exactly leaves no rounding loss behind, and with
             // no rounding loss there is nothing that could move the object. Offsetting by less than the encoding
             // can represent guarantees there is some, which is the state a settling object is normally left in.
             yield return DriveAuthority(k_UnrepresentableOffset, 1);
 
-            yield return WaitForConditionOrTimeOut(AllInstancesCaughtUp);
-            AssertOnTimeout("Non-authority instances did not catch up to the authority after the travel phase.");
+            yield return WaitForConditionOrTimeOut(NonAuthorityCaughtUp);
+            AssertOnTimeout("The non-authority instance did not catch up to the authority after the travel phase.");
 
-            // Nothing moves for the rest of the test, so the authority's last direction was forwards. Checking for
-            // backwards movement rather than for drift from a starting point means the instances are still free to
-            // finish interpolating towards the authority without that counting against them.
+            // Nothing moves for the rest of the test, so the last direction the authority moved was forwards.
+            // Checking for backwards movement rather than for drift from a starting point means the instance is
+            // still free to finish interpolating towards the authority without that counting against it.
             BeginSampling();
-            for (var i = 0; i < 120; i++)
+            for (var i = 0; i < k_StationaryFrames; i++)
             {
                 SampleForRegression();
                 yield return null;