fix: NetworkAnimator does not allow disabling or enabling parameters for synch #3586
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…-or-enabling-parameter-synch
Work in progress.
…-or-enabling-parameter-synch
Renamed the new method `ToggleParameterSync` to `EnableParameterSynchronization`. Added overridden version of `EnableParameterSynchronization` that accepts a string for the name and changed the original method to accept the hash value of the parameter name. Did some minor clean up in the `NetworkAnimatorStateChangeHandler.NetworkUpdate` method. Added some additional comments in various places.
…er-synch' of https://github.com/Unity-Technologies/com.unity.netcode.gameobjects into fix/networkanimator-allow-disabling-or-enabling-parameter-synch
Reverted the change in HiddenScriptEditor. Moved away from a NetworkAnimator custom editor to a custom property drawer to handle displaying the Animator parameters to be synchronized. Handling the population of parameters and populating the NetworkAnimator.AnimatorParameterEntries during OnValidate. Had to re-apply unchecking the excluded parameter in some prefabs for testing purposes.
…-or-enabling-parameter-synch
…-or-enabling-parameter-synch
…-or-enabling-parameter-synch
…-or-enabling-parameter-synch
5 tasks
…-or-enabling-parameter-synch
…-or-enabling-parameter-synch
…-or-enabling-parameter-synch
…-or-enabling-parameter-synch
Converting all legacy ClientRpc and ServerRpc usages over to the newer universal RPC format. Using a pre-allocated Persistent RpcTargetGroup as opposed to the List. When using `NetworkAnimator.SetTrigger` with a client-server network topology and it is the server or host invoking the method, then go ahead and set the trigger since it no longer sends itself an RPC/no longer invokes the InternalSetTrigger via RPC.
…-or-enabling-parameter-synch
EmandM
reviewed
Nov 3, 2025
| /// Optionally, you can still derive from <see cref="NetworkAnimator"/> and override the <see cref="OnIsServerAuthoritative"/> method. | ||
| /// </summary> | ||
| /// <returns><see cref="true"/> or <see cref="false"/></returns> | ||
| public bool IsServerAuthoritative() |
There was a problem hiding this comment.
Are we sure we want to make this public?
| } | ||
|
|
||
| return !DistributedAuthorityMode; | ||
| return m_LocalNetworkManager && m_LocalNetworkManager.DistributedAuthorityMode ? true : AuthorityMode == AuthorityModes.Server; |
There was a problem hiding this comment.
I'm not normally the one to suggest this but I think this might be easier to read if it's not in one line.
Suggested change
| return m_LocalNetworkManager && m_LocalNetworkManager.DistributedAuthorityMode ? true : AuthorityMode == AuthorityModes.Server; | |
| if (m_LocalNetworkManager && m_LocalNetworkManager.DistributedAuthorityMode) | |
| { | |
| return true; | |
| } | |
| return AuthorityMode == AuthorityModes.Server; |
| /// </summary> | ||
| [Rpc(SendTo.NotAuthority)] | ||
| internal void SendParametersUpdateRpc(ParametersUpdateMessage parametersUpdate) | ||
| [Rpc(SendTo.NotAuthority, AllowTargetOverride = true, InvokePermission = RpcInvokePermission.Owner)] |
There was a problem hiding this comment.
This will be changing the logic (might be intended). the default for [Rpc(SendTo.NotAuthority)] is InvokePermission = RpcInvokePermission.Everyone
There was a problem hiding this comment.
This just assures that in a distributed authority session (when this is only invoked) the owner/authority is the only instance that can send parameter updates (i.e. a more "secure" NetworkAnimator for distributed authority) to the non-authority instances.
| /// </summary> | ||
| [ServerRpc] | ||
| private void SendAnimStateServerRpc(AnimationMessage animationMessage, ServerRpcParams serverRpcParams = default) | ||
| [Rpc(SendTo.Server, AllowTargetOverride = true)] |
There was a problem hiding this comment.
This also might have been an intentional change.
The default for [ServerRpc] would be RpcInvokePermission.Owner
Suggested change
| [Rpc(SendTo.Server, AllowTargetOverride = true)] | |
| [Rpc(SendTo.Server, AllowTargetOverride = true, InvokePermission = RpcInvokePermission.Owner)] |
| /// </summary> | ||
| [ServerRpc] | ||
| internal void SendAnimTriggerServerRpc(AnimationTriggerMessage animationTriggerMessage, ServerRpcParams serverRpcParams = default) | ||
| [Rpc(SendTo.Server, AllowTargetOverride = true)] |
There was a problem hiding this comment.
Also here.
Suggested change
| [Rpc(SendTo.Server, AllowTargetOverride = true)] | |
| [Rpc(SendTo.Server, AllowTargetOverride = true, InvokePermission = RpcInvokePermission.Owner)] |
| /// <param name="animationTriggerMessage">the payload containing the trigger data to apply</param> | ||
| [Rpc(SendTo.NotAuthority)] | ||
| internal void SendAnimTriggerRpc(AnimationTriggerMessage animationTriggerMessage) | ||
| [Rpc(SendTo.NotAuthority, AllowTargetOverride = true, InvokePermission = RpcInvokePermission.Owner)] |
There was a problem hiding this comment.
Also here, this will be changing the previous behaviour.
Comment on lines
+1803
to
+1815
| if (!IsSpawned || serverAuthoritative && IsServer || !serverAuthoritative && IsOwner) | ||
| { | ||
| for (int i = 0; i < m_CachedAnimatorParameters.Length; i++) | ||
| { | ||
| var cachedParameter = m_CachedAnimatorParameters[i]; | ||
| if (cachedParameter.Hash == parameterNameHash) | ||
| { | ||
| cachedParameter.Exclude = !isEnabled; | ||
| m_CachedAnimatorParameters[i] = cachedParameter; | ||
| break; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Any chance we can swap this to an early return?
Suggested change
| if (!IsSpawned || serverAuthoritative && IsServer || !serverAuthoritative && IsOwner) | |
| { | |
| for (int i = 0; i < m_CachedAnimatorParameters.Length; i++) | |
| { | |
| var cachedParameter = m_CachedAnimatorParameters[i]; | |
| if (cachedParameter.Hash == parameterNameHash) | |
| { | |
| cachedParameter.Exclude = !isEnabled; | |
| m_CachedAnimatorParameters[i] = cachedParameter; | |
| break; | |
| } | |
| } | |
| } | |
| if (IsSpawned && (serverAuthoritative && IsServer || !serverAuthoritative && IsOwner)) | |
| { | |
| // not valid to edit | |
| return; | |
| } | |
| for (int i = 0; i < m_CachedAnimatorParameters.Length; i++) | |
| { | |
| var cachedParameter = m_CachedAnimatorParameters[i]; | |
| if (cachedParameter.Hash == parameterNameHash) | |
| { | |
| cachedParameter.Exclude = !isEnabled; | |
| m_CachedAnimatorParameters[i] = cachedParameter; | |
| break; | |
| } | |
| } |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose of this PR
This PR addresses the primary issue of NetworkAnimator not providing a way to exclude specific parameters from synchronization to provide users the ability to handle the synchronization of the excluded parameters themselves or to handle them via a calculation performed on non-authority instances.
This PR also includes:
Jira ticket:
MTTB-1485
fix: #3530
Changelog
NetworkAnimator.AuthorityModewhich allows you to select whether theNetworkAnimatorwill use a server or owner authority model for state updates (likeNetworkTransform).Animatorparameters theNetworkAnimatorshould synchronize. This can be done via the inspector view interface or during runtime viaNetworkAnimator.EnableParameterSynchronization.RpcAttributealong with the appropriateSendToparameter.Documentation
Testing & QA
Functional Testing
Manual testing :
Manual testing doneAutomated tests:
Covered by existing automated testsCovered by new automated testsDoes the change require QA team to:
Review automated tests?Execute manual tests?If any boxes above are checked, please add QA as a PR reviewer.
Backports
No backport is needed as this sustainability update will only be available in v2.x.