fix: dual triggers generating false state transition [MTTB-48] #2999
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
This fixes the issue where a trigger that transitions to a state with a trigger that transitions back to the original state could cause NetworkAnimator to generate a false layer to layer state transition which would not only generate an additional message after the original trigger message, but would cause the non-authority instances to log a warning message.
This includes additional assets needed to test this fix manually as well as some of the assets included will be used in an integration test.
NoelStephensUnity
changed the title
fluong6
reviewed
Aug 6, 2024
| @@ -832,7 +832,8 @@ private void CheckForStateChange(int layer) | |||
| stateChangeDetected = true; | |||
| //Debug.Log($"[Cross-Fade] To-Hash: {nt.fullPathHash} | TI-Duration: ({tt.duration}) | TI-Norm: ({tt.normalizedTime}) | From-Hash: ({m_AnimationHash[layer]}) | SI-FPHash: ({st.fullPathHash}) | SI-Norm: ({st.normalizedTime})"); | |||
| } | |||
| else if (!tt.anyState && tt.fullPathHash != m_TransitionHash[layer]) | |||
| else if (!tt.anyState && tt.fullPathHash != m_TransitionHash[layer] && (!m_DestinationStateToTransitioninfo.ContainsKey(layer) || | |||
| (m_DestinationStateToTransitioninfo.ContainsKey(layer) && m_DestinationStateToTransitioninfo[layer].ContainsKey(nt.fullPathHash)))) | |||
There was a problem hiding this comment.
Nitpick: I wonder if we can split the condition into simpler checks, which makes it easier to understand.
else if (!tt.anyState && tt.fullPathHash != m_TransitionHash[layer])
{
bool containsLayer = m_DestinationStateToTransitioninfo.ContainsKey(layer);
bool containsFullPathHash = containsLayer && m_DestinationStateToTransitioninfo[layer].ContainsKey(nt.fullPathHash);
if (!containsLayer || containsFullPathHash)
{
// first time in this transition for this layer
m_TransitionHash[layer] = tt.fullPathHash;
}
There was a problem hiding this comment.
Yeah... it would really be nice to have access to some of the Animator stuff that is available in the editor at runtime...but it gets boiled down to these condensed states.
Although, I think you made me think more about this and that it probably needs some comments...
👍
There was a problem hiding this comment.
Added some additional comments about that crazy logic... but basically it is avoiding from sending a animation transition that was a trigger to state or trigger from state back to Idle(state) transition which is already sent when one of the two triggers was already set (and sent via RPC).
fluong6
approved these changes
Aug 6, 2024
NoelStephensUnity
deleted the
fix/dual-triggers-generating-false-state-transition
branch
NoelStephensUnity
added a commit
that referenced
this pull request
Aug 13, 2024
NoelStephensUnity
added a commit
that referenced
this pull request
Aug 13, 2024
* fix This is an up-port of the #2999 NetworkAnimator fix. * test The ci runtime and editor manual tests for this fix. * update Adding changelog entry.
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.
This fixes the issue where a trigger that transitions to a state with a trigger that transitions back to the original state could cause NetworkAnimator to generate a false layer to layer state transition which would not only generate an additional message after the original trigger message, but would cause the non-authority instances to log a warning message.
MTTB-48
fix: #2828
Changelog
NetworkAnimatorinstances and cause a warning message to be logged.Testing and Documentation