Skip to content

fix: prevent loading stale profiles on realm change - #8834

Merged
lorenzo-ranciaffi merged 4 commits into
devfrom
fix/8827-remove-stale-profiles
May 21, 2026
Merged

fix: prevent loading stale profiles on realm change#8834
lorenzo-ranciaffi merged 4 commits into
devfrom
fix/8827-remove-stale-profiles

Conversation

@lorenzo-ranciaffi

@lorenzo-ranciaffi lorenzo-ranciaffi commented May 20, 2026

Copy link
Copy Markdown
Contributor

Pull Request Description

Fixes #8827

What does this PR change?

This PR makes sure that on realm change, profiles that are being downloaded for the previous realm, are cancelled (RemoteProfiles.TryDownloadAsync is a fire & forget) so that when they are completed, they don't get included into remoteProfiles leading to "ghost avatars".

Test Steps

  1. Follow repro steps of the linked issue
  2. Smoke test, everything should operate as normal (especially in terms of remote avatars across teleports between worlds and Genesis City)

Quality Checklist

  • Changes have been tested locally
  • Documentation has been updated (if required)
  • Performance impact has been considered
  • For SDK features: Test scene is included

Code Review Reference

Please review our Branch & PR Standards before submitting. It explains the automated review flow, QA/DEV approval requirements, and what each label does — especially useful for first-time contributors.

@github-actions

github-actions Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

@lorenzo-ranciaffi

Copy link
Copy Markdown
Contributor Author

@claude review

@claude

This comment has been minimized.

@lorenzo-ranciaffi
lorenzo-ranciaffi marked this pull request as ready for review May 20, 2026 15:28
@lorenzo-ranciaffi
lorenzo-ranciaffi requested review from a team as code owners May 20, 2026 15:28
@github-actions
github-actions Bot requested a review from anicalbano May 20, 2026 15:29
@claude

This comment has been minimized.

@claude

claude Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Claude finished @lorenzo-ranciaffi's task in 3m 22s —— View job


PR Review

  • Read CLAUDE.md and docs/README.md
  • Read changed files and diff
  • Analyze and post review

Root-cause assessment: PASS ✅

This PR fixes the root cause. Fire-and-forget TryDownloadAsync calls issued for the previous realm completed after realm change and injected stale RemoteProfile entries into the shared list, creating ghost avatars. The fix correctly:

  1. Lifts RemoteAnnouncements and RemoteProfiles instantiation out of MultiplayerPlugin.InjectToWorld() so they can be shared with the teleport pipeline.
  2. Adds Reset() to both, cancelling all in-flight CTSes and clearing buffered state.
  3. Calls Reset() synchronously during RemoveRemoteEntitiesTeleportOperation, before ForceRemoveAll, so no stale data survives into the new realm.
  4. Adds the cts.IsCancellationRequested || !pendingProfiles.TryGetValue(...) guard in TryDownloadAsync so async continuations resuming after Reset() exit safely without writing to remoteProfiles.

All previously flagged issues (multi-line comment blocks, grammar error) have been resolved ✅.


Blocking issues

None.


Implementation notes (non-blocking)

remoteMetadata move in DynamicWorldContainer: The instantiation was correctly moved earlier (line ~448) to satisfy the new dependency ordering. All transitive dependencies (roomHub at line 435, messagePipesHub at line 446, profilesRepository at line 362) are available at that point.

finally-block / new-realm race (theoretical): After Reset() clears pendingProfiles, if a new-realm download is initiated for the same walletId with the same profileVersion before an old download's finally block runs, the old finally could remove the new entry from pendingProfiles. The new download's try guard (!pendingProfiles.TryGetValue(...)) would then cause it to return early. The avatar would simply be re-requested on the next announcement cycle. Probability is very low given the disconnect-reconnect timing between realms, and the impact is one missed frame of avatar loading — not a ghost avatar. Not worth addressing here.

Bunch<T> lifecycle: Bunch<T> wraps the underlying remoteProfiles list by reference and clears it on Dispose(). Calling Reset() mid-frame discards any buffered profiles for that frame, which is the desired behaviour during realm transition. All access is single-threaded (main thread), so no concurrency concern.


REVIEW_RESULT: PASS ✅
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Modifies async fire-and-forget UniTaskVoid cancellation flow, CancellationTokenSource lifecycle, and plugin/container DI wiring across 6 files.
QA_REQUIRED: YES

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Claude reviewed this PR and found no blocking issues, but assessed it as complex — human DEV review is still required before merging.

@lorenzo-ranciaffi lorenzo-ranciaffi moved this from In Progress to With QA / Awaiting Review in Explorer Alpha May 20, 2026
@m3taphysics

Copy link
Copy Markdown
Contributor

PR #8834, run #26175361330

Builds: Windows change, Windows baseline, macOS change, macOS baseline

Framework 13 i7

Metric Change Baseline Delta Improvement
Samples 2700 2699
CPU average 33.3 ms 33.3 ms -0.0 ms 0.0%
CPU 1% worst 33.4 ms 34.0 ms -0.6 ms 1.6%
CPU 0.1% worst 33.6 ms 37.6 ms -4.0 ms 10.6% 🟢
GPU average 7.5 ms 7.7 ms -0.1 ms 1.8%
GPU 1% worst 19.4 ms 19.6 ms -0.2 ms 0.9%
GPU 0.1% worst 25.5 ms 23.7 ms 1.8 ms -7.7% 🔴

@github-project-automation github-project-automation Bot moved this from With QA / Awaiting Review to In Progress in Explorer Alpha May 21, 2026

@DafGreco DafGreco left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✔️ PR reviewed and approved by QA on both platforms following instructions playing both happy and un-happy path

Regressions for this ticket had been performed in order to verify that the normal flow is working as expected:

  • [ ✔️] Backpack and wearables in world
  • [ ✔️] Emotes in world and in backpack
  • [ ✔️] Teleport with map/coordinates/Jump In
  • [✔️ ] Chat and multiplayer
  • [ ✔️] Profile card
  • [ ✔️] Camera
  • [ ✔️] Skybox
  • [ ✔️] Settings

Evidence:

thread with evidence

Player (23).log

Image

@lorenzo-ranciaffi
lorenzo-ranciaffi merged commit f6af5c8 into dev May 21, 2026
16 of 20 checks passed
@lorenzo-ranciaffi
lorenzo-ranciaffi deleted the fix/8827-remove-stale-profiles branch May 21, 2026 13:01
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Explorer Alpha May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[QA] Explorer | Ghost avatars from Genesis Plaza appear in World after first teleport of the session

5 participants