fix: Reset NetworkClient upon shutdown (#3491)

This PR addresses some NetworkManager instance clean up when shutdown
including the ConnectionManager's NetworkClient.
This PR also includes a clean up for NetworkObject upon despawning in
the event one is using an object pool.

[MTT-12384](https://jira.unity3d.com/browse/MTT-12384)

## Changelog

- Fixed: Issue where `NetworkClient` could persist some settings if
re-using the same `NetworkManager` instance.
- Fixed: Issue where a pooled `NetworkObject` was not resetting the
internal latest parent property when despawned.

## Testing and Documentation

- Includes integration test updates.
- No documentation changes or additions were necessary.

<!-- Uncomment and mark items off with a * if this PR deprecates any
API:
### Deprecated API
- [ ] An `[Obsolete]` attribute was added along with a `(RemovedAfter
yyyy-mm-dd)` entry.
- [ ] An [api updater] was added.
- [ ] Deprecation of the API is explained in the CHANGELOG.
- [ ] The users can understand why this API was removed and what they
should use instead.
-->

## Backport
This requires a back port for pertinent/similar areas. (#3494)

<!-- If this is a backport:
 - Add the following to the PR title: "\[Backport\] ..." .
 - Link to the original PR.
If this needs a backport - state this here
If a backport is not needed please provide the reason why.
If the "Backports" section is not present it will lead to a CI test
failure.
-->

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -12,6 +12,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed issue where `NetworkClient` could persist some settings if re-using the same `NetworkManager` instance. (#3491)
+- Fixed issue where a pooled `NetworkObject` was not resetting the internal latest parent property when despawned. (#3491)
 - Fixed issue where the initial client synchronization pre-serialization process was not excluding spawned `NetworkObject` instances that already had pending visibility for the client being synchronized. (#3488)
 - Fixed issue where there was a potential for a small memory leak in the `ConnectionApprovedMessage`. (#3486)
 

com.unity.netcode.gameobjects/Runtime/Connection/NetworkConnectionManager.cs

@@ -1512,8 +1512,11 @@ internal void Shutdown()
                 }
             }
 
+            // Reset the approved and connectd flags
             LocalClient.IsApproved = false;
             LocalClient.IsConnected = false;
+
+            // Clear all lists
             ConnectedClients.Clear();
             ConnectedClientIds.Clear();
             ConnectedClientsList.Clear();

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

@@ -1590,8 +1590,8 @@ internal void ShutdownInternal()
             // In the event shutdown is invoked within OnClientStopped or OnServerStopped, set it to false again
             m_ShuttingDown = false;
 
-            // Reset the client's roles
-            ConnectionManager.LocalClient.SetRole(false, false);
+            // Completely reset the NetworkClient
+            ConnectionManager.LocalClient = new NetworkClient();
 
             // This cleans up the internal prefabs list
             NetworkConfig?.Prefabs?.Shutdown();

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

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Runtime.CompilerServices;
+using System.Text;
 using Unity.Netcode.Components;
 #if UNITY_EDITOR
 using UnityEditor;
@@ -1959,10 +1960,18 @@ public void Despawn(bool destroy = true)
             {
                 behavior.MarkVariablesDirty(false);
             }
-
             NetworkManager.SpawnManager.DespawnObject(this, destroy);
         }
 
+        internal void ResetOnDespawn()
+        {
+            // Always clear out the observers list when despawned
+            Observers.Clear();
+            IsSpawned = false;
+            DeferredDespawnTick = 0;
+            m_LatestParent = null;
+        }
+
         /// <summary>
         /// Removes all ownership of an object from any client. Can only be called from server
         /// </summary>
@@ -2695,6 +2704,17 @@ internal static void VerifyParentingStatus()
                 if (OrphanChildren.Count > 0)
                 {
                     NetworkLog.LogWarning($"{nameof(NetworkObject)} ({OrphanChildren.Count}) children not resolved to parents by the end of frame");
+                    if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
+                    {
+                        var builder = new StringBuilder();
+                        builder.AppendLine("Orphaned Children:");
+                        foreach (var child in OrphanChildren)
+                        {
+                            builder.Append($"| {child} ");
+                        }
+                        builder.AppendLine("|");
+                        NetworkLog.LogWarning(builder.ToString());
+                    }
                 }
             }
         }
@@ -2744,7 +2764,7 @@ internal NetworkBehaviour GetNetworkBehaviourAtOrderIndex(ushort index)
                 }
                 if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
                 {
-                    var currentKnownChildren = new System.Text.StringBuilder();
+                    var currentKnownChildren = new StringBuilder();
                     currentKnownChildren.Append($"Known child {nameof(NetworkBehaviour)}s:");
                     for (int i = 0; i < ChildNetworkBehaviours.Count; i++)
                     {

com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs

@@ -1717,8 +1717,8 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec
                 }
             }
 
-            networkObject.IsSpawned = false;
-            networkObject.DeferredDespawnTick = 0;
+            // Reset the NetworkObject when despawned.
+            networkObject.ResetOnDespawn();
 
             if (SpawnedObjects.Remove(networkObject.NetworkObjectId))
             {
@@ -1730,9 +1730,6 @@ internal void OnDespawnObject(NetworkObject networkObject, bool destroyGameObjec
                 RemovePlayerObject(networkObject, destroyGameObject);
             }
 
-            // Always clear out the observers list when despawned
-            networkObject.Observers.Clear();
-
             var gobj = networkObject.gameObject;
             if (destroyGameObject && gobj != null)
             {

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

@@ -46,7 +46,7 @@ public enum ClientDisconnectType
         private ulong m_ClientId;
 
 
-        public DisconnectTests(OwnerPersistence ownerPersistence)
+        public DisconnectTests(OwnerPersistence ownerPersistence) : base(HostOrServer.Host)
         {
             m_OwnerPersistence = ownerPersistence;
         }
@@ -202,6 +202,15 @@ public IEnumerator ClientPlayerDisconnected([Values] ClientDisconnectType client
             yield return WaitForConditionOrTimeOut(TransportIdCleanedUp);
             AssertOnTimeout("Timed out waiting for transport and client id mappings to be cleaned up!");
 
+
+            if (clientNetworkManager.ConnectionManager != null)
+            {
+                Assert.False(clientNetworkManager.ConnectionManager.LocalClient.IsClient, $"{clientNetworkManager.name} still has IsClient setting!");
+                Assert.False(clientNetworkManager.ConnectionManager.LocalClient.IsConnected, $"{clientNetworkManager.name} still has IsConnected setting!");
+                Assert.False(clientNetworkManager.ConnectionManager.LocalClient.ClientId != 0, $"{clientNetworkManager.name} still has ClientId ({clientNetworkManager.ConnectionManager.LocalClient.ClientId}) setting!");
+                Assert.False(clientNetworkManager.ConnectionManager.LocalClient.IsApproved, $"{clientNetworkManager.name} still has IsApproved setting!");
+                Assert.IsNull(clientNetworkManager.ConnectionManager.LocalClient.PlayerObject, $"{clientNetworkManager.name} still has Player assigned!");
+            }
             // Validate the host-client generates a OnClientDisconnected event when it shutsdown.
             // Only test when the test run is the client disconnecting from the server (otherwise the server will be shutdown already)
             if (clientDisconnectType == ClientDisconnectType.ClientDisconnectsFromServer)
@@ -215,6 +224,15 @@ public IEnumerator ClientPlayerDisconnected([Values] ClientDisconnectType client
 
                 Assert.IsTrue(m_DisconnectedEvent.ContainsKey(m_ServerNetworkManager), $"Could not find the server {nameof(NetworkManager)} disconnect event entry!");
                 Assert.IsTrue(m_DisconnectedEvent[m_ServerNetworkManager].ClientId == NetworkManager.ServerClientId, $"Expected ClientID {m_ClientId} but found ClientID {m_DisconnectedEvent[m_ServerNetworkManager].ClientId} for the server {nameof(NetworkManager)} disconnect event entry!");
+                yield return s_DefaultWaitForTick;
+                if (m_ServerNetworkManager.ConnectionManager != null)
+                {
+                    Assert.False(m_ServerNetworkManager.ConnectionManager.LocalClient.IsClient, $"{m_ServerNetworkManager.name} still has IsClient setting!");
+                    Assert.False(m_ServerNetworkManager.ConnectionManager.LocalClient.IsConnected, $"{m_ServerNetworkManager.name} still has IsConnected setting!");
+                    Assert.False(m_ServerNetworkManager.ConnectionManager.LocalClient.ClientId != 0, $"{m_ServerNetworkManager.name} still has ClientId ({clientNetworkManager.ConnectionManager.LocalClient.ClientId}) setting!");
+                    Assert.False(m_ServerNetworkManager.ConnectionManager.LocalClient.IsApproved, $"{m_ServerNetworkManager.name} still has IsApproved setting!");
+                    Assert.IsNull(m_ServerNetworkManager.ConnectionManager.LocalClient.PlayerObject, $"{m_ServerNetworkManager.name} still has Player assigned!");
+                }
             }
         }
     }

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

@@ -260,22 +260,23 @@ protected override IEnumerator OnTearDown()
         public IEnumerator TestOnNetworkSpawnCallbacks()
         {
             // [Host-Side] Get the Host owned instance
-            var serverInstance = m_PlayerNetworkObjects[m_ServerNetworkManager.LocalClientId][m_ServerNetworkManager.LocalClientId].GetComponent<TrackOnSpawnFunctions>();
+            var authorityNetworkManager = GetAuthorityNetworkManager();
+            var authorityInstance = m_PlayerNetworkObjects[authorityNetworkManager.LocalClientId][authorityNetworkManager.LocalClientId].GetComponent<TrackOnSpawnFunctions>();
 
             foreach (var client in m_ClientNetworkManagers)
             {
-                var clientRpcTests = m_PlayerNetworkObjects[client.LocalClientId][m_ServerNetworkManager.LocalClientId].gameObject.GetComponent<TrackOnSpawnFunctions>();
+                var clientRpcTests = m_PlayerNetworkObjects[client.LocalClientId][authorityNetworkManager.LocalClientId].gameObject.GetComponent<TrackOnSpawnFunctions>();
                 Assert.IsNotNull(clientRpcTests);
                 m_ClientTrackOnSpawnInstances.Add(clientRpcTests);
             }
 
             // -------------- step 1 check player spawn despawn
 
             // check spawned on server
-            Assert.AreEqual(1, serverInstance.OnNetworkSpawnCalledCount);
+            Assert.AreEqual(1, authorityInstance.OnNetworkSpawnCalledCount);
 
             // safety check server despawned
-            Assert.AreEqual(0, serverInstance.OnNetworkDespawnCalledCount);
+            Assert.AreEqual(0, authorityInstance.OnNetworkDespawnCalledCount);
 
             // Conditional check for clients spawning or despawning
             var checkSpawnCondition = false;
@@ -314,10 +315,10 @@ bool HasConditionBeenMet()
             Assert.False(s_GlobalTimeoutHelper.TimedOut, "Timed out while waiting for client side spawns!");
 
             // despawn on server.  However, since we'll be using this object later in the test, don't delete it
-            serverInstance.GetComponent<NetworkObject>().Despawn(false);
+            authorityInstance.GetComponent<NetworkObject>().Despawn(false);
 
             // check despawned on server
-            Assert.AreEqual(1, serverInstance.OnNetworkDespawnCalledCount);
+            Assert.AreEqual(1, authorityInstance.OnNetworkDespawnCalledCount);
             // we now expect the clients to each have despawned once
             expectedDespawnCount = 1;
 
@@ -329,24 +330,24 @@ bool HasConditionBeenMet()
             Assert.False(s_GlobalTimeoutHelper.TimedOut, "Timed out while waiting for client side despawns!");
 
             //----------- step 2 check spawn and destroy again
-            serverInstance.GetComponent<NetworkObject>().Spawn();
+            authorityInstance.GetComponent<NetworkObject>().Spawn();
             // wait a tick
             yield return s_DefaultWaitForTick;
             // check spawned again on server this is 2 because we are reusing the object which was already spawned once.
-            Assert.AreEqual(2, serverInstance.OnNetworkSpawnCalledCount);
+            Assert.AreEqual(2, authorityInstance.OnNetworkSpawnCalledCount);
 
             checkSpawnCondition = true;
             yield return WaitForConditionOrTimeOut(HasConditionBeenMet);
 
             Assert.False(s_GlobalTimeoutHelper.TimedOut, "Timed out while waiting for client side spawns! (2nd pass)");
 
             // destroy the server object
-            Object.Destroy(serverInstance.gameObject);
+            Object.Destroy(authorityInstance.gameObject);
 
             yield return s_DefaultWaitForTick;
 
             // check whether despawned was called again on server instance
-            Assert.AreEqual(2, serverInstance.OnNetworkDespawnCalledCount);
+            Assert.AreEqual(2, authorityInstance.OnNetworkDespawnCalledCount);
 
             checkSpawnCondition = false;
             yield return WaitForConditionOrTimeOut(HasConditionBeenMet);
@@ -386,5 +387,50 @@ public override void OnNetworkDespawn()
                 OnNetworkDespawnCalledCount++;
             }
         }
+
+        private bool AllClientsSpawnedObject()
+        {
+            foreach (var networkManager in m_NetworkManagers)
+            {
+                if (!networkManager.SpawnManager.SpawnedObjects.ContainsKey(m_SpawnedInstanceId))
+                {
+                    return false;
+                }
+            }
+            return true;
+        }
+
+        private bool AllClientsDespawnedObject()
+        {
+            foreach (var networkManager in m_NetworkManagers)
+            {
+                if (networkManager.SpawnManager.SpawnedObjects.ContainsKey(m_SpawnedInstanceId))
+                {
+                    return false;
+                }
+            }
+            return true;
+        }
+
+        private ulong m_SpawnedInstanceId;
+        [UnityTest]
+        public IEnumerator NetworkObjectResetOnDespawn()
+        {
+            var authorityNetworkManager = GetAuthorityNetworkManager();
+            var instance = SpawnObject(m_ObserverPrefab, authorityNetworkManager).GetComponent<NetworkObject>();
+            m_SpawnedInstanceId = instance.NetworkObjectId;
+            yield return WaitForConditionOrTimeOut(AllClientsSpawnedObject);
+            AssertOnTimeout($"Not all clients spawned an instance of {instance.name}!");
+
+            instance.Despawn(false);
+
+            yield return WaitForConditionOrTimeOut(AllClientsDespawnedObject);
+            AssertOnTimeout($"Not all clients de-spawned an instance of {instance.name}!");
+
+            Assert.IsNull(instance.GetNetworkParenting(), "Last parent was not reset!");
+
+            Object.Destroy(instance.gameObject);
+        }
+
     }
 }

com.unity.netcode.gameobjects/Tests/Runtime/Prefabs/NetworkPrefabOverrideTests.cs

@@ -321,9 +321,12 @@ bool ClientDisconnected(ulong clientId)
                     {
                         continue;
                     }
-
+                    if (manager.SpawnManager == null)
+                    {
+                        continue;
+                    }
                     var clientOwnedObjects = manager.SpawnManager.SpawnedObjects.Where((c) => c.Value.OwnerClientId == clientId).ToList();
-                    if (clientOwnedObjects.Count > 0)
+                    if (clientOwnedObjects != null && clientOwnedObjects.Count > 0)
                     {
                         return false;
                     }