Skip to content
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

fix: Performance improvements for triggering emotes #5987

Merged
merged 4 commits into from
Nov 30, 2023

Conversation

Kinerius
Copy link
Contributor

@Kinerius Kinerius commented Nov 29, 2023

What does this PR change?

  • Fixed an issue that caused other avatars to trigger emotes to the current player
  • Restored emote loop replication functionality
  • Reduced allocations caused when AvatarModel changes, which is produced after any emote being triggered:

Before:
image

After:
image

The remaining 380 B comes from an intended data copy which its intent is not documented.

How to test the changes?

  • Check that looping emotes are still working when spawning near a player that already triggered that emote
  • Check that triggering an emote is visible in other clients
  • Check that avatar pointer events work correctly (hover and click)
  • Check that avatars are updated correctly after changing a wearable
  • ⚠️ IMPORTANT ⚠️ Make sure you, other guests, or users using THIS branch, do not see yourselves doing random emotes, all other users that are not using this branch may be seen using some weird emotes making them float.

Our Code Review Standards

https://github.com/decentraland/unity-renderer/blob/master/docs/code-review-standards.md

Copilot summary

🤖[deprecated] Generated by Copilot at 8fa6e9c

The pull request improves the avatar system by adding and using a baseAvatarReferences component that stores the base avatar data, and by simplifying and optimizing the code for the avatar anchors, expressions, and emotes. It also removes unused or redundant code and parameters, and follows the coding style conventions. The changes affect several files, such as AvatarShape.cs, AvatarAnchorPoints.cs, AvatarAnimatorLegacy.cs, and AvatarSceneEmoteHandler.cs, as well as the AvatarShape.prefab asset and the AvatarSceneEmoteHandlerShould.cs test file.

Enabled back the functionality of emote loop replication
Fixed allocations on avatar anchors setup
@Kinerius Kinerius self-assigned this Nov 29, 2023
@Kinerius Kinerius marked this pull request as ready for review November 29, 2023 20:56
@Kinerius Kinerius requested a review from a team as a code owner November 29, 2023 20:56
Copy link
Contributor

@anicalbano anicalbano left a comment

Choose a reason for hiding this comment

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

The PR has been reviewed by QA and it's working as expected! 😄

30.11.2023_09.38.45_REC.mp4
30.11.2023_09.30.43_REC.mp4

Copy link
Member

@pravusjif pravusjif left a comment

Choose a reason for hiding this comment

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

let's merge it!

@Kinerius Kinerius merged commit dc9841a into dev Nov 30, 2023
2 checks passed
@Kinerius Kinerius deleted the fix/emotes-performance branch November 30, 2023 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants