diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md
index 8de02a4326..7c947f8d2f 100644
--- a/com.unity.netcode.gameobjects/CHANGELOG.md
+++ b/com.unity.netcode.gameobjects/CHANGELOG.md
@@ -10,15 +10,14 @@ Additional documentation and release notes are available at [Multiplayer Documen
### Added
-
### Fixed
- Fixed issue where a `NetworkTransform` using full precision state updates was losing transform state updates when interpolation was enabled. (#2624)
- Fixed issue where `NetworkObject.SpawnWithObservers` was not being honored for late joining clients. (#2623)
- Fixed issue where invoking `NetworkManager.Shutdown` multiple times, depending upon the timing, could cause an exception. (#2622)
+- 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
-
+### Changed
## [1.5.1] - 2023-06-07
diff --git a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs
index 2e5c6250c6..a901fec70e 100644
--- a/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs
+++ b/com.unity.netcode.gameobjects/Runtime/Core/NetworkManager.cs
@@ -1035,6 +1035,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();
}
diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs
index 8b966a213a..c7ddc60a4a 100644
--- a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs
+++ b/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)
{
@@ -227,43 +215,6 @@ public NetworkObject GetPlayerNetworkObject(ulong clientId)
return null;
}
- internal void RemoveOwnership(NetworkObject networkObject)
- {
- if (!NetworkManager.IsServer)
- {
- throw new NotServerException("Only the server can change ownership");
- }
-
- if (!networkObject.IsSpawned)
- {
- throw new SpawnStateException("Object is not spawned");
- }
-
- // If we made it here then we are the server and if the server is determined to already be the owner
- // then ignore the RemoveOwnership invocation.
- if (networkObject.OwnerClientId == NetworkManager.ServerClientId)
- {
- return;
- }
-
- networkObject.OwnerClientId = NetworkManager.ServerClientId;
-
- // Server removes the entry and takes over ownership before notifying
- UpdateOwnershipTable(networkObject, NetworkManager.ServerClientId, true);
-
- var message = new ChangeOwnershipMessage
- {
- NetworkObjectId = networkObject.NetworkObjectId,
- OwnerClientId = networkObject.OwnerClientId
- };
- var size = NetworkManager.ConnectionManager.SendMessage(ref message, NetworkDelivery.ReliableSequenced, NetworkManager.ConnectedClientsIds);
-
- foreach (var client in NetworkManager.ConnectedClients)
- {
- NetworkManager.NetworkMetrics.TrackOwnershipChangeSent(client.Key, networkObject, size);
- }
- }
-
///
/// Helper function to get a network client for a clientId from the NetworkManager.
/// On the server this will check the list.
@@ -289,6 +240,11 @@ private bool TryGetNetworkClient(ulong clientId, out NetworkClient networkClient
return false;
}
+ internal void RemoveOwnership(NetworkObject networkObject)
+ {
+ ChangeOwnership(networkObject, NetworkManager.ServerClientId);
+ }
+
internal void ChangeOwnership(NetworkObject networkObject, ulong clientId)
{
if (!NetworkManager.IsServer)
@@ -301,14 +257,21 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId)
throw new SpawnStateException("Object is not spawned");
}
+ // Assign the new owner
networkObject.OwnerClientId = clientId;
+ // Always notify locally on the server when ownership is lost
+ 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,
diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkBehaviourGenericTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkBehaviourGenericTests.cs
index cea941a9b4..ab48aaab09 100644
--- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkBehaviourGenericTests.cs
+++ b/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();
diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOwnershipTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOwnershipTests.cs
index 1f4e3c7471..319415d782 100644
--- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOwnershipTests.cs
+++ b/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();
@@ -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
///
[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();
@@ -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).");