fix: RemoveOwnership not locally notifying server that it gained ownership [MTT-6377] [MTT-6937][MTT-6936]

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -10,10 +10,14 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Added
 
+
 ### Fixed
-- Fixed a failing UTP test that was failing when you install Unity Transport package 2.0.0 or newer.
 
-## Changed
+- Fixed issue where removing ownership would not notify the server that it gained ownership. This also resolves the issue where an owner authoritative NetworkTransform would not properly initialize upon removing ownership from a remote client. (#2618)
+
+
+### Changed
+
 
 ## [1.5.1] - 2023-06-07
 

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

@@ -1029,6 +1029,8 @@ internal void ShutdownInternal()
         // Ensures that the NetworkManager is cleaned up before OnDestroy is run on NetworkObjects and NetworkBehaviours when quitting the application.
         private void OnApplicationQuit()
         {
+            // Make sure ShutdownInProgress returns true during this time
+            m_ShuttingDown = true;
             OnDestroy();
         }
 

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

@@ -113,12 +113,6 @@ internal void UpdateOwnershipTable(NetworkObject networkObject, ulong newOwner,
                     // Remove the previous owner's entry
                     OwnershipToObjectsTable[previousOwner].Remove(networkObject.NetworkObjectId);
 
-                    // Server or Host alway invokes the lost ownership notification locally
-                    if (NetworkManager.IsServer)
-                    {
-                        networkObject.InvokeBehaviourOnLostOwnership();
-                    }
-
                     // If we are removing the entry (i.e. despawning or client lost ownership)
                     if (isRemoving)
                     {
@@ -143,12 +137,6 @@ internal void UpdateOwnershipTable(NetworkObject networkObject, ulong newOwner,
             {
                 // Add the new ownership entry
                 OwnershipToObjectsTable[newOwner].Add(networkObject.NetworkObjectId, networkObject);
-
-                // Server or Host always invokes the gained ownership notification locally
-                if (NetworkManager.IsServer)
-                {
-                    networkObject.InvokeBehaviourOnGainedOwnership();
-                }
             }
             else if (isRemoving)
             {
@@ -248,9 +236,23 @@ internal void RemoveOwnership(NetworkObject networkObject)
 
             networkObject.OwnerClientId = NetworkManager.ServerClientId;
 
+            // If there is no BehaviourUpdater and we are shutting down then return early
+            if (NetworkManager.BehaviourUpdater == null && NetworkManager.ShutdownInProgress)
+            {
+                return;
+            }
+
+            // This was not being done when ownership was removed but was being done when ownership changed
+            // Mark variables as dirty and add the NetworkObject for a delta update
+            networkObject.MarkVariablesDirty(true);
+            NetworkManager.BehaviourUpdater.AddForUpdate(networkObject);
+
             // Server removes the entry and takes over ownership before notifying
             UpdateOwnershipTable(networkObject, NetworkManager.ServerClientId, true);
 
+            //Notify the local server that it gained ownership
+            networkObject.InvokeBehaviourOnGainedOwnership();
+
             var message = new ChangeOwnershipMessage
             {
                 NetworkObjectId = networkObject.NetworkObjectId,
@@ -301,14 +303,27 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId)
                 throw new SpawnStateException("Object is not spawned");
             }
 
+            // Determine if the server is gong to lose ownership
+            var serverWillLoseOwnership = networkObject.OwnerClientId == NetworkManager.LocalClientId;
+
+            // Assign the new owner
             networkObject.OwnerClientId = clientId;
 
+            // Notify locally that the server lost ownership
+            if (serverWillLoseOwnership)
+            {
+                networkObject.InvokeBehaviourOnLostOwnership();
+            }
+
             networkObject.MarkVariablesDirty(true);
             NetworkManager.BehaviourUpdater.AddForUpdate(networkObject);
 
             // Server adds entries for all client ownership
             UpdateOwnershipTable(networkObject, networkObject.OwnerClientId);
 
+            // Always notify locally on the server when a new owner is assigned
+            networkObject.InvokeBehaviourOnGainedOwnership();
+
             var message = new ChangeOwnershipMessage
             {
                 NetworkObjectId = networkObject.NetworkObjectId,

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

@@ -67,7 +67,6 @@ public IEnumerator ValidatedDisableddNetworkBehaviourWarning()
             // Set the child object to be inactive in the hierarchy
             childObject.SetActive(false);
 
-            LogAssert.Expect(LogType.Warning, $"{childObject.name} is disabled! Netcode for GameObjects does not support disabled NetworkBehaviours! The {childBehaviour.GetType().Name} component was skipped during ownership assignment!");
             LogAssert.Expect(LogType.Warning, $"{childObject.name} is disabled! Netcode for GameObjects does not support spawning disabled NetworkBehaviours! The {childBehaviour.GetType().Name} component was skipped during spawn!");
 
             parentNetworkObject.Spawn();

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

@@ -42,6 +42,12 @@ public class NetworkObjectOwnershipTests : NetcodeIntegrationTest
 
         public NetworkObjectOwnershipTests(HostOrServer hostOrServer) : base(hostOrServer) { }
 
+        public enum OwnershipChecks
+        {
+            Change,
+            Remove
+        }
+
         protected override void OnServerAndClientsCreated()
         {
             m_OwnershipPrefab = CreateNetworkObjectPrefab("OnwershipPrefab");
@@ -62,7 +68,7 @@ public void TestPlayerIsOwned()
         }
 
         [UnityTest]
-        public IEnumerator TestOwnershipCallbacks()
+        public IEnumerator TestOwnershipCallbacks([Values] OwnershipChecks ownershipChecks)
         {
             m_OwnershipObject = SpawnObject(m_OwnershipPrefab, m_ServerNetworkManager);
             m_OwnershipNetworkObject = m_OwnershipObject.GetComponent<NetworkObject>();
@@ -109,7 +115,17 @@ public IEnumerator TestOwnershipCallbacks()
             serverComponent.ResetFlags();
             clientComponent.ResetFlags();
 
-            serverObject.ChangeOwnership(NetworkManager.ServerClientId);
+            if (ownershipChecks == OwnershipChecks.Change)
+            {
+                // Validates that when ownership is changed back to the server it will get an OnGainedOwnership notification
+                serverObject.ChangeOwnership(NetworkManager.ServerClientId);
+            }
+            else
+            {
+                // Validates that when ownership is removed the server gets an OnGainedOwnership notification
+                serverObject.RemoveOwnership();
+            }
+
             yield return s_DefaultWaitForTick;
 
             Assert.That(serverComponent.OnGainedOwnershipFired);
@@ -125,7 +141,7 @@ public IEnumerator TestOwnershipCallbacks()
         /// Verifies that switching ownership between several clients works properly
         /// </summary>
         [UnityTest]
-        public IEnumerator TestOwnershipCallbacksSeveralClients()
+        public IEnumerator TestOwnershipCallbacksSeveralClients([Values] OwnershipChecks ownershipChecks)
         {
             // Build our message hook entries tables so we can determine if all clients received spawn or ownership messages
             var messageHookEntriesForSpawn = new List<MessageHookEntry>();
@@ -247,8 +263,17 @@ bool WaitForClientsToSpawnNetworkObject()
                 previousClientComponent = currentClientComponent;
             }
 
-            // Now change ownership back to the server
-            serverObject.ChangeOwnership(NetworkManager.ServerClientId);
+            if (ownershipChecks == OwnershipChecks.Change)
+            {
+                // Validates that when ownership is changed back to the server it will get an OnGainedOwnership notification
+                serverObject.ChangeOwnership(NetworkManager.ServerClientId);
+            }
+            else
+            {
+                // Validates that when ownership is removed the server gets an OnGainedOwnership notification
+                serverObject.RemoveOwnership();
+            }
+
             yield return WaitForConditionOrTimeOut(ownershipMessageHooks);
             Assert.False(s_GlobalTimeoutHelper.TimedOut, $"Timed out waiting for all clients to receive the {nameof(ChangeOwnershipMessage)} message (back to server).");