chore: remove the today environment - #9885
Conversation
Removes DecentralandEnvironment.Today and everything that special-cased it: the org/today host-mixture pinning in DecentralandUrlsSource, the gateway environment gate (now flag-only), GatekeeperMode.Today, TODAY_DOMAIN, the ChatEnvironmentValidator realm-change ban (and its now-dead environment dependency), the chain/manifest/server-env mappings, tests, and doc mentions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🚦 CI Status
Windows and Mac build successful in Unity Cloud! You can find a link to the downloadable artifact below. Warnings not reduced: 12215 => 12218 — remove at least 4 warnings to merge. Warnings/errors in files changed by this PR (8)
|
decentraland-bot
left a comment
decentraland-bot
left a comment
There was a problem hiding this comment.
STEP 1 — Context loaded
Scope: 21 files changed (+57 −213). Pure removal of the dead Today environment (decentraland.today) across shared enums, interfaces, URL resolution, gateway routing, chat validation, chain pinning, tests, and docs. No new units introduced.
Subsystem docs reviewed: CLAUDE.md, docs/app-arguments.md, docs/custom-base-domain.md.
STEP 2 — Root-cause check: PASS
The problem is dead code: the Today environment is no longer in use and its host/domain infrastructure is decommissioned. The diff removes it cleanly — this is the correct action, not a symptom workaround.
STEP 3 — Design & integration: PASS
No new long-lived units introduced — this PR only removes code. The design simplifications are sound:
ChatEnvironmentValidatorloses itsDecentralandEnvironmentdependency — correct, since the Today early-return was its only use. Callers updated.GatewayUrlsSource—SUPPORTED_ENVSgate removed. The array was{ Org, Zone, Custom }, which is exactly the set of remaining environments, soenvSupportedwas always true. Removing the gate and making fields non-nullable is the right cleanup.DecentralandUrlsSource.BaseDomain— changed from{ get; private set; }to{ get; }. The Today constructor block that mutated it post-construction was the only writer. Correct.System.Linqremoved fromGatewayUrlsSource—SUPPORTED_ENVS.Contains()was its only use. Good per CLAUDE.md (no LINQ in hot paths).
No teardown/subscription concerns — the PR removes subscriptions and arms, it doesn't add them.
STEP 4 — Member audit
BaseDomain { get; }— consumed throughoutDecentralandUrlsSourceandGatewayUrlsSourcefor host composition and trust checks. Changing from{ get; private set; }to{ get; }is a pure tightening; no consumers break.ChatEnvironmentValidator(IDecentralandUrlsSource)— single-parameter constructor. Both callers (ChatContainer.Create,ChatTeleporterShould.SetUp) updated. Clean.GatewayUrlsSource.enabled— simplified fromenvSupported && flagtoflag. Behaviorally equivalent sinceenvSupportedwas alwaystruefor remaining envs.
No new public members introduced.
STEP 5 — Line-level review
Pass A — Blocking issues: None (P0/P1)
All switch expressions remain exhaustive (Org, Zone, Custom, _ => throw). Nullable-to-non-nullable transition in GatewayUrlsSource is correct — the constructor now unconditionally initializes all fields. Feature-flag gating logic is behaviorally identical. No bugs, no security issues, no resource leaks.
Pass B — Design smells
One P2 finding — see inline comment on DecentralandEnvironment.cs.
[P2] DecentralandEnvironment uses implicit ordinal values while being serialized by index — GatekeeperMode correctly uses explicit values (Org = 0, Zone = 1, Localhost = 3, Custom = 4), making it resilient to member removal. DecentralandEnvironment does not, so removing Today shifts Custom from ordinal 3 to 2. The existing XML comment on Custom warns "New values must be appended: the enum is serialized by index on MainSceneLoader" — the PR necessarily breaks this append-only contract. While Main.unity serializes 0 (Org) so no checked-in asset breaks, adding explicit values now (like GatekeeperMode already does) would make the enum resilient and honest about its serialization contract.
Security review: PASS
Removing decentraland.today from ALL_DOMAINS reduces the trust surface — correct for a decommissioned domain. Host-suffix validation, deep-link allowlist, chain pinning, and gateway feature-flag gating are all preserved and unaffected. No new attack surface.
STEP 6 — Complexity
COMPLEX. Modifies shared enums and interfaces used across multiple assemblies.
STEP 7 — QA assessment
QA_REQUIRED: YES. Runtime code is modified: URL resolution, chat validation, gateway routing, chain pinning.
STEP 8 — Non-blocking warnings
None. Main.unity is not in the changed files.
STEP 9 — Verdict
REVIEW_RESULT: PASS ✅
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Modifies shared enums and interfaces (DecentralandEnvironment, GatekeeperMode, IDecentralandUrlsSource) used across multiple assemblies (Chat, Infrastructure, NetworkDefinitions, Web3), plus URL resolution and gateway routing logic.
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. |
Signed-off-by: Juan Ignacio Molteni <juanignaciomolteni@gmail.com>
decentraland-bot
left a comment
decentraland-bot
left a comment
There was a problem hiding this comment.
Re-review — STEP 1: Context loaded
Scope: 21 files changed (+57 −213). Pure removal of the dead Today environment (decentraland.today). Three commits on branch:
3b02588— original removal (chore: remove the today environment)de7051c— addresses previous review P2: assign explicit values toDecentralandEnvironmentenum6953632— mergedevinto branch
Previous review (2026-08-27) PASSED with one P2 (enum explicit values). This re-review verifies the fix and re-evaluates the full diff.
STEP 2 — Root-cause check: PASS
Unchanged from prior review. The problem is dead code — the Today environment is decommissioned. The diff removes it cleanly.
STEP 3 — Design & integration: PASS
No new units introduced. All design simplifications remain sound:
ChatEnvironmentValidator—DecentralandEnvironmentconstructor param removed; onlyIDecentralandUrlsSourceremains. Both callers updated (ChatContainer.Create,ChatTeleporterShould.SetUp).GatewayUrlsSource—SUPPORTED_ENVSgate removed. Fields correctly promoted from nullable to non-nullable (constructor now unconditionally initializes them).System.Linqremoved (its only use wasSUPPORTED_ENVS.Contains()).DecentralandUrlsSource.BaseDomain—{ get; private set; }→{ get; }. Correct — the Today mixture was the only post-construction writer.DecentralandEnvironment— explicit values now assigned (Org = 0, Zone = 1, Custom = 2).Custommoves from implicit ordinal 3 to explicit 2; this is safe because: (a)MainSceneLoaderserializes0(Org), (b)Customis only assigned at runtime viaApplyBaseDomainArg, never persisted, (c) CLI parsing usesEnum.TryParseby string name, not integer.GatekeeperMode—Today = 2removed; gap preserved (Org = 0, Zone = 1, Localhost = 3, Custom = 4). Correct.
No teardown/subscription concerns — the PR only removes code.
STEP 4 — Member audit
All members from the previous review re-verified. No new public members introduced. The enabled property in GatewayUrlsSource simplified from envSupported && flag to flag — behaviorally equivalent since envSupported was always true for remaining environments.
STEP 5 — Line-level review
Pass A — Blocking issues: None (P0/P1)
All switch expressions remain exhaustive (Org, Zone, Custom, _ => throw). Nullable-to-non-nullable transitions are correct. Feature-flag gating logic is behaviorally identical. No bugs, no security issues, no resource leaks.
Pass B — Design smells
[P2] Stale XML comment on DecentralandEnvironment.Custom — see inline comment. The comment still says "New values must be appended: the enum is serialized by index" but with explicit values now assigned, the append-only restriction is lifted. The previous review's inline comment specifically requested this update alongside the explicit values.
Security review: PASS
Full security audit conducted (parallel agent). Removing decentraland.today from ALL_DOMAINS correctly reduces the trust surface. Host-suffix validation (HostHasSuffix), deep-link allowlist, chain pinning, and gateway feature-flag gating are all preserved and unaffected. No new attack surface, no secrets exposed.
STEP 6 — Complexity
COMPLEX. Modifies shared enums and interfaces used across multiple assemblies.
STEP 7 — QA assessment
QA_REQUIRED: YES. Runtime code is modified: URL resolution, chat validation, gateway routing, chain pinning.
STEP 8 — Non-blocking warnings
None. Main.unity is not in the changed files.
Previous review P2 status
✅ Enum lacks explicit values — addressed in commit de7051c. DecentralandEnvironment now has Org = 0, Zone = 1, Custom = 2.
STEP 9 — Verdict
REVIEW_RESULT: PASS ✅
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Modifies shared enums and interfaces (DecentralandEnvironment, GatekeeperMode, IDecentralandUrlsSource) used across multiple assemblies (Chat, Infrastructure, NetworkDefinitions, Web3), plus URL resolution and gateway routing logic.
QA_REQUIRED: YES
Reviewed by Jarvis 🤖 · Requested by Juan Ignacio Molteni [Dalkia] (<@U03JSUQ5Z7U>) via Slack
Signed-off-by: Juan Ignacio Molteni <juanignaciomolteni@gmail.com>
|
PR #9885, run #33191568941 Overall: ✅ no significant changes Builds: Windows change, Windows baseline, macOS change, macOS baseline How to read this table
Intel Core i5
Apple M1
|
Ludmilafantaniella
left a comment
Ludmilafantaniella
left a comment
There was a problem hiding this comment.
✅Smoke test performed:
- ✔️ 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
- ✔️ Camera
- ✔️ Skybox
3 participants
Pull Request Description
What does this PR change?
Removes the
todayenvironment (decentraland.today) from the client.Todaywas a hybrid environment — a catalyst replicating org that served a handful of asset/comms hosts from.todayand everything else from org — and it is no longer in use.What goes away:
DecentralandEnvironment.TodayandGatekeeperMode.Today(explicit values on the remainingGatekeeperModemembers keep serialized data stable).IDecentralandUrlsSource.TODAY_DOMAIN, anddecentraland.todaydrops out ofALL_DOMAINS, so.todayhosts are no longer trusted by deep-link host checks.DecentralandUrlsSource: the constructor block that pre-resolved and pinned today's org/today host mixture, plus theisTodayEnvironmentbranches in the optimized-assets/abgen resolution.BaseDomainis now a get-only property — the today mixture was the only reason it was writable.GatewayUrlsSource: theSUPPORTED_ENVSgate existed only to keep today off the gateway; with every remaining environment supported, the gate is dead code and gateway routing is now gated by theuse-gatewayfeature flag alone (no behavior change for org/zone/custom).ChatEnvironmentValidator: the "cannot change realms in Today" early return; that made its environment dependency dead, so the constructor parameter is removed too (callers updated).Todayarms inChainUtils.PinnedNetworkOf,WorldManifestProvider,LiveKitChatMessagesBus, andChatReactionsFactory.docs/app-arguments.md,docs/custom-base-domain.md, and the perf-report docs.Notes for reviewers:
--dclenv todaynow failsEnum.TryParseinMainSceneLoader.ParseEnvironmentand is logged and ignored, like any other unknown value — the client falls back to the serialized default.TodayshiftsCustom's serialized index from 3 to 2 onMainSceneLoader.decentralandEnvironment. The checked-inMain.unityserializes0(Org), so no asset changes; only a local uncommitted scene tweaked to Today/Custom would be affected, one time.Test Instructions
Steps (standard run):
metaforge explorer run XXXX # ← replace with this PR numberExpected result: Client boots into org exactly as before — this PR only deletes the today path; org/zone/custom resolution, gateway routing, chain pinning, and chat teleport validation are unchanged for the remaining environments.
Additional Testing Notes
--dclenv todayshould log the ignored value at startup and continue on the default environment.DecentralandUrlsSourceShould,ChainUtilsShould, andChatEnvironmentValidatorShould.Quality Checklist
Code Review Reference
Please review our Branch & PR Standards before submitting. It explains the automated review flow, QA/DEV approval requirements, and what each label does — especially useful for first-time contributors.
🤖 Generated with Claude Code