fix: bugsweep week 2026-W36 (fable redo) - #10083
Draft
decentraland-bot wants to merge 6 commits into
Draft
decentraland-bot wants to merge 6 commits into
decentraland-bot wants to merge 6 commits into
Conversation
Issue: #9916 Title: System.NullReferenceException: Object reference not set to an instance of an object. Sentry: UNITY-EXPLORER-PW7 (https://decentraland.sentry.io/issues/7701581691/) Repro / context: startup blocklist check with the ReportUser feature flag enabled. Crash at BlocklistCheckStartupOperation.cs:30 reading `banStatusData.isBanned`. Root cause: ApplicationBlocklistGuard.IsUserBlocklistedAsync declares a non-nullable contract (UniTask<GetBanStatusData>) and three of its four return sites honor it by returning `new GetBanStatusData { isBanned = false }`. The fourth return blindly forwarded `result.Value.data`, a schema-optional wire field: when the moderation endpoint returns a successful response whose body omits `data` (missing key / "data": null / empty envelope), Newtonsoft leaves `data` null while `result.Success` is true, so the error guard does not fire. The null propagates out and both consumers (BlocklistCheckStartupOperation:30, BannedNotificationHandler:67) dereference `.isBanned` on their non-null-typed value and crash. Fix: normalize the nullable wire field at the single boundary that owns the non-nullable contract, mirroring the method's three existing fallback returns, so callers can safely trust the annotation. Guard is at the publisher, not relocated into the two subscribers, and does not add a defensive check against the (proven non-null) `result.Value`.
Issue: #10001 Title: DCL.Diagnostics.EcsSystemException: [MainCameraSystem] Sentry: UNITY-EXPLORER-Q04 (https://decentraland.sentry.io/issues/7715252408/) Repro / context: NullReferenceException wrapped as EcsSystemException from MainCameraSystem's HandleActiveVirtualCameraLookAtChange query, on scenes carrying a virtual camera. Root cause: the query dereferenced cameraData.CinemachineBrain!.ActiveVirtualCamera.VirtualCameraGameObject unconditionally. CinemachineBrain.ActiveVirtualCamera is a transient, genuinely nullable Cinemachine runtime property: it is null whenever the brain has no live vcam (during scene load before the brain's first resolve, during blends/transitions, or a frame where all vcams are momentarily disabled). The query runs for every PBVirtualCamera entity each throttled update, so a null ActiveVirtualCamera dereferences into an NRE. Fix: honor the property's nullability by reading it into a local and returning early when it is null. This is nullability-honest (Cinemachine owns the publisher; there is no scene-side value to guard) and mirrors the existing in-repo idiom in BillboardSystem, which already null-guards CinemachineBrain?.ActiveVirtualCamera before touching VirtualCameraGameObject. Semantics are preserved: with no active vcam this entity is not the active one, so the intended skip is exactly correct. The other operands on the line (virtualCameraInstance, CinemachineBrain) remain non-null per their contracts and are left untouched.
Issue: #10007 Title: System.InvalidOperationException: The current SynchronizationContext may not be used as a TaskScheduler. Sentry: UNITY-EXPLORER-Q0D (https://decentraland.sentry.io/issues/7715704900/) Repro / context: exception thrown from ENetTransport.DisconnectAsync, reached via the handshake-failure path (PulseMultiplayerService.ConnectInternalAsync -> HandshakeAsync -> PulseMultiplayerService.DisconnectAsync -> ENetTransport.DisconnectAsync). Root cause: the public DisconnectAsync converts its inner Task with `.AsUniTask()`, whose default `useCurrentSynchronizationContext: true` eagerly calls TaskScheduler.FromCurrentSynchronizationContext() on the calling thread. The ENet routing loop runs on the thread pool (DCLTask.RunOnThreadPool(configureAwait: false)), and the handshake completion resumes DisconnectAsync on that context-less thread-pool thread, where SynchronizationContext.Current is null and FromCurrentSynchronizationContext() throws. Fix: pass `useCurrentSynchronizationContext: false` so the disconnect continuation is not marshalled back to a sync context it does not have. This mirrors the codebase's own documented remedy for the identical hazard in DCLWebSocket.cs. Disconnect has no main-thread requirement and the ENet transport is single-thread-pool by design, so no behavior changes beyond removing the invalid marshalling.
…9937) Issues (same root cause, one fix): - #9922 — System.NullReferenceException (RestrictedJsApiPermissionsProvider .ctor) — Sentry UNITY-EXPLORER-PWF (https://decentraland.sentry.io/issues/7702775015/) - #9937 — System.NullReferenceException (SmartWearableCache) — Sentry UNITY-EXPLORER-PWK (https://decentraland.sentry.io/issues/7703252679/) Repro / context: loading a smart wearable whose scene.json omits the optional "requiredPermissions" key. Two distinct crash sites, one cause: - #9922: LoadSmartWearableSceneSystem passes the field into new RestrictedJsApiPermissionsProvider(...), whose ctor does `foreach (string permission in permissions)` and dereferences null. - #9937: SmartWearableCache.BuildCacheItemAsync does `permissions.Contains(...)` on the same field after parsing metadata. Root cause: SceneMetadata.requiredPermissions is a schema-optional wire field declared as a non-nullable List<string>. When scene.json omits the key, Newtonsoft leaves it null, so the non-null declaration lies and every unguarded consumer crashes. The canonical reader SceneData.HasRequiredPermission already null-guards it, proving the field is genuinely nullable at runtime. Fix: guarantee the field non-null at the publisher (the DTO) by defaulting it to an empty list. Newtonsoft keeps the initializer when the key is absent and populates it when present, so the non-null declaration becomes honest and all consumers are fixed at once without scattering null-checks into each subscriber. Behaviour is unchanged: an empty permission set denies every restricted JS API (RestrictedJsApiPermissionsProvider.CanInvoke* all return false) and HasRequiredPermission returns false, matching the prior null-guarded semantics. Verified all four readers (SmartWearableCache, LoadSmartWearableSceneSystem, SmartWearableAuthorizationPopupController, SceneData) treat an empty list identically to the previous null.
#9953) Issue: #9953 Title: DCL.Diagnostics.EcsSystemException: [LightSourceApplyPropertiesSystem] Sentry: UNITY-EXPLORER-PTZ (https://decentraland.sentry.io/issues/7696501391/) Repro / context: NullReferenceException wrapped as EcsSystemException from LightSourceApplyPropertiesSystem while a light-source entity is being torn down. Root cause: the UpdateLightSource and ResolveTexturePromise queries match entities on LightSourceComponent (+PBLightSource) without filtering [None(typeof(DeleteEntityIntention))], violating the project ECS rule "always filter out DeleteEntityIntention". The publisher LightSourceLifecycleSystem.ReleaseDestroyedLightSource (runs [UpdateBefore] via [All(DeleteEntityIntention)]) returns the pooled Light to its pool but does not remove LightSourceComponent, and DeleteEntityIntention can defer actual destruction across frames (DeferDeletion). So the entity keeps flowing through UpdateLightSource, which dereferences the released/ destroyed Light (lightSourceInstance.enabled = ...); under IL2CPP a destroyed UnityEngine.Object dereference surfaces as NullReferenceException. Fix: add [None(typeof(DeleteEntityIntention))] to both queries so an entity marked for deletion is skipped once its Light has been released, mirroring the established idiom in sibling systems (e.g. PropagateAvatarLocomotionOverridesSystem, UpdateMediaPlayerSystem). This is a structural ECS filter at the correct layer, not a subscriber null-check.
Issue: #9967 Title: Emojis not visible in chat bubble - missing emoji font asset Repro: send a chat message containing an emoji with chat bubbles enabled; the emoji renders in the chat window but is broken/invisible in the over-avatar bubble, and Player.log repeats "No suitable font asset found for emoji support in the provided text." Reproducible 10/10 on Windows and Mac (v0.175.0-alpha-main). Root cause: chat bubbles and the chat window are two different text pipelines. The window is TextMeshPro and gets its emoji fallback fonts at runtime (FallbackFontsProvider -> TMP_Settings). The bubble is a UI Toolkit Label on NametagElement, whose panel (DCLNametagsPanelSettings) is the only panel overriding textSettings, pointing at DCLTextSettings (a PanelTextSettings). That asset already wires the UITK emoji sprite atlas (emojis32_uitk in m_EmojiFallbackTextAssets and m_DefaultSpriteAsset) but has the master toggle m_EnableEmojiSupport set to 0, so TextCore never substitutes emoji glyphs and raw codepoints render as missing. Fix: flip m_EnableEmojiSupport from 0 to 1 in DCLTextSettings.asset (a text-serialized YAML ScriptableObject; a one-field change). The atlas wiring already present becomes effective. DCLTextSettings is referenced only by DCLNametagsPanelSettings, so the change is scoped to the nametag/ chat-bubble panel. Needs a QA visual check: emoji in nearby chat renders in the over-avatar bubble.
Contributor
🚦 CI StatusWindows and Mac built successfully in Unity Cloud.
Warnings not reduced: 12025 => 12027 — remove at least 3 warnings to merge. Warnings/errors in files changed by this PR (43)Lint run · took 26m 36s All Unity tests passed ✅
Tests time sums the test cases; Job time is the job's wall clock including checkout, licensing and asset import. Slowest tests
Full report: run summary · results + editor logs: editmode · playmode 🏁 Bare-metal benchmark finished — run #34791230804. Full reportPR #10083, run #34791230804 Overall: ✅ no significant changes Builds: Windows change, Windows baseline, macOS change, macOS baseline How to read this table
Apple M1
Intel Core i5
On demand — comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Description
What does this PR change?
Fable redo of the W36 bug sweep — an alternative to draft PR #10080, requested by the sweep owner to compare model outputs; the batch (issues opened Aug 31 – Sep 6, 2026) is identical. Merge exactly one of the two PRs and close the other: they change overlapping files, and this one is a superset (it adds the #9967 fix).
Each fix is its own commit whose body carries the issue link, Sentry link, repro and root-cause analysis. This PR is a draft: the red Lint job is intentionally left for the team's manual lint pass; full human DEV review and QA remain required before it is marked ready and merged.
Fixed (7 issues)
b8fdb462d)5db93f0fc)67329bfd4)bb7df9365)acfddecbb)0559f43d5)Not worked on (28 issues)
These were collected by the sweep but not fixed; each is reported with its reason (never silently dropped):
3 further issues from the week were out of scope by assignee (assigned to non-sweep owners) and are not listed here.
Test Instructions
Per-fix validation is via the CI gate (Unity build + tests, forced with the
force-buildlabel on this draft) plus human DEV review and QA. Each fix commit body documents the specific crash/repro it addresses. The #9967 fix (chat-bubble emoji) needs a QA visual check: send an emoji in nearby chat with bubbles enabled and confirm it renders over the avatar.Quality Checklist
Code Review Reference
Automated review flow, QA/DEV approval requirements, and label meanings: see the repo Branch & PR Standards.
Requested by Alejandro Jimenez via Slack (redo on Fable; branch postfixed
-fableper request).