fix: Hide spammy & debug draw NetworkTransform behind define [MTT-1262] (#1286)

* chore: Hide spammy & debug draw NetworkTransform behind define

- Moves debug draw & local-write warnings in NT behind a debug define
- Fixes warning message format
- Fixes teleport exception message

Future changes not addressed here:
- Debug draw should be via gizmos, not the current implementation
- Spammy log messages shouldn't be so spammy, and shouldn't be so expensive to detect

* fix: Test log verification workaround.

We have a test which verifies log output. Updated the test to match same parameters (that is, define guard and developer log level guard).

These tests need to be properly refactored post-1.0

* test: guard regex using in test file with NGO_TRANSFORM_DEBUG

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

Author: Jesse Olmer

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

@@ -298,8 +298,6 @@ public void NetworkSerialize<T>(BufferSerializer<T> serializer) where T : IReade
         private int m_LastSentTick;
         private NetworkTransformState m_LastSentState;
 
-        private const string k_NoAuthorityMessage = "A local change to {dirtyField} without authority detected, reverting back to latest interpolated network state!";
-
 
         /// <summary>
         /// Tries updating the server authoritative transform, only if allowed.
@@ -852,25 +850,28 @@ protected virtual void Update()
 
                 if (!CanCommitToTransform)
                 {
+#if NGO_TRANSFORM_DEBUG
                     if (m_CachedNetworkManager.LogLevel == LogLevel.Developer)
                     {
+                        // TODO: This should be a component gizmo - not some debug draw based on log level
                         var interpolatedPosition = new Vector3(m_PositionXInterpolator.GetInterpolatedValue(), m_PositionYInterpolator.GetInterpolatedValue(), m_PositionZInterpolator.GetInterpolatedValue());
                         Debug.DrawLine(interpolatedPosition, interpolatedPosition + Vector3.up, Color.magenta, k_DebugDrawLineTime, false);
-                    }
 
-                    // try to update previously consumed NetworkState
-                    // if we have any changes, that means made some updates locally
-                    // we apply the latest ReplNetworkState again to revert our changes
-                    var oldStateDirtyInfo = ApplyTransformToNetworkStateWithInfo(ref m_PrevNetworkState, 0, m_Transform);
-
-                    // there is a bug in this code, as we the message is dumped out under odd circumstances
-                    if (oldStateDirtyInfo.isPositionDirty || oldStateDirtyInfo.isScaleDirty || (oldStateDirtyInfo.isRotationDirty && SyncRotAngleX && SyncRotAngleY && SyncRotAngleZ))
-                    {
-                        // ignoring rotation dirty since quaternions will mess with euler angles, making this impossible to determine if the change to a single axis comes
-                        // from an unauthorized transform change or euler to quaternion conversion artifacts.
-                        var dirtyField = oldStateDirtyInfo.isPositionDirty ? "position" : oldStateDirtyInfo.isRotationDirty ? "rotation" : "scale";
-                        Debug.LogWarning(dirtyField + k_NoAuthorityMessage, this);
+                        // try to update previously consumed NetworkState
+                        // if we have any changes, that means made some updates locally
+                        // we apply the latest ReplNetworkState again to revert our changes
+                        var oldStateDirtyInfo = ApplyTransformToNetworkStateWithInfo(ref m_PrevNetworkState, 0, m_Transform);
+
+                        // there are several bugs in this code, as we the message is dumped out under odd circumstances
+                        if (oldStateDirtyInfo.isPositionDirty || oldStateDirtyInfo.isScaleDirty || (oldStateDirtyInfo.isRotationDirty && SyncRotAngleX && SyncRotAngleY && SyncRotAngleZ))
+                        {
+                            // ignoring rotation dirty since quaternions will mess with euler angles, making this impossible to determine if the change to a single axis comes
+                            // from an unauthorized transform change or euler to quaternion conversion artifacts.
+                            var dirtyField = oldStateDirtyInfo.isPositionDirty ? "position" : oldStateDirtyInfo.isRotationDirty ? "rotation" : "scale";
+                            Debug.LogWarning($"A local change to {dirtyField} without authority detected, reverting back to latest interpolated network state!", this);
+                        }
                     }
+#endif
 
                     // Apply updated interpolated value
                     ApplyInterpolatedNetworkStateToTransform(m_ReplicatedNetworkState.Value, m_Transform);
@@ -887,7 +888,7 @@ public void Teleport(Vector3 newPosition, Quaternion newRotation, Vector3 newSca
         {
             if (!CanCommitToTransform)
             {
-                throw new Exception("Teleport not allowed, " + k_NoAuthorityMessage);
+                throw new Exception("Teleport not allowed");
             }
 
             var newRotationEuler = newRotation.eulerAngles;

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

@@ -1,6 +1,8 @@
 using System;
 using System.Collections;
+#if NGO_TRANSFORM_DEBUG
 using System.Text.RegularExpressions;
+#endif
 using Unity.Netcode.Components;
 using NUnit.Framework;
 // using Unity.Netcode.Samples;
@@ -45,6 +47,13 @@ public override IEnumerator Setup()
                 }
             });
 
+#if NGO_TRANSFORM_DEBUG
+            // Log assert for writing without authority is a developer log...
+            // TODO: This is why monolithic test base classes and test helpers are an anti-pattern - this is part of an individual test case setup but is separated from the code verifying it!
+            m_ServerNetworkManager.LogLevel = LogLevel.Developer;
+            m_ClientNetworkManagers[0].LogLevel = LogLevel.Developer;
+#endif
+
             // This is the *SERVER VERSION* of the *CLIENT PLAYER*
             var serverClientPlayerResult = new MultiInstanceHelpers.CoroutineResultWrapper<NetworkObject>();
             yield return MultiInstanceHelpers.Run(MultiInstanceHelpers.GetNetworkObjectByRepresentation(x => x.IsPlayerObject && x.OwnerClientId == m_ClientNetworkManagers[0].LocalClientId, m_ServerNetworkManager, serverClientPlayerResult));
@@ -167,7 +176,10 @@ public IEnumerator TestCantChangeTransformFromOtherSideAuthority([Values] bool t
             yield return null; // one frame
 
             Assert.AreEqual(Vector3.zero, otherSideNetworkTransform.transform.position, "got authority error, but other side still moved!");
+#if NGO_TRANSFORM_DEBUG
+            // TODO: This should be a separate test - verify 1 behavior per test
             LogAssert.Expect(LogType.Warning, new Regex(".*without authority detected.*"));
+#endif
         }
 
         /*