Skip to content

Commit bda34c5

Browse files
NoelStephensUnityNoelStephensUnity
committed
Merge branch 'develop-2.0.0' into feat/add-singleplayer-transport-component
2 parents 06063cc + 964e0ca commit bda34c5

File tree

6 files changed

+132
-15
lines changed

6 files changed

+132
-15
lines changed

‎com.unity.netcode.gameobjects/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
1515

1616
### Fixed
1717

18+
- Fixed issue where invoking `NetworkObject.NetworkShow` and `NetworkObject.ChangeOwnership` consecutively within the same call stack location could result in an unnecessary change in ownership error message generated on the target client side. (#3468)
1819
- Fixed issue where `NetworkVariable`s on a `NetworkBehaviour` could fail to synchronize changes if one has `NetworkVariableUpdateTraits` set and is dirty but is not ready to send. (#3466)
1920
- Fixed inconsistencies in the `OnSceneEvent` callback. (#3458)
2021
- Fixed issues with the `NetworkBehaviour` and `NetworkVariable` length safety checks. (#3405)

‎com.unity.netcode.gameobjects/Runtime/Messaging/Messages/ChangeOwnershipMessage.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ private void HandleOwnershipChange(ref NetworkContext context)
333333
if (networkObject.OwnerClientId == OwnerClientId)
334334
{
335335
// Log error and then ignore the message
336-
UnityEngine.Debug.LogError($"Client-{context.SenderId} ({RequestClientId}) sent unnecessary ownership changed message for {NetworkObjectId}.");
336+
NetworkLog.LogError($"[Receiver: Client-{networkManager.LocalClientId}][Sender: Client-{context.SenderId}][RID: {RequestClientId}] Detected unnecessary ownership changed message for {networkObject.name} (NID:{NetworkObjectId}).");
337337
return;
338338
}
339339

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

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,7 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool
565565
}
566566

567567
var size = 0;
568+
568569
if (NetworkManager.DistributedAuthorityMode)
569570
{
570571
var message = new ChangeOwnershipMessage
@@ -578,24 +579,31 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool
578579
OwnershipFlags = (ushort)networkObject.Ownership,
579580
};
580581
// If we are connected to the CMB service or not the DAHost (i.e. pure DA-Clients only)
582+
581583
if (NetworkManager.CMBServiceConnection || !NetworkManager.DAHost)
582584
{
583585
// Always update the network properties in distributed authority mode for the client gaining ownership
584586
for (int i = 0; i < networkObject.ChildNetworkBehaviours.Count; i++)
585587
{
586588
networkObject.ChildNetworkBehaviours[i].UpdateNetworkProperties();
587589
}
590+
591+
// Populate valid target client identifiers that should receive this change in ownership message.
592+
message.ClientIds = NetworkManager.ConnectedClientsIds.Where((c) => !IsObjectVisibilityPending(c, ref networkObject) && networkObject.IsNetworkVisibleTo(c)).ToArray();
593+
message.ClientIdCount = message.ClientIds.Length;
594+
588595
size = NetworkManager.ConnectionManager.SendMessage(ref message, NetworkDelivery.ReliableSequenced, NetworkManager.ServerClientId);
589596
NetworkManager.NetworkMetrics.TrackOwnershipChangeSent(NetworkManager.LocalClientId, networkObject, size);
590597
}
591598
else // We are the DAHost so broadcast the ownership change
592599
{
593600
foreach (var client in NetworkManager.ConnectedClients)
594601
{
595-
if (client.Value.ClientId == NetworkManager.ServerClientId)
602+
if (client.Value.ClientId == NetworkManager.ServerClientId || IsObjectVisibilityPending(client.Key, ref networkObject))
596603
{
597604
continue;
598605
}
606+
599607
if (networkObject.IsNetworkVisibleTo(client.Value.ClientId))
600608
{
601609
size = NetworkManager.ConnectionManager.SendMessage(ref message, NetworkDelivery.ReliableSequenced, client.Value.ClientId);
@@ -613,8 +621,17 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool
613621
};
614622
foreach (var client in NetworkManager.ConnectedClients)
615623
{
624+
if (client.Value.ClientId == NetworkManager.ServerClientId || IsObjectVisibilityPending(client.Key, ref networkObject))
625+
{
626+
continue;
627+
}
616628
if (networkObject.IsNetworkVisibleTo(client.Value.ClientId))
617629
{
630+
if (client.Key != client.Value.ClientId)
631+
{
632+
NetworkLog.LogError($"[Client-{client.Key}] Client key ({client.Key}) does not match the {nameof(NetworkClient)} client Id {client.Value.ClientId}! Client-{client.Key} will not receive ownership changed message!");
633+
continue;
634+
}
618635
size = NetworkManager.ConnectionManager.SendMessage(ref message, NetworkDelivery.ReliableSequenced, client.Value.ClientId);
619636
NetworkManager.NetworkMetrics.TrackOwnershipChangeSent(client.Key, networkObject, size);
620637
}
@@ -639,6 +656,27 @@ internal void ChangeOwnership(NetworkObject networkObject, ulong clientId, bool
639656
}
640657
}
641658

659+
/// <summary>
660+
/// Will determine if a client has been granted visibility for a NetworkObject but
661+
/// the <see cref="CreateObjectMessage"/> has yet to be generated for it. Under this case,
662+
/// the client might not need to be sent a message (i.e. <see cref="ChangeOwnershipMessage")
663+
/// </summary>
664+
/// <param name="clientId">the client to check</param>
665+
/// <param name="networkObject">the <see cref="NetworkObject"/> to check if it is pending show</param>
666+
[System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.AggressiveInlining)]
667+
internal bool IsObjectVisibilityPending(ulong clientId, ref NetworkObject networkObject)
668+
{
669+
if (NetworkManager.DistributedAuthorityMode && ClientsToShowObject.ContainsKey(networkObject))
670+
{
671+
return ClientsToShowObject[networkObject].Contains(clientId);
672+
}
673+
else if (ObjectsToShowToClient.ContainsKey(clientId))
674+
{
675+
return ObjectsToShowToClient[clientId].Contains(networkObject);
676+
}
677+
return false;
678+
}
679+
642680
internal bool HasPrefab(NetworkObject.SceneObject sceneObject)
643681
{
644682
if (!NetworkManager.NetworkConfig.EnableSceneManagement || !sceneObject.IsSceneObject)

‎com.unity.netcode.gameobjects/Tests/Runtime/Metrics/OwnershipChangeMetricsTests.cs‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,6 @@ public IEnumerator TrackOwnershipChangeSentMetric()
7373

7474
var ownershipChangeSent = metricValues.First();
7575
Assert.AreEqual(networkObject.NetworkObjectId, ownershipChangeSent.NetworkId.NetworkId);
76-
Assert.AreEqual(Server.LocalClientId, ownershipChangeSent.Connection.Id);
77-
Assert.AreEqual(0, ownershipChangeSent.BytesCount);
78-
79-
// The first metric is to the server(self), so its size is now correctly reported as 0.
80-
// Let's check the last one instead, to have a valid value
81-
ownershipChangeSent = metricValues.Last();
82-
Assert.AreEqual(networkObject.NetworkObjectId, ownershipChangeSent.NetworkId.NetworkId);
8376
Assert.AreEqual(Client.LocalClientId, ownershipChangeSent.Connection.Id);
8477

8578
var serializedLength = GetWriteSizeForOwnerChange(networkObject, 1);

‎com.unity.netcode.gameobjects/Tests/Runtime/NetworkShowHideTests.cs‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -753,5 +753,88 @@ public IEnumerator NetworkShowHideAroundListModify()
753753
Compare(ShowHideObject.ObjectsPerClientId[0].MyList, ShowHideObject.ObjectsPerClientId[2].MyList);
754754
}
755755
}
756+
757+
private GameObject m_OwnershipObject;
758+
private NetworkObject m_OwnershipNetworkObject;
759+
private NetworkManager m_NewOwner;
760+
private ulong m_ObjectId;
761+
762+
763+
private bool AllObjectsSpawnedOnClients()
764+
{
765+
foreach (var networkManager in m_NetworkManagers)
766+
{
767+
if (!networkManager.SpawnManager.SpawnedObjects.ContainsKey(m_OwnershipNetworkObject.NetworkObjectId))
768+
{
769+
return false;
770+
}
771+
}
772+
return true;
773+
}
774+
775+
private bool ObjectHiddenOnNonAuthorityClients()
776+
{
777+
foreach (var networkManager in m_NetworkManagers)
778+
{
779+
if (networkManager.LocalClientId == m_OwnershipNetworkObject.OwnerClientId)
780+
{
781+
continue;
782+
}
783+
if (networkManager.SpawnManager.SpawnedObjects.ContainsKey(m_OwnershipNetworkObject.NetworkObjectId))
784+
{
785+
return false;
786+
}
787+
}
788+
return true;
789+
}
790+
791+
private bool OwnershipHasChanged()
792+
{
793+
if (!m_NewOwner.SpawnManager.SpawnedObjects.ContainsKey(m_ObjectId))
794+
{
795+
return false;
796+
}
797+
return m_NewOwner.SpawnManager.SpawnedObjects[m_ObjectId].OwnerClientId == m_NewOwner.LocalClientId;
798+
}
799+
800+
801+
[UnityTest]
802+
public IEnumerator NetworkShowAndChangeOwnership()
803+
{
804+
var authority = GetAuthorityNetworkManager();
805+
806+
m_OwnershipObject = SpawnObject(m_PrefabToSpawn, authority);
807+
m_OwnershipNetworkObject = m_OwnershipObject.GetComponent<NetworkObject>();
808+
809+
yield return WaitForConditionOrTimeOut(AllObjectsSpawnedOnClients);
810+
AssertOnTimeout("Timed out waiting for all clients to spawn the ownership object!");
811+
812+
VerboseDebug($"Hiding object {m_OwnershipNetworkObject.NetworkObjectId} on all clients");
813+
foreach (var client in m_NetworkManagers)
814+
{
815+
if (client == authority)
816+
{
817+
continue;
818+
}
819+
m_OwnershipNetworkObject.NetworkHide(client.LocalClientId);
820+
}
821+
822+
yield return WaitForConditionOrTimeOut(ObjectHiddenOnNonAuthorityClients);
823+
AssertOnTimeout("Timed out waiting for all clients to hide the ownership object!");
824+
825+
m_NewOwner = GetNonAuthorityNetworkManager();
826+
Assert.AreNotEqual(m_OwnershipNetworkObject.OwnerClientId, m_NewOwner.LocalClientId, $"Client-{m_NewOwner.LocalClientId} should not have ownership of object {m_OwnershipNetworkObject.NetworkObjectId}!");
827+
Assert.False(m_NewOwner.SpawnManager.SpawnedObjects.ContainsKey(m_OwnershipNetworkObject.NetworkObjectId), $"Client-{m_NewOwner.LocalClientId} should not have object {m_OwnershipNetworkObject.NetworkObjectId} spawned!");
828+
829+
// Run NetworkShow and ChangeOwnership directly after one-another
830+
VerboseDebug($"Calling {nameof(NetworkObject.NetworkShow)} on object {m_OwnershipNetworkObject.NetworkObjectId} for client {m_NewOwner.LocalClientId}");
831+
m_OwnershipNetworkObject.NetworkShow(m_NewOwner.LocalClientId);
832+
VerboseDebug($"Calling {nameof(NetworkObject.ChangeOwnership)} on object {m_OwnershipNetworkObject.NetworkObjectId} for client {m_NewOwner.LocalClientId}");
833+
m_OwnershipNetworkObject.ChangeOwnership(m_NewOwner.LocalClientId);
834+
m_ObjectId = m_OwnershipNetworkObject.NetworkObjectId;
835+
yield return WaitForConditionOrTimeOut(OwnershipHasChanged);
836+
AssertOnTimeout($"Timed out waiting for clients-{m_NewOwner.LocalClientId} to gain ownership of object {m_OwnershipNetworkObject.NetworkObjectId}!");
837+
VerboseDebug($"Client {m_NewOwner.LocalClientId} now owns object {m_OwnershipNetworkObject.NetworkObjectId}!");
838+
}
756839
}
757840
}

‎com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformTests.cs‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ namespace Unity.Netcode.RuntimeTests
8080
internal class NetworkTransformTests : NetworkTransformBase
8181
{
8282
protected const int k_TickRate = 60;
83+
84+
protected const int k_DefaultTimeTravelFrames = 1000;
8385
/// <summary>
8486
/// Constructor
8587
/// </summary>
@@ -749,11 +751,11 @@ public void TestAuthoritativeTransformChangeOneAtATime([Values] TransformSpace t
749751
if (overideState == OverrideState.CommitToTransform)
750752
{
751753
// Wait for the deltas to be pushed
752-
success = WaitForConditionOrTimeOutWithTimeTravel(() => m_AuthoritativeTransform.StatePushed, 600);
754+
success = WaitForConditionOrTimeOutWithTimeTravel(() => m_AuthoritativeTransform.StatePushed, k_DefaultTimeTravelFrames);
753755
Assert.True(success, $"[Position] Timed out waiting for state to be pushed ({m_AuthoritativeTransform.StatePushed})!");
754756
}
755757

756-
success = WaitForConditionOrTimeOutWithTimeTravel(() => PositionsMatch(), 600);
758+
success = WaitForConditionOrTimeOutWithTimeTravel(() => PositionsMatch(), k_DefaultTimeTravelFrames);
757759
Assert.True(success, $"Timed out waiting for positions to match {m_AuthoritativeTransform.transform.position} | {m_NonAuthoritativeTransform.transform.position}");
758760

759761
// test rotation
@@ -784,12 +786,12 @@ public void TestAuthoritativeTransformChangeOneAtATime([Values] TransformSpace t
784786
if (overideState == OverrideState.CommitToTransform)
785787
{
786788
// Wait for the deltas to be pushed
787-
success = WaitForConditionOrTimeOutWithTimeTravel(() => m_AuthoritativeTransform.StatePushed, 600);
789+
success = WaitForConditionOrTimeOutWithTimeTravel(() => m_AuthoritativeTransform.StatePushed, k_DefaultTimeTravelFrames);
788790
Assert.True(success, $"[Rotation] Timed out waiting for state to be pushed ({m_AuthoritativeTransform.StatePushed})!");
789791
}
790792

791793
// Make sure the values match
792-
success = WaitForConditionOrTimeOutWithTimeTravel(() => RotationsMatch(), 600);
794+
success = WaitForConditionOrTimeOutWithTimeTravel(() => RotationsMatch(), k_DefaultTimeTravelFrames);
793795
Assert.True(success, $"Timed out waiting for rotations to match");
794796

795797
m_AuthoritativeTransform.StatePushed = false;
@@ -818,12 +820,12 @@ public void TestAuthoritativeTransformChangeOneAtATime([Values] TransformSpace t
818820
if (overideState == OverrideState.CommitToTransform)
819821
{
820822
// Wait for the deltas to be pushed
821-
success = WaitForConditionOrTimeOutWithTimeTravel(() => m_AuthoritativeTransform.StatePushed, 600);
823+
success = WaitForConditionOrTimeOutWithTimeTravel(() => m_AuthoritativeTransform.StatePushed, k_DefaultTimeTravelFrames);
822824
Assert.True(success, $"[Rotation] Timed out waiting for state to be pushed ({m_AuthoritativeTransform.StatePushed})!");
823825
}
824826

825827
// Make sure the scale values match
826-
success = WaitForConditionOrTimeOutWithTimeTravel(() => ScaleValuesMatch(), 600);
828+
success = WaitForConditionOrTimeOutWithTimeTravel(() => ScaleValuesMatch(), k_DefaultTimeTravelFrames);
827829
Assert.True(success, $"Timed out waiting for scale values to match");
828830
}
829831

0 commit comments

Comments
 (0)