fix: correct SDK version field in current-scene debug panel#9385
fix: correct SDK version field in current-scene debug panel#9385decentraland-bot wants to merge 8 commits into
Conversation
The "SDK 6:" marker reflected whether the player was standing on any scene (True/False), not the scene's actual runtime version. It now reports "6" or "7" via ISceneData.IsSdk7(), which already parses scene.json's runtimeVersion.
decentraland-bot
added
the
ext-contribution
|
Build failed! Check the logs to see what went wrong. |
|
Slack notification sent to #explorer-ext-contributions for external review. |
There was a problem hiding this comment.
PR Review — #9385: fix: correct SDK version field in current-scene debug panel
STEP 2 — Root-cause check ✅
The debug panel's "SDK 6:" row was wired to show bool.FalseString / bool.TrueString based on whether any scene was active — it never reported the actual SDK version. This PR fixes the root cause by querying ISceneData.IsSdk7() (the canonical API, used throughout the codebase) and displaying the real version. PASS.
STEP 3 — Design & integration ✅
No new units, state, or lifecycle management introduced. The change lives entirely within UpdateCurrentSceneSystem, which already owns all current-scene debug panel bindings. The ElementBinding<string> is a 1:1 rename from sdk6Binding → sdkVersionBinding, following the identical pattern of its sibling bindings (sceneNameBinding, sceneParcelsBinding, etc.). PASS.
STEP 4 — Member audit ✅
sdkVersionBinding is a private readonly field. Consumers:
- Constructor — initialization and widget registration
RefreshSceneDebugInfo()— value update
No new public members introduced. No single-use accessors, no absent-≠-false predicates. No issues.
STEP 5 — Line-level review ✅
A. Blocking-issue scan: None found.
- Null safety:
currentActiveSceneis null-checked before.SceneData.IsSdk7()access. ✅ - Allocations: String literals
"7"and"6"are interned;NO_DATA_STRINGis an existingconst. No allocations in the update path. ✅ - Naming:
sdkVersionBindingfollows camelCase for private fields per CLAUDE.md. ✅ - Subscriptions/teardown: No new subscriptions, events, or disposables added. ✅
- Nullability: No nullability contract changes. ✅
B. Design/encapsulation smells: None found.
- No magic values —
NO_DATA_STRINGis a named constant;"7"and"6"are the literal SDK version identifiers being displayed. - No naming issues — the rename from
sdk6BindingtosdkVersionBindingcorrectly generalizes the field name.
STEP 6 — Complexity: SIMPLE
Single file, 4 additions / 4 deletions. Renames a field and fixes a value computation in debug-only UI. No ECS structural changes, no async, no new dependencies.
STEP 7 — QA: YES
Modifies runtime code that affects what the user sees in the debug panel. Test plan in the PR description is appropriate.
STEP 8 — Warnings
None. Main scene not modified.
Security review ✅
No secrets, injection vectors, auth changes, or sensitive data exposure. The field displays non-sensitive, publicly-known scene metadata (SDK version 6 or 7) in a debug-only panel.
Sub-agent reviews
| Agent | Verdict |
|---|---|
| Security Sentinel | Clean — no findings |
| Architecture Strategist | Clean — correctly scoped |
| Pattern Recognition | Clean — canonical API, proper naming |
| Code Simplicity | Clean — appropriately minimal |
REVIEW_RESULT: PASS ✅
COMPLEXITY: SIMPLE
COMPLEXITY_REASON: Renames a debug-panel binding and fixes its display value in a single existing system — no ECS, async, or architectural changes
QA_REQUIRED: YES
Reviewed by Jarvis 🤖 · Requested by Mikhail Agapov (<@U04A7TYN13L>) via Slack
Both UpdateCurrentSceneSystem and DebugViewCurrentSceneSystem share the same CURRENT_SCENE widget via GetOrAddWidget, but both called SetVisibilityBinding with their own instance. Only the last binding set gets Connect()-ed at build time, leaving UpdateCurrentSceneSystem's binding permanently disconnected (IsConnectedAndExpanded = false). This prevented RefreshSceneDebugInfo from ever running. Remove the redundant visibility binding from UpdateCurrentSceneSystem and let DebugViewCurrentSceneSystem (PR #8663) own it.
SDK6 scenes always resolve to SHOWING_LOD in VisualSceneStateResolver and never get a SceneFacade, so currentActiveScene can only be SDK7. Remove the unreachable IsSdk7() branch.
|
Warnings count reduced: 19087 => 14692 Warnings/errors in files changed by this PR (1)
|
Signed-off-by: Juan Ignacio Molteni <juanignaciomolteni@gmail.com>
DafGreco
left a comment
DafGreco
left a comment
There was a problem hiding this comment.
✔️ 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:
- [ ✔️] Log In/Log Out
- [ ✔️] Backpack and wearables in world
- [ ✔️] Emotes in world and in backpack
- [✔️ ] Teleport with map/coordinates/Jump In
- [ ✔️] Chat and multiplayer
- [✔️ ] Profile card
Evidence:
Working properly on both platforms. SDK6 scenes won't have any data logged but SDK7 scenes will have information
4 participants
Summary
All scene info fields stuck empty — when
DebugViewCurrentSceneSystem(PR #8663) was added, it also calledSetVisibilityBinding()on the sharedCURRENT_SCENEwidget. SinceDebugWidgetBuilderonly keeps the last binding,UpdateCurrentSceneSystem's binding never gotConnect()-ed, soIsConnectedAndExpandedwas alwaysfalseandRefreshSceneDebugInfo()never ran. Removed the redundant visibility binding;DebugViewCurrentSceneSystemnow owns it.Test plan
truefalse<No data>when not standing on any sceneDebugViewCurrentSceneSystemstill work