diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 7105a6e705..4bfdff566e 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -36,6 +36,7 @@ Additional documentation and release notes are available at [Multiplayer Documen - Fixed throwing an exception in `OnNetworkUpdate` causing other `OnNetworkUpdate` calls to not be executed. (#1739) - Fixed synchronization when Time.timeScale is set to 0. This changes timing update to use unscaled deltatime. Now network updates rate are independent from the local time scale. (#2171) - Fixed not sending all NetworkVariables to all clients when a client connects to a server. (#1987) +- Fixed IsOwner/IsOwnedByServer being wrong on the server after calling RemoveOwnership (#2211) ## [1.0.2] - 2022-09-12 diff --git a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs index 4b6a214a9a..91c5a52147 100644 --- a/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs +++ b/com.unity.netcode.gameobjects/Runtime/Spawning/NetworkSpawnManager.cs @@ -213,11 +213,11 @@ internal void RemoveOwnership(NetworkObject networkObject) return; } + networkObject.OwnerClientId = NetworkManager.ServerClientId; + // Server removes the entry and takes over ownership before notifying UpdateOwnershipTable(networkObject, NetworkManager.ServerClientId, true); - networkObject.OwnerClientId = NetworkManager.ServerClientId; - var message = new ChangeOwnershipMessage { NetworkObjectId = networkObject.NetworkObjectId, diff --git a/com.unity.netcode.gameobjects/TestHelpers/Runtime/MessageHooksConditional.cs b/com.unity.netcode.gameobjects/TestHelpers/Runtime/MessageHooksConditional.cs index 71791d2b6e..733cae8c90 100644 --- a/com.unity.netcode.gameobjects/TestHelpers/Runtime/MessageHooksConditional.cs +++ b/com.unity.netcode.gameobjects/TestHelpers/Runtime/MessageHooksConditional.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Linq; using NUnit.Framework; @@ -86,10 +87,19 @@ internal void AssignMessageType() where T : INetworkMessage Initialize(); } + internal void AssignMessageType(Type type) + { + MessageType = type.Name; + m_MessageReceiptCheck = (message) => + { + return message.GetType() == type; + }; + Initialize(); + } + public MessageHookEntry(NetworkManager networkManager) { m_NetworkManager = networkManager; } } } - diff --git a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs index ee085737d3..a83d0bdb18 100644 --- a/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs +++ b/com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs @@ -762,6 +762,39 @@ protected IEnumerator WaitForClientsConnectedOrTimeOut() yield return WaitForClientsConnectedOrTimeOut(m_ClientNetworkManagers); } + internal IEnumerator WaitForMessageReceived(List wiatForReceivedBy) where T : INetworkMessage + { + // Build our message hook entries tables so we can determine if all clients received spawn or ownership messages + var messageHookEntriesForSpawn = new List(); + foreach (var clientNetworkManager in wiatForReceivedBy) + { + var messageHook = new MessageHookEntry(clientNetworkManager); + messageHook.AssignMessageType(); + messageHookEntriesForSpawn.Add(messageHook); + } + // Used to determine if all clients received the CreateObjectMessage + var hooks = new MessageHooksConditional(messageHookEntriesForSpawn); + yield return WaitForConditionOrTimeOut(hooks); + } + + internal IEnumerator WaitForMessagesReceived(List messagesInOrder, List wiatForReceivedBy) + { + // Build our message hook entries tables so we can determine if all clients received spawn or ownership messages + var messageHookEntriesForSpawn = new List(); + foreach (var clientNetworkManager in wiatForReceivedBy) + { + foreach (var message in messagesInOrder) + { + var messageHook = new MessageHookEntry(clientNetworkManager); + messageHook.AssignMessageType(message); + messageHookEntriesForSpawn.Add(messageHook); + } + } + // Used to determine if all clients received the CreateObjectMessage + var hooks = new MessageHooksConditional(messageHookEntriesForSpawn); + yield return WaitForConditionOrTimeOut(hooks); + } + /// /// Creates a basic NetworkObject test prefab, assigns it to a new /// NetworkPrefab entry, and then adds it to the server and client(s) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectNetworkClientOwnedObjectsTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectNetworkClientOwnedObjectsTests.cs index fcdbfd886a..bcdd613102 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectNetworkClientOwnedObjectsTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectNetworkClientOwnedObjectsTests.cs @@ -9,6 +9,11 @@ namespace Unity.Netcode.RuntimeTests { public class NetworkObjectNetworkClientOwnedObjectsTests : NetcodeIntegrationTest { + private class DummyNetworkBehaviour : NetworkBehaviour + { + + } + protected override int NumberOfClients => 1; private NetworkPrefab m_NetworkPrefab; protected override void OnServerAndClientsCreated() @@ -16,6 +21,7 @@ protected override void OnServerAndClientsCreated() // create prefab var gameObject = new GameObject("ClientOwnedObject"); var networkObject = gameObject.AddComponent(); + gameObject.AddComponent(); NetcodeIntegrationTestHelpers.MakeNetworkObjectTestPrefab(networkObject); m_NetworkPrefab = (new NetworkPrefab() @@ -39,7 +45,7 @@ public IEnumerator ChangeOwnershipOwnedObjectsAddTest() serverObject.Spawn(); // Provide enough time for the client to receive and process the spawned message. - yield return s_DefaultWaitForTick; + yield return WaitForMessageReceived(m_ClientNetworkManagers.ToList()); // The object is owned by server Assert.False(m_ServerNetworkManager.SpawnManager.GetClientOwnedObjects(m_ClientNetworkManagers[0].LocalClientId).Any(x => x.NetworkObjectId == serverObject.NetworkObjectId)); @@ -48,13 +54,71 @@ public IEnumerator ChangeOwnershipOwnedObjectsAddTest() serverObject.ChangeOwnership(m_ClientNetworkManagers[0].LocalClientId); // Provide enough time for the client to receive and process the change in ownership message. - yield return s_DefaultWaitForTick; + yield return WaitForMessageReceived(m_ClientNetworkManagers.ToList()); // Ensure it's now added to the list - yield return WaitForConditionOrTimeOut(() => m_ClientNetworkManagers[0].SpawnManager.GetClientOwnedObjects(m_ClientNetworkManagers[0].LocalClientId).Any(x => x.NetworkObjectId == serverObject.NetworkObjectId)); - Assert.False(s_GlobalTimeoutHelper.TimedOut, $"Timed out waiting for client to gain ownership!"); Assert.True(m_ClientNetworkManagers[0].SpawnManager.GetClientOwnedObjects(m_ClientNetworkManagers[0].LocalClientId).Any(x => x.NetworkObjectId == serverObject.NetworkObjectId)); Assert.True(m_ServerNetworkManager.SpawnManager.GetClientOwnedObjects(m_ClientNetworkManagers[0].LocalClientId).Any(x => x.NetworkObjectId == serverObject.NetworkObjectId)); } + + [UnityTest] + public IEnumerator WhenOwnershipIsChanged_OwnershipValuesUpdateCorrectly() + { + NetworkObject serverObject = Object.Instantiate(m_NetworkPrefab.Prefab).GetComponent(); + serverObject.NetworkManagerOwner = m_ServerNetworkManager; + serverObject.Spawn(); + + // Provide enough time for the client to receive and process the spawned message. + yield return WaitForMessageReceived(m_ClientNetworkManagers.ToList()); + + // The object is owned by server + Assert.False(m_ServerNetworkManager.SpawnManager.GetClientOwnedObjects(m_ClientNetworkManagers[0].LocalClientId).Any(x => x.NetworkObjectId == serverObject.NetworkObjectId)); + + // Change the ownership + serverObject.ChangeOwnership(m_ClientNetworkManagers[0].LocalClientId); + + // Provide enough time for the client to receive and process the change in ownership message. + yield return WaitForMessageReceived(m_ClientNetworkManagers.ToList()); + + Assert.IsFalse(serverObject.IsOwner); + Assert.IsFalse(serverObject.IsOwnedByServer); + Assert.AreEqual(m_ClientNetworkManagers[0].LocalClientId, serverObject.OwnerClientId); + + var serverBehaviour = serverObject.GetComponent(); + Assert.IsFalse(serverBehaviour.IsOwner); + Assert.IsFalse(serverBehaviour.IsOwnedByServer); + Assert.AreEqual(m_ClientNetworkManagers[0].LocalClientId, serverBehaviour.OwnerClientId); + + var clientObject = Object.FindObjectsOfType().Where((obj) => obj.NetworkManagerOwner == m_ClientNetworkManagers[0]).FirstOrDefault(); + + Assert.IsNotNull(clientObject); + Assert.IsTrue(clientObject.IsOwner); + Assert.IsFalse(clientObject.IsOwnedByServer); + Assert.AreEqual(m_ClientNetworkManagers[0].LocalClientId, clientObject.OwnerClientId); + + var clientBehaviour = clientObject.GetComponent(); + Assert.IsTrue(clientBehaviour.IsOwner); + Assert.IsFalse(clientBehaviour.IsOwnedByServer); + Assert.AreEqual(m_ClientNetworkManagers[0].LocalClientId, clientBehaviour.OwnerClientId); + + serverObject.RemoveOwnership(); + + // Provide enough time for the client to receive and process the change in ownership message. + yield return WaitForMessageReceived(m_ClientNetworkManagers.ToList()); + + Assert.IsTrue(serverObject.IsOwner); + Assert.IsTrue(serverObject.IsOwnedByServer); + Assert.AreEqual(NetworkManager.ServerClientId, serverObject.OwnerClientId); + Assert.IsTrue(serverBehaviour.IsOwner); + Assert.IsTrue(serverBehaviour.IsOwnedByServer); + Assert.AreEqual(NetworkManager.ServerClientId, serverBehaviour.OwnerClientId); + + Assert.IsFalse(clientObject.IsOwner); + Assert.IsTrue(clientObject.IsOwnedByServer); + Assert.AreEqual(NetworkManager.ServerClientId, clientObject.OwnerClientId); + Assert.IsFalse(clientBehaviour.IsOwner); + Assert.IsTrue(clientBehaviour.IsOwnedByServer); + Assert.AreEqual(NetworkManager.ServerClientId, clientBehaviour.OwnerClientId); + } } }