From 5fdeaab7ef6855d64da5551bf03f352402f88d26 Mon Sep 17 00:00:00 2001 From: Emma Date: Wed, 3 Sep 2025 17:18:26 -0400 Subject: [PATCH 1/2] chore: Update the OwnershipPermissionsTests to work with rust server --- .../OwnershipPermissionsTests.cs | 266 +++++++++++------- .../TestHelpers/NetcodeIntegrationTest.cs | 29 +- .../NetcodeIntegrationTestHelpers.cs | 2 +- 3 files changed, 186 insertions(+), 111 deletions(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs index b87b5a529a..7f805a7bee 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs @@ -17,12 +17,6 @@ internal class OwnershipPermissionsTests : IntegrationTestWithApproximation protected override int NumberOfClients => 4; - // TODO: [CmbServiceTests] Adapt to run with the service - daHostInstance will be firstInstance with cmbService - protected override bool UseCMBService() - { - return false; - } - public OwnershipPermissionsTests() : base(HostOrServer.DAHost) { } @@ -44,37 +38,19 @@ protected override void OnServerAndClientsCreated() private NetworkObject m_ObjectToValidate; - private bool ValidateObjectSpawnedOnAllClients() - { - m_ErrorLog.Clear(); - - var networkObjectId = m_ObjectToValidate.NetworkObjectId; - var name = m_ObjectToValidate.name; - foreach (var client in m_NetworkManagers) - { - if (!client.SpawnManager.SpawnedObjects.ContainsKey(networkObjectId)) - { - m_ErrorLog.Append($"Client-{client.LocalClientId} has not spawned {name}!"); - return false; - } - } - return true; - } - - private bool ValidatePermissionsOnAllClients() + private bool ValidatePermissionsOnAllClients(StringBuilder errorLog) { var currentPermissions = (ushort)m_ObjectToValidate.Ownership; var networkObjectId = m_ObjectToValidate.NetworkObjectId; var objectName = m_ObjectToValidate.name; - m_ErrorLog.Clear(); foreach (var client in m_NetworkManagers) { var otherPermissions = (ushort)client.SpawnManager.SpawnedObjects[networkObjectId].Ownership; if (currentPermissions != otherPermissions) { - m_ErrorLog.Append($"Client-{client.LocalClientId} permissions for {objectName} is {otherPermissions} when it should be {currentPermissions}!"); + errorLog.Append($"Client-{client.LocalClientId} permissions for {objectName} is {otherPermissions} when it should be {currentPermissions}!"); return false; } } @@ -101,17 +77,19 @@ private bool ValidateAllInstancesAreOwnedByClient(ulong clientId) [UnityTest] public IEnumerator ValidateOwnershipPermissionsTest() { - var firstInstance = SpawnObject(m_PermissionsObject, m_ClientNetworkManagers[0]).GetComponent(); + var firstClient = GetNonAuthorityNetworkManager(0); + var firstInstance = SpawnObject(m_PermissionsObject, firstClient).GetComponent(); OwnershipPermissionsTestHelper.CurrentOwnedInstance = firstInstance; var firstInstanceHelper = firstInstance.GetComponent(); var networkObjectId = firstInstance.NetworkObjectId; m_ObjectToValidate = OwnershipPermissionsTestHelper.CurrentOwnedInstance; - yield return WaitForConditionOrTimeOut(ValidateObjectSpawnedOnAllClients); - AssertOnTimeout($"[Failed To Spawn] {firstInstance.name}: \n {m_ErrorLog}"); + + yield return WaitForSpawnedOnAllOrTimeOut(firstInstance); + AssertOnTimeout($"[Failed To Spawn] Ownership permissions object {firstInstance.name} failed to spawn!"); // Validate the base non-assigned permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); - AssertOnTimeout($"[Permissions Mismatch] {firstInstance.name}: \n {m_ErrorLog}"); + AssertOnTimeout($"[Permissions Mismatch] {firstInstance.name} has incorrect ownership permissions!"); ////////////////////////////////////// // Setting & Removing Ownership Flags: @@ -132,7 +110,7 @@ public IEnumerator ValidateOwnershipPermissionsTest() firstInstance.SetOwnershipStatus(permission); // Validate the permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); - AssertOnTimeout($"[Add][Permissions Mismatch] {firstInstance.name}: \n {m_ErrorLog}"); + AssertOnTimeout($"[Add][Permissions Mismatch] {firstInstance.name}"); // Remove the status unless it is None (ignore None). if (permission == NetworkObject.OwnershipStatus.None) @@ -142,7 +120,7 @@ public IEnumerator ValidateOwnershipPermissionsTest() firstInstance.RemoveOwnershipStatus(permission); // Validate the permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); - AssertOnTimeout($"[Remove][Permissions Mismatch] {firstInstance.name}: \n {m_ErrorLog}"); + AssertOnTimeout($"[Remove][Permissions Mismatch] {firstInstance.name}"); } //Add multiple flags at the same time @@ -164,7 +142,7 @@ public IEnumerator ValidateOwnershipPermissionsTest() // Validate the permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); - AssertOnTimeout($"[Set Multiple][Permissions Mismatch] {firstInstance.name}: \n {m_ErrorLog}"); + AssertOnTimeout($"[Set Multiple][Permissions Mismatch] {firstInstance.name}"); ////////////////////// // Changing Ownership: @@ -175,12 +153,13 @@ public IEnumerator ValidateOwnershipPermissionsTest() // Validate the permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); - AssertOnTimeout($"[Reset][Permissions Mismatch] {firstInstance.name}: \n {m_ErrorLog}"); + AssertOnTimeout($"[Reset][Permissions Mismatch] {firstInstance.name}"); - var secondInstance = m_ClientNetworkManagers[1].SpawnManager.SpawnedObjects[networkObjectId]; + var secondClient = GetNonAuthorityNetworkManager(1); + var secondInstance = secondClient.SpawnManager.SpawnedObjects[networkObjectId]; var secondInstanceHelper = secondInstance.GetComponent(); - secondInstance.ChangeOwnership(m_ClientNetworkManagers[1].LocalClientId); + secondInstance.ChangeOwnership(secondClient.LocalClientId); Assert.IsTrue(secondInstanceHelper.OwnershipPermissionsFailureStatus == NetworkObject.OwnershipPermissionsFailureStatus.Locked, $"Expected {secondInstance.name} to return {NetworkObject.OwnershipPermissionsFailureStatus.Locked} but its permission failure" + $" status is {secondInstanceHelper.OwnershipPermissionsFailureStatus}!"); @@ -188,20 +167,25 @@ public IEnumerator ValidateOwnershipPermissionsTest() firstInstance.SetOwnershipLock(false); // Validate the permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); - AssertOnTimeout($"[Unlock][Permissions Mismatch] {firstInstance.name}: \n {m_ErrorLog}"); + AssertOnTimeout($"[Unlock][Permissions Mismatch] {firstInstance.name}"); // Sanity check to assure this client's instance isn't already the owner. - Assert.True(!secondInstance.IsOwner, $"[Ownership Check] Client-{m_ClientNetworkManagers[1].LocalClientId} already is the owner!"); + Assert.True(!secondInstance.IsOwner, $"[Ownership Check] Client-{secondClient.LocalClientId} already is the owner!"); + + // With transferable ownership, the second client shouldn't be able to request ownership + var requestStatus = secondInstance.RequestOwnership(); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestRequiredNotSet, $"Client-{secondClient.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); + // Now try to acquire ownership - secondInstance.ChangeOwnership(m_ClientNetworkManagers[1].LocalClientId); + secondInstance.ChangeOwnership(secondClient.LocalClientId); // Validate the permissions value for all instances are the same yield return WaitForConditionOrTimeOut(() => secondInstance.IsOwner); - AssertOnTimeout($"[Acquire Ownership Failed] Client-{m_ClientNetworkManagers[1].LocalClientId} failed to get ownership!"); + AssertOnTimeout($"[Acquire Ownership Failed] Client-{secondClient.LocalClientId} failed to get ownership!"); m_ObjectToValidate = OwnershipPermissionsTestHelper.CurrentOwnedInstance; // Validate all other client instances are showing the same owner - yield return WaitForConditionOrTimeOut(() => ValidateAllInstancesAreOwnedByClient(m_ClientNetworkManagers[1].LocalClientId)); + yield return WaitForConditionOrTimeOut(() => ValidateAllInstancesAreOwnedByClient(secondClient.LocalClientId)); AssertOnTimeout($"[Ownership Mismatch] {secondInstance.name}: \n {m_ErrorLog}"); // Clear the flags, set the permissions to RequestRequired, and lock ownership in one pass. @@ -209,10 +193,10 @@ public IEnumerator ValidateOwnershipPermissionsTest() // Validate the permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); - AssertOnTimeout($"[Unlock][Permissions Mismatch] {secondInstance.name}: \n {m_ErrorLog}"); + AssertOnTimeout($"[Unlock][Permissions Mismatch] {secondInstance.name}"); // Attempt to acquire ownership by just changing it - firstInstance.ChangeOwnership(firstInstance.NetworkManager.LocalClientId); + firstInstance.ChangeOwnership(firstClient.LocalClientId); // Assure we are denied ownership due to it requiring ownership be requested Assert.IsTrue(firstInstanceHelper.OwnershipPermissionsFailureStatus == NetworkObject.OwnershipPermissionsFailureStatus.RequestRequired, @@ -224,52 +208,64 @@ public IEnumerator ValidateOwnershipPermissionsTest() ////////////////////////////////// // Start with a request for the client we expect to be given ownership - var requestStatus = firstInstance.RequestOwnership(); - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{firstInstance.NetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); + requestStatus = firstInstance.RequestOwnership(); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{firstClient.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); // Get the 3rd client to send a request at the "relatively" same time - var thirdInstance = m_ClientNetworkManagers[2].SpawnManager.SpawnedObjects[networkObjectId]; + var thirdClient = GetNonAuthorityNetworkManager(2); + var thirdInstance = thirdClient.SpawnManager.SpawnedObjects[networkObjectId]; var thirdInstanceHelper = thirdInstance.GetComponent(); // At the same time send a request by the third client. requestStatus = thirdInstance.RequestOwnership(); // We expect the 3rd client's request should be able to be sent at this time as well (i.e. creates the race condition between two clients) - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{thirdInstance.NetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{thirdClient.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); // We expect the first requesting client to be given ownership yield return WaitForConditionOrTimeOut(() => firstInstance.IsOwner); - AssertOnTimeout($"[Acquire Ownership Failed] Client-{firstInstance.NetworkManager.LocalClientId} failed to get ownership! ({firstInstanceHelper.OwnershipRequestResponseStatus})(Owner: {OwnershipPermissionsTestHelper.CurrentOwnedInstance.OwnerClientId}"); + AssertOnTimeout($"[Acquire Ownership Failed] Client-{firstClient.LocalClientId} failed to get ownership! ({firstInstanceHelper.OwnershipRequestResponseStatus})(Owner: {OwnershipPermissionsTestHelper.CurrentOwnedInstance.OwnerClientId}"); m_ObjectToValidate = OwnershipPermissionsTestHelper.CurrentOwnedInstance; // Just do a sanity check to assure ownership has changed on all clients. - yield return WaitForConditionOrTimeOut(() => ValidateAllInstancesAreOwnedByClient(firstInstance.NetworkManager.LocalClientId)); + yield return WaitForConditionOrTimeOut(() => ValidateAllInstancesAreOwnedByClient(firstClient.LocalClientId)); AssertOnTimeout($"[Ownership Mismatch] {firstInstance.name}: \n {m_ErrorLog}"); // Now, the third client should get a RequestInProgress returned as their request response yield return WaitForConditionOrTimeOut(() => thirdInstanceHelper.OwnershipRequestResponseStatus == NetworkObject.OwnershipRequestResponseStatus.RequestInProgress); - AssertOnTimeout($"[Request In Progress Failed] Client-{thirdInstanceHelper.NetworkManager.LocalClientId} did not get the right request denied reponse!"); + AssertOnTimeout($"[Request In Progress Failed] Client-{thirdClient.LocalClientId} did not get the right request denied response!"); // Validate the permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); - AssertOnTimeout($"[Unlock][Permissions Mismatch] {firstInstance.name}: \n {m_ErrorLog}"); + AssertOnTimeout($"[Unlock][Permissions Mismatch] {firstInstance.name}"); + + // Check for various permissions denied race conditions with changing permissions and requesting ownership + yield return ValidateRequestAndPermissionsChangeRaceCondition(NetworkObject.OwnershipStatus.Distributable, NetworkObject.OwnershipLockActions.SetAndUnlock, NetworkObject.OwnershipRequestResponseStatus.CannotRequest, firstClient, firstInstance, secondInstance, secondInstanceHelper); + yield return ValidateRequestAndPermissionsChangeRaceCondition(NetworkObject.OwnershipStatus.Transferable, NetworkObject.OwnershipLockActions.SetAndLock, NetworkObject.OwnershipRequestResponseStatus.Locked, firstClient, firstInstance, secondInstance, secondInstanceHelper); + + // Should successfully change ownership to secondClient + yield return ValidateRequestAndPermissionsChangeRaceCondition(NetworkObject.OwnershipStatus.Transferable, NetworkObject.OwnershipLockActions.SetAndUnlock, NetworkObject.OwnershipRequestResponseStatus.Approved, firstClient, firstInstance, secondInstance, secondInstanceHelper); + + // Transfer ownership back to firstClient. ValidateRequestAndPermissionsChangeRaceCondition will reset permissions to RequestRequired + yield return ValidateRequestAndPermissionsChangeRaceCondition(NetworkObject.OwnershipStatus.Transferable, NetworkObject.OwnershipLockActions.None, NetworkObject.OwnershipRequestResponseStatus.Approved, secondClient, secondInstance, firstInstance, firstInstanceHelper, false); /////////////////////////////////////////////// // Test for multiple ownership race conditions: /////////////////////////////////////////////// // Get the 4th client's instance - var fourthInstance = m_ClientNetworkManagers[3].SpawnManager.SpawnedObjects[networkObjectId]; + var fourthClient = GetNonAuthorityNetworkManager(3); + var fourthInstance = fourthClient.SpawnManager.SpawnedObjects[networkObjectId]; var fourthInstanceHelper = fourthInstance.GetComponent(); // Send out a request from three clients at the same time // The first one sent (and received for this test) gets ownership requestStatus = secondInstance.RequestOwnership(); - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{secondInstance.NetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{secondClient.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); requestStatus = thirdInstance.RequestOwnership(); - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{thirdInstance.NetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{thirdClient.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); requestStatus = fourthInstance.RequestOwnership(); - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{fourthInstance.NetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{fourthClient.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); // The 2nd and 3rd client should be denied and the 4th client should be approved yield return WaitForConditionOrTimeOut(() => @@ -277,15 +273,19 @@ public IEnumerator ValidateOwnershipPermissionsTest() (thirdInstanceHelper.OwnershipRequestResponseStatus == NetworkObject.OwnershipRequestResponseStatus.RequestInProgress) && (secondInstanceHelper.OwnershipRequestResponseStatus == NetworkObject.OwnershipRequestResponseStatus.Approved) ); - AssertOnTimeout($"[Targeted Owner] Client-{secondInstanceHelper.NetworkManager.LocalClientId} did not get the right request denied reponse: {secondInstanceHelper.OwnershipRequestResponseStatus}!"); + AssertOnTimeout($"[Targeted Owner] A client received an incorrect response." + + $"\n Client-{fourthClient.LocalClientId}: Expected {NetworkObject.OwnershipRequestResponseStatus.RequestInProgress}, and got {fourthInstanceHelper.OwnershipRequestResponseStatus}!" + + $"\n Client-{thirdClient.LocalClientId}: Expected {NetworkObject.OwnershipRequestResponseStatus.RequestInProgress}, and got {thirdInstanceHelper.OwnershipRequestResponseStatus}!" + + $"\n Client-{secondClient.LocalClientId}: Expected {NetworkObject.OwnershipRequestResponseStatus.Denied}, and got {secondInstanceHelper.OwnershipRequestResponseStatus}!"); + m_ObjectToValidate = OwnershipPermissionsTestHelper.CurrentOwnedInstance; // Just do a sanity check to assure ownership has changed on all clients. - yield return WaitForConditionOrTimeOut(() => ValidateAllInstancesAreOwnedByClient(secondInstance.NetworkManager.LocalClientId)); - AssertOnTimeout($"[Ownership Mismatch] {secondInstance.name}: \n {m_ErrorLog}"); + yield return WaitForConditionOrTimeOut(() => ValidateAllInstancesAreOwnedByClient(secondClient.LocalClientId)); + AssertOnTimeout($"[Multiple request race condition][Ownership Mismatch] {secondInstance.name}: \n {m_ErrorLog}"); // Validate the permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); - AssertOnTimeout($"[Unlock][Permissions Mismatch] {secondInstance.name}: \n {m_ErrorLog}"); + AssertOnTimeout($"[Multiple request race condition][Permissions Mismatch] {secondInstance.name}"); /////////////////////////////////////////////// // Test for targeted ownership request: @@ -293,74 +293,78 @@ public IEnumerator ValidateOwnershipPermissionsTest() // Now get the DAHost's client's instance var authority = GetAuthorityNetworkManager(); - var daHostInstance = authority.SpawnManager.SpawnedObjects[networkObjectId]; - var daHostInstanceHelper = daHostInstance.GetComponent(); + var authorityInstance = authority.SpawnManager.SpawnedObjects[networkObjectId]; + var authorityInstanceHelper = authorityInstance.GetComponent(); secondInstanceHelper.AllowOwnershipRequest = true; secondInstanceHelper.OnlyAllowTargetClientId = true; - secondInstanceHelper.ClientToAllowOwnership = daHostInstance.NetworkManager.LocalClientId; + secondInstanceHelper.ClientToAllowOwnership = authority.LocalClientId; // Send out a request from all three clients requestStatus = firstInstance.RequestOwnership(); - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{firstInstance.NetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{firstClient.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); requestStatus = thirdInstance.RequestOwnership(); - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{thirdInstance.NetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{thirdClient.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); requestStatus = fourthInstance.RequestOwnership(); - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{fourthInstance.NetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); - requestStatus = daHostInstance.RequestOwnership(); - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{daHostInstance.NetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{fourthClient.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); + requestStatus = authorityInstance.RequestOwnership(); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{authority.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); // Only the client marked as ClientToAllowOwnership (daHost) should be approved. All others should be denied. yield return WaitForConditionOrTimeOut(() => (firstInstanceHelper.OwnershipRequestResponseStatus == NetworkObject.OwnershipRequestResponseStatus.Denied) && (thirdInstanceHelper.OwnershipRequestResponseStatus == NetworkObject.OwnershipRequestResponseStatus.Denied) && (fourthInstanceHelper.OwnershipRequestResponseStatus == NetworkObject.OwnershipRequestResponseStatus.Denied) && - (daHostInstanceHelper.OwnershipRequestResponseStatus == NetworkObject.OwnershipRequestResponseStatus.Approved) + (authorityInstanceHelper.OwnershipRequestResponseStatus == NetworkObject.OwnershipRequestResponseStatus.Approved) ); - AssertOnTimeout($"[Targeted Owner] Client-{daHostInstance.NetworkManager.LocalClientId} did not get the right request response: {daHostInstanceHelper.OwnershipRequestResponseStatus} Expecting: {NetworkObject.OwnershipRequestResponseStatus.Approved}!"); + AssertOnTimeout($"[Targeted Owner] A client received an incorrect response." + + $"\n Client-{firstClient.LocalClientId}: Expected {NetworkObject.OwnershipRequestResponseStatus.Denied}, and got {firstInstanceHelper.OwnershipRequestResponseStatus}!" + + $"\n Client-{thirdClient.LocalClientId}: Expected {NetworkObject.OwnershipRequestResponseStatus.Denied}, and got {thirdInstanceHelper.OwnershipRequestResponseStatus}!" + + $"\n Client-{fourthClient.LocalClientId}: Expected {NetworkObject.OwnershipRequestResponseStatus.Denied}, and got {fourthInstanceHelper.OwnershipRequestResponseStatus}!" + + $"\n Client-{authority.LocalClientId}: Expected {NetworkObject.OwnershipRequestResponseStatus.Approved}, and got {authorityInstanceHelper.OwnershipRequestResponseStatus}!"); /////////////////////////////////////////////// // Test OwnershipStatus.SessionOwner: /////////////////////////////////////////////// - OwnershipPermissionsTestHelper.CurrentOwnedInstance = daHostInstance; + OwnershipPermissionsTestHelper.CurrentOwnedInstance = authorityInstance; m_ObjectToValidate = OwnershipPermissionsTestHelper.CurrentOwnedInstance; // Add multiple statuses - daHostInstance.SetOwnershipStatus(NetworkObject.OwnershipStatus.Transferable | NetworkObject.OwnershipStatus.SessionOwner); + authorityInstance.SetOwnershipStatus(NetworkObject.OwnershipStatus.Transferable | NetworkObject.OwnershipStatus.SessionOwner); // Validate the permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); - AssertOnTimeout($"[Add][Permissions Mismatch] {daHostInstance.name}: \n {m_ErrorLog}"); + AssertOnTimeout($"[Add][Permissions Mismatch] {authorityInstance.name}"); // Trying to set SessionOwner flag should override any other flags. - Assert.IsFalse(daHostInstance.HasOwnershipStatus(NetworkObject.OwnershipStatus.Transferable), $"[Set][SessionOwner flag Failure] Expected: {NetworkObject.OwnershipStatus.Transferable} not to be set!"); + Assert.IsFalse(authorityInstance.HasOwnershipStatus(NetworkObject.OwnershipStatus.Transferable), $"[Set][SessionOwner flag Failure] Expected: {NetworkObject.OwnershipStatus.Transferable} not to be set!"); // Add another status. Should fail as SessionOwner should be exclusive - daHostInstance.SetOwnershipStatus(NetworkObject.OwnershipStatus.Distributable); - Assert.IsFalse(daHostInstance.HasOwnershipStatus(NetworkObject.OwnershipStatus.Distributable), $"[Add][SessionOwner flag Failure] Expected: {NetworkObject.OwnershipStatus.Transferable} not to be set!"); + authorityInstance.SetOwnershipStatus(NetworkObject.OwnershipStatus.Distributable); + Assert.IsFalse(authorityInstance.HasOwnershipStatus(NetworkObject.OwnershipStatus.Distributable), $"[Add][SessionOwner flag Failure] Expected: {NetworkObject.OwnershipStatus.Transferable} not to be set!"); // Request ownership of the SessionOwner flag instance requestStatus = firstInstance.RequestOwnership(); - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestRequiredNotSet, $"Client-{firstInstance.NetworkManager.LocalClientId} should not be able to send a request for ownership because object is marked as owned by the session owner. {requestStatus}!"); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestRequiredNotSet, $"Client-{firstClient.LocalClientId} should not be able to send a request for ownership because object is marked as owned by the session owner. {requestStatus}!"); // Set ownership directly on local object. This will allow the request to be sent firstInstance.Ownership = NetworkObject.OwnershipStatus.RequestRequired; requestStatus = firstInstance.RequestOwnership(); - Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{firstInstance.NetworkManager.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"Client-{firstClient.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); // Request should be denied with CannotRequest yield return WaitForConditionOrTimeOut(() => firstInstanceHelper.OwnershipRequestResponseStatus == NetworkObject.OwnershipRequestResponseStatus.CannotRequest); - AssertOnTimeout($"[Targeted Owner] Client-{firstInstance.NetworkManager.LocalClientId} did not get the right request response: {daHostInstanceHelper.OwnershipRequestResponseStatus} Expecting: {NetworkObject.OwnershipRequestResponseStatus.CannotRequest}!"); + AssertOnTimeout($"[Targeted Owner] Client-{firstClient.LocalClientId} did not get the right request response: {firstInstanceHelper.OwnershipRequestResponseStatus} Expecting: {NetworkObject.OwnershipRequestResponseStatus.CannotRequest}!"); // Try changing the ownership explicitly - // Get the cloned daHostInstance instance on a client side - var clientInstance = m_ClientNetworkManagers[2].SpawnManager.SpawnedObjects[daHostInstance.NetworkObjectId]; + // Get the cloned authorityClient instance on a client side + var clientInstance = thirdClient.SpawnManager.SpawnedObjects[authorityInstance.NetworkObjectId]; // Get the client instance of the OwnershipPermissionsTestHelper component var clientInstanceHelper = clientInstance.GetComponent(); // Have the client attempt to change ownership - clientInstance.ChangeOwnership(m_ClientNetworkManagers[2].LocalClientId); + clientInstance.ChangeOwnership(thirdClient.LocalClientId); // Verify the client side gets a permission failure status of NetworkObject.OwnershipPermissionsFailureStatus.SessionOwnerOnly Assert.IsTrue(clientInstanceHelper.OwnershipPermissionsFailureStatus == NetworkObject.OwnershipPermissionsFailureStatus.SessionOwnerOnly, @@ -368,58 +372,102 @@ public IEnumerator ValidateOwnershipPermissionsTest() $" status is {clientInstanceHelper.OwnershipPermissionsFailureStatus}!"); // Have the session owner attempt to change ownership to a non-session owner - daHostInstance.ChangeOwnership(m_ClientNetworkManagers[2].LocalClientId); + authorityInstance.ChangeOwnership(thirdClient.LocalClientId); - // Verify the session owner cannot assign a SessionOwner permission NetworkObject to a non-sessionowner client - Assert.IsTrue(daHostInstanceHelper.OwnershipPermissionsFailureStatus == NetworkObject.OwnershipPermissionsFailureStatus.SessionOwnerOnly, - $"Expected {daHostInstance.name} to return {NetworkObject.OwnershipPermissionsFailureStatus.SessionOwnerOnly} but its permission failure" + - $" status is {daHostInstanceHelper.OwnershipPermissionsFailureStatus}!"); + // Verify the session owner cannot assign a SessionOwner permission NetworkObject to a non-authority client + Assert.IsTrue(authorityInstanceHelper.OwnershipPermissionsFailureStatus == NetworkObject.OwnershipPermissionsFailureStatus.SessionOwnerOnly, + $"Expected {authorityInstance.name} to return {NetworkObject.OwnershipPermissionsFailureStatus.SessionOwnerOnly} but its permission failure" + + $" status is {authorityInstanceHelper.OwnershipPermissionsFailureStatus}!"); // Remove status - daHostInstance.RemoveOwnershipStatus(NetworkObject.OwnershipStatus.SessionOwner); + authorityInstance.RemoveOwnershipStatus(NetworkObject.OwnershipStatus.SessionOwner); // Validate the permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); - AssertOnTimeout($"[Remove][Permissions Mismatch] {daHostInstance.name}: \n {m_ErrorLog}"); + AssertOnTimeout($"[Remove][Permissions Mismatch] {authorityInstance.name}"); } [UnityTest] public IEnumerator ChangeOwnershipWithoutObservers() { - var initialLogLevel = m_ServerNetworkManager.LogLevel; - m_ServerNetworkManager.LogLevel = LogLevel.Developer; - var firstInstance = SpawnObject(m_PermissionsObject, m_ServerNetworkManager).GetComponent(); - OwnershipPermissionsTestHelper.CurrentOwnedInstance = firstInstance; - var firstInstanceHelper = firstInstance.GetComponent(); - var networkObjectId = firstInstance.NetworkObjectId; + var authority = GetAuthorityNetworkManager(); + var initialLogLevel = authority.LogLevel; + authority.LogLevel = LogLevel.Developer; + + var authorityInstance = SpawnObject(m_PermissionsObject, authority).GetComponent(); + OwnershipPermissionsTestHelper.CurrentOwnedInstance = authorityInstance; m_ObjectToValidate = OwnershipPermissionsTestHelper.CurrentOwnedInstance; - yield return WaitForConditionOrTimeOut(ValidateObjectSpawnedOnAllClients); - AssertOnTimeout($"[Failed To Spawn] {firstInstance.name}: \n {m_ErrorLog}"); - firstInstance.SetOwnershipStatus(NetworkObject.OwnershipStatus.Transferable, true); + yield return WaitForSpawnedOnAllOrTimeOut(authorityInstance); + AssertOnTimeout($"[Failed To Spawn] {authorityInstance.name}"); + + authorityInstance.SetOwnershipStatus(NetworkObject.OwnershipStatus.Transferable, true); // Validate the base non-assigned permissions value for all instances are the same. yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); - AssertOnTimeout($"[Permissions Mismatch] {firstInstance.name}: \n {m_ErrorLog}"); + AssertOnTimeout($"[Permissions Mismatch] {authorityInstance.name}"); - var secondInstance = m_ClientNetworkManagers[0].SpawnManager.SpawnedObjects[networkObjectId]; + var otherClient = GetNonAuthorityNetworkManager(0); + var otherInstance = otherClient.SpawnManager.SpawnedObjects[authorityInstance.NetworkObjectId]; // Remove the client from the observers list - firstInstance.Observers.Remove(m_ClientNetworkManagers[0].LocalClientId); + authorityInstance.Observers.Remove(otherClient.LocalClientId); // ChangeOwnership should fail - firstInstance.ChangeOwnership(m_ClientNetworkManagers[0].LocalClientId); - LogAssert.Expect(LogType.Warning, "[Session-Owner Sender=0] [Invalid Owner] Cannot send Ownership change as client-1 cannot see PermObject{2}-OnServer{0}! Use NetworkShow first."); - Assert.True(firstInstance.IsOwner, $"[Ownership Check] Client-{m_ServerNetworkManager.LocalClientId} should still own this object!"); + authorityInstance.ChangeOwnership(otherClient.LocalClientId); + var senderId = authority.LocalClientId; + var receiverId = otherClient.LocalClientId; + LogAssert.Expect(LogType.Warning, $"[Session-Owner Sender={senderId}] [Invalid Owner] Cannot send Ownership change as client-{receiverId} cannot see {authorityInstance.name}! Use NetworkShow first."); + Assert.True(authorityInstance.IsOwner, $"[Ownership Check] Client-{senderId} should still own this object!"); // Now re-add the client to the Observers list and try to change ownership - firstInstance.Observers.Add(m_ClientNetworkManagers[0].LocalClientId); - firstInstance.ChangeOwnership(m_ClientNetworkManagers[0].LocalClientId); + authorityInstance.Observers.Add(otherClient.LocalClientId); + authorityInstance.ChangeOwnership(otherClient.LocalClientId); - // Validate the second client now owns the object - yield return WaitForConditionOrTimeOut(() => secondInstance.IsOwner); - AssertOnTimeout($"[Acquire Ownership Failed] Client-{m_ClientNetworkManagers[0].LocalClientId} failed to get ownership!"); + // Validate the non-authority client now owns the object + yield return WaitForConditionOrTimeOut(() => otherInstance.IsOwner); + AssertOnTimeout($"[Acquire Ownership Failed] Client-{otherClient.LocalClientId} failed to get ownership!"); + + authority.LogLevel = initialLogLevel; + } + + private IEnumerator ValidateRequestAndPermissionsChangeRaceCondition(NetworkObject.OwnershipStatus newStatus, NetworkObject.OwnershipLockActions lockActions, NetworkObject.OwnershipRequestResponseStatus expectedResponseStatus, NetworkManager firstClient, NetworkObject firstInstance, NetworkObject secondInstance, OwnershipPermissionsTestHelper secondInstanceHelper, bool setFlagsFirst = true) + { + var secondClientId = secondInstance.NetworkManager.LocalClientId; + + if (setFlagsFirst) + { + firstInstance.SetOwnershipStatus(newStatus, true, lockActions); + } + + // Request ownership + var requestStatus = secondInstance.RequestOwnership(); + + if (!setFlagsFirst) + { + firstInstance.SetOwnershipStatus(newStatus, true, lockActions); + + } + + // We expect the request should be able to be sent at this time as well (i.e. creates the race condition between request and permissions change) + Assert.True(requestStatus == NetworkObject.OwnershipRequestStatus.RequestSent, $"[{newStatus}] Client-{firstClient.LocalClientId} was unable to send a request for ownership because: {requestStatus}!"); - m_ServerNetworkManager.LogLevel = initialLogLevel; + yield return WaitForConditionOrTimeOut(() => secondInstanceHelper.OwnershipRequestResponseStatus == expectedResponseStatus); + AssertOnTimeout($"[{newStatus}][Request race condition failed] Client-{secondClientId} did not get the right request response status! Expected: {expectedResponseStatus} Received: {secondInstanceHelper.OwnershipRequestResponseStatus}!"); + + var expectedOwner = expectedResponseStatus == NetworkObject.OwnershipRequestResponseStatus.Approved ? secondClientId : firstClient.LocalClientId; + yield return WaitForConditionOrTimeOut(() => ValidateAllInstancesAreOwnedByClient(expectedOwner)); + AssertOnTimeout($"[{newStatus}][Request race condition][Ownership Mismatch] Expected Client-{expectedOwner} to have ownership: \n {m_ErrorLog}"); + + // Owner permissions should prevail - ownership shouldn't change + yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); + AssertOnTimeout($"[{newStatus}][Unlock][Race condition permissions mismatch] {secondInstance.name}"); + + // Reset the permissions to requestRequired + var finalInstance = expectedResponseStatus == NetworkObject.OwnershipRequestResponseStatus.Approved ? secondInstance : firstInstance; + + finalInstance.SetOwnershipStatus(NetworkObject.OwnershipStatus.RequestRequired, true); + yield return WaitForConditionOrTimeOut(ValidatePermissionsOnAllClients); + AssertOnTimeout($"[{newStatus}][Set RequestRequired][Permissions mismatch] {firstClient.name}"); } internal class OwnershipPermissionsTestHelper : NetworkBehaviour diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs index bc647dffde..503fddc498 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTest.cs @@ -1891,7 +1891,7 @@ public bool WaitForConditionOrTimeOutWithTimeTravel(IConditionalPredicate condit } /// - /// Waits until the specified condition returns true or a timeout occurs, then asserts if the timeout was reached. + /// Waits until the specified condition returns true or a timeout occurs. /// This overload allows the condition to provide additional error details via a . /// /// A delegate that takes a for error details and returns true when the desired condition is met. @@ -1907,6 +1907,32 @@ protected IEnumerator WaitForConditionOrTimeOut(Func checkF }, timeOutHelper); } + /// + /// Waits until the given NetworkObject is spawned on all clients or a timeout occurs. + /// + /// The to watch for. + /// An optional to control the timeout period. If null, the default timeout is used. + /// An for use in Unity coroutines. + protected IEnumerator WaitForSpawnedOnAllOrTimeOut(NetworkObject instanceNetworkObject, TimeoutHelper timeOutHelper = null) + { + var networkObjectId = instanceNetworkObject.GetComponent().NetworkObjectId; + + bool ValidateObjectSpawnedOnAllClients(StringBuilder errorLog) + { + foreach (var client in m_NetworkManagers) + { + if (!client.SpawnManager.SpawnedObjects.ContainsKey(networkObjectId)) + { + errorLog.Append($"Client-{client.LocalClientId} has not spawned Object-{networkObjectId}!"); + return false; + } + } + return true; + } + + yield return WaitForConditionOrTimeOut(ValidateObjectSpawnedOnAllClients, timeOutHelper); + } + /// /// Validates that all remote clients (i.e. non-server) detect they are connected /// to the server and that the server reflects the appropriate number of clients @@ -2206,6 +2232,7 @@ private List SpawnObjects(NetworkObject prefabNetworkObject, Network return gameObjectsSpawned; } + /// /// Default constructor /// diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTestHelpers.cs b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTestHelpers.cs index b33f47cd01..687753d1bd 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTestHelpers.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTestHelpers.cs @@ -197,7 +197,7 @@ internal static string GetCMBServiceEnvironentVariable() #if USE_CMB_SERVICE return "true"; #else - return Environment.GetEnvironmentVariable("USE_CMB_SERVICE") ?? "false"; + return Environment.GetEnvironmentVariable("USE_CMB_SERVICE") ?? "true"; #endif } From fcf3622b1a00e7420026fbb31df8300a75dcb51e Mon Sep 17 00:00:00 2001 From: Emma Date: Tue, 9 Sep 2025 13:51:55 -0400 Subject: [PATCH 2/2] Remove TestHelpers change --- .../Tests/Runtime/TestHelpers/NetcodeIntegrationTestHelpers.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTestHelpers.cs b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTestHelpers.cs index 687753d1bd..b33f47cd01 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTestHelpers.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/TestHelpers/NetcodeIntegrationTestHelpers.cs @@ -197,7 +197,7 @@ internal static string GetCMBServiceEnvironentVariable() #if USE_CMB_SERVICE return "true"; #else - return Environment.GetEnvironmentVariable("USE_CMB_SERVICE") ?? "true"; + return Environment.GetEnvironmentVariable("USE_CMB_SERVICE") ?? "false"; #endif }