Skip to content

fix: Resolving Sync Issues and adding ResetTrigger support #1327

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.

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
merged 9 commits into from
Oct 22, 2021
Merged
10 changes: 10 additions & 0 deletions com.unity.netcode.gameobjects/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)

Additional documentation and release notes are available at [Multiplayer Documentation](https://docs-multiplayer.unity3d.com).

## [1.0.0-pre.3] - 2021-10-22

### Added

- ResetTrigger function to NetworkAnimator

### Fixed

- Overflow exception when syncing Animator state.

## [1.0.0-pre.2] - 2020-12-20

### Added
Expand Down
56 changes: 35 additions & 21 deletions com.unity.netcode.gameobjects/Components/NetworkAnimator.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Unity.Collections;
using Unity.Collections.LowLevel.Unsafe;

using UnityEngine;

namespace Unity.Netcode.Components
Expand Down Expand Up @@ -38,11 +39,12 @@ public void NetworkSerialize<T>(BufferSerializer<T> serializer) where T : IReade
internal struct AnimationTriggerMessage : INetworkSerializable
{
public int Hash;
public bool Reset;

public void NetworkSerialize<T>(BufferSerializer<T> serializer) where T : IReaderWriter
{
serializer.SerializeValue(ref Hash);

serializer.SerializeValue(ref Reset);
}
}

Expand Down Expand Up @@ -286,6 +288,12 @@ private bool CheckAnimStateChanged(out int stateHash, out float normalizedTime)
return false;
}

/* $AS TODO: Right now we are not checking for changed values this is because
the read side of this function doesn't have similar logic which would cause
an overflow read because it doesn't know if the value is there or not. So
there needs to be logic to track which indexes changed in order for there
to be proper value change checking. Will revist in 1.1.0.
*/
private unsafe bool WriteParameters(FastBufferWriter writer, bool autoSend)
{
if (m_CachedAnimatorParameters == null)
Expand All @@ -308,39 +316,27 @@ private unsafe bool WriteParameters(FastBufferWriter writer, bool autoSend)
var valueInt = m_Animator.GetInteger(hash);
fixed (void* value = cacheValue.Value)
{
var oldValue = UnsafeUtility.AsRef<int>(value);
if (valueInt != oldValue)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess because we can't trust the value hasn't changed because of the reset?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I will revisit this for 1.1.0... I have potential solution but its a little more involved so this unblocks the issue where we get an overflow exception on the read side of things. Luckily by default we don't auto sync anything so in most cases we don't send a bunch of updates unless the user opts in for that, and we will always send a full update when we sync state which should be true but I would like to come back to this for 1.1.0

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sg, can I suggest pasting what you just said in a comment around it?

{
UnsafeUtility.WriteArrayElement(value, 0, valueInt);
BytePacker.WriteValuePacked(writer, (uint)valueInt);
}
UnsafeUtility.WriteArrayElement(value, 0, valueInt);
BytePacker.WriteValuePacked(writer, (uint)valueInt);
}
}
else if (cacheValue.Type == AnimationParamEnumWrapper.AnimatorControllerParameterBool)
{
var valueBool = m_Animator.GetBool(hash);
fixed (void* value = cacheValue.Value)
{
var oldValue = UnsafeUtility.AsRef<bool>(value);
if (valueBool != oldValue)
{
UnsafeUtility.WriteArrayElement(value, 0, valueBool);
writer.WriteValueSafe(valueBool);
}
UnsafeUtility.WriteArrayElement(value, 0, valueBool);
writer.WriteValueSafe(valueBool);
}
}
else if (cacheValue.Type == AnimationParamEnumWrapper.AnimatorControllerParameterFloat)
{
var valueFloat = m_Animator.GetFloat(hash);
fixed (void* value = cacheValue.Value)
{
var oldValue = UnsafeUtility.AsRef<float>(value);
if (valueFloat != oldValue)
{
UnsafeUtility.WriteArrayElement(value, 0, valueFloat);

writer.WriteValueSafe(valueFloat);
}
UnsafeUtility.WriteArrayElement(value, 0, valueFloat);
writer.WriteValueSafe(valueFloat);
}
}
}
Expand Down Expand Up @@ -432,23 +428,41 @@ private unsafe void SendAnimStateClientRpc(AnimationMessage animSnapshot, Client
[ClientRpc]
private void SendAnimTriggerClientRpc(AnimationTriggerMessage animSnapshot, ClientRpcParams clientRpcParams = default)
{
m_Animator.SetTrigger(animSnapshot.Hash);
if (animSnapshot.Reset)
{
m_Animator.ResetTrigger(animSnapshot.Hash);
}
else
{
m_Animator.SetTrigger(animSnapshot.Hash);
}
}

public void SetTrigger(string triggerName)
{
SetTrigger(Animator.StringToHash(triggerName));
}

public void SetTrigger(int hash)
public void SetTrigger(int hash, bool reset = false)
{
var animMsg = new AnimationTriggerMessage();
animMsg.Hash = hash;
animMsg.Reset = reset;

if (IsServer)
{
SendAnimTriggerClientRpc(animMsg);
}
}

public void ResetTrigger(string triggerName)
{
ResetTrigger(Animator.StringToHash(triggerName));
}

public void ResetTrigger(int hash)
{
SetTrigger(hash, true);
}
}
}