fixes based off of code review

Author: EmandM

com.unity.netcode.gameobjects/Runtime/Logging/ContextualLogger.cs

@@ -18,15 +18,20 @@ namespace Unity.Netcode.Logging
     /// </summary>
     internal class ContextualLogger
     {
+        private const string k_CompilationCondition = "UNITY_ASSERTIONS";
+
         private const string k_NetcodeHeader = "[Netcode] ";
-        private readonly bool m_UseCompatibilityMode;
         private readonly Object m_Object;
         private readonly LogBuilder m_Builder = new();
 
         private LogContextNetworkManager m_ManagerContext;
         private readonly GenericContext m_LoggerContext;
 
-        private const string k_CompilationCondition = "UNITY_ASSERTIONS";
+        /// <summary>
+        /// Compatibility mode with the old behavior of NetworkLog
+        /// TODO: remove this when enough of the codebase is using the new log system
+        /// </summary>
+        private readonly bool m_UseCompatibilityMode;
 
         /// <summary>
         /// Creates a minimally configured contextual logger
@@ -54,11 +59,13 @@ public ContextualLogger(Object inspectorObject, [NotNull] NetworkManager network
             m_LoggerContext = GenericContext.Create();
         }
 
-        [Conditional(k_CompilationCondition)]
-        internal void UpdateNetworkManagerContext(NetworkManager manager)
+        /// Used for the NetworkLog
+        internal ContextualLogger(NetworkManager networkManager,  bool useCompatibilityMode)
         {
-            m_ManagerContext.Dispose();
-            m_ManagerContext = new LogContextNetworkManager(manager);
+            m_UseCompatibilityMode = useCompatibilityMode;
+            m_ManagerContext = new LogContextNetworkManager(networkManager);
+            m_Object = networkManager;
+            m_LoggerContext = GenericContext.Create();
         }
 
         [Conditional(k_CompilationCondition)]

com.unity.netcode.gameobjects/Runtime/Logging/NetworkLog.cs

@@ -17,17 +17,24 @@ internal struct LogConfiguration
     public static class NetworkLog
     {
         internal static LogConfiguration Config;
-        private static readonly ContextualLogger k_Log = new(true);
+        private static ContextualLogger s_Log = new(true);
 
-        internal static void SetNetworkManager(NetworkManager networkManager)
+        /// <summary>
+        /// Configures the NetworkLog for integration tests.
+        /// </summary>
+        internal static void ConfigureIntegrationTestLogging(NetworkManager networkManager, bool enableVerboseDebug = false)
         {
-            k_Log.UpdateNetworkManagerContext(networkManager);
+            // useCompatibilityMode when verboseDebug is not enabled
+            s_Log = new ContextualLogger(networkManager, !enableVerboseDebug);
+            // This setting will do nothing if the logger is created with useCompatibilityMode=true
+            Config.LogNetworkManagerRole = enableVerboseDebug;
         }
+
         /// <summary>
         /// Gets the current log level.
         /// </summary>
         /// <value>The current log level.</value>
-        // [Obsolete("Use the LogLevel directly on the NetworkManager instead")]
+        // TODO: Work on deprecating this field.
         public static LogLevel CurrentLogLevel => NetworkManager.Singleton == null ? LogLevel.Normal : NetworkManager.Singleton.LogLevel;
 
         // internal logging
@@ -37,27 +44,27 @@ internal static void SetNetworkManager(NetworkManager networkManager)
         /// </summary>
         /// <param name="message">The message to log</param>
         [HideInCallstack]
-        public static void LogInfo(string message) => k_Log.Info(new Context(LogLevel.Normal, message, true));
+        public static void LogInfo(string message) => s_Log.Info(new Context(LogLevel.Normal, message, true));
         [HideInCallstack]
-        internal static void LogInfo(Context context) => k_Log.Info(context);
+        internal static void LogInfo(Context context) => s_Log.Info(context);
 
         /// <summary>
         /// Locally logs a warning log with Netcode prefixing.
         /// </summary>
         /// <param name="message">The message to log</param>
         [HideInCallstack]
-        public static void LogWarning(string message) => k_Log.Warning(new Context(LogLevel.Error, message, true));
+        public static void LogWarning(string message) => s_Log.Warning(new Context(LogLevel.Error, message, true));
         [HideInCallstack]
-        internal static void LogWarning(Context context) => k_Log.Warning(context);
+        internal static void LogWarning(Context context) => s_Log.Warning(context);
 
         /// <summary>
         /// Locally logs a error log with Netcode prefixing.
         /// </summary>
         /// <param name="message">The message to log</param>
         [HideInCallstack]
-        public static void LogError(string message) => k_Log.Error(new Context(LogLevel.Error, message, true));
+        public static void LogError(string message) => s_Log.Error(new Context(LogLevel.Error, message, true));
         [HideInCallstack]
-        internal static void LogError(Context context) => k_Log.Error(context);
+        internal static void LogError(Context context) => s_Log.Error(context);
 
         // internal static void Log(LogLevel level, object message, Object gameObject) => Logger.Log($"[Netcode] {message} ({(int)level})");
 
@@ -66,28 +73,28 @@ internal static void SetNetworkManager(NetworkManager networkManager)
         /// </summary>
         /// <param name="message">The message to log</param>
         [HideInCallstack]
-        public static void LogInfoServer(string message) => k_Log.InfoServer(new Context(LogLevel.Normal, message, true));
+        public static void LogInfoServer(string message) => s_Log.InfoServer(new Context(LogLevel.Normal, message, true));
 
         /// <summary>
         /// Logs an info log locally and on the session owner if possible.
         /// </summary>
         /// <param name="message">The message to log</param>
         [HideInCallstack]
-        public static void LogInfoSessionOwner(string message) => k_Log.InfoServer(new Context(LogLevel.Normal, message, true));
+        public static void LogInfoSessionOwner(string message) => s_Log.InfoServer(new Context(LogLevel.Normal, message, true));
 
         /// <summary>
         /// Logs a warning log locally and on the server if possible.
         /// </summary>
         /// <param name="message">The message to log</param>
         [HideInCallstack]
-        public static void LogWarningServer(string message) => k_Log.WarningServer(new Context(LogLevel.Error, message, true));
+        public static void LogWarningServer(string message) => s_Log.WarningServer(new Context(LogLevel.Error, message, true));
 
         /// <summary>
         /// Logs an error log locally and on the server if possible.
         /// </summary>
         /// <param name="message">The message to log</param>
         [HideInCallstack]
-        public static void LogErrorServer(string message) => k_Log.ErrorServer(new Context(LogLevel.Error, message, true));
+        public static void LogErrorServer(string message) => s_Log.ErrorServer(new Context(LogLevel.Error, message, true));
 
         internal static LogType GetMessageLogType(UnityEngine.LogType engineLogType)
         {

com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectDestroyTests.cs

@@ -152,7 +152,7 @@ public IEnumerator TestNetworkObjectClientDestroy([Values] ClientDestroyObject c
                 // The non-authority client is =NOT= allowed to destroy any spawned object it does not
                 // have authority over during runtime.
                 LogAssert.ignoreFailingMessages = true;
-                NetworkLog.SetNetworkManager(nonAuthorityClient);
+                NetworkLog.ConfigureIntegrationTestLogging(nonAuthorityClient);
                 Object.Destroy(clientPlayerClone.gameObject);
             }
 

com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs

@@ -594,7 +594,6 @@ private void InternalOnOneTimeSetup()
             IntegrationTestSceneHandler.VerboseDebugMode = m_EnableVerboseDebug;
             NetworkManagerHelper.VerboseDebugMode = m_EnableVerboseDebug;
             VerboseDebug($"Entering {nameof(OneTimeSetup)}");
-            // NetworkLog.Config.LogNetworkManagerRole = true;
 
             m_NetworkManagerInstatiationMode = OnSetIntegrationTestMode();
 
@@ -809,7 +808,7 @@ protected void CreateServerAndClients(int numberOfClients)
             m_NumberOfClients = numberOfClients;
             m_ClientNetworkManagers = clients;
             m_ServerNetworkManager = server;
-            NetworkLog.SetNetworkManager(server);
+            NetworkLog.ConfigureIntegrationTestLogging(server, m_EnableVerboseDebug);
 
             var managers = clients.ToList();
             if (!m_UseCmbService)