From d56910c8f07bc2eb2abcc33433ed81232bd7dce2 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Mon, 14 Oct 2024 13:58:35 -0500 Subject: [PATCH 1/5] fix This fixes the issue when using a client-server network topology spawning a player with nested NetworkTransform components would result in loss of the child NetworkTransform component(s) settings/flags which would end up being serialized to the owner client (thus causing invalid synchronization results). --- .../Runtime/Components/NetworkTransform.cs | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Components/NetworkTransform.cs b/com.unity.netcode.gameobjects/Runtime/Components/NetworkTransform.cs index d4d1f93a98..cb439e7e9b 100644 --- a/com.unity.netcode.gameobjects/Runtime/Components/NetworkTransform.cs +++ b/com.unity.netcode.gameobjects/Runtime/Components/NetworkTransform.cs @@ -1487,7 +1487,6 @@ protected override void OnSynchronize(ref BufferSerializer serializer) HalfVectorRotation = new HalfVector4(), HalfVectorScale = new HalfVector3(), NetworkDeltaPosition = new NetworkDeltaPosition(), - }; if (serializer.IsWriter) @@ -3050,12 +3049,45 @@ protected internal override void InternalOnNetworkSessionSynchronized() base.InternalOnNetworkSessionSynchronized(); } + private void ApplyPlayerTransformState() + { + SynchronizeState.InLocalSpace = InLocalSpace; + SynchronizeState.UseInterpolation = Interpolate; + SynchronizeState.QuaternionSync = UseQuaternionSynchronization; + SynchronizeState.UseHalfFloatPrecision = UseHalfFloatPrecision; + SynchronizeState.QuaternionCompression = UseQuaternionCompression; + SynchronizeState.UsePositionSlerp = SlerpPosition; + } + /// /// For dynamically spawned NetworkObjects, when the non-authority instance's client is already connected and /// the SynchronizeState is still pending synchronization then we want to finalize the synchornization at this time. /// protected internal override void InternalOnNetworkPostSpawn() { + // This is a special case for client-server where a server is spawning a player but has yet to serialize anything. + // When the server detects that: + // - We are not in a distributed authority session (DAHost check). + // - This is the first/root NetworkTransform. + // - The NetworkObject is a player object (i.e. server-side instantiated). + // - The player object is not the local player object (i.e. spawning a player for another client). + // - We are in owner authoritative mode. + // - SynchronizeState is set to false. + // Then we want to: + // - Force the "IsSynchronizing" flag so the NetworkTransform has its state updated properly and runs through the initialization again. + // - Make sure the SynchronizingState is updated to the instantiated prefab's default flags/settings. + if (NetworkManager.IsServer && !NetworkManager.DistributedAuthorityMode && m_IsFirstNetworkTransform && NetworkObject.IsPlayerObject && !NetworkObject.IsLocalPlayer && !OnIsServerAuthoritative() && !SynchronizeState.IsSynchronizing) + { + // Assure the first/root NetworkTransform has the synchronizing flag set so the server runs through the final post initialization steps + SynchronizeState.IsSynchronizing = true; + // Assure the SynchronizeState matches the initial prefab's values for each associated NetworkTransfrom (this includes root + all children) + foreach (var child in NetworkObject.NetworkTransforms) + { + child.ApplyPlayerTransformState(); + } + // Now fall through to the final synchronization portion of the spawning for NetworkTransform + } + if (!CanCommitToTransform && NetworkManager.IsConnectedClient && SynchronizeState.IsSynchronizing) { NonAuthorityFinalizeSynchronization(); From bd12e136b6a74b20c8f489bd551afd42f5b64067 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Mon, 14 Oct 2024 14:01:22 -0500 Subject: [PATCH 2/5] update Adding the changelog entry. --- com.unity.netcode.gameobjects/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index dc89f55dfb..85d9cdf15b 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -18,6 +18,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed +- Fixed issue with child `NetworkTransform` components clearing their initial prefab settings when in owner authoritative mode which resulted in improper synchronization. - Fixed issue with service not getting synchronized with in-scene placed `NetworkObject` instances when a session owner starts a `SceneEventType.Load` event. (#3096) - Fixed issue with the in-scene network prefab instance update menu tool where it was not properly updating scenes when invoked on the root prefab instance. (#3092) - Fixed issue where applying the position and/or rotation to the `NetworkManager.ConnectionApprovalResponse` when connection approval and auto-spawn player prefab were enabled would not apply the position and/or rotation when the player prefab was instantiated. (#3078) From b5a38a39de8d199e1ebbe51068dffddb9ceeeb4a Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Mon, 14 Oct 2024 14:20:11 -0500 Subject: [PATCH 3/5] fix The issue is not just for local players but for all prefabs that have nested NetworkTransforms, are owner authoritative, are not owned by the server, and the session instance is using a client-server network topology. --- .../Runtime/Components/NetworkTransform.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/com.unity.netcode.gameobjects/Runtime/Components/NetworkTransform.cs b/com.unity.netcode.gameobjects/Runtime/Components/NetworkTransform.cs index cb439e7e9b..a103b0fc4b 100644 --- a/com.unity.netcode.gameobjects/Runtime/Components/NetworkTransform.cs +++ b/com.unity.netcode.gameobjects/Runtime/Components/NetworkTransform.cs @@ -3065,18 +3065,17 @@ private void ApplyPlayerTransformState() /// protected internal override void InternalOnNetworkPostSpawn() { - // This is a special case for client-server where a server is spawning a player but has yet to serialize anything. + // This is a special case for client-server where a server is spawning an owner authoritative NetworkObject but has yet to serialize anything. // When the server detects that: // - We are not in a distributed authority session (DAHost check). // - This is the first/root NetworkTransform. - // - The NetworkObject is a player object (i.e. server-side instantiated). - // - The player object is not the local player object (i.e. spawning a player for another client). // - We are in owner authoritative mode. - // - SynchronizeState is set to false. + // - The NetworkObject is not owned by the server. + // - The SynchronizeState.IsSynchronizing is set to false. // Then we want to: // - Force the "IsSynchronizing" flag so the NetworkTransform has its state updated properly and runs through the initialization again. // - Make sure the SynchronizingState is updated to the instantiated prefab's default flags/settings. - if (NetworkManager.IsServer && !NetworkManager.DistributedAuthorityMode && m_IsFirstNetworkTransform && NetworkObject.IsPlayerObject && !NetworkObject.IsLocalPlayer && !OnIsServerAuthoritative() && !SynchronizeState.IsSynchronizing) + if (NetworkManager.IsServer && !NetworkManager.DistributedAuthorityMode && m_IsFirstNetworkTransform && !OnIsServerAuthoritative() && !IsOwner && !SynchronizeState.IsSynchronizing) { // Assure the first/root NetworkTransform has the synchronizing flag set so the server runs through the final post initialization steps SynchronizeState.IsSynchronizing = true; From 26c82e2fe6f63661f4300a6ed50c09044bc725dd Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Mon, 14 Oct 2024 17:44:04 -0500 Subject: [PATCH 4/5] test Adding a test to validate this fix. --- .../NetworkTransformOwnershipTests.cs | 275 ++++++++++++++++++ 1 file changed, 275 insertions(+) diff --git a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformOwnershipTests.cs b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformOwnershipTests.cs index 8f65f34694..31105efadd 100644 --- a/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformOwnershipTests.cs +++ b/com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformOwnershipTests.cs @@ -2,6 +2,7 @@ using System.Collections; using System.Collections.Generic; using System.Linq; +using System.Text; using NUnit.Framework; using Unity.Netcode.Components; using Unity.Netcode.TestHelpers.Runtime; @@ -539,5 +540,279 @@ protected override bool OnIsServerAuthoritative() } } } + + [TestFixture(HostOrServer.DAHost, NetworkTransform.AuthorityModes.Owner)] // Validate the NetworkTransform owner authoritative mode fix using distributed authority + [TestFixture(HostOrServer.Host, NetworkTransform.AuthorityModes.Server)] // Validate we have not impacted NetworkTransform server authoritative mode + [TestFixture(HostOrServer.Host, NetworkTransform.AuthorityModes.Owner)] // Validate the NetworkTransform owner authoritative mode fix using client-server + internal class NestedNetworkTransformTests : IntegrationTestWithApproximation + { + private const int k_NestedChildren = 5; + protected override int NumberOfClients => 2; + + private GameObject m_SpawnObject; + + private NetworkTransform.AuthorityModes m_AuthorityMode; + + private StringBuilder m_ErrorLog = new StringBuilder(); + + private List m_NetworkManagers = new List(); + private List m_SpawnedObjects = new List(); + + public NestedNetworkTransformTests(HostOrServer hostOrServer, NetworkTransform.AuthorityModes authorityMode) : base(hostOrServer) + { + m_AuthorityMode = authorityMode; + } + + /// + /// Creates a player prefab with several nested NetworkTransforms + /// + protected override void OnCreatePlayerPrefab() + { + var networkTransform = m_PlayerPrefab.AddComponent(); + networkTransform.AuthorityMode = m_AuthorityMode; + var parent = m_PlayerPrefab; + // Add several nested NetworkTransforms + for (int i = 0; i < k_NestedChildren; i++) + { + var nestedChild = new GameObject(); + nestedChild.transform.parent = parent.transform; + var nestedNetworkTransform = nestedChild.AddComponent(); + nestedNetworkTransform.AuthorityMode = m_AuthorityMode; + nestedNetworkTransform.InLocalSpace = true; + parent = nestedChild; + } + base.OnCreatePlayerPrefab(); + } + + private void RandomizeObjectTransformPositions(GameObject gameObject) + { + var networkObject = gameObject.GetComponent(); + Assert.True(networkObject.ChildNetworkBehaviours.Count > 0); + + foreach (var networkTransform in networkObject.NetworkTransforms) + { + networkTransform.gameObject.transform.position = GetRandomVector3(-15.0f, 15.0f); + } + } + + /// + /// Randomizes each player's position when validating distributed authority + /// + /// + private GameObject FetchLocalPlayerPrefabToSpawn() + { + RandomizeObjectTransformPositions(m_PlayerPrefab); + return m_PlayerPrefab; + } + + /// + /// Randomizes the player position when validating client-server + /// + /// + /// + private void ConnectionApprovalHandler(NetworkManager.ConnectionApprovalRequest connectionApprovalRequest, NetworkManager.ConnectionApprovalResponse connectionApprovalResponse) + { + connectionApprovalResponse.Approved = true; + connectionApprovalResponse.CreatePlayerObject = true; + RandomizeObjectTransformPositions(m_PlayerPrefab); + connectionApprovalResponse.Position = GetRandomVector3(-15.0f, 15.0f); + } + + protected override void OnServerAndClientsCreated() + { + // Create a prefab to spawn with each NetworkManager as the owner + m_SpawnObject = CreateNetworkObjectPrefab("SpawnObj"); + var networkTransform = m_SpawnObject.AddComponent(); + networkTransform.AuthorityMode = m_AuthorityMode; + var parent = m_SpawnObject; + // Add several nested NetworkTransforms + for (int i = 0; i < k_NestedChildren; i++) + { + var nestedChild = new GameObject(); + nestedChild.transform.parent = parent.transform; + var nestedNetworkTransform = nestedChild.AddComponent(); + nestedNetworkTransform.AuthorityMode = m_AuthorityMode; + nestedNetworkTransform.InLocalSpace = true; + parent = nestedChild; + } + + if (m_DistributedAuthority) + { + if (!UseCMBService()) + { + m_ServerNetworkManager.OnFetchLocalPlayerPrefabToSpawn = FetchLocalPlayerPrefabToSpawn; + } + + foreach (var client in m_ClientNetworkManagers) + { + client.OnFetchLocalPlayerPrefabToSpawn = FetchLocalPlayerPrefabToSpawn; + } + } + else + { + m_ServerNetworkManager.NetworkConfig.ConnectionApproval = true; + m_ServerNetworkManager.ConnectionApprovalCallback += ConnectionApprovalHandler; + foreach (var client in m_ClientNetworkManagers) + { + client.NetworkConfig.ConnectionApproval = true; + } + } + + base.OnServerAndClientsCreated(); + } + + /// + /// Validates the transform positions of two NetworkObject instances + /// + /// the local instance (source of truth) + /// the remote instance + /// + private bool ValidateTransforms(NetworkObject current, NetworkObject testing) + { + if (current.ChildNetworkBehaviours.Count == 0 || testing.ChildNetworkBehaviours.Count == 0) + { + return false; + } + + for (int i = 0; i < current.NetworkTransforms.Count - 1; i++) + { + var transformA = current.NetworkTransforms[i].transform; + var transformB = testing.NetworkTransforms[i].transform; + if (!Approximately(transformA.position, transformB.position)) + { + m_ErrorLog.AppendLine($"TransformA Position {transformA.position} != TransformB Position {transformB.position}"); + return false; + } + if (!Approximately(transformA.localPosition, transformB.localPosition)) + { + m_ErrorLog.AppendLine($"TransformA Local Position {transformA.position} != TransformB Local Position {transformB.position}"); + return false; + } + if (transformA.parent != null) + { + if (current.NetworkTransforms[i].InLocalSpace != testing.NetworkTransforms[i].InLocalSpace) + { + m_ErrorLog.AppendLine($"NetworkTransform-{current.OwnerClientId}-{current.NetworkTransforms[i].NetworkBehaviourId} InLocalSpace ({current.NetworkTransforms[i].InLocalSpace}) is different from the remote instance version on Client-{testing.NetworkManager.LocalClientId}!"); + return false; + } + } + } + return true; + } + + /// + /// Validates all player instances spawned with the correct positions including all nested NetworkTransforms + /// When running in server authority mode we are validating this fix did not impact that. + /// + private bool AllClientInstancesSynchronized() + { + m_ErrorLog.Clear(); + + foreach (var current in m_NetworkManagers) + { + var currentPlayer = current.LocalClient.PlayerObject; + var currentNetworkObjectId = currentPlayer.NetworkObjectId; + foreach (var testing in m_NetworkManagers) + { + if (currentPlayer == testing.LocalClient.PlayerObject) + { + continue; + } + + if (!testing.SpawnManager.SpawnedObjects.ContainsKey(currentNetworkObjectId)) + { + m_ErrorLog.AppendLine($"Failed to find Client-{currentPlayer.OwnerClientId}'s player instance on Client-{testing.LocalClientId}!"); + return false; + } + + var remoteInstance = testing.SpawnManager.SpawnedObjects[currentNetworkObjectId]; + if (!ValidateTransforms(currentPlayer, remoteInstance)) + { + m_ErrorLog.AppendLine($"Failed to validate Client-{currentPlayer.OwnerClientId} against its remote instance on Client-{testing.LocalClientId}!"); + return false; + } + } + } + return true; + } + + /// + /// Validates that dynamically spawning works the same. + /// When running in server authority mode we are validating this fix did not impact that. + /// + /// + private bool AllSpawnedObjectsSynchronized() + { + m_ErrorLog.Clear(); + + foreach (var current in m_SpawnedObjects) + { + var currentNetworkObject = current.GetComponent(); + var currentNetworkObjectId = currentNetworkObject.NetworkObjectId; + foreach (var testing in m_NetworkManagers) + { + if (currentNetworkObject.OwnerClientId == testing.LocalClientId) + { + continue; + } + + if (!testing.SpawnManager.SpawnedObjects.ContainsKey(currentNetworkObjectId)) + { + m_ErrorLog.AppendLine($"Failed to find Client-{currentNetworkObject.OwnerClientId}'s player instance on Client-{testing.LocalClientId}!"); + return false; + } + + var remoteInstance = testing.SpawnManager.SpawnedObjects[currentNetworkObjectId]; + if (!ValidateTransforms(currentNetworkObject, remoteInstance)) + { + m_ErrorLog.AppendLine($"Failed to validate Client-{currentNetworkObject.OwnerClientId} against its remote instance on Client-{testing.LocalClientId}!"); + return false; + } + } + } + return true; + } + + /// + /// Validates that spawning player and dynamically spawned prefab instances with nested NetworkTransforms + /// synchronizes properly in both client-server and distributed authority when using owner authoritative mode. + /// + [UnityTest] + public IEnumerator NestedNetworkTransformSpawnPositionTest() + { + if (!m_DistributedAuthority || (m_DistributedAuthority && !UseCMBService())) + { + m_NetworkManagers.Add(m_ServerNetworkManager); + } + m_NetworkManagers.AddRange(m_ClientNetworkManagers); + + yield return WaitForConditionOrTimeOut(AllClientInstancesSynchronized); + AssertOnTimeout($"Failed to synchronize all client instances!\n{m_ErrorLog}"); + + foreach (var networkManager in m_NetworkManagers) + { + // Randomize the position + RandomizeObjectTransformPositions(m_SpawnObject); + + // Create an instance owned by the specified networkmanager + m_SpawnedObjects.Add(SpawnObject(m_SpawnObject, networkManager)); + } + // Randomize the position once more just to assure we are instantiating remote instances + // with a completely different position + RandomizeObjectTransformPositions(m_SpawnObject); + yield return WaitForConditionOrTimeOut(AllSpawnedObjectsSynchronized); + AssertOnTimeout($"Failed to synchronize all spawned NetworkObject instances!\n{m_ErrorLog}"); + m_SpawnedObjects.Clear(); + m_NetworkManagers.Clear(); + } + + protected override IEnumerator OnTearDown() + { + // In case there was a failure, go ahead and clear these lists out for any pending TextFixture passes + m_SpawnedObjects.Clear(); + m_NetworkManagers.Clear(); + return base.OnTearDown(); + } + } } #endif From 67fb1d27ec8a9c615f38ee1162099de494c18c25 Mon Sep 17 00:00:00 2001 From: NoelStephensUnity Date: Mon, 14 Oct 2024 17:46:16 -0500 Subject: [PATCH 5/5] update adding PR number to changelog entry --- com.unity.netcode.gameobjects/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 85d9cdf15b..9567bc8844 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -18,7 +18,7 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Fixed -- Fixed issue with child `NetworkTransform` components clearing their initial prefab settings when in owner authoritative mode which resulted in improper synchronization. +- Fixed issue with nested `NetworkTransform` components clearing their initial prefab settings when in owner authoritative mode on the server side while using a client-server network topology which resulted in improper synchronization of the nested `NetworkTransform` components. (#3099) - Fixed issue with service not getting synchronized with in-scene placed `NetworkObject` instances when a session owner starts a `SceneEventType.Load` event. (#3096) - Fixed issue with the in-scene network prefab instance update menu tool where it was not properly updating scenes when invoked on the root prefab instance. (#3092) - Fixed issue where applying the position and/or rotation to the `NetworkManager.ConnectionApprovalResponse` when connection approval and auto-spawn player prefab were enabled would not apply the position and/or rotation when the player prefab was instantiated. (#3078)