style

Verbosity and clarity pass for the tests added.

Author: NoelStephensUnity

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

@@ -12,11 +12,9 @@ namespace Unity.Netcode.RuntimeTests
     /// measurement sequences and compares them step for step.
     /// </summary>
     /// <remarks>
-    /// The two exist in parallel: the managed one continues to serve
-    /// <see cref="TransformSyncModes.PerInstance"/> and the native one serves
-    /// <see cref="TransformSyncModes.Batched"/>, because the managed one cannot run inside a job. Unlike the
-    /// delta check, which the two synchronization modes genuinely share, there is nothing structural stopping
-    /// these two from drifting apart. This is what stops it.
+    /// The two exist in parallel: the managed one serves <see cref="TransformSyncModes.PerInstance"/> and the
+    /// native one serves <see cref="TransformSyncModes.Batched"/>, because the managed one cannot run inside a
+    /// job. Nothing structural keeps them from drifting apart.
     /// </remarks>
     // These tests do not need to run against the Rust server.
     [IgnoreIfServiceEnvironmentVariableSet]
@@ -33,18 +31,14 @@ internal class NativeInterpolatorTests
         private const float k_Tolerance = 1E-4f;
 
         /// <summary>
-        /// Allowed while a value is still in motion, for the two paths that are known to be sensitive rather
-        /// than exact.
+        /// Allowed while a value is still in motion, for the two paths that are equivalent rather than exact.
         /// </summary>
         /// <remarks>
-        /// Vector slerp is the one replacement in <see cref="NetworkTransformMath"/> that is equivalent rather
-        /// than exact, so its small per step difference compounds through the interpolator's feedback.
+        /// Vector slerp is the one <see cref="NetworkTransformMath"/> replacement that is equivalent rather
+        /// than exact, so its small per step difference compounds through the interpolator's feedback.<br />
         /// Quaternion smooth dampening converts to euler angles, dampens each angle, and converts back every
-        /// frame; near a gimbal transition a sub thousandth of a degree difference in the conversion is enough
-        /// to select a different (equally valid) euler representative, after which the two dampen toward
-        /// different angles. The managed implementation is just as fragile there, so this is the two diverging
-        /// under a shared weakness rather than one of them being wrong.<br /><br />
-        /// What matters is that neither drifts permanently, which is what the settle phase asserts.
+        /// frame. Near a gimbal transition a sub thousandth of a degree difference selects a different, equally
+        /// valid, euler representative, and the two then dampen toward different angles.
         /// </remarks>
         private const float k_TransientTolerance = 5.0f;
 
@@ -88,9 +82,6 @@ private NativeInterpolatorState CreateState(InterpolatorValueKind kind, bool isS
             };
         }
 
-        /// <summary>
-        /// A deterministic motion path, so a failure is reproducible.
-        /// </summary>
         private static Vector3 PositionAt(int tick)
         {
             return new Vector3(
@@ -334,19 +325,14 @@ public void BufferOverflowKeepsNewestMeasurement()
         }
 
         /// <summary>
-        /// An instance that stops being the authority part way through a session resets its interpolator, and
-        /// the measurements that follow are stamped with the tick they were authored on.<br />
-        /// Those stamps are older than the reset, so a seeded baseline would leave both ordering guards
-        /// rejecting them. Neither implementation seeds one, and this holds them to that.
+        /// An interpolator reset part way through a session still accepts the measurements that follow it.
         /// </summary>
         /// <remarks>
-        /// This is the shape of an ownership transfer away from the local instance. It happens to the server
-        /// under a server authoritative motion model and to the previous owner under an owner authoritative
-        /// one.<br />
-        /// It is reproduced here rather than only through
-        /// <c>NetworkTransformSyncModeParityTests.OwnershipChangeKeepsReplicating</c> because the failure is
-        /// entirely internal to the interpolator. Once the buffered measurements are all older than a seeded
-        /// baseline, no amount of elapsed time recovers it.
+        /// The reset stamps its baseline with the local time, while the measurements that follow carry the
+        /// older tick they were authored on. See
+        /// <see cref="BufferedLinearInterpolator{T}.ResetTo(T, double)"/>.<br />
+        /// This is the shape of an ownership transfer away from the local instance: the server under server
+        /// authority, the previous owner under owner authority.
         /// </remarks>
         [Test]
         public void ResetPartWayThroughSessionStillAcceptsOlderStampedMeasurements([Values] bool useManaged)

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

@@ -12,12 +12,10 @@ namespace Unity.Netcode.RuntimeTests
     /// Measures how closely <see cref="NetworkTransformMath"/> agrees with the engine math it replaces.
     /// </summary>
     /// <remarks>
-    /// The replacements exist because the engine equivalents are native bindings that Burst cannot compile.
-    /// Some of them are ports of implementations that are managed C# in the engine and are expected to match
-    /// exactly; the rest are mathematically equivalent but cannot be verified as bit identical because the
-    /// engine's operation order is not observable.<br /><br />
-    /// Each test reports the worst disagreement it found, so a failure states the actual measured error rather
-    /// than just that a threshold was crossed.
+    /// The replacements exist because the engine equivalents are native bindings Burst cannot compile.<br />
+    /// Some are ports of engine code that is managed C#, and those are expected to match exactly.<br />
+    /// The rest are mathematically equivalent, and cannot be verified as bit identical because the engine's
+    /// operation order is not observable.
     /// </remarks>
     // These tests do not need to run against the Rust server.
     [IgnoreIfServiceEnvironmentVariableSet]
@@ -41,10 +39,8 @@ internal class NetworkTransformMathTests
         /// over the value ranges used here, but not bit for bit.
         /// </summary>
         /// <remarks>
-        /// The remaining difference is one rounding step, not an algorithmic one: every other ported function
-        /// (including the scalar <see cref="NetworkTransformMath.SmoothDampAngle"/>, which shares this
-        /// arithmetic) matches exactly. Chasing it would mean guessing at how the engine's build contracts
-        /// multiply and add, and a difference this size is far below anything interpolation can express.
+        /// Not worth closing: it would mean guessing at how the engine's build contracts multiply and add, and
+        /// a difference this size is far below anything interpolation can express.
         /// </remarks>
         private const float k_SingleUlpTolerance = 1E-5f;
 

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

@@ -15,11 +15,8 @@ namespace Unity.Netcode.RuntimeTests
     /// configurations that drive its serialization branches.
     /// </summary>
     /// <remarks>
-    /// This exists as the wire format baseline. Any change to how a state is written (whether intentional or
-    /// not) will fail <see cref="NetworkTransformStateSerializationBaseline"/> with the new signatures, which
-    /// makes an unintended wire change impossible to land silently.<br /><br />
-    /// To (re)generate the baseline: run this test, copy the C# array literal it prints on failure into
-    /// <see cref="k_ExpectedSignatures"/>, and verify every changed entry is an intended wire format change.
+    /// To regenerate: run this test, copy the array literal it prints on failure into
+    /// <see cref="k_ExpectedSignatures"/>, and confirm every changed entry is an intended wire format change.
     /// </remarks>
     // These tests do not need to run against the Rust server.
     [IgnoreIfServiceEnvironmentVariableSet]
@@ -28,9 +25,6 @@ internal class NetworkTransformStateBaselineTests
         /// <summary>
         /// One entry per case in <see cref="BuildStateMatrix"/>, formatted as "name|byteLength|fnv1aHash".
         /// </summary>
-        /// <remarks>
-        /// Empty until generated. See the remarks on the class for how to populate it.
-        /// </remarks>
         private static readonly string[] k_ExpectedSignatures =
         {
             "FullPrecision.AllAxes|42|B454AEF3",
@@ -406,8 +400,8 @@ public void NetworkTransformStateSerializationBaseline()
         /// Verifies every configuration in the matrix survives a write and read back.
         /// </summary>
         /// <remarks>
-        /// The baseline above proves the bytes did not change. This proves those bytes still round trip, which
-        /// keeps a baseline that was regenerated against a broken serializer from being accepted.
+        /// The baseline proves the bytes did not change. This proves they still round trip, so a baseline
+        /// regenerated against a broken serializer is not silently accepted.
         /// </remarks>
         [Test]
         public void NetworkTransformStateSerializationRoundTrip()
@@ -460,10 +454,9 @@ public void NetworkTransformStateSerializationRoundTrip()
         /// <see cref="FlagStates.ReliableSequenced"/>.
         /// </summary>
         /// <remarks>
-        /// <see cref="NetworkTransformState.UpdateReliability"/> is what derives ReliableSequenced, and
-        /// <see cref="Serialize"/> takes the state by value and calls it on that copy.<br />
-        /// The flag therefore reaches the bytes and the state read back from them, but never the source state
-        /// the case holds. Comparing it against the source would always fail.
+        /// <see cref="NetworkTransformState.UpdateReliability"/> derives ReliableSequenced, and
+        /// <see cref="Serialize"/> takes the state by value and calls it on that copy. The flag reaches the
+        /// bytes and the state read back from them, but never the source state the case holds.
         /// </remarks>
         private static void AssertFlagsSurvived(StateCase stateCase, NetworkTransformState deserialized)
         {

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

@@ -12,9 +12,8 @@ namespace Unity.Netcode.RuntimeTests
     /// fixed for the duration of a session.
     /// </summary>
     /// <remarks>
-    /// Nothing covered this before: every other fixture reaches the mode through the same assignment the
-    /// harness makes, so a delivery path that never ran would still leave them all green. The two assertions
-    /// here are the ones that fail if it stops arriving, or if it starts arriving in the middle of a session.
+    /// Every other fixture reaches the mode through the same assignment the harness makes, so none of them
+    /// can see a delivery path that never ran.
     /// </remarks>
     // These tests do not need to run against the Rust server, and batching is client-server only.
     [IgnoreIfServiceEnvironmentVariableSet]
@@ -46,9 +45,6 @@ protected override void OnServerAndClientsCreated()
             base.OnServerAndClientsCreated();
         }
 
-        /// <summary>
-        /// Spawns an instance and returns the authority side <see cref="NetworkTransform"/>.
-        /// </summary>
         private NetworkTransform SpawnMover()
         {
             var instance = Object.Instantiate(m_MoverPrefab);
@@ -71,10 +67,6 @@ private bool IsRegisteredForBatching(NetworkTransform networkTransform)
         /// The authored mode has to reach every <see cref="NetworkManager"/> and route the instances that
         /// spawn under it.
         /// </summary>
-        /// <remarks>
-        /// This is the one that would have caught the mode being configured but never delivered: the traffic
-        /// stayed per instance while everything reporting on the configuration said otherwise.
-        /// </remarks>
         [UnityTest]
         public IEnumerator AuthoredModeReachesTheRuntime()
         {
@@ -92,12 +84,6 @@ public IEnumerator AuthoredModeReachesTheRuntime()
         /// <summary>
         /// Writing the mode while a session is running applies to the next session, not this one.
         /// </summary>
-        /// <remarks>
-        /// The field is public so a project can offer it in its own pre-session UI, which means a mid-session
-        /// write has to be inert rather than trusted. Taking effect immediately would leave already spawned
-        /// instances registered under one mode while new ones use the other, and would move the connection
-        /// configuration hash out from under clients still joining.
-        /// </remarks>
         [UnityTest]
         public IEnumerator ModeChangedDuringASessionDoesNotAffectIt()
         {