fix: player objects do not honor the DontDestroyWithOwner property [MTT-4458] (Unity-Technologies#2225)

* fix
This fixes the issue with player objects not honoring the DontDestroyWithOwner property.
The summary for DontDestroyWithOwner was incorrect.  When true the NetworkObject's ownership is given back to the server.

* test
Adding a test to verify that when a client disconnects and the client's player object has DontDestroyWithOwner set to true that the NetworkObject's ownership is transferred back to the client.

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -21,6 +21,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed the issue where player objects were not taking the `DontDestroyWithOwner` property into consideration when a client disconnected. (#2225)
 - Fixed issue #1924 where `UnityTransport` would fail to restart after a first failure (even if what caused the initial failure was addressed). (#2220)
 - Fixed issue where `NetworkTransform.SetStateServerRpc` and `NetworkTransform.SetStateClientRpc` were not honoring local vs world space settings when applying the position and rotation. (#2203)
 - Fixed ILPP `TypeLoadException` on WebGL on MacOS Editor and potentially other platforms. (#2199)

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

@@ -1951,13 +1951,20 @@ private void OnClientDisconnectFromServer(ulong clientId)
                     var playerObject = networkClient.PlayerObject;
                     if (playerObject != null)
                     {
-                        if (PrefabHandler.ContainsHandler(ConnectedClients[clientId].PlayerObject.GlobalObjectIdHash))
+                        if (!playerObject.DontDestroyWithOwner)
                         {
-                            PrefabHandler.HandleNetworkPrefabDestroy(ConnectedClients[clientId].PlayerObject);
+                            if (PrefabHandler.ContainsHandler(ConnectedClients[clientId].PlayerObject.GlobalObjectIdHash))
+                            {
+                                PrefabHandler.HandleNetworkPrefabDestroy(ConnectedClients[clientId].PlayerObject);
+                            }
+                            else
+                            {
+                                Destroy(playerObject.gameObject);
+                            }
                         }
                         else
                         {
-                            Destroy(playerObject.gameObject);
+                            playerObject.RemoveOwnership();
                         }
                     }
 

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

@@ -129,7 +129,7 @@ internal void GenerateGlobalObjectIdHash()
 
         /// <summary>
         /// Whether or not to destroy this object if it's owner is destroyed.
-        /// If false, the objects ownership will be given to the server.
+        /// If true, the objects ownership will be given to the server.
         /// </summary>
         public bool DontDestroyWithOwner;
 

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

@@ -60,5 +60,53 @@ private void OnClientDisconnectCallback(ulong obj)
         {
             m_ClientDisconnected = true;
         }
+
+        [UnityTest]
+        public IEnumerator ClientDisconnectPlayerObjectCleanup()
+        {
+            // create server and client instances
+            NetcodeIntegrationTestHelpers.Create(1, out NetworkManager server, out NetworkManager[] clients);
+
+            // create prefab
+            var gameObject = new GameObject("PlayerObject");
+            var networkObject = gameObject.AddComponent<NetworkObject>();
+            networkObject.DontDestroyWithOwner = true;
+            NetcodeIntegrationTestHelpers.MakeNetworkObjectTestPrefab(networkObject);
+
+            server.NetworkConfig.PlayerPrefab = gameObject;
+
+            for (int i = 0; i < clients.Length; i++)
+            {
+                clients[i].NetworkConfig.PlayerPrefab = gameObject;
+            }
+
+            // start server and connect clients
+            NetcodeIntegrationTestHelpers.Start(false, server, clients);
+
+            // wait for connection on client side
+            yield return NetcodeIntegrationTestHelpers.WaitForClientsConnected(clients);
+
+            // wait for connection on server side
+            yield return NetcodeIntegrationTestHelpers.WaitForClientConnectedToServer(server);
+
+            // disconnect the remote client
+            m_ClientDisconnected = false;
+
+            server.OnClientDisconnectCallback += OnClientDisconnectCallback;
+
+            var serverSideClientPlayer = server.ConnectedClients[clients[0].LocalClientId].PlayerObject;
+
+            // Stopping the client is the same as the client disconnecting
+            NetcodeIntegrationTestHelpers.StopOneClient(clients[0]);
+
+            var timeoutHelper = new TimeoutHelper();
+            yield return NetcodeIntegrationTest.WaitForConditionOrTimeOut(() => m_ClientDisconnected, timeoutHelper);
+
+            // ensure the object was destroyed
+            Assert.True(serverSideClientPlayer.IsOwnedByServer, $"The client's player object's ownership was not transferred back to the server!");
+
+            // cleanup
+            NetcodeIntegrationTestHelpers.Destroy();
+        }
     }
 }