fix: masked emote looping#9081
Conversation
|
Windows and Mac build successful in Unity Cloud! You can find a link to the downloadable artifact below. |
This comment has been minimized.
This comment has been minimized.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| if (shouldPlay && masked.CurrentEmoteReference == null) | ||
| ReplayMaskedEmote(ref masked, ec); | ||
| { | ||
| if (emoteStorage.TryGetElement(masked.EmoteUrn.Shorten(), out IEmote emote) && emote.IsLooping()) | ||
| ReplayMaskedEmote(ref masked, ec); | ||
| } |
There was a problem hiding this comment.
The IsLooping() guard here triggers a second emoteStorage.TryGetElement call — ReplayMaskedEmote already does its own TryGetElement on line 216. Moving the looping check inside ReplayMaskedEmote would eliminate the redundant lookup and co-locate all replay preconditions in one place (alongside IsLoading, body-shape checks, etc.).
| if (shouldPlay && masked.CurrentEmoteReference == null) | |
| ReplayMaskedEmote(ref masked, ec); | |
| { | |
| if (emoteStorage.TryGetElement(masked.EmoteUrn.Shorten(), out IEmote emote) && emote.IsLooping()) | |
| ReplayMaskedEmote(ref masked, ec); | |
| } | |
| if (shouldPlay && masked.CurrentEmoteReference == null) | |
| ReplayMaskedEmote(ref masked, ec); |
And in ReplayMaskedEmote, add the looping guard right after the storage lookup:
private void ReplayMaskedEmote(ref CharacterMaskedEmoteComponent masked, CharacterEmoteComponent emoteComponent)
{
if (!emoteStorage.TryGetElement(masked.EmoteUrn.Shorten(), out IEmote emote)) return;
if (!emote.IsLooping()) return; // <-- add here
if (emote.IsLoading) return;
...This keeps UpdateMaskedEmoteVisibility simple and ReplayMaskedEmote self-contained — consistent with how ConsumeMaskedEmoteIntent is structured.
|
🔍 Claude reviewed this PR and found no blocking issues, but assessed it as complex — human DEV review is still required before merging. |
|
Claude finished @nicoecheza's task in 1m 32s —— View job PR Review: fix/masked-emote-loop
Step 2 — Root-cause checkProblem: Non-looping scene-triggered masked emotes were being replayed when the player re-entered the scene or when a full-body emote / glide ended. Fix: This update (the Step 3 — Design & integrationThe guard placement is now correct. Step 5 — Line-level reviewNon-blocking —
Because the guard at line 216 already returned if Simplest fix — replace the redundant call with a const bool isLooping = true;Or inline Step 6 — ComplexityCOMPLEX — modifies a query method within an ECS Step 7 — QA assessmentQA_REQUIRED: YES — directly affects user-visible emote playback (whether a one-shot masked emote incorrectly replays on scene re-entry or after a full-body emote ends). REVIEW_RESULT: PASS ✅ 💰 Review cost: $0.10 · 💰 Review cost: $0.26 · |
| if (!emoteStorage.TryGetElement(masked.EmoteUrn.Shorten(), out IEmote emote)) return; | ||
| if (emote.IsLoading) return; | ||
|
|
||
| if (!emote.IsLooping()) return; |
There was a problem hiding this comment.
The masked emote system is supposed to keep playing it even if the avatar plays another full body emote. The loop state its already considered here:
So with this change, i guess that if the masked emote is not a loop emote, it will not end playing the animation if another action is made, either emote or glide
There was a problem hiding this comment.
If the attempt is to fix the non-stopping masked emotes from this scene, then i think the problem is in how it tries to stop the emote. Its not using stopEmote({}) as its used here: https://github.com/decentraland/sdk7-test-scenes/pull/59/changes#diff-8a277014b5464a07850d78daffc446f7e7bacbf56390348825a59f6901e7b39eR4
2 participants
No description provided.