Skip to content

fix: remove auto-send animation parameters #1746

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
merged 4 commits into from
Feb 24, 2022
Merged

fix: remove auto-send animation parameters #1746

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c2e75b7
fix: remove auto-send animation parameters
mattwalsh-unity Feb 23, 2022
6827ba3
Merge branch 'develop' into fix/remove-animation-autosend
mattwalsh-unity Feb 23, 2022
00eb364
Merge branch 'develop' into fix/remove-animation-autosend
mattwalsh-unity Feb 23, 2022
d5d088a
Merge branch 'develop' into fix/remove-animation-autosend
mattwalsh-unity Feb 23, 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
2 changes: 1 addition & 1 deletion com.unity.netcode.gameobjects/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
- Fixed The NetworkConfig's checksum hash includes the NetworkTick so that clients with a different tickrate than the server are identified and not allowed to connect (#1728)
- Fixed OwnedObjects not being properly modified when using ChangeOwnership (#1731)
- Improved performance in NetworkAnimator (#1735)
- Fixed display over "always sync" network animator parameters even when an animator controller override is in use (#1735)
- Removed the "always sync" network animator (aka "autosend") parameters (#1746)

## [1.0.0-pre.5] - 2022-01-26

Expand Down
142 changes: 17 additions & 125 deletions com.unity.netcode.gameobjects/Components/NetworkAnimator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,6 @@ public void NetworkSerialize<T>(BufferSerializer<T> serializer) where T : IReade
}
}

internal struct AnimationParametersMessage : INetworkSerializable
{
public byte[] Parameters;

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

internal struct AnimationTriggerMessage : INetworkSerializable
{
public int Hash;
Expand All @@ -49,51 +39,22 @@ public void NetworkSerialize<T>(BufferSerializer<T> serializer) where T : IReade
}

[SerializeField] private Animator m_Animator;
[SerializeField] private uint m_ParameterSendBits;
[SerializeField] private float m_SendRate = 0.1f;

public Animator Animator
{
get { return m_Animator; }
set
{
m_Animator = value;
ResetParameterOptions();
}
}

/*
* AutoSend is the ability to select which parameters linked to this animator
* get replicated on a regular basis regardless of a state change. The thinking
* behind this is that many of the parameters people use are usually booleans
* which result in a state change and thus would cause a full sync of state.
* Thus if you really care about a parameter syncing then you need to be explicit
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately I can't ask Andrew what he meant by "if you really care", but apart from adding redundancy to cover unreliable delivery (which we don't use for this), the only scenario I can think of here is if you could go from state A to state B and back to state A all between 2 sequential FixedUpdate calls (and also not using our SetTrigger), in which case we wouldn't pick it up and the connected players would stay on 'A' missing the ultra-short 'B' transition.

* by selecting it in the inspector when an NetworkAnimator is selected.
*/
public void SetParameterAutoSend(int index, bool value)
{
if (value)
{
m_ParameterSendBits |= (uint)(1 << index);
}
else
{
m_ParameterSendBits &= (uint)(~(1 << index));
}
}

public bool GetParameterAutoSend(int index)
{
return (m_ParameterSendBits & (uint)(1 << index)) != 0;
}

private bool m_SendMessagesAllowed = false;

// Animators only support up to 32 params
public static int K_MaxAnimationParams = 32;

private int m_TransitionHash;
private double m_NextSendTime = 0.0f;

private int m_AnimationHash;
public int AnimationHash { get => m_AnimationHash; }
Expand All @@ -105,7 +66,7 @@ private unsafe struct AnimatorParamCache
public fixed byte Value[4]; // this is a max size of 4 bytes
}

// 128bytes per Animator
// 128 bytes per Animator
private FastBufferWriter m_ParameterWriter = new FastBufferWriter(K_MaxAnimationParams * sizeof(float), Allocator.Persistent);
private NativeArray<AnimatorParamCache> m_CachedAnimatorParameters;

Expand All @@ -124,17 +85,6 @@ static AnimationParamEnumWrapper()
}
}

internal void ResetParameterOptions()
{

if (NetworkLog.CurrentLogLevel <= LogLevel.Developer)
{
NetworkLog.LogInfoServer("ResetParameterOptions");
}

m_ParameterSendBits = 0;
}

public override void OnDestroy()
{
if (m_CachedAnimatorParameters.IsCreated)
Expand Down Expand Up @@ -162,14 +112,17 @@ public override void OnNetworkSpawn()

if (m_Animator.IsParameterControlledByCurve(parameter.nameHash))
{
//we are ignoring parameters that are controlled by animation curves - syncing the layer states indirectly syncs the values that are driven by the animation curves
// we are ignoring parameters that are controlled by animation curves - syncing the layer
// states indirectly syncs the values that are driven by the animation curves
continue;
}

var cacheParam = new AnimatorParamCache();
var cacheParam = new AnimatorParamCache
{
Type = UnsafeUtility.EnumToInt(parameter.type),
Hash = parameter.nameHash
};

cacheParam.Type = UnsafeUtility.EnumToInt(parameter.type);
cacheParam.Hash = parameter.nameHash;
unsafe
{
switch (parameter.type)
Expand Down Expand Up @@ -213,49 +166,24 @@ private void FixedUpdate()
float normalizedTime;
if (!CheckAnimStateChanged(out stateHash, out normalizedTime))
{
// We only want to check and send if we don't have any other state to since
// as we will sync all params as part of the state sync
CheckAndSend();

return;
}

var animMsg = new AnimationMessage();
animMsg.StateHash = stateHash;
animMsg.NormalizedTime = normalizedTime;
var animMsg = new AnimationMessage
{
StateHash = stateHash,
NormalizedTime = normalizedTime
};

m_ParameterWriter.Seek(0);
m_ParameterWriter.Truncate();

WriteParameters(m_ParameterWriter, false);
WriteParameters(m_ParameterWriter);
animMsg.Parameters = m_ParameterWriter.ToArray();

SendAnimStateClientRpc(animMsg);
}

private void CheckAndSend()
{
var networkTime = NetworkManager.ServerTime.Time;
if (m_SendMessagesAllowed && m_SendRate != 0 && m_NextSendTime < networkTime)
{
m_NextSendTime = networkTime + m_SendRate;

m_ParameterWriter.Seek(0);
m_ParameterWriter.Truncate();

if (WriteParameters(m_ParameterWriter, true))
{
// we then sync the params we care about
var animMsg = new AnimationParametersMessage()
{
Parameters = m_ParameterWriter.ToArray()
};

SendParamsClientRpc(animMsg);
}
}
}

private bool CheckAnimStateChanged(out int stateHash, out float normalizedTime)
{
stateHash = 0;
Expand Down Expand Up @@ -297,20 +225,10 @@ the read side of this function doesn't have similar logic which would cause
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)
private unsafe void WriteParameters(FastBufferWriter writer)
{
if (m_CachedAnimatorParameters == null)
{
return false;
}

for (int i = 0; i < m_CachedAnimatorParameters.Length; i++)
{
if (autoSend && !GetParameterAutoSend(i))
{
continue;
}

ref var cacheValue = ref UnsafeUtility.ArrayElementAsRef<AnimatorParamCache>(m_CachedAnimatorParameters.GetUnsafePtr(), i);
var hash = cacheValue.Hash;

Expand Down Expand Up @@ -343,24 +261,12 @@ private unsafe bool WriteParameters(FastBufferWriter writer, bool autoSend)
}
}
}

// If we do not write any values to the writer then we should not send any data
return writer.Length > 0;
}

private unsafe void ReadParameters(FastBufferReader reader, bool autoSend)
private unsafe void ReadParameters(FastBufferReader reader)
{
if (m_CachedAnimatorParameters == null)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this can never be null

{
return;
}

for (int i = 0; i < m_CachedAnimatorParameters.Length; i++)
{
if (autoSend && !GetParameterAutoSend(i))
{
continue;
}
ref var cacheValue = ref UnsafeUtility.ArrayElementAsRef<AnimatorParamCache>(m_CachedAnimatorParameters.GetUnsafePtr(), i);
var hash = cacheValue.Hash;

Expand Down Expand Up @@ -394,20 +300,6 @@ private unsafe void ReadParameters(FastBufferReader reader, bool autoSend)
}
}

[ClientRpc]
private unsafe void SendParamsClientRpc(AnimationParametersMessage animSnapshot, ClientRpcParams clientRpcParams = default)
{
if (animSnapshot.Parameters != null)
{
// We use a fixed value here to avoid the copy of data from the byte buffer since we own the data
fixed (byte* parameters = animSnapshot.Parameters)
{
var reader = new FastBufferReader(parameters, Allocator.None, animSnapshot.Parameters.Length);
ReadParameters(reader, true);
}
}
}

[ClientRpc]
private unsafe void SendAnimStateClientRpc(AnimationMessage animSnapshot, ClientRpcParams clientRpcParams = default)
{
Expand All @@ -423,7 +315,7 @@ private unsafe void SendAnimStateClientRpc(AnimationMessage animSnapshot, Client
fixed (byte* parameters = animSnapshot.Parameters)
{
var reader = new FastBufferReader(parameters, Allocator.None, animSnapshot.Parameters.Length);
ReadParameters(reader, false);
ReadParameters(reader);
}
}
}
Expand Down
98 changes: 4 additions & 94 deletions com.unity.netcode.gameobjects/Editor/NetworkAnimatorEditor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,113 +1,23 @@
using System;
using Unity.Netcode.Components;
using UnityEditor;
using UnityEditor.Animations;
using UnityEngine;

namespace Unity.Netcode.Editor
{
public static class TextUtility
{
public static GUIContent TextContent(string name, string tooltip)
{
var newContent = new GUIContent(name);
newContent.tooltip = tooltip;
return newContent;
}

public static GUIContent TextContent(string name)
{
return new GUIContent(name);
}
}

[CustomEditor(typeof(NetworkAnimator), true)]
[CanEditMultipleObjects]
public class NetworkAnimatorEditor : UnityEditor.Editor
{
private NetworkAnimator m_AnimSync;
[NonSerialized] private bool m_Initialized;
private SerializedProperty m_AnimatorProperty;
private GUIContent m_AnimatorLabel;

private void Init()
{
if (m_Initialized)
{
return;
}

m_Initialized = true;
m_AnimSync = target as NetworkAnimator;

m_AnimatorProperty = serializedObject.FindProperty("m_Animator");
m_AnimatorLabel = TextUtility.TextContent("Animator", "The Animator component to synchronize.");
}

public override void OnInspectorGUI()
{
Init();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't seem worth it to me to go to the trouble of tracking init and not re-drawing. This isn't remotely performance noticeable, and makes the code harder to grok.

serializedObject.Update();
DrawControls();
serializedObject.ApplyModifiedProperties();
}

private void DrawControls()
{
EditorGUI.BeginChangeCheck();
EditorGUILayout.PropertyField(m_AnimatorProperty, m_AnimatorLabel);
if (EditorGUI.EndChangeCheck())
{
m_AnimSync.ResetParameterOptions();
}

if (m_AnimSync.Animator == null)
{
return;
}
var label = new GUIContent("Animator", "The Animator component to synchronize");
EditorGUILayout.PropertyField(serializedObject.FindProperty("m_Animator"), label);
EditorGUI.EndChangeCheck();

AnimatorController controller;
var overrideController = m_AnimSync.Animator.runtimeAnimatorController as AnimatorOverrideController;
if (overrideController != null)
{
controller = overrideController.runtimeAnimatorController as AnimatorController;
}
else
{
controller = m_AnimSync.Animator.runtimeAnimatorController as AnimatorController;
}

if (controller != null)
{
var showWarning = false;
EditorGUI.indentLevel += 1;
int i = 0;

foreach (var p in controller.parameters)
{
if (i >= NetworkAnimator.K_MaxAnimationParams)
{
showWarning = true;
break;
}

bool oldSend = m_AnimSync.GetParameterAutoSend(i);
bool send = EditorGUILayout.Toggle(p.name, oldSend);
if (send != oldSend)
{
m_AnimSync.SetParameterAutoSend(i, send);
EditorUtility.SetDirty(target);
}
i += 1;
}

if (showWarning)
{
EditorGUILayout.HelpBox($"NetworkAnimator can only select between the first {NetworkAnimator.K_MaxAnimationParams} parameters in a mecanim controller", MessageType.Warning);
}

EditorGUI.indentLevel -= 1;
}
serializedObject.ApplyModifiedProperties();
}
}
}