Skip to content

Commit 2749dbb

Browse files
NoelStephensUnityEmandMnetcode-ci-service
authored
fix: [Up-port] mixed authority nested NetworkTransform stops updating non-authority nested NetworkTransforms (#4170)
* fix: [Up-port] mixed authority nested NetworkTransform stops updating non-authority instances Up-port of #4169. The NetworkManager update registration is per-NetworkObject while the authority motion model is per-NetworkTransform, so the first authority instance to initialize removed the entire NetworkObject from the update group and its non-authority siblings stopped being updated. Gate the removal on whether any NetworkTransform on the NetworkObject is still non-authority for that same update. * test: assert the fixed update registration baseline before transferring ownership Without it the post-transfer check passes whether the NetworkObject was unregistered or was never registered for the fixed update to begin with, so it could not tell a transition apart from nothing having happened. * test: collapse the mixed authority fixtures and add GetManagersInstance The root authority mode moved from a TestFixture argument into an array walked inside the test, which takes the fixture count from 4 to 2 and the case count from 8 to 2. Only HostOrServer still needs a session per value. That required moving the nested NetworkTransform components off the player prefab and onto two prefabs spawned and despawned per case, so the owner is now an explicit client rather than every player in turn. NetcodeIntegrationTest gains GetManagersInstance for resolving a NetworkObject relative to a NetworkManager, which asserts rather than throwing KeyNotFoundException when the instance is missing. * test: name the instance in assertion messages instead of its client identifier A dedicated server and a host both have a LocalClientId of 0, so a "Client-0" prefix did not say which instance an assertion failed on. NetcodeIntegrationTest gains GetDisplayName, which returns "Server", "Host" or "Client-" and the identifier. GetManagersInstance and the mixed authority assertion messages use it. Also matches the CHANGELOG entry to the develop-2.0.0 wording. --------- Co-authored-by: Emma <emma.mcmillan@unity3d.com> Co-authored-by: Unity Netcode CI <74025435+netcode-ci-service@users.noreply.github.com>
1 parent e9481cf commit 2749dbb

6 files changed

Lines changed: 242 additions & 52 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
2121
- Issue where moving a `NetworkObject` into another scene made the clients that did not observe it log "Trying to synchronize NetworkObjectId but it was not spawned". The scene migration is now only sent to the clients that observe the `NetworkObject`. (#4176)
2222
- Issue where a `NetworkObject` that was moved into another scene while hidden from a client spawned in that client's active scene when it was shown with `NetworkShow`, instead of the scene it is in on the server. (#4176)
2323
- Fixed issue where a prefab added to a `NetworkPrefabsList` while a session was running was registered more than once and logged a duplicate `GlobalObjectIdHash` error. (#4184)
24+
- Fixed an issue where mixed authority nested `NetworkTransform` instances could stop child/nested instances from updating because an authoritative `NetworkTransform` (root or otherwise) would remove the `NetworkObject` from the update group, preventing non-authoritative instances from updating their state on the authority side. (#4169)
2425

2526
### Security
2627

‎com.unity.netcode.gameobjects/Runtime/Components/NetworkTransform.cs‎

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3753,6 +3753,38 @@ private void ResetInterpolatedStateToCurrentAuthoritativeState()
37533753
m_ScaleInterpolator.ResetTo(transform.parent, transform.localScale, serverTime);
37543754
}
37553755

3756+
/// <summary>
3757+
/// Determines if this <see cref="NetworkObject"/> has any <see cref="NetworkTransform"/> instances that are non-authority and are updated during the same update stage.
3758+
/// </summary>
3759+
/// <remarks>
3760+
/// See <see cref="InternalInitialization"/> to better understand how the <paramref name="forUpdate"/> parameter is used to determine which update stage to check for non-authority <see cref="NetworkTransform"/> instances.
3761+
/// </remarks>
3762+
/// <param name="forUpdate">true to check the instances updated during the standard update and false to check the instances updated during the fixed update.</param>
3763+
/// <returns>true if a non-authority NetworkTransform exists on this NetworkObject and false if there are none.</returns>
3764+
private bool HasNonAuthorityNetworkTransform(bool forUpdate)
3765+
{
3766+
var networkTransforms = NetworkObject.NetworkTransforms;
3767+
for (int i = 0; i < networkTransforms.Count; i++)
3768+
{
3769+
var networkTransform = networkTransforms[i];
3770+
#if COM_UNITY_MODULES_PHYSICS || COM_UNITY_MODULES_PHYSICS2D
3771+
// If the update stages don't match, then skip this instance.
3772+
// Reference:
3773+
// forUpdate is true for the standard update and false for the fixed update.
3774+
// m_UseRigidbodyForMotion is false for the standard update and true for the fixed update.
3775+
if (forUpdate == networkTransform.m_UseRigidbodyForMotion)
3776+
{
3777+
continue;
3778+
}
3779+
#endif
3780+
if (!(networkTransform.IsServerAuthoritative() ? networkTransform.IsServer : networkTransform.IsOwner))
3781+
{
3782+
return true;
3783+
}
3784+
}
3785+
return false;
3786+
}
3787+
37563788
/// <summary>
37573789
/// The internal initialization method to allow for internal API adjustments
37583790
/// </summary>
@@ -3822,8 +3854,12 @@ internal virtual void InternalInitialization(bool isOwnershipChange = false)
38223854

38233855
if (CanCommitToTransform)
38243856
{
3825-
// Make sure authority doesn't get added to updates (no need to do this on the authority side)
3826-
m_CachedNetworkManager.NetworkTransformRegistration(NetworkObject, forUpdate, false);
3857+
// If there are no non-authority NetworkTransform instances on this NetworkObject using this update, then remove this instance from the NetworkManager's update list.
3858+
// Otherwise, we need to keep it registered for updates so the non-authority instances will process their received state updates and apply them to the transform.
3859+
if (!HasNonAuthorityNetworkTransform(forUpdate))
3860+
{
3861+
m_CachedNetworkManager.NetworkTransformRegistration(NetworkObject, forUpdate, false);
3862+
}
38273863
if (UseHalfFloatPrecision)
38283864
{
38293865
m_HalfPositionState = new NetworkDeltaPosition(currentPosition, m_CachedNetworkManager.ServerTime.Tick, math.bool3(SyncPositionX, SyncPositionY, SyncPositionZ));

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

Lines changed: 94 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,97 +1,141 @@
11
using System.Collections;
22
using System.Text;
3+
using NUnit.Framework;
34
using Unity.Netcode.Components;
45
using Unity.Netcode.TestHelpers.Runtime;
56
using UnityEngine;
67
using UnityEngine.TestTools;
78

89
namespace Unity.Netcode.RuntimeTests
910
{
11+
[TestFixture(HostOrServer.Host)]
12+
[TestFixture(HostOrServer.Server)]
1013
internal class NetworkTransformMixedAuthorityTests : IntegrationTestWithApproximation
1114
{
1215
private const float k_MotionMagnitude = 5.5f;
1316
private const int k_Iterations = 4;
1417

1518
protected override int NumberOfClients => 2;
1619

17-
private StringBuilder m_ErrorMsg = new StringBuilder();
18-
19-
protected override void OnCreatePlayerPrefab()
20+
/// <summary>
21+
/// The root's authority mode for each case. The nested child uses the inverse.
22+
/// </summary>
23+
private static readonly NetworkTransform.AuthorityModes[] k_RootAuthorityModes =
2024
{
21-
m_PlayerPrefab.AddComponent<NetworkTransform>();
25+
NetworkTransform.AuthorityModes.Server,
26+
NetworkTransform.AuthorityModes.Owner,
27+
};
28+
29+
private GameObject[] m_MixedAuthorityPrefabs;
2230

23-
var childGameObject = new GameObject();
24-
childGameObject.transform.parent = m_PlayerPrefab.transform;
25-
var childNetworkTransform = childGameObject.AddComponent<NetworkTransform>();
26-
childNetworkTransform.AuthorityMode = NetworkTransform.AuthorityModes.Owner;
27-
childNetworkTransform.InLocalSpace = true;
31+
private StringBuilder m_ErrorMsg = new StringBuilder();
2832

29-
base.OnCreatePlayerPrefab();
33+
public NetworkTransformMixedAuthorityTests(HostOrServer hostOrServer) : base(hostOrServer)
34+
{
3035
}
3136

32-
private void MovePlayers()
37+
protected override void OnServerAndClientsCreated()
3338
{
34-
foreach (var networkManager in m_NetworkManagers)
39+
m_MixedAuthorityPrefabs = new GameObject[k_RootAuthorityModes.Length];
40+
for (int i = 0; i < k_RootAuthorityModes.Length; i++)
3541
{
36-
var direction = GetRandomVector3(-1.0f, 1.0f);
37-
var playerObject = networkManager.LocalClient.PlayerObject;
38-
var playerObjectId = networkManager.LocalClient.PlayerObject.NetworkObjectId;
39-
// Server authoritative
40-
var serverPlayerClone = m_ServerNetworkManager.SpawnManager.SpawnedObjects[playerObjectId];
41-
serverPlayerClone.transform.position += direction * k_MotionMagnitude;
42-
// Owner authoritative
43-
var childTransform = networkManager.LocalClient.PlayerObject.transform.GetChild(0);
44-
childTransform.localPosition += direction * k_MotionMagnitude;
42+
var rootAuthorityMode = k_RootAuthorityModes[i];
43+
var prefab = CreateNetworkObjectPrefab($"MixedAuthority-{rootAuthorityMode}Root");
44+
prefab.AddComponent<NetworkTransform>().AuthorityMode = rootAuthorityMode;
45+
46+
var childGameObject = new GameObject();
47+
childGameObject.transform.parent = prefab.transform;
48+
var childNetworkTransform = childGameObject.AddComponent<NetworkTransform>();
49+
childNetworkTransform.AuthorityMode = InverseOf(rootAuthorityMode);
50+
childNetworkTransform.InLocalSpace = true;
51+
52+
m_MixedAuthorityPrefabs[i] = prefab;
4553
}
54+
55+
base.OnServerAndClientsCreated();
56+
}
57+
58+
private static NetworkTransform.AuthorityModes InverseOf(NetworkTransform.AuthorityModes authorityMode)
59+
{
60+
return authorityMode == NetworkTransform.AuthorityModes.Server ? NetworkTransform.AuthorityModes.Owner : NetworkTransform.AuthorityModes.Server;
61+
}
62+
63+
/// <summary>
64+
/// Returns the instance with authority over a <see cref="NetworkTransform"/> set to the given authority mode.
65+
/// </summary>
66+
private NetworkObject GetAuthorityInstance(NetworkObject instance, NetworkManager owner, NetworkTransform.AuthorityModes authorityMode)
67+
{
68+
return GetManagersInstance(authorityMode == NetworkTransform.AuthorityModes.Server ? m_ServerNetworkManager : owner, instance);
4669
}
4770

48-
private bool AllInstancePositionsMatch()
71+
private bool AllInstancePositionsMatch(NetworkObject instance, NetworkManager owner, NetworkTransform.AuthorityModes rootAuthorityMode)
4972
{
5073
m_ErrorMsg.Clear();
74+
var authorityRootPosition = GetAuthorityInstance(instance, owner, rootAuthorityMode).transform.position;
75+
var authorityChildPosition = GetAuthorityInstance(instance, owner, InverseOf(rootAuthorityMode)).transform.GetChild(0).localPosition;
76+
77+
// The authority instances are compared too. An instance with authority over one nested
78+
// NetworkTransform is still non-authority for the other.
5179
foreach (var networkManager in m_NetworkManagers)
5280
{
53-
var playerObject = networkManager.LocalClient.PlayerObject;
54-
var playerObjectId = networkManager.LocalClient.PlayerObject.NetworkObjectId;
55-
var serverRootPosition = m_ServerNetworkManager.SpawnManager.SpawnedObjects[playerObjectId].transform.position;
56-
var ownerChildPosition = networkManager.LocalClient.PlayerObject.transform.GetChild(0).localPosition;
57-
foreach (var client in m_NetworkManagers)
58-
{
59-
if (client == networkManager)
60-
{
61-
continue;
62-
}
63-
var playerClone = client.SpawnManager.SpawnedObjects[playerObjectId];
64-
var cloneRootPosition = playerClone.transform.position;
65-
var cloneChildPosition = playerClone.transform.GetChild(0).localPosition;
81+
var clone = GetManagersInstance(networkManager, instance);
82+
var cloneRootPosition = clone.transform.position;
83+
var cloneChildPosition = clone.transform.GetChild(0).localPosition;
6684

67-
if (!Approximately(serverRootPosition, cloneRootPosition))
68-
{
69-
m_ErrorMsg.AppendLine($"[{playerObject.name}][{playerClone.name}] Root mismatch ({GetVector3Values(serverRootPosition)})({GetVector3Values(cloneRootPosition)})!");
70-
}
85+
if (!Approximately(authorityRootPosition, cloneRootPosition))
86+
{
87+
m_ErrorMsg.AppendLine($"[{rootAuthorityMode}Root][{GetDisplayName(networkManager)}] Root mismatch ({GetVector3Values(authorityRootPosition)})({GetVector3Values(cloneRootPosition)})!");
88+
}
7189

72-
if (!Approximately(ownerChildPosition, cloneChildPosition))
73-
{
74-
m_ErrorMsg.AppendLine($"[{playerObject.name}][{playerClone.name}] Child mismatch ({GetVector3Values(ownerChildPosition)})({GetVector3Values(cloneChildPosition)})!");
75-
}
90+
if (!Approximately(authorityChildPosition, cloneChildPosition))
91+
{
92+
m_ErrorMsg.AppendLine($"[{rootAuthorityMode}Root][{GetDisplayName(networkManager)}] Child mismatch ({GetVector3Values(authorityChildPosition)})({GetVector3Values(cloneChildPosition)})!");
7693
}
7794
}
7895
return m_ErrorMsg.Length == 0;
7996
}
8097

8198
/// <summary>
8299
/// Client-Server Only
83-
/// Validates that mixed authority is working properly
84-
/// Root -- Server Authoritative
85-
/// |--Child -- Owner Authoritative
100+
/// Validates that mixed authority is working properly for both arrangements:
101+
/// Root -- Server or Owner authoritative
102+
/// |--Child -- The inverse of the root's authority mode
86103
/// </summary>
87104
[UnityTest]
88105
public IEnumerator MixedAuthorityTest()
89106
{
90-
for (int i = 0; i < k_Iterations; i++)
107+
// A client owns the instance so the owner authoritative half is never also the server.
108+
var owner = m_ClientNetworkManagers[0];
109+
for (int i = 0; i < k_RootAuthorityModes.Length; i++)
91110
{
92-
MovePlayers();
93-
yield return WaitForConditionOrTimeOut(AllInstancePositionsMatch);
94-
AssertOnTimeout($"Transforms failed to synchronize!");
111+
var rootAuthorityMode = k_RootAuthorityModes[i];
112+
var instance = SpawnObject(m_MixedAuthorityPrefabs[i], owner).GetComponent<NetworkObject>();
113+
yield return WaitForSpawnedOnAllOrTimeOut(instance);
114+
AssertOnTimeout($"[{rootAuthorityMode}Root] Failed to spawn {instance.name} on all clients!");
115+
116+
// An instance stays registered for updates while any of its nested NetworkTransform components is non-authority.
117+
foreach (var networkManager in m_NetworkManagers)
118+
{
119+
var clone = GetManagersInstance(networkManager, instance);
120+
var hasNonAuthority = false;
121+
foreach (var networkTransform in clone.NetworkTransforms)
122+
{
123+
hasNonAuthority |= !networkTransform.CanCommitToTransform;
124+
}
125+
Assert.AreEqual(hasNonAuthority, networkManager.NetworkTransformUpdate.ContainsKey(instance.NetworkObjectId), $"[{rootAuthorityMode}Root][{GetDisplayName(networkManager)}] Unexpected update registration!");
126+
}
127+
128+
for (int iteration = 0; iteration < k_Iterations; iteration++)
129+
{
130+
var direction = GetRandomVector3(-1.0f, 1.0f);
131+
GetAuthorityInstance(instance, owner, rootAuthorityMode).transform.position += direction * k_MotionMagnitude;
132+
GetAuthorityInstance(instance, owner, InverseOf(rootAuthorityMode)).transform.GetChild(0).localPosition += direction * k_MotionMagnitude;
133+
134+
yield return WaitForConditionOrTimeOut(() => AllInstancePositionsMatch(instance, owner, rootAuthorityMode));
135+
AssertOnTimeout($"[{rootAuthorityMode}Root] Transforms failed to synchronize!\n{m_ErrorMsg}");
136+
}
137+
138+
instance.Despawn();
95139
}
96140
}
97141
}

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

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
#if COM_UNITY_MODULES_PHYSICS
2+
using System.Collections;
3+
using NUnit.Framework;
4+
using Unity.Netcode.Components;
5+
using Unity.Netcode.TestHelpers.Runtime;
6+
using UnityEngine;
7+
using UnityEngine.TestTools;
8+
9+
namespace Unity.Netcode.RuntimeTests
10+
{
11+
internal class NetworkTransformMixedMotionModelTests : NetcodeIntegrationTest
12+
{
13+
protected override int NumberOfClients => 2;
14+
15+
private GameObject m_MixedMotionModelPrefab;
16+
17+
protected override void OnServerAndClientsCreated()
18+
{
19+
m_MixedMotionModelPrefab = CreateNetworkObjectPrefab("MixedMotionModel");
20+
21+
// The root is owner authoritative and driven by the rigidbody, which places it in the fixed update registration
22+
var rootNetworkTransform = m_MixedMotionModelPrefab.AddComponent<NetworkTransform>();
23+
rootNetworkTransform.AuthorityMode = NetworkTransform.AuthorityModes.Owner;
24+
var rigidbody = m_MixedMotionModelPrefab.AddComponent<Rigidbody>();
25+
rigidbody.useGravity = false;
26+
rigidbody.detectCollisions = false;
27+
m_MixedMotionModelPrefab.AddComponent<NetworkRigidbody>().UseRigidBodyForMotion = true;
28+
29+
// The nested child is server authoritative and driven by the transform, which places it in the update registration
30+
var childGameObject = new GameObject();
31+
childGameObject.transform.parent = m_MixedMotionModelPrefab.transform;
32+
var childNetworkTransform = childGameObject.AddComponent<NetworkTransform>();
33+
childNetworkTransform.AuthorityMode = NetworkTransform.AuthorityModes.Server;
34+
childNetworkTransform.InLocalSpace = true;
35+
36+
base.OnServerAndClientsCreated();
37+
}
38+
39+
/// <summary>
40+
/// A NetworkObject that mixes both the authority motion model and the rigidbody motion model has each nested
41+
/// NetworkTransform registered under a different update. Gaining authority over the instance in one update
42+
/// should not leave it registered for the other.
43+
/// </summary>
44+
[UnityTest]
45+
public IEnumerator UpdateRegistrationFollowsMotionModel()
46+
{
47+
var instance = SpawnObject(m_MixedMotionModelPrefab, m_ServerNetworkManager).GetComponent<NetworkObject>();
48+
yield return WaitForSpawnedOnAllOrTimeOut(instance);
49+
AssertOnTimeout($"Failed to spawn {instance.name} on all clients!");
50+
51+
var newOwner = m_ClientNetworkManagers[0];
52+
53+
// Establish the baseline before ownership is transferred, otherwise the check below would still pass if this instance was never registered for the fixed update to begin with.
54+
Assert.True(newOwner.NetworkTransformFixedUpdate.ContainsKey(instance.NetworkObjectId), $"Client-{newOwner.LocalClientId} should initially be registered for the fixed update!");
55+
56+
instance.ChangeOwnership(newOwner.LocalClientId);
57+
yield return WaitForConditionOrTimeOut(() => newOwner.SpawnManager.SpawnedObjects[instance.NetworkObjectId].OwnerClientId == newOwner.LocalClientId);
58+
AssertOnTimeout($"Client-{newOwner.LocalClientId} never gained ownership of {instance.name}!");
59+
60+
// The new owner is the authority for the rigidbody driven root, so nothing on this instance needs the fixed
61+
// update any longer. The server authoritative child still needs the standard update.
62+
Assert.False(newOwner.NetworkTransformFixedUpdate.ContainsKey(instance.NetworkObjectId), $"Client-{newOwner.LocalClientId} is still registered for the fixed update!");
63+
Assert.True(newOwner.NetworkTransformUpdate.ContainsKey(instance.NetworkObjectId), $"Client-{newOwner.LocalClientId} is not registered for the update!");
64+
}
65+
}
66+
}
67+
#endif

‎com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformMixedMotionModelTests.cs.meta‎

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)