Skip to content

fix: Set state private RPC handlers were not honoring local vs world space assignment of position and rotation #2203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
NoelStephensUnity merged 6 commits into from
Sep 14, 2022
Merged

fix: Set state private RPC handlers were not honoring local vs world space assignment of position and rotation #2203

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
95581ed
fix
NoelStephensUnity Sep 14, 2022
5b037ed
update changelog
NoelStephensUnity Sep 14, 2022
6a3ffd4
test
NoelStephensUnity Sep 14, 2022
904ec6a
update
NoelStephensUnity Sep 14, 2022
89a209f
test
NoelStephensUnity Sep 14, 2022
b39ccce
update
NoelStephensUnity Sep 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions com.unity.netcode.gameobjects/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Additional documentation and release notes are available at [Multiplayer Documen

### Fixed

- Fixed issue where the `SetStateServerRpc` and `SetStateClientRpc` were not honoring local vs world space settings when applying the position and rotation. (#2203)
Comment thread
NoelStephensUnity marked this conversation as resolved.
Outdated
- Fixed ILPP `TypeLoadException` on WebGL on MacOS Editor and potentially other platforms. (#2199)
- Implicit conversion of NetworkObjectReference to GameObject will now return null instead of throwing an exception if the referenced object could not be found (i.e., was already despawned) (#2158)
- Fixed warning resulting from a stray NetworkAnimator.meta file (#2153)
Expand Down
13 changes: 2 additions & 11 deletions com.unity.netcode.gameobjects/Components/NetworkTransform.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1169,11 +1169,7 @@ private void SetStateInternal(Vector3 pos, Quaternion rot, Vector3 scale, bool s
private void SetStateClientRpc(Vector3 pos, Quaternion rot, Vector3 scale, bool shouldTeleport, ClientRpcParams clientRpcParams = default)
{
// Server dictated state is always applied
transform.position = pos;
transform.rotation = rot;
transform.localScale = scale;
m_LocalAuthoritativeNetworkState.IsTeleportingNextFrame = shouldTeleport;
TryCommitTransform(transform, m_CachedNetworkManager.LocalTime.Time);
SetStateInternal(pos, rot, scale, shouldTeleport);
}

/// <summary>
Expand All @@ -1190,12 +1186,7 @@ private void SetStateServerRpc(Vector3 pos, Quaternion rot, Vector3 scale, bool
{
(pos, rot, scale) = OnClientRequestChange(pos, rot, scale);
}

transform.position = pos;
transform.rotation = rot;
transform.localScale = scale;
m_LocalAuthoritativeNetworkState.IsTeleportingNextFrame = shouldTeleport;
TryCommitTransform(transform, m_CachedNetworkManager.LocalTime.Time);
SetStateInternal(pos, rot, scale, shouldTeleport);
}

/// <summary>
Expand Down
69 changes: 48 additions & 21 deletions com.unity.netcode.gameobjects/Tests/Runtime/NetworkTransform/NetworkTransformTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,8 @@ public enum TransformSpace
public enum OverrideState
{
Update,
CommitToTransform
CommitToTransform,
SetState
}

/// <summary>
Expand Down Expand Up @@ -299,9 +300,8 @@ private IEnumerator WaitForAllChildrenLocalTransformValuesToMatch()
/// parented under another NetworkTransform
/// </summary>
[UnityTest]
public IEnumerator NetworkTransformParentedLocalSpaceTest([Values] Interpolation interpolation, [Values] OverrideState overideState)
public IEnumerator NetworkTransformParentedLocalSpaceTest([Values] Interpolation interpolation)
{
var overrideUpdate = overideState == OverrideState.CommitToTransform;
m_AuthoritativeTransform.Interpolate = interpolation == Interpolation.EnableInterpolate;
m_NonAuthoritativeTransform.Interpolate = interpolation == Interpolation.EnableInterpolate;
var authoritativeChildObject = SpawnObject(m_ChildObjectToBeParented.gameObject, m_AuthoritativeTransform.NetworkManager);
Expand Down Expand Up @@ -334,15 +334,28 @@ public IEnumerator NetworkTransformParentedLocalSpaceTest([Values] Interpolation
/// Validates that moving, rotating, and scaling the authority side with a single
/// tick will properly synchronize the non-authoritative side with the same values.
/// </summary>
private IEnumerator MoveRotateAndScaleAuthority(Vector3 position, Vector3 rotation, Vector3 scale)
private IEnumerator MoveRotateAndScaleAuthority(Vector3 position, Vector3 rotation, Vector3 scale, OverrideState overrideState)
{
m_AuthoritativeTransform.transform.position = position;
yield return null;
var authoritativeRotation = m_AuthoritativeTransform.transform.rotation;
authoritativeRotation.eulerAngles = rotation;
m_AuthoritativeTransform.transform.rotation = authoritativeRotation;
yield return null;
m_AuthoritativeTransform.transform.localScale = scale;
switch (overrideState)
{
case OverrideState.SetState:
{
m_AuthoritativeTransform.SetState(position, Quaternion.Euler(rotation), scale);
break;
}
case OverrideState.Update:
default:
{
m_AuthoritativeTransform.transform.position = position;
yield return null;
var authoritativeRotation = m_AuthoritativeTransform.transform.rotation;
authoritativeRotation.eulerAngles = rotation;
m_AuthoritativeTransform.transform.rotation = authoritativeRotation;
yield return null;
m_AuthoritativeTransform.transform.localScale = scale;
break;
}
}
}

/// <summary>
Expand Down Expand Up @@ -400,7 +413,6 @@ protected override void OnNewClientCreated(NetworkManager networkManager)
[UnityTest]
public IEnumerator NetworkTransformMultipleChangesOverTime([Values] TransformSpace testLocalTransform, [Values] OverrideState overideState)
{
var overrideUpdate = overideState == OverrideState.CommitToTransform;
m_AuthoritativeTransform.InLocalSpace = testLocalTransform == TransformSpace.Local;

var positionStart = new Vector3(1.0f, 0.5f, 2.0f);
Expand All @@ -422,7 +434,7 @@ public IEnumerator NetworkTransformMultipleChangesOverTime([Values] TransformSpa
yield return WaitForNextTick();

// Apply deltas
MoveRotateAndScaleAuthority(position, rotation, scale);
MoveRotateAndScaleAuthority(position, rotation, scale, overideState);

// Wait for deltas to synchronize on non-authoritative side
yield return WaitForPositionRotationAndScaleToMatch(4);
Expand Down Expand Up @@ -455,7 +467,7 @@ public IEnumerator NetworkTransformMultipleChangesOverTime([Values] TransformSpa
// to apply both deltas within the same tick period.
yield return WaitForNextTick();

MoveRotateAndScaleAuthority(position, rotation, scale);
MoveRotateAndScaleAuthority(position, rotation, scale, overideState);
yield return WaitForPositionRotationAndScaleToMatch(4);
}

Expand All @@ -471,7 +483,7 @@ public IEnumerator NetworkTransformMultipleChangesOverTime([Values] TransformSpa
rotation = rotationStart * i;
scale = scaleStart * i;

MoveRotateAndScaleAuthority(position, rotation, scale);
MoveRotateAndScaleAuthority(position, rotation, scale, overideState);
}

yield return WaitForPositionRotationAndScaleToMatch(1);
Expand All @@ -486,7 +498,7 @@ public IEnumerator NetworkTransformMultipleChangesOverTime([Values] TransformSpa
position = positionStart * i;
rotation = rotationStart * i;
scale = scaleStart * i;
MoveRotateAndScaleAuthority(position, rotation, scale);
MoveRotateAndScaleAuthority(position, rotation, scale, overideState);
}
yield return WaitForPositionRotationAndScaleToMatch(1);
}
Expand All @@ -513,32 +525,47 @@ public IEnumerator TestAuthoritativeTransformChangeOneAtATime([Values] Transform

Assert.AreEqual(Vector3.zero, m_NonAuthoritativeTransform.transform.position, "server side pos should be zero at first"); // sanity check

authPlayerTransform.position = new Vector3(10, 20, 30);
if (overrideUpdate)
var nextPosition = new Vector3(10, 20, 30);
if (overideState != OverrideState.SetState)
{
authPlayerTransform.position = nextPosition;
m_OwnerTransform.CommitToTransform();
}
else
{
m_OwnerTransform.SetState(nextPosition, null, null, m_AuthoritativeTransform.Interpolate);
}

yield return WaitForConditionOrTimeOut(PositionsMatch);
AssertOnTimeout($"Timed out waiting for positions to match");

// test rotation
Assert.AreEqual(Quaternion.identity, m_NonAuthoritativeTransform.transform.rotation, "wrong initial value for rotation"); // sanity check

authPlayerTransform.rotation = Quaternion.Euler(45, 40, 35); // using euler angles instead of quaternions directly to really see issues users might encounter
if (overrideUpdate)
var nextRotation = Quaternion.Euler(45, 40, 35); // using euler angles instead of quaternions directly to really see issues users might encounter
if (overideState != OverrideState.SetState)
{
authPlayerTransform.rotation = nextRotation;
m_OwnerTransform.CommitToTransform();
}
else
{
m_OwnerTransform.SetState(null, nextRotation, null, m_AuthoritativeTransform.Interpolate);
}

yield return WaitForConditionOrTimeOut(RotationsMatch);
AssertOnTimeout($"Timed out waiting for rotations to match");

authPlayerTransform.localScale = new Vector3(2, 3, 4);
var nextScale = new Vector3(2, 3, 4);
if (overrideUpdate)
{
authPlayerTransform.localScale = nextScale;
m_OwnerTransform.CommitToTransform();
}
else
{
m_OwnerTransform.SetState(null, null, nextScale, m_AuthoritativeTransform.Interpolate);
}

yield return WaitForConditionOrTimeOut(ScaleValuesMatch);
AssertOnTimeout($"Timed out waiting for scale values to match");
Expand Down