Skip to content

fix: NetworkAnimator destinationStateMachine exception [MTT-5083] #2309

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 29 commits into from
Jan 12, 2023
Merged

fix: NetworkAnimator destinationStateMachine exception [MTT-5083] #2309

merged 29 commits into from
Jan 12, 2023

Conversation

NoelStephensUnity
Copy link
Collaborator

@NoelStephensUnity NoelStephensUnity commented Nov 14, 2022 •
edited
Loading

This resolves the issue where NetworkAnimator was not checking for AnimatorStateTtansition.destinationStateMachine, which was causing an exception, and adding any possible states with triggers defined within it. This also prevents the server from sending itself messages about animation state changing (#2305).

MTT-5083
MTT-5122

GitHub Issues Resolved:
#2305
#2293
#2374
#2366
#2368

Changelog

  • Fixed: NetworkAnimator issue where it was not checking for AnimatorStateTtansition.destinationStateMachine and any possible sub-states defined within it.
  • Fixed: NetworkAnimator issue where the host client was receiving the ClientRpc animation updates when the host was the owner.
  • Fixed: NetworkAnimator issue with using pooled objects and when specific properties are cleaned during despawn and destroy.
  • Fixed: issue where NetworkAnimator was checking for animation changes when the associated NetworkObject was not spawned.

Testing and Documentation

  • Includes integration test updates.
  • Includes manual test updates.

@NoelStephensUnity
fix
5500c6b 
This fixes the issue where NetworkAnimator was not taking into consideration `AnimatorStateTtansition.destinationStateMachine`.  Now, NetworkAnimator parses each destinationStateMachine's states for trigger transitions and includes those in the transition table.
@NoelStephensUnity
fix
353893b 
passing setTrigger bool to internal method.
@NoelStephensUnity
test manual
c9ff07c 
Manual test that validates this fix.
@NoelStephensUnity
fix
870be01 
Only send animation updates to clients and not host client when animation state changes.
@NoelStephensUnity
test
fb975dc 
MTT-5083
Updating test to validate states defined within a destinationStateMachine are triggering.
@NoelStephensUnity
update
2a49e2b 
MTT-5083
Adding entry for this fix.
@NoelStephensUnity
update
b379dd2 
MTT-5083
Adding the PR number to the changelog entry.
@NoelStephensUnity NoelStephensUnity marked this pull request as ready for review November 14, 2022 20:45
@NoelStephensUnity NoelStephensUnity requested a review from a team as a code owner November 14, 2022 20:45
jeffreyrainy
jeffreyrainy previously approved these changes Nov 15, 2022
View reviewed changes
Copy link
Contributor

@jeffreyrainy jeffreyrainy left a comment •
edited
Loading

Choose a reason for hiding this comment

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

.

jeffreyrainy
jeffreyrainy reviewed Nov 16, 2022
View reviewed changes
Copy link
Contributor

@jeffreyrainy jeffreyrainy left a comment

Choose a reason for hiding this comment

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

.

jeffreyrainy
jeffreyrainy reviewed Nov 16, 2022
View reviewed changes
Copy link
Contributor

@jeffreyrainy jeffreyrainy left a comment

Choose a reason for hiding this comment

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

Mistakenly approved wrong PR, please wait for actual review before acting on this approval.

@jeffreyrainy jeffreyrainy dismissed their stale review November 16, 2022 01:06

approval was by mistake 🤷

@NoelStephensUnity
Merge branch 'develop' into fix/networkanimator-destinationstatemachi…
7eb8316 
…ne-exception
@NoelStephensUnity
update and fix
4b6ed27 
This includes some updates to NetworkAnimator that will remove the need to synchronize client's using RPCs but instead use the new OnSynchronize method.

Fixed an issue with using pooled objects and when specific properties are cleaned during despawn and destroy.
@NoelStephensUnity
test
8810b31 
Made components visible to testproject again although I might try to figure out a better way to handle these two ways of testing.
Updated the NetworkAnimatorTests to acquire the state info from NetworkAnimator.SynchronizationStateInfo.
NoelStephensUnity and others added 9 commits November 20, 2022 13:40
@NoelStephensUnity
fix
07f8942 
Making the NGO Components AssemblyInfo adjustment only expose internals if NGO_COMPONENTS_ASSEMBLY_RUNTIME_VISIBILITY is defined.
@NoelStephensUnity
test
4e36786 
Removed enabling verbose debug in LateJoinSynchronizationTest
@NoelStephensUnity
fix
4314dd6 
Adjusting to original serialized format for backwards compatibility.
@NoelStephensUnity
Merge branch 'develop' into fix/networkanimator-destinationstatemachi…
7f5d45b 
…ne-exception
@NoelStephensUnity
update
19bf656 
removed the AnimationSynchronizationData class as it really was not needed.
@NoelStephensUnity
Merge branch 'develop' into fix/networkanimator-destinationstatemachi…
ddd03f0 
…ne-exception
@NoelStephensUnity
Merge branch 'develop' into fix/networkanimator-destinationstatemachi…
442ea04 
…ne-exception
@NoelStephensUnity
Merge branch 'develop' into fix/networkanimator-destinationstatemachi…
6bafef4 
…ne-exception
@NoelStephensUnity
fix
5ce321c 
This fixes the issue where NetworkAnimator will not check for animation changes unless the associated NetworkObject is spawned.
@NoelStephensUnity NoelStephensUnity enabled auto-merge (squash) December 12, 2022 17:14
lpmaurice
lpmaurice previously approved these changes Dec 14, 2022
View reviewed changes
@NoelStephensUnity NoelStephensUnity mentioned this pull request Jan 10, 2023
Closed
@NoelStephensUnity
update
3d764ec 
Removing the NGO_COMPONENTS_ASSEMBLY_RUNTIME_VISIBILITY from the scriptingDefineSymbols.
Replacing it with UNITY_INCLUDE_TESTS in the components AssemblyInfo.cs file.
Adding an additional remark about the context of when CheckForAnimatorChanges is invoked.
NoelStephensUnity and others added 3 commits January 10, 2023 11:01
@NoelStephensUnity
update
46bf588 
Migrating all test related assemblies within the UNITY_INCLUDE_TESTS define region.
@NoelStephensUnity
update
ba515d8 
Minor fix for weapon attack option in manual test.
@NoelStephensUnity
Merge branch 'develop' into fix/networkanimator-destinationstatemachi…
cd110e6 
…ne-exception
@NoelStephensUnity
update
bd5b81c 
updating what was fixed in this PR
@NoelStephensUnity NoelStephensUnity enabled auto-merge (squash) January 12, 2023 18:15
@NoelStephensUnity NoelStephensUnity merged commit 4cd5092 into develop Jan 12, 2023
@NoelStephensUnity NoelStephensUnity deleted the fix/networkanimator-destinationstatemachine-exception branch January 12, 2023 20:45
This was referenced Jan 12, 2023
Closed
Closed
Closed
@armstrongl armstrongl mentioned this pull request Jan 25, 2023
Closed
@NoelStephensUnity NoelStephensUnity mentioned this pull request Apr 6, 2023
Closed
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

@jeffreyrainy jeffreyrainy jeffreyrainy approved these changes

@lpmaurice lpmaurice lpmaurice left review comments

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
@NoelStephensUnity @lpmaurice @0xFA11 @jeffreyrainy @netcode-ci-service