fix: Not sending ChangeOwnership messages to clients that have an object hidden (#2251)

* fix: Not sending ChangeOwnership messages to clients that have an object hidden

Author: jeffreyrainy

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -28,6 +28,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Fixed
 
+- Fixed issue caused when changing ownership of objects hidden to some clients (#2242)
 - Fixed issue where an in-scene placed NetworkObject would not invoke NetworkBehaviour.OnNetworkSpawn if the GameObject was disabled when it was despawned. (#2239)
 - Fixed issue where clients were not rebuilding the `NetworkConfig` hash value for each unique connection request. (#2226)
 - Fixed the issue where player objects were not taking the `DontDestroyWithOwner` property into consideration when a client disconnected. (#2225)

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

@@ -278,11 +278,14 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId)
                 NetworkObjectId = networkObject.NetworkObjectId,
                 OwnerClientId = networkObject.OwnerClientId
             };
-            var size = NetworkManager.SendMessage(ref message, NetworkDelivery.ReliableSequenced, NetworkManager.ConnectedClientsIds);
 
             foreach (var client in NetworkManager.ConnectedClients)
             {
-                NetworkManager.NetworkMetrics.TrackOwnershipChangeSent(client.Key, networkObject, size);
+                if (networkObject.IsNetworkVisibleTo(client.Value.ClientId))
+                {
+                    var size = NetworkManager.SendMessage(ref message, NetworkDelivery.ReliableSequenced, client.Value.ClientId);
+                    NetworkManager.NetworkMetrics.TrackOwnershipChangeSent(client.Key, networkObject, size);
+                }
             }
         }
 

com.unity.netcode.gameobjects/Tests/Runtime/Metrics/OwnershipChangeMetricsTests.cs

@@ -61,6 +61,13 @@ public IEnumerator TrackOwnershipChangeSentMetric()
             var ownershipChangeSent = metricValues.First();
             Assert.AreEqual(networkObject.NetworkObjectId, ownershipChangeSent.NetworkId.NetworkId);
             Assert.AreEqual(Server.LocalClientId, ownershipChangeSent.Connection.Id);
+            Assert.AreEqual(0, ownershipChangeSent.BytesCount);
+
+            // The first metric is to the server(self), so its size is now correctly reported as 0.
+            // Let's check the last one instead, to have a valid value
+            ownershipChangeSent = metricValues.Last();
+            Assert.AreEqual(networkObject.NetworkObjectId, ownershipChangeSent.NetworkId.NetworkId);
+            Assert.AreEqual(Client.LocalClientId, ownershipChangeSent.Connection.Id);
             Assert.AreEqual(FastBufferWriter.GetWriteSize<ChangeOwnershipMessage>() + k_MessageHeaderSize, ownershipChangeSent.BytesCount);
         }
 

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

@@ -306,5 +306,37 @@ public IEnumerator NetworkHideDespawnTest()
 
             LogAssert.NoUnexpectedReceived();
         }
+
+        [UnityTest]
+        public IEnumerator NetworkHideChangeOwnership()
+        {
+            ShowHideObject.ClientTargetedNetworkObjects.Clear();
+            ShowHideObject.ClientIdToTarget = m_ClientNetworkManagers[1].LocalClientId;
+            ShowHideObject.Silent = true;
+
+            var spawnedObject1 = SpawnObject(m_PrefabToSpawn, m_ServerNetworkManager);
+            m_NetSpawnedObject1 = spawnedObject1.GetComponent<NetworkObject>();
+
+            m_NetSpawnedObject1.GetComponent<ShowHideObject>().MyNetworkVariable.Value++;
+            // Hide an object to a client
+            m_NetSpawnedObject1.NetworkHide(m_ClientNetworkManagers[1].LocalClientId);
+
+            yield return WaitForConditionOrTimeOut(() => ShowHideObject.ClientTargetedNetworkObjects.Count == 0);
+
+            // Change ownership while the object is hidden to some
+            m_NetSpawnedObject1.ChangeOwnership(m_ClientNetworkManagers[0].LocalClientId);
+
+            // The two-second wait is actually needed as there's a potential warning of unhandled message after 1 second
+            yield return new WaitForSeconds(1.25f);
+
+            LogAssert.NoUnexpectedReceived();
+
+            // Show the object again to check nothing unexpected happens
+            m_NetSpawnedObject1.NetworkShow(m_ClientNetworkManagers[1].LocalClientId);
+
+            yield return WaitForConditionOrTimeOut(() => ShowHideObject.ClientTargetedNetworkObjects.Count == 1);
+
+            Assert.True(ShowHideObject.ClientTargetedNetworkObjects[0].OwnerClientId == m_ClientNetworkManagers[0].LocalClientId);
+        }
     }
 }