-
Notifications
You must be signed in to change notification settings - Fork 450
fix: server doesn't apply triggers to itself; add host and server Animation tests (#1746) #1751
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
Changes from all commits
b6b1908
64e6f70
e29a9eb
fd13121
62996fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,10 +18,21 @@ public class NetworkAnimatorTests : BaseMultiInstanceTest | |
private Animator m_PlayerOnServerAnimator; | ||
private Animator m_PlayerOnClientAnimator; | ||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so I'm not sure how changes in this test file actually tests against the fix proposed here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before, we just called |
||
[UnitySetUp] | ||
public override IEnumerator Setup() | ||
{ | ||
yield return StartSomeClientsAndServerWithPlayers(useHost: true, nbClients: NbClients, | ||
// this is treacherous...normally BaseMultiInstance calls StartSomeClientsAndServerWithPlayers for you | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @NoelStephensUnity this was what I was trying to convey in our slack thread There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a way to handle this a little more gracefully in the NetcodeIntegraitonTest, and was why I included an example of how to do this in the documentation (because the way it is currently can be problematic to figure out the best way to achieve the same kind of results). |
||
// in its version of Setup. In this case, I want to alternatively test Server and Host mode via parameters | ||
// in each test. However I cannot inject the host / server mode into the corresponding Setup calls, hence | ||
// I wrote and manually call an Init function. However, if I don't write this null version of setup | ||
// I will get 2 calls to StartSomeClientsAndServerWithPlayers, which wrecks everything. | ||
yield return null; | ||
} | ||
|
||
public IEnumerator Init(bool hostMode) | ||
{ | ||
yield return StartSomeClientsAndServerWithPlayers(useHost: hostMode, nbClients: NbClients, | ||
updatePlayerPrefab: playerPrefab => | ||
{ | ||
// ideally, we would build up the AnimatorController entirely in code and not need an asset, | ||
|
@@ -63,8 +74,10 @@ private bool HasClip(Animator animator, string clipName) | |
} | ||
|
||
[UnityTest] | ||
public IEnumerator AnimationTriggerReset([Values(true, false)] bool asHash) | ||
public IEnumerator AnimationTriggerReset([Values(true, false)] bool asHash, [Values(true, false)] bool hostMode) | ||
{ | ||
yield return Init(hostMode); | ||
|
||
// We have "UnboundTrigger" purposely not bound to any animations so we can test resetting. | ||
// If we used a trigger that was bound to a transition, then the trigger would reset as soon as the | ||
// transition happens. This way it will stay stuck on | ||
|
@@ -114,9 +127,12 @@ public IEnumerator AnimationTriggerReset([Values(true, false)] bool asHash) | |
Assert.False(s_GloabalTimeOutHelper.TimedOut, "Timed out on client reset check"); | ||
} | ||
|
||
|
||
[UnityTest] | ||
public IEnumerator AnimationStateSyncTest() | ||
public IEnumerator AnimationStateSyncTest([Values(true, false)] bool hostMode) | ||
{ | ||
yield return Init(hostMode); | ||
|
||
// check that we have started in the default state | ||
Assert.True(m_PlayerOnServerAnimator.GetCurrentAnimatorStateInfo(0).IsName("DefaultState")); | ||
Assert.True(m_PlayerOnClientAnimator.GetCurrentAnimatorStateInfo(0).IsName("DefaultState")); | ||
|
@@ -137,8 +153,10 @@ public IEnumerator AnimationStateSyncTest() | |
} | ||
|
||
[UnityTest] | ||
public IEnumerator AnimationStateSyncTriggerTest([Values(true, false)] bool asHash) | ||
public IEnumerator AnimationStateSyncTriggerTest([Values(true, false)] bool asHash, [Values(true, false)] bool hostMode) | ||
{ | ||
yield return Init(hostMode); | ||
|
||
string triggerString = "TestTrigger"; | ||
int triggerHash = Animator.StringToHash(triggerString); | ||
|
||
|
@@ -172,8 +190,9 @@ public IEnumerator AnimationStateSyncTriggerTest([Values(true, false)] bool asHa | |
} | ||
|
||
[UnityTest] | ||
public IEnumerator AnimationStateSyncTestWithOverride() | ||
public IEnumerator AnimationStateSyncTestWithOverride([Values(true, false)] bool hostMode) | ||
{ | ||
yield return Init(hostMode); | ||
// set up the animation override controller | ||
var overrideController = Resources.Load("TestAnimatorOverrideController") as AnimatorOverrideController; | ||
m_PlayerOnServer.GetComponent<Animator>().runtimeAnimatorController = overrideController; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the missing server animation update bug fix