feat: network animator Trigger parameter support #872

pdeschain · 2021-06-01T16:07:28Z

Epic Link: https://jira.unity3d.com/browse/MTT-586

Added functionality:

Trigger parameters can now be correctly synced via NetworkAnimators

Improvements:

Removed boxing and replaced animator data storage with strongly typed containers

Fixes:
https://jira.unity3d.com/browse/MTT-715 - we are no longer syncing parameters that are driven by animation curves. These parameters are being indirectly set when we sync layer state, so no reason to sync these.

I have also removed the ability to pick parameters of the animator that are being synced - all of them are synced now.

In order to implement the logic that can capture the value of Animator Trigger parameters we have to poll for these values on every frame - Trigger values are transient and can be consumed by the Mecanim internals at any update after they are set to True, thus making it impossible for us to actually detect them being flipped on. Thus we poll for Trigger changes in FixedUpdate - this point in Unity update loop currently seems to be the best time to check for Triggers regardless of the update type the Animator is using.

In order to check the value of a Trigger we rely on an undocumented feature of Animator.GetBool that will retrieve the value of the Trigger as though it was a bool (which it actually is, just with an added semantic of being immediately reset to False).

Initially an attempt was made to avoid syncing parameters at all and to just sync the Animator layer state, but it turned out to be insufficient to make the Animator on the other side to correctly replicate the visuals - Animator parameters are being re-evaluated by the update loop of Mecanim state machine, which happens every frame - and if the parameters aren't in sync, then certain undesirable transitions could be happening, thus breaking the animations on the non-auth peers' side.

In order to play with the new animator:

Build and launch
navigate to the ManualTests->NetworkAnimator
Do the same in Editor (or start Play mode from Assets/Tests/Manual/NetworkAnimatorTests/NetworkAnimatorEnhancement.unity scene).

… than before, supports Triggers but currently does not rely on netvars

…nity.multiplayer.mlapi into feature/networkanimator-trigger-overrides-support

…nity.multiplayer.mlapi into feature/network-animator-triggers

…eird - we can't wait on serializing data till the tick end, this way we will most likely loose some of the netvar values which is potentially bad for trigger-based animations

…nity.multiplayer.mlapi into feature/network-animator-triggers

…ty that's driven by animator curves There is no reason to sync these parameter values - they are being indirectly updated via the layer state sync.

0xFA11 · 2021-06-07T16:30:05Z

just an early heads-up, I was planning to remove Unity-Chan! entirely from the testproject. medium/big assets make Yamato spend more time in tests due to importing and I think both for testing and the sake of package being smaller, we need to use primitives and small assets when it's absolutely required (I know, I put that in and made that mistake in the first place but will correct my mistake).
we can chat about this later, just wanted to plug it in here too.

pdeschain · 2021-06-08T20:24:02Z

@MFatihMAR Aye-aye. I will replace UnityChan usage with something more lightweight after we're done with the draft review stage.

com.unity.multiplayer.mlapi/Editor/NetworkAnimatorEditor.cs

com.unity.multiplayer.mlapi/Prototyping/NetworkAnimatorVar.cs

LukeStampfli

I think the improvements to NetworkAnimator are great and we should merge them for now to improve the animator which already exists and solve some of our user needs.

I don't think we are there yet with the NetworkAnimatorVar. I'm not convinced about including parameters in the animator state and the overall approach in general. I think this part needs some more exploration and potentially a bigger effort for creating a next generation network animator.

…nity.multiplayer.mlapi into feature/network-animator-triggers

…nd replaced it with a nice spinning cube.

LukeStampfli

Great work! NetworkAnimator looks much more clean now than before.

0xFA11 · 2021-07-05T13:02:40Z

com.unity.multiplayer.mlapi/Editor/NetworkAnimatorEditor.cs

-{
-    [CustomEditor(typeof(NetworkAnimator), true)]
-    [CanEditMultipleObjects]
-    public class NetworkAnimatorEditor : Editor


is there anything replacing this or do you think default inspector UI (Editor) is good enough now?

com.unity.multiplayer.mlapi/Prototyping/NetworkAnimator.cs

0xFA11 · 2021-07-05T13:10:31Z

testproject/Assets/Tests/Manual/NetworkAnimatorTests/AnimatedCubeController.cs

+        {
+            if (IsOwner)
+            {
+                if(Input.GetKeyDown(KeyCode.C))


you might consider running ./standards.py --fix once to format all source files :)

Suggested change

if(Input.GetKeyDown(KeyCode.C))

if (Input.GetKeyDown(KeyCode.C))

0xFA11

overall looks like a good refactor to me!
I left some minor comments & questions here and there.

one more thing though, would you also remove Unity-Chan! stuff in this PR as it was mainly there for testing/showing NetworkAnimator working but now we have simple cube anims replacing it?

…ity-Technologies/com.unity.multiplayer.mlapi into test/multiprocess-tests/orchestration * 'test/multiprocess-tests/orchestration' of github.com:Unity-Technologies/com.unity.multiplayer.mlapi: Update testproject/Assets/Tests/Runtime/MultiprocessRuntime/Helpers/BuildMultiprocessTestPlayer.cs rename test scene changing root menu no longer ignore '[Ss]treamingAssets/buildInfo.txt' feat: network animator Trigger parameter support (#872) # Conflicts: # testproject/Assets/Tests/Runtime/MultiprocessRuntime/Helpers/BuildMultiprocessTestPlayer.cs

…rocess-tests/base-multiprocess-tests * test/multiprocess-tests/orchestration: Update testproject/Assets/Tests/Runtime/MultiprocessRuntime/Helpers/BuildMultiprocessTestPlayer.cs rename test scene # changing root menu PR suggestions no longer ignore '[Ss]treamingAssets/buildInfo.txt' feat: network animator Trigger parameter support (#872) # Conflicts: # testproject/ProjectSettings/EditorBuildSettings.asset

…est/multiprocess-tests/execute-step-in-context * test/multiprocess-tests/base-multiprocess-tests: proper rename for scene rename for test scene Update testproject/Assets/Tests/Runtime/MultiprocessRuntime/Helpers/BuildMultiprocessTestPlayer.cs rename test scene # changing root menu PR suggestions no longer ignore '[Ss]treamingAssets/buildInfo.txt' feat: network animator Trigger parameter support (#872)

…est/multiprocess-tests/adding-perf-tests-for-spawn * test/multiprocess-tests/execute-step-in-context: proper rename for scene rename for test scene Update testproject/Assets/Tests/Runtime/MultiprocessRuntime/Helpers/BuildMultiprocessTestPlayer.cs rename test scene # changing root menu PR suggestions no longer ignore '[Ss]treamingAssets/buildInfo.txt' feat: network animator Trigger parameter support (#872) # Conflicts: # testproject/Assets/Tests/Runtime/MultiprocessRuntime.meta # testproject/Assets/Tests/Runtime/MultiprocessRuntime/Helpers.meta # testproject/Assets/Tests/Runtime/MultiprocessRuntime/Helpers/BuildMultiprocessTestPlayer.cs

…to test/multiprocess-tests/adding-doc-on-how-to-use * test/multiprocess-tests/adding-perf-tests-for-spawn: proper rename for scene rename for test scene Update testproject/Assets/Tests/Runtime/MultiprocessRuntime/Helpers/BuildMultiprocessTestPlayer.cs rename test scene # changing root menu PR suggestions no longer ignore '[Ss]treamingAssets/buildInfo.txt' feat: network animator Trigger parameter support (#872)

…nsform * develop: test: Perf tests part 3. Adding ExecuteStepInContext for better test readability (#924) test: Perf tests part 2. Adding Test Coordinator and base test class (#923) fix: (MLAPI.Serialization) 'specified cast is not valid.' on NetworkW… (#951) test: Perf tests part 1. Basis for multiprocess tests process orchestration. (#922) feat: network animator Trigger parameter support (#872)

pdeschain and others added 11 commits May 19, 2021 16:34

Added enhanced version of NetworkAnimator that allocates less garbage…

950b33e

… than before, supports Triggers but currently does not rely on netvars

Merge branch 'develop' of https://github.com/Unity-Technologies/com.u…

78dbb5f

…nity.multiplayer.mlapi into feature/networkanimator-trigger-overrides-support

#

bdc12ed

#

c4f4c14

Merge branch 'develop' of https://github.com/Unity-Technologies/com.u…

ef3ad16

…nity.multiplayer.mlapi into feature/network-animator-triggers

#

d33665a

NetworkAnimator updated with all the latest goodies. Var version is w…

90d18b2

…eird - we can't wait on serializing data till the tick end, this way we will most likely loose some of the netvar values which is potentially bad for trigger-based animations

Merge branch 'develop' of https://github.com/Unity-Technologies/com.u…

30190b7

…nity.multiplayer.mlapi into feature/network-animator-triggers

Test scene now shows both netvar and rpc-based network animators

cc1f66b

Fixed an issue when N.A. tries to set the value of an animator proepr…

5487435

…ty that's driven by animator curves There is no reason to sync these parameter values - they are being indirectly updated via the layer state sync.

Merge branch 'develop' into feature/network-animator-triggers

643be92

pdeschain requested review from SamuelBellomo, LukeStampfli, NoelStephensUnity and 0xFA11 June 8, 2021 20:20

LukeStampfli reviewed Jun 9, 2021

View reviewed changes

com.unity.multiplayer.mlapi/Editor/NetworkAnimatorEditor.cs Show resolved Hide resolved

LukeStampfli reviewed Jun 9, 2021

View reviewed changes

com.unity.multiplayer.mlapi/Prototyping/NetworkAnimatorVar.cs Outdated Show resolved Hide resolved

LukeStampfli reviewed Jun 9, 2021

View reviewed changes

pdeschain and others added 4 commits June 28, 2021 11:59

Merge branch 'develop' into feature/network-animator-triggers

f85229b

Merge branch 'develop' of https://github.com/Unity-Technologies/com.u…

7887d6d

…nity.multiplayer.mlapi into feature/network-animator-triggers

Removed NetowrkAnimatorVar, removed dependency on Unity Chan prefab a…

8d3b5e7

…nd replaced it with a nice spinning cube.

Added some text with instruction on how to use the scene in runtime

f5e80df

pdeschain marked this pull request as ready for review June 30, 2021 16:41

pdeschain requested review from LukeStampfli and removed request for 0xFA11, LukeStampfli, SamuelBellomo and NoelStephensUnity June 30, 2021 16:41

pdeschain requested review from LukeStampfli, NoelStephensUnity and SamuelBellomo July 1, 2021 16:47

Merge branch 'develop' into feature/network-animator-triggers

914a7aa

LukeStampfli approved these changes Jul 5, 2021

View reviewed changes

0xFA11 reviewed Jul 5, 2021

View reviewed changes

com.unity.multiplayer.mlapi/Prototyping/NetworkAnimator.cs Outdated Show resolved Hide resolved

0xFA11 reviewed Jul 5, 2021

View reviewed changes

com.unity.multiplayer.mlapi/Prototyping/NetworkAnimator.cs Show resolved Hide resolved

0xFA11 reviewed Jul 5, 2021

View reviewed changes

0xFA11 approved these changes Jul 5, 2021

View reviewed changes

Removed UnityChan and changed some variable naming + formatting pass

f342d2d

pdeschain merged commit be0ca06 into develop Jul 6, 2021

pdeschain deleted the feature/network-animator-triggers branch July 6, 2021 19:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: network animator Trigger parameter support #872

feat: network animator Trigger parameter support #872

Uh oh!

pdeschain commented Jun 1, 2021 •

edited

Loading

Uh oh!

0xFA11 commented Jun 7, 2021

Uh oh!

pdeschain commented Jun 8, 2021

Uh oh!

Uh oh!

Uh oh!

LukeStampfli left a comment

Uh oh!

LukeStampfli left a comment

Uh oh!

0xFA11 Jul 5, 2021

Uh oh!

Uh oh!

Uh oh!

0xFA11 Jul 5, 2021

Uh oh!

0xFA11 left a comment

Uh oh!

Uh oh!

	if(Input.GetKeyDown(KeyCode.C))
	if (Input.GetKeyDown(KeyCode.C))

feat: network animator Trigger parameter support #872

feat: network animator Trigger parameter support #872

Uh oh!

Conversation

pdeschain commented Jun 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xFA11 commented Jun 7, 2021

Uh oh!

pdeschain commented Jun 8, 2021

Uh oh!

Uh oh!

Uh oh!

LukeStampfli left a comment

Choose a reason for hiding this comment

Uh oh!

LukeStampfli left a comment

Choose a reason for hiding this comment

Uh oh!

0xFA11 Jul 5, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

0xFA11 Jul 5, 2021

Choose a reason for hiding this comment

Uh oh!

0xFA11 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pdeschain commented Jun 1, 2021 •

edited

Loading