fix: change ownership of owner-read network variables mtt-5009 #2286 (#2301)

Author: jeffreyrainy

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

@@ -270,6 +270,9 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId)
 
             networkObject.OwnerClientId = clientId;
 
+            networkObject.MarkVariablesDirty(true);
+            NetworkManager.BehaviourUpdater.AddForUpdate(networkObject);
+
             // Server adds entries for all client ownership
             UpdateOwnershipTable(networkObject, networkObject.OwnerClientId);
 

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

@@ -794,17 +794,23 @@ public IEnumerator WhenSpawnTriggeredMessagesAreDeferredBeforeThePrefabIsAdded_A
 
             serverObject.GetComponent<NetworkObject>().ChangeOwnership(m_ClientNetworkManagers[0].LocalClientId);
 
-            yield return WaitForAllClientsToReceive<ChangeOwnershipMessage>();
+            yield return WaitForAllClientsToReceive<ChangeOwnershipMessage, NetworkVariableDeltaMessage>();
+
+            // wait three ticks
+            yield return NetcodeIntegrationTestHelpers.WaitForTicks(m_ServerNetworkManager, 3);
+            yield return NetcodeIntegrationTestHelpers.WaitForTicks(m_ClientNetworkManagers[0], 3);
 
             // Validate messages are deferred and pending
             foreach (var client in m_ClientNetworkManagers)
             {
                 var manager = (TestDeferredMessageManager)client.DeferredMessageManager;
                 Assert.IsTrue(manager.DeferMessageCalled);
                 Assert.IsFalse(manager.ProcessTriggersCalled);
-                Assert.AreEqual(4, manager.DeferredMessageCountTotal());
-                Assert.AreEqual(3, manager.DeferredMessageCountForType(IDeferredMessageManager.TriggerType.OnSpawn));
-                Assert.AreEqual(3, manager.DeferredMessageCountForKey(IDeferredMessageManager.TriggerType.OnSpawn, serverObject.GetComponent<NetworkObject>().NetworkObjectId));
+
+                Assert.AreEqual(5, manager.DeferredMessageCountTotal());
+
+                Assert.AreEqual(4, manager.DeferredMessageCountForType(IDeferredMessageManager.TriggerType.OnSpawn));
+                Assert.AreEqual(4, manager.DeferredMessageCountForKey(IDeferredMessageManager.TriggerType.OnSpawn, serverObject.GetComponent<NetworkObject>().NetworkObjectId));
                 Assert.AreEqual(1, manager.DeferredMessageCountForType(IDeferredMessageManager.TriggerType.OnAddPrefab));
                 Assert.AreEqual(1, manager.DeferredMessageCountForKey(IDeferredMessageManager.TriggerType.OnAddPrefab, serverObject.GetComponent<NetworkObject>().GlobalObjectIdHash));
                 AddPrefabsToClient(client);

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

@@ -13,6 +13,7 @@ public class ShowHideObject : NetworkBehaviour
         public static List<ShowHideObject> ClientTargetedNetworkObjects = new List<ShowHideObject>();
         public static ulong ClientIdToTarget;
         public static bool Silent;
+        public static int ValueAfterOwnershipChange = 0;
 
         public static NetworkObject GetNetworkObjectById(ulong networkObjectId)
         {
@@ -57,13 +58,35 @@ public override void OnNetworkDespawn()
 
         public NetworkVariable<int> MyNetworkVariable;
         public NetworkList<int> MyListSetOnSpawn;
+        public NetworkVariable<int> MyOwnerReadNetworkVariable;
+        static public NetworkManager NetworkManagerOfInterest;
+
+        internal static int GainOwnershipCount = 0;
 
         private void Awake()
         {
+            // Debug.Log($"Awake {NetworkManager.LocalClientId}");
             MyNetworkVariable = new NetworkVariable<int>();
             MyNetworkVariable.OnValueChanged += Changed;
 
             MyListSetOnSpawn = new NetworkList<int>();
+
+            MyOwnerReadNetworkVariable = new NetworkVariable<int>(readPerm: NetworkVariableReadPermission.Owner);
+            MyOwnerReadNetworkVariable.OnValueChanged += OwnerReadChanged;
+        }
+
+        public override void OnGainedOwnership()
+        {
+            GainOwnershipCount++;
+            base.OnGainedOwnership();
+        }
+
+        public void OwnerReadChanged(int before, int after)
+        {
+            if (NetworkManager == NetworkManagerOfInterest)
+            {
+                ValueAfterOwnershipChange = after;
+            }
         }
 
         public void Changed(int before, int after)
@@ -328,5 +351,48 @@ public IEnumerator NetworkHideChangeOwnership()
 
             Assert.True(ShowHideObject.ClientTargetedNetworkObjects[0].OwnerClientId == m_ClientNetworkManagers[0].LocalClientId);
         }
+
+        [UnityTest]
+        public IEnumerator NetworkHideChangeOwnershipNotHidden()
+        {
+            ShowHideObject.ClientTargetedNetworkObjects.Clear();
+            ShowHideObject.ClientIdToTarget = m_ClientNetworkManagers[1].LocalClientId;
+            ShowHideObject.Silent = true;
+
+            var spawnedObject1 = SpawnObject(m_PrefabToSpawn, m_ServerNetworkManager);
+            m_NetSpawnedObject1 = spawnedObject1.GetComponent<NetworkObject>();
+
+            // wait for host to have spawned and gained ownership
+            while (ShowHideObject.GainOwnershipCount == 0)
+            {
+                yield return new WaitForSeconds(0.0f);
+            }
+
+            // change the value
+            m_NetSpawnedObject1.GetComponent<ShowHideObject>().MyOwnerReadNetworkVariable.Value++;
+
+            // wait for three ticks
+            yield return NetcodeIntegrationTestHelpers.WaitForTicks(m_ServerNetworkManager, 3);
+
+            // check we'll actually be changing owners
+            Assert.False(ShowHideObject.ClientTargetedNetworkObjects[0].OwnerClientId == m_ClientNetworkManagers[0].LocalClientId);
+
+            // only check for value change on one specific client
+            ShowHideObject.NetworkManagerOfInterest = m_ClientNetworkManagers[0];
+
+            // change ownership
+            m_NetSpawnedObject1.ChangeOwnership(m_ClientNetworkManagers[0].LocalClientId);
+
+            // wait three ticks
+            yield return NetcodeIntegrationTestHelpers.WaitForTicks(m_ServerNetworkManager, 3);
+            yield return NetcodeIntegrationTestHelpers.WaitForTicks(m_ClientNetworkManagers[0], 3);
+
+            // verify ownership changed
+            Assert.True(ShowHideObject.ClientTargetedNetworkObjects[0].OwnerClientId == m_ClientNetworkManagers[0].LocalClientId);
+
+            // verify the expected client got the OnValueChanged. (Only client 1 sets this value)
+            Assert.True(ShowHideObject.ValueAfterOwnershipChange == 1);
+        }
+
     }
 }