Skip to content

fix: Resolving Sync Issues and adding ResetTrigger support #1327

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 9 commits into from
Oct 22, 2021
Merged

fix: Resolving Sync Issues and adding ResetTrigger support #1327

merged 9 commits into from
Oct 22, 2021

Conversation

andrews-unity
Copy link
Contributor

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

This PR allows user to properly Reset a trigger, this also fixes an issue with an overflow read issue where values were not syncing when the value didn't change with full state changes.

PR Checklist

  • Have you added a backport label (if needed)? For example, the type:backport-release-* label. After you backport the PR, the label changes to stat:backported-release-*.
  • Have you updated the changelog? Each package has a CHANGELOG.md file.
  • Have you updated or added the documentation for your PR? When you add a new feature, change a property name, or change the behavior of a feature, it's best practice to include related documentation changes in the same PR or a link to the documentation repo PR if this is a manual update.

Changelog

com.unity.netcode.gameobjects

  • Added: ResetTrigger function to NetworkAnimator
  • Fixed: Overflow exception when syncing Animator state.

Testing and Documentation

  • No tests have been added.

@andrews-unity andrews-unity changed the title Fix/add resettrigger animator fix: Resolving Sync Issues and adding ResetTrigger support Oct 20, 2021
@andrews-unity andrews-unity marked this pull request as ready for review October 21, 2021 20:54
mattwalsh-unity
mattwalsh-unity reviewed Oct 21, 2021
View reviewed changes
com.unity.netcode.gameobjects/Components/NetworkAnimator.cs
@@ -308,39 +310,27 @@ private unsafe bool WriteParameters(FastBufferWriter writer, bool autoSend)
var valueInt = m_Animator.GetInteger(hash);
fixed (void* value = cacheValue.Value)
{
var oldValue = UnsafeUtility.AsRef<int>(value);
if (valueInt != oldValue)
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess because we can't trust the value hasn't changed because of the reset?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, I will revisit this for 1.1.0... I have potential solution but its a little more involved so this unblocks the issue where we get an overflow exception on the read side of things. Luckily by default we don't auto sync anything so in most cases we don't send a bunch of updates unless the user opts in for that, and we will always send a full update when we sync state which should be true but I would like to come back to this for 1.1.0

Copy link
Contributor

Choose a reason for hiding this comment

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

sg, can I suggest pasting what you just said in a comment around it?

@andrews-unity andrews-unity enabled auto-merge (squash) October 21, 2021 21:05
@andrews-unity
Copy link
Contributor Author

sg, can I suggest pasting what you just said in a comment around it?

Done!

@andrews-unity andrews-unity enabled auto-merge (squash) October 22, 2021 03:55
@andrews-unity andrews-unity merged commit 5fcb793 into develop Oct 22, 2021
@andrews-unity andrews-unity deleted the fix/add-resettrigger-animator branch October 22, 2021 05:27
andrews-unity added a commit that referenced this pull request Oct 22, 2021
@andrews-unity
fix: Resolving Sync Issues and adding ResetTrigger support (#1327)
c5db1e3 
* fix: Adding the ability to call ResetTrigger for a NetworkAnimator.
* chore: fix out of sync values when no value changes where happening.
@andrews-unity andrews-unity mentioned this pull request Oct 22, 2021
Merged
3 tasks
andrews-unity added a commit that referenced this pull request Oct 23, 2021
@andrews-unity
fix: Resolving Sync Issues and adding ResetTrigger support (#1327) (#…
1dc2e5a 
…1344)

* fix: Adding the ability to call ResetTrigger for a NetworkAnimator.
* chore: fix out of sync values when no value changes where happening.
@aazzolini aazzolini mentioned this pull request Oct 23, 2021
Closed
@aazzolini aazzolini mentioned this pull request Oct 28, 2021
Closed
mollstam pushed a commit to Keepsake-Games/com.unity.netcode.gameobjects that referenced this pull request Feb 13, 2023
@andrews-unity
fix: Resolving Sync Issues and adding ResetTrigger support (Unity-Tec…
769d329 
…hnologies#1327)

* fix: Adding the ability to call ResetTrigger for a NetworkAnimator.
* chore: fix out of sync values when no value changes where happening.
mollstam pushed a commit to Keepsake-Games/com.unity.netcode.gameobjects that referenced this pull request Feb 13, 2023
@andrews-unity
fix: Resolving Sync Issues and adding ResetTrigger support (Unity-Tec…
820062a 
…hnologies#1327) (Unity-Technologies#1344)

* fix: Adding the ability to call ResetTrigger for a NetworkAnimator.
* chore: fix out of sync values when no value changes where happening.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

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

Successfully merging this pull request may close these issues.
2 participants
@andrews-unity @mattwalsh-unity