fix: smart wearable crash - #9755
Conversation
🚦 CI StatusWindows and Mac build successful in Unity Cloud! You can find a link to the downloadable artifact below. Warnings count reduced: 13156 => 13155 Warnings/errors in files changed by this PR (3)All Unity tests passed ✅
|
decentraland-bot
left a comment
There was a problem hiding this comment.
Step 2 — Root-cause check
PASS. The crash (Sentry UNITY-EXPLORER-PM0, #9753) occurred because RunScenesForEquippedWearablesAsync called World.CachePlayer() which throws when the player entity does not exist yet. The player profile is not guaranteed to be resolved by the time LoadingStatus.LoadingStage.Completed fires. The fix introduces TryGetPlayerProfile to check availability before accessing the profile, and defers the start-up to the next completed-loading transition when the profile is absent. This addresses the root cause — the incorrect assumption that the profile was always available at loading completion.
Step 3 — Design & integration
PASS. No new long-lived units introduced. All changes are within the existing SmartWearableSystem — the system already owns the smart-wearable lifecycle (loading, unloading, authorization, scene-change handling) and these changes add proper guards and cancellation scoping to that lifecycle.
New members:
CachePlayerEntityOrNull(WorldExtensions.cs) — thin utility that complements the existingCachePlayer()method, using the existingGetSingleInstanceEntityOrNulloverload withstrict: false. Not a lifecycle manager; no ownership concerns.sessionCts,runScenesCts— CancellationTokenSources scoped to the session and per-scene-run respectively, alongside the pre-existingoutfitEquipCts. All three have distinct cancellation semantics and consolidating any pair would cause unintended cross-cancellation.
Teardown / consumption trace — all subscriptions and CTS accounted for:
| Opener | Teardown location |
|---|---|
EquipWearableEvent += (L104) |
OnDispose L114 |
UnEquipWearableEvent += (L105) |
OnDispose L115 |
EquipOutfitEvent += (L106) |
OnDispose L116 |
PortableExperienceUnloaded += (L107) |
OnDispose L117 |
CurrentStage.OnUpdate += (L108) |
OnDispose L118 · OnLoadingStatusChanged L412 · OnIdentityCleared L500 |
CurrentScene.OnUpdate += (L413) |
OnDispose L119 · OnIdentityCleared L499 |
OnIdentityCleared += (L109) |
OnDispose L120 |
outfitEquipCts field |
OnDispose L122 · OnIdentityCleared L484 |
sessionCts field |
OnDispose L123 · OnIdentityCleared L485 |
runScenesCts field |
OnDispose L124 · OnIdentityCleared L486 |
All openers have corresponding teardown. The defensive -= before += in OnIdentityCleared (L499-501) correctly prevents double-subscription when OnLoadingStatusChanged was never consumed (profile-unavailable case). This also fixes a pre-existing bug where the old code always did += without a preceding -=, risking double-subscription if the handler had not yet unsubscribed itself.
Step 4 — Member audit
| Member | Consumers | Verdict |
|---|---|---|
CachePlayerEntityOrNull (WorldExtensions) |
TryGetPlayerProfile (1) |
Single-use today but natural complement to CachePlayer() in a shared utility; legitimate for reuse |
TryGetPlayerProfile (private) |
HandleSceneChange (L377), OnLoadingStatusChanged (L405) |
2 consumers; well-scoped helper |
sessionCts (private field) |
OnEquipWearable, OnUnEquipWearable, OnDispose, OnIdentityCleared |
4 sites; distinct session-scoped lifetime |
runScenesCts (private field) |
HandleSceneChange, OnLoadingStatusChanged, OnDispose, OnIdentityCleared |
4 sites; distinct per-run lifetime |
No single-use-merge, absent≠false, re-derive, or redundant-guard issues.
Step 5 — Line-level review
No P0 or P1 issues found.
No P2 issues requiring suggestion blocks. Minor non-blocking observations:
-
Explicit
ct.IsCancellationRequestedchecks after awaits (e.g. L139, L474): Some are technically redundant given the outercatch (OperationCanceledException). However, the checks at L160 (guardingWorld.Addafter a thread hop) and L462 (guarding cache mutations after authorization) are genuinely justified — they prevent side effects that must not execute after session end. The pattern is applied consistently across the file and serves as documentation of cancellation intent. Not flagging. -
Deferred retry gap (L399-416): If the loading stage reaches
Completedbut the profile is absent, the handler stays subscribed and waits for the nextCompletedtransition. If the profile becomes available shortly after (but the stage stays atCompleted), smart wearables won't start until the next scene change triggersHandleSceneChange. This is strictly better than the pre-existing crash, and the window is very short (the profile is expected to arrive around the same time as loading completion). Worth noting for future hardening — subscribing to the profile's own availability signal (if one exists) would close this gap — but not blocking.
Security review: No security issues found. No secrets, no injection surfaces, no auth/authz changes, no sensitive data in logs.
Step 6 — Complexity
COMPLEX.
Step 7 — QA assessment
QA_REQUIRED: YES. Changes affect runtime smart-wearable loading/unloading — user-facing behavior on login, logout, and scene transitions.
Step 8 — Non-blocking warnings
None. Main scene not modified.
Step 9 — Verdict
REVIEW_RESULT: PASS ✅
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Modifies async/UniTask cancellation flows, CTS lifecycle management, and event subscription patterns in SmartWearableSystem
QA_REQUIRED: YES
Reviewed by Jarvis 🤖 · Requested by decentraland-bot via GitHub
|
🔍 Jarvis reviewed this PR and found no blocking issues, but assessed it as complex — human DEV review is still required before merging. |
|
PR #9755, run #31819170782 Builds: Windows change, Windows baseline, macOS change, macOS baseline How to read this table
Intel Core i5
Exception breakdown
Apple M1
|
|
PR reviewed and approved by QA on both platforms following the PR test instructions. ✅ Build: Test results:
Unrelated errors noted (do not affect verdict):
Verdict: PASS ✅ Player 9755.log 14.08.2026_15.12.55_REC.windows.9755.mp414.08.2026_15.33.28_REC.-.9755.mac.mp4 |
What does this PR change?
Fixes a crash on startup where the game could hard-crash right after loading finished. Closes #9753 (Sentry
UNITY-EXPLORER-PM0).The Smart Wearable system starts the scenes for your equipped smart wearables as soon as loading completes. It assumed the player profile was always ready by then, which isn't guaranteed — when it wasn't, the game crashed.
Changes:
Test Instructions
Expected result: The game loads into world without crashing, and equipped smart wearables start as usual.
Also worth checking: log out and back in, and walk across a few scene borders with a smart wearable equipped — no crashes, and the wearable's scene still loads.