fix: use the launched realm for local scene development scene room metadata - #9503
fix: use the launched realm for local scene development scene room metadata#9503gonpombo8 wants to merge 4 commits into
Conversation
…tadata The scene room metadata source read a hardcoded http://127.0.0.1:8000 instead of the realm the client was launched with, so it never reached a scene server started on any other port.
decentraland-bot
added
the
ext-contribution
|
Windows and Mac build successful in Unity Cloud! You can find a link to the downloadable artifact below. |
|
Warnings count reduced: 14054 => 14047 |
|
Slack notification sent to #explorer-ext-contributions for external review. |
|
All Unity tests passed ✅
|
decentraland-bot
left a comment
decentraland-bot
left a comment
There was a problem hiding this comment.
PR Review: fix: use the launched realm for local scene development scene room metadata
Step 2 — Root-cause check
PASS. LocalSceneDevelopmentSceneRoomMetaDataSource.MetaDataAsync hardcoded IRealmNavigator.LOCALHOST (http://127.0.0.1:8000) as the base URL, ignoring the actual realm from the deep link. When sdk-commands start --port 8003 (or any non-default port) is used, every metadata fetch cycle fails with Local scene server unreachable at http://127.0.0.1:8000. The fix threads the already-parsed realm from DynamicWorldParams.LocalSceneDevelopmentRealm through to the metadata source constructor, addressing the cause directly.
Step 3 — Design & integration
PASS. No new types or lifecycle management introduced. The change threads an existing, already-populated value through the established container pipeline:
Bootstraper.cs → DynamicWorldParams.LocalSceneDevelopmentRealm → DynamicWorldContainer → CommsContainer.Create → LocalSceneDevelopmentSceneRoomMetaDataSource
The data originates from RealmUrls.LocalSceneDevelopmentRealmAsync(), which reads the deep-link realm parameter already normalized by ApplicationParametersParser.
Other consumers checked:
LocalSceneDevelopmentPlayground.cs:49(editor-only demo) — correctly continues to use the no-arg constructor, falling back to the default localhost. This is deliberate for the editor playground.- No other call sites exist for
LocalSceneDevelopmentSceneRoomMetaDataSource.
The optional parameter (string? realm = null) with string.IsNullOrWhiteSpace fallback preserves full backward compatibility.
Teardown trace: No new subscriptions, events, connections, or disposables introduced. The realm field is a plain readonly string — no teardown needed.
Step 4 — Member audit
realmfield (private readonly string): consumed once inMetaDataAsync:34to constructURLDomain baseUrl. Single-use within the class is appropriate — it replaces a hardcoded constant. Not a derived predicate or single-use intermediate; it's a configuration value stored at construction time.- No new public properties or accessors added.
Step 5 — Line-level review
No blocking issues found across both passes.
Pass A (blocking categories):
- Code quality: naming follows conventions (
realmcamelCase private field,localSceneDevelopmentRealmcamelCase parameter).readonlyapplied correctly. - Bugs: the
!null-forgiving operator at line 27 is correct —string.IsNullOrWhiteSpacereturning false guaranteesrealmis non-null. - Security: no new attack surface. The realm URL was already accepted from deep-link params and used elsewhere (
LocalSceneDevelopmentPlugin.cs:46). This PR threads it to one additional consumer. Local development path only, client-side, no credentials attached. - Performance: no allocations in hot paths. The
realmfield is set once in the constructor. - Error handling: fallback to
IRealmNavigator.LOCALHOSTwhen realm is null/empty matches pre-existing behavior. - Resource leaks: no new subscriptions, connections, or disposables.
- Nullability:
string? realm = nullparameter correctly annotated as nullable; stored field is non-nullablestringafter the guard — contract is sound.
Pass B (design/encapsulation smells): None. No magic values introduced, no naming issues, no encapsulation leaks, no construction smells.
Security review: No secrets, injection risks, auth/authz issues, or sensitive data exposure. The realm URL is validated for null/whitespace and used only to construct local HTTP requests to the scene server.
Step 6 — Complexity
SIMPLE. Three files, 15 additions / 6 deletions. Straightforward parameter threading with no ECS, async, plugin, or architectural changes.
Step 7 — QA assessment
YES. The change modifies runtime code in the comms/multiplayer path that affects local scene development connectivity.
Step 8 — Non-blocking warnings
None. Main scene not modified.
Step 9 — Verdict
REVIEW_RESULT: PASS ✅
COMPLEXITY: SIMPLE
COMPLEXITY_REASON: Threads an existing parameter through the container pipeline to the local scene development metadata source — no ECS, async, or architectural changes.
QA_REQUIRED: YES
Reviewed by Jarvis 🤖 · Requested by gonpombo8 via Slack
The warning ratchet requires strictly fewer warnings than the S3 baseline (14087 vs branch 14089). Suppress InconsistentNaming for the wire-format DTO fields and fix four APIClient local names to shed 7.
|
✅ PR reviewed and approved by QA on both platforms following the PR test instructions. Build: Test results:
Unrelated errors noted (do not affect verdict): None Verdict: PASS ✅ Windows: Player port 8003 windows (1).log 
Mac: 
|
5 participants
Summary
LocalSceneDevelopmentSceneRoomMetaDataSourcebuilt its base URL from the hardcodedIRealmNavigator.LOCALHOSTconstant (http://127.0.0.1:8000) rather than the realm the client was actually launched with:sdk-commands startonly defaults to port 8000 — it takes--port, and tools that embed it pick their own port (the Creator Hub uses a random ephemeral one). Whenever the scene server is not on 8000, this source can never reach it, soMetaDataAsyncfails every cycle and the GateKeeper scene room never connects for local scene development.The realm is already parsed from the deep link and carried as
DynamicWorldParams.LocalSceneDevelopmentRealm(it's whatLocalSceneDevelopmentPluginuses viaRealmUrls.LocalSceneDevelopmentRealmAsync), so this threads that value throughCommsContainer.Createinto the metadata source. The parameter is optional and falls back toIRealmNavigator.LOCALHOSTwhen empty, so existing behaviour is unchanged when no realm is supplied.Repro
npm start -- --port 8003 # any port other than 8000then launch the desktop client from the printed deep link. Before this change the client logs, on repeat:
Note it reports
:8000even though the deep link's realm says:8003. With the fix it targetshttp://127.0.0.1:8003and the scene room connects.What could break
SceneRoomMetaDataSource(play mode) is untouched.LocalSceneDevelopmentPlayground(editor-only demo) still uses the loopback default deliberately.ApplicationParametersParser(trailing/stripped, macOShttp//protocol patched), so it has the same shape as the constant it replaces andURLDomain.Appendbehaves identically.How to test
npm iand thennpm run start -- --port 800373,-8): https://github.com/decentraland/unity-explorer/blob/dev/docs/how-to-connect-to-a-local-scene.mdMetadataAsync error Local scene server unreachable at http://127.0.0.1:8000