fix: networklog can throw exception when no networkmanager exists (#3917)

* fix

Fixing exception that could be thrown in NetworkLog if there is no NetworkManager instance.
Adding additional check in NetworkObject.OnNetworkBehaviourDestroyed to assure nothing is ever logged about destroying a NetworkBehaviour when shutting down and the NetworkObject is still considered spawned.

* test

The test that validates the NetworkLog fix.

* update

Adding change log entry for this fix.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -23,6 +23,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed issue where attempts to use `NetworkLog` when there is no `NetworkManager` instance would result in an exception. (#3917)
 
 ### Security
 

com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs

@@ -1020,6 +1020,14 @@ private void OnTransformParentChanged()
             NetworkManagerCheckForParent();
         }
 
+        /// <summary>
+        /// For testing purposes when you need the singleton to be null
+        /// </summary>
+        internal static void ResetSingleton()
+        {
+            Singleton = null;
+        }
+
         /// <summary>
         /// Set this NetworkManager instance as the static NetworkManager singleton
         /// </summary>

com.unity.netcode.gameobjects/Runtime/Core/NetworkObject.cs

@@ -3647,7 +3647,8 @@ internal void OnNetworkBehaviourDestroyed(NetworkBehaviour networkBehaviour)
         {
             if (networkBehaviour.IsSpawned && IsSpawned)
             {
-                if (NetworkManagerOwner.LogLevel <= LogLevel.Developer)
+                // Only log this warning if we are not shutting down.
+                if (!NetworkManagerOwner.ShutdownInProgress && NetworkManagerOwner.LogLevel <= LogLevel.Developer)
                 {
                     NetworkLog.LogWarning($"{nameof(NetworkBehaviour)}-{networkBehaviour.name} is being destroyed while {nameof(NetworkObject)}-{name} is still spawned! (could break state synchronization)");
                 }

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

@@ -113,14 +113,21 @@ private static void LogServer(string message, LogType logType)
             }
         }
 
+        private const string k_HeaderStart = "Netcode";
         private static string Header()
         {
             var networkManager = NetworkManagerOverride ??= NetworkManager.Singleton;
-            if (networkManager.DistributedAuthorityMode)
+            if (networkManager != null)
             {
-                return "Session-Owner";
+                if (networkManager.DistributedAuthorityMode)
+                {
+                    return $"{k_HeaderStart}-Session-Owner";
+                }
+                return $"{k_HeaderStart}-Server";
             }
-            return "Netcode-Server";
+
+            // If NetworkManager no longer exists, then return the generic header
+            return k_HeaderStart;
         }
 
         internal static void LogInfoServerLocal(string message, ulong sender) => Debug.Log($"[{Header()} Sender={sender}] {message}");

com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs

@@ -445,7 +445,7 @@ public IEnumerator ChangeOwnershipWithoutObservers()
             authorityInstance.ChangeOwnership(otherClient.LocalClientId);
             var senderId = authority.LocalClientId;
             var receiverId = otherClient.LocalClientId;
-            LogAssert.Expect(LogType.Warning, $"[Session-Owner Sender={senderId}] [Invalid Owner] Cannot send Ownership change as client-{receiverId} cannot see {authorityInstance.name}! Use NetworkShow first.");
+            LogAssert.Expect(LogType.Warning, $"[Netcode-Session-Owner Sender={senderId}] [Invalid Owner] Cannot send Ownership change as client-{receiverId} cannot see {authorityInstance.name}! Use NetworkShow first.");
             Assert.True(authorityInstance.IsOwner, $"[Ownership Check] Client-{senderId} should still own this object!");
 
             // Now re-add the client to the Observers list and try to change ownership

com.unity.netcode.gameobjects/Tests/Runtime/NetworkLogTests.cs

@@ -0,0 +1,47 @@
+using System.Collections;
+using Unity.Netcode.TestHelpers.Runtime;
+using UnityEngine.TestTools;
+
+namespace Unity.Netcode.RuntimeTests
+{
+    /// <summary>
+    /// Validates edge cases with <see cref="NetworkLog"/>
+    /// </summary>
+    internal class NetworkLogTests : NetcodeIntegrationTest
+    {
+        protected override int NumberOfClients => 0;
+        private bool m_ServerStopped;
+
+        /// <summary>
+        /// Validates that if no <see cref="NetworkManager"/> exists,
+        /// you can still use NetworkLog with the caveat when one does
+        /// not exist it will only log locally.
+        /// (is topology agnostic)
+        /// </summary>
+        [UnityTest]
+        public IEnumerator UseNetworkLogWithNoNetworkManager()
+        {
+            m_ServerStopped = false;
+            var authority = GetAuthorityNetworkManager();
+            authority.OnServerStopped += OnServerStopped;
+            authority.Shutdown();
+            yield return WaitForConditionOrTimeOut(() => m_ServerStopped);
+            AssertOnTimeout($"Timed out waiting for {nameof(NetworkManager)} to stop!");
+            // Assure it is destroyed.
+            UnityEngine.Object.Destroy(authority);
+            authority = null;
+
+            // Clear out the singleton to assure NetworkLog has no references to a NetworkManager
+            NetworkManager.ResetSingleton();
+
+            // Validate you can use NetworkLog without any NetworkManager instance.
+            NetworkLog.LogInfoServer($"Test a message to the server with no  {nameof(NetworkManager)}.");
+            // No exceptions thrown is considered passing.
+        }
+
+        private void OnServerStopped(bool obj)
+        {
+            m_ServerStopped = true;
+        }
+    }
+}