feat: retire public scene viewers and Unity renderer - #3489
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Coverage Report for CI Build 33640788098Coverage decreased (-0.001%) to 54.038%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
decentraland-bot
left a comment
decentraland-bot
left a comment
There was a problem hiding this comment.
Code Review — PR #3489: feat: retire public scene viewers and Unity renderer
Branch: sunset/2-retire-viewers → sunset/1-behavior-sunset
Files changed: 65 (+61 −8669)
CI: ✅ All checks passing (audit, test 1588/1588, Vercel preview deployed)
Coverage: Patch 100% (2/2 lines covered); overall -0.001% to 53.83% (expected for deletion)
Summary
Clean, well-scoped second layer of the scene editor sunset. Removes the public 3D scene viewers (SceneViewPage, SceneListPage/PoolCard), the SDK6 EditorPage (Unity renderer's last consumer), the Preview/ViewPort components, and ~25 MB of prebuilt Unity static assets. Retired URLs redirect to /scenes instead of 404ing. Shared Editor/EditorWindow types are relocated to modules/editor/types.ts so surviving sagas keep compiling. The LOCALSTORAGE_TUTORIAL_KEY constant moves to TutorialModal. All translation files (en/es/zh) are cleaned up consistently.
Findings
[P2 — Minor] Stale process.argv[3] index in scripts/postinstall.cjs
The removed code consumed process.argv[2] (editor path). The remaining ECS path still reads process.argv[3], which is now the wrong positional index. Since the script is invoked without arguments (node ./scripts/postinstall.cjs), the require.resolve(...) fallback always kicks in, so this is not a runtime bug — but it's a confusing leftover. Consider updating to process.argv[2] for clarity.
[P2 — Minor] injectScript in routing/utils.ts may now be unused
The only call site (injectSceneEditorScripts in editor/utils.ts) was removed. injectScript is still exported from routing/utils.ts but no longer imported anywhere in the codebase. Consider removing it in this PR or a follow-up to complete the cleanup.
[P2 — Minor] modules/pool/ module still wired into root reducer/sagas
The pool reducer, saga, and state are still registered in modules/common/reducer.ts, sagas.ts, and types.ts, but ScenesPage was the only external consumer of loadPoolsRequest and getPoolList. After this PR, modules/pool/ has no external consumers (only modules/poolGroup/ remains, which is separate). This is likely intentional — deferred to a future sunset layer — but worth flagging. The pool saga still runs and may fire unnecessary network requests.
[P2 — Minor] locations.poolSearch(), poolView(), sceneView() — retained only for redirects
These location helpers in routing/locations.ts are now exclusively consumed by the <Redirect> entries and tests. Fine to keep for readability, but worth noting they're redirect-only now.
[P2 — Minor] Redirect tests cover desktop only
The new AppRoutes.spec.tsx tests verify retired routes redirect on desktop (innerWidth = 1280) but don't test mobile behavior. On mobile, these paths fall through to the <Route component={MobilePage} /> catch-all, which is correct behavior — but a mobile test would document that intent.
What looks good
- Route strategy is solid — redirects instead of 404s for retired URLs, placed correctly after all real routes in the
<Switch>. - Type relocation is clean —
Editor/EditorWindowmoved tomodules/editor/types.tswhere they semantically belong.UnityKeyboardEventis already defined in the same file (line 34), so no missing type reference. LOCALSTORAGE_TUTORIAL_KEYrelocation — correctly moved to its only remaining consumer (TutorialModal), preserving the'builder-tutorial'value.decentraland-ecsandsrc/ecsScene/*intentionally retained — export/download andItemEditorPagestill need them.- Translation cleanup is consistent across all 3 language files (en, es, zh) — both
from_scene_poolandeditor_previewsections removed. - No breaking API changes — purely internal component/route removal with backward-compatible redirects.
- No security issues found — no secrets, no XSS vectors, no open redirects. Removing the Unity script injection actually reduces attack surface.
Verdict
✅ APPROVE — No P0 or P1 issues. The P2 items (process.argv[3] index, orphaned injectScript, pool module cleanup) are low-impact cleanup opportunities that can land in this PR or a follow-up. The deletion is thorough, the surviving code compiles and tests pass, and the redirect strategy is correct.
Reviewed by Jarvis 🤖 · Requested by Gabriel Díaz (<@U03MGHMAJL8>) via Slack
| const editorPath = path.resolve(__dirname, process.argv[2] || require.resolve('decentraland-ecs/artifacts/editor')) | ||
| const targetEditorPath = path.resolve(__dirname, '../public/editor.js') | ||
|
|
||
| const ecsPath = path.resolve(__dirname, process.argv[3] || require.resolve('decentraland-ecs/dist/src/index.min.js')) |
There was a problem hiding this comment.
[P2] process.argv[3] is a stale index — the process.argv[2] consumer (editor path) was removed in this PR. Since the script is invoked without arguments (node ./scripts/postinstall.cjs), the require.resolve(...) fallback always works, so no runtime breakage. But if anyone ever passes a CLI arg, they'd have to skip index 2 for no reason.
Consider updating to process.argv[2]:
| const ecsPath = path.resolve(__dirname, process.argv[3] || require.resolve('decentraland-ecs/dist/src/index.min.js')) | |
| const ecsPath = path.resolve(__dirname, process.argv[2] || require.resolve('decentraland-ecs/dist/src/index.min.js')) |
Delete SceneViewPage, the SceneListPage pool gallery, ViewPort, Preview, and EditorPage, plus the Unity renderer assets (public/unity, editor.js, UnityLoader.js) and the postinstall step that copied editor.js. Redirect the retired /pools and /view routes to the scene list. Move the shared Editor/EditorWindow types into modules/editor and drop the dead pool section from the Scenes page.
fd66f83 to
a199aa5
Compare
3 participants
Feature Description
Second layer of the web scene editor sunset. Retires the public 3D scene viewers and the Unity renderer that powered them. The pool gallery (
/pools) and the scene/pool viewers (/view/...) are removed, along with the SDK6EditorPage— which was the renderer's last remaining consumer — and the ~25 MB of prebuilt Unity assets. Net −8669 lines.User Story
As a Decentraland creator, the web builder no longer offers the in-browser 3D preview or the community scene pool (those experiences move to Creator Hub); my own scenes still list, download, and open normally.
Type of Change
Implementation Details
Base:
sunset/1-behavior-sunset. Almost entirely deletion. The Unity renderer shipped as prebuilt static files (no npm package), so the win here is deleting those assets and the code that injected them.Architecture
SceneViewPageandEditorPageboth renderedViewPort → Preview, which injected the Unity scripts. Removing both consumers letsViewPort,Preview, and thepublic/unityassets go. The sharedEditor/EditorWindowtypes that lived inPreviewwere moved tomodules/editor/types.tsso the surviving sagas keep compiling.decentraland-ecsandsrc/ecsScene/*are kept — the export/download path and the wearableItemEditorPagestill use them.Key Files Changed
src/components/SceneViewPage/,SceneListPage/(incl.PoolCard/),ViewPort/,Preview/src/components/EditorPage/LOCALSTORAGE_TUTORIAL_KEYmoved toTutorialModal)public/unity/,public/editor.js,public/UnityLoader.jsscripts/postinstall.cjsno longer copieseditor.jssrc/routing/AppRoutes/AppRoutes.tsx/pools+/viewroutes (mobile + desktop); redirect them to/scenessrc/components/ScenesPage/ScenesPage.*src/components/ProjectCard/ProjectCard.tsxsrc/modules/editor/{types,utils,sagas}.tsEditor/EditorWindowtypes; dropinjectSceneEditorScriptsAPI Changes
No API changes.
Database Changes
How to Test
/pools,/view/<id>,/view/pool/<id>— each redirects to/scenes; confirm (Network tab) nothing requestsunity/*,editor.js, orUnityLoader.js./scenesno longer shows the "from scene pool" section; your scenes still list, download, and open.ItemEditorPage) — it still loads and renders (guards the sharedmodules/editor/decentraland-ecs).Edge Cases Considered
decentraland-ecsandsrc/ecsScene/*.rawremain intact so scene export/download keeps working.Checklist
AppRoutes.spec.tsx)Feature Flag
Related Issues
Part of the web scene editor sunset stack. Base:
sunset/1-behavior-sunset.Screenshots / Demo
Deletion of viewer surfaces; retired URLs redirect to
/scenes.Deployment Notes
Ships ~25 MB less in
public/(Unity binaries +editor.js).postinstallstill copies the SDK6ecs.js.rawthe export path needs.