Skip to content

feat: NetworkAnimator Cleanup #1281

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

Jump to bottom
Merged
merged 10 commits into from
Oct 14, 2021
Merged

feat: NetworkAnimator Cleanup #1281

merged 10 commits into from
Oct 14, 2021

Conversation

andrews-unity
Copy link
Contributor

@andrews-unity andrews-unity commented Oct 11, 2021 •
edited
Loading

This is an attempt to clean up NetworkAnimator

  • Fixed an issue where we didn't sync when going into a transition state which is possible and we didn't even support at all in the current one
  • We no longer sync all params by default unless you do a state change either full state or transition state. This is because 90% of the params you will sync are bools which just result in a state change.
  • We now allow the dev to pick which parameters get auto-synced which means that after some interval it will check to see if the values have changed and if so it will sync that param (as well as other params that are set to sync)
  • Triggers do not sync with other params, they are an explicit RPC
  • If nothing changes we don't send anything
  • zero runtime GC
  • Server Auth only

Changelog

com.unity.netcode.gameobjects

  • Changed: Re-worked NetworkAnimator to reduce overhead as well as proper correctness when it comes to state changes.
  • Changed: NetworkAnimator no longer supports client authority.

Testing and Documentation

  • No tests have been added.
  • Includes edits to existing public API documentation.

mattwalsh-unity
mattwalsh-unity suggested changes Oct 13, 2021
View reviewed changes
com.unity.netcode.gameobjects/Components/NetworkAnimator.cs
}
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably easier to read if you didn't have this 'false' and then had the stuff on line 249 in an 'else'

0xFA11
0xFA11 reviewed Oct 13, 2021
View reviewed changes
com.unity.netcode.gameobjects/Components/com.unity.netcode.components.asmdef
Comment on lines -5 to +16
"Unity.Netcode.Runtime"
]
"Unity.Netcode.Runtime",
"Unity.Collections"
],
"includePlatforms": [],
"excludePlatforms": [],
"allowUnsafeCode": true,
"overrideReferences": false,
"precompiledReferences": [],
"autoReferenced": true,
"defineConstraints": [],
"versionDefines": [],
"noEngineReferences": false
Copy link
Contributor

Choose a reason for hiding this comment

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

this diff could be as small & simple as:

-         "Unity.Netcode.Runtime"
+         "Unity.Netcode.Runtime",
+         "Unity.Collections"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well and the allow unsafe code :P but this is Unity saving the asmdef :P

0xFA11
0xFA11 reviewed Oct 13, 2021
View reviewed changes
0xFA11
0xFA11 reviewed Oct 13, 2021
View reviewed changes
com.unity.netcode.gameobjects/Components/NetworkAnimator.cs
@@ -78,12 +86,13 @@ public bool GetParameterAutoSend(int index)
}

// Animators only support up to 32 params
private const int k_MaxAnimationParams = 32;
public static int K_MaxAnimationParams = 32;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm Mr. Nit (optional though)

Suggested change
public static int K_MaxAnimationParams = 32;
public static int MaxAnimationParamCount = 32;

0xFA11
0xFA11 approved these changes Oct 13, 2021
View reviewed changes
Copy link
Contributor

@0xFA11 0xFA11 left a comment

Choose a reason for hiding this comment

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

parking my 🚀 here

Cosmin-B
Cosmin-B approved these changes Oct 13, 2021
View reviewed changes
Copy link
Contributor

@Cosmin-B Cosmin-B left a comment

Choose a reason for hiding this comment

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

Love it!

mattwalsh-unity
mattwalsh-unity approved these changes Oct 13, 2021
View reviewed changes
Copy link
Contributor

@mattwalsh-unity mattwalsh-unity left a comment

Choose a reason for hiding this comment

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

Terrific!

@andrews-unity andrews-unity merged commit 4bb6d3a into develop Oct 14, 2021
@andrews-unity andrews-unity deleted the feature/network-anim-exp-take-2 branch October 14, 2021 01:17
@Shivang44
Copy link

Shivang44 commented Dec 26, 2021 •
edited
Loading

Server Auth only
Changed: NetworkAnimator no longer supports client authority.

@andrews-unity Can we/should we add more documentation on this here (https://docs-multiplayer.unity3d.com/docs/components/networkanimator)? I spent quite a few hours wondering why the NetworkAnimator was not syncing the bools set on my local player game object.

Thanks!

mollstam pushed a commit to Keepsake-Games/com.unity.netcode.gameobjects that referenced this pull request Feb 13, 2023
@andrews-unity
feat: NetworkAnimator Cleanup (Unity-Technologies#1281)
b20e1dd 
* feat: NetworkAnimator Cleanup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xFA11 0xFA11 0xFA11 approved these changes

@Cosmin-B Cosmin-B Cosmin-B approved these changes

@mattwalsh-unity mattwalsh-unity mattwalsh-unity approved these changes

@JesseOlmer JesseOlmer Awaiting requested review from JesseOlmer

@pdeschain pdeschain Awaiting requested review from pdeschain

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@andrews-unity @Shivang44 @0xFA11 @Cosmin-B @mattwalsh-unity