fix: Ownership table entries not being removed on clients [MTT-2974] (#1838)

* fix

Spawn manager not removing entries from ownership table

* Update CHANGELOG.md

MTT-2974

* fix

Removing the ownership check in InvokeBehaviourOnLostOwnership because clients still need to update their table when the owner has changed to remove their ownership entry in their local ownership table.

* test

updating the integration test to check and make sure that the OwnershipToObjectsTable is having entries removed when ownership is lost.

* update

removing some debug code

* update

adding PR number

Author: NoelStephensUnity

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -21,6 +21,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 - Removed `com.unity.modules.animation`, `com.unity.modules.physics` and `com.unity.modules.physics2d` dependencies from the package (#1812)
 
 ### Fixed
+- Fixed issue where entries were not being removed from the NetworkSpawnManager.OwnershipToObjectsTable. (#1838)
 - Fixed clarity for NetworkSceneManager client side notification when it receives a scene hash value that does not exist in its local hash table. (#1828)
 - Fixed client throws a key not found exception when it times out using UNet or UTP. (#1821)
 - Fixed network variable updates are no longer limited to 32,768 bytes when NetworkConfig.EnsureNetworkVariableLengthSafety is enabled. The limits are now determined by what the transport can send in a message. (#1811)

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

@@ -515,8 +515,8 @@ public void ChangeOwnership(ulong newOwnerClientId)
 
         internal void InvokeBehaviourOnLostOwnership()
         {
-            // Server already handles this earlier, hosts should ignore
-            if (!NetworkManager.IsServer && NetworkManager.LocalClientId == OwnerClientId)
+            // Server already handles this earlier, hosts should ignore, all clients should update
+            if (!NetworkManager.IsServer)
             {
                 NetworkManager.SpawnManager.UpdateOwnershipTable(this, OwnerClientId, true);
             }
@@ -529,7 +529,7 @@ internal void InvokeBehaviourOnLostOwnership()
 
         internal void InvokeBehaviourOnGainedOwnership()
         {
-            // Server already handles this earlier, hosts should ignore
+            // Server already handles this earlier, hosts should ignore and only client owners should update
             if (!NetworkManager.IsServer && NetworkManager.LocalClientId == OwnerClientId)
             {
                 NetworkManager.SpawnManager.UpdateOwnershipTable(this, OwnerClientId);

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

@@ -113,6 +113,10 @@ internal void UpdateOwnershipTable(NetworkObject networkObject, ulong newOwner,
                     networkObject.InvokeBehaviourOnGainedOwnership();
                 }
             }
+            else if (isRemoving)
+            {
+                OwnershipToObjectsTable[previousOwner].Remove(networkObject.NetworkObjectId);
+            }
             else if (NetworkManager.LogLevel == LogLevel.Developer)
             {
                 NetworkLog.LogWarning($"Setting ownership twice? Client-ID {previousOwner} already owns NetworkObject ID {networkObject.NetworkObjectId}!");

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

@@ -192,18 +192,25 @@ public IEnumerator TestOwnershipCallbacksSeveralClients()
                 Assert.That(m_ServerNetworkManager.ConnectedClients.ContainsKey(clientId));
                 serverObject.ChangeOwnership(clientId);
                 yield return s_DefaultWaitForTick;
-
                 Assert.That(serverComponent.OnLostOwnershipFired);
                 Assert.That(serverComponent.OwnerClientId, Is.EqualTo(clientId));
                 // Wait for all clients to receive the CreateObjectMessage
                 yield return WaitForConditionOrTimeOut(ownershipMessageHooks);
                 Assert.False(s_GlobalTimeoutHelper.TimedOut, $"Timed out waiting for all clients to receive the {nameof(ChangeOwnershipMessage)} message.");
 
+                var previousNetworkManager = m_ServerNetworkManager;
                 if (previousClientComponent != null)
                 {
+                    // Once we have a previousClientComponent, we want to verify the server is keeping track for the removal of ownership in the OwnershipToObjectsTable
+                    Assert.That(!m_ServerNetworkManager.SpawnManager.OwnershipToObjectsTable[m_ServerNetworkManager.LocalClientId].ContainsKey(serverObject.NetworkObjectId));
+                    previousNetworkManager = previousClientComponent.NetworkManager;
                     Assert.That(previousClientComponent.OnLostOwnershipFired);
                     Assert.That(previousClientComponent.OwnerClientId, Is.EqualTo(clientId));
                 }
+
+                // Assure the previous owner is no longer in the local table of the previous owner.
+                Assert.That(!previousNetworkManager.SpawnManager.OwnershipToObjectsTable[previousNetworkManager.LocalClientId].ContainsKey(serverObject.NetworkObjectId));
+
                 var currentClientComponent = clientObjects[clientIndex].GetComponent<NetworkObjectOwnershipComponent>();
                 Assert.That(currentClientComponent.OnGainedOwnershipFired);