feat: add PBExplorerUiEventsResult and relocate ExplorerUi enum - #455
Open
popuz wants to merge 2 commits into
Open
feat: add PBExplorerUiEventsResult and relocate ExplorerUi enum#455popuz wants to merge 2 commits into
popuz wants to merge 2 commits into
Conversation
Adds the PBExplorerUiEventsResult component (ecs_component_id 1220) so scenes can observe when fullscreen explorer panels open and close, and relocates the ExplorerUi enum out of the restricted-actions API into decentraland/sdk/components/common/explorer_ui.proto. The relocation is required, not cosmetic. js-sdk-toolchain's RPC API generator (scripts/rpc-api-generation/index.ts:203) hardcodes decentraland/sdk/components/common/ as the only path whose proto types collapse to a single nominal TypeScript identity. With the enum left in restricted_actions.proto a scene would hold two incomparable ExplorerUi types -- one from ~system/RestrictedActions, one from @dcl/ecs. After the move, restricted_actions.gen.ts and explorer_ui_events_result.gen.ts both import ExplorerUi from the same generated module. The move and the new component must ship together: relocating the enum without a component proto importing it leaves it ungenerated in @dcl/ecs and re-declared in apis.d.ts, which is a regression. Wire compatibility is untouched -- field number 1, values 0-6, and wire types are all unchanged. buf breaking with WIRE_JSON (the policy configured in proto/buf.yaml) passes. Expected red check: validate-compatibility runs scripts/check-proto-compabitility.sh, which compares fully-qualified type names textually and is stricter than the repo's configured buf policy. It reports a source-level break for the `ui` field. The break is real but has zero consumers -- openExplorerUi merged 2026-07-28 and the latest stable @dcl/sdk (7.25.0, published 2026-07-27) does not contain the API, so only next/experimental tarballs are affected. A team override is requested rather than suppressing the check. Co-Authored-By: Claude <noreply@anthropic.com>
Test this pull request
|
`ui` and `event` are independent axes, so every pair of the two is representable. The previous comment invited per-panel feature events into the same oneof at field 20+, which would have made pairs such as `ui = EU_MAP` carrying a backpack event perfectly legal, and pushed the burden of recognising and ignoring them onto every consumer. State the rule instead: a variant belongs in this oneof only if it is meaningful for every value of `ui`. Panel-specific events get their own result component, where the component identity carries the panel and the contradiction cannot be expressed. That also matches how the rest of the protocol scales -- PBUiDropdownResult, PBUiInputResult, PBTriggerAreaResult, PBVideoEvent and PBAudioEvent are separate components rather than variants of one shared event bus. Comment-only change; the wire format is untouched.
2 tasks
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.
What
Adds the
PBExplorerUiEventsResultgrow-only SDK component (id 1220) so scenes can observe when a fullscreen explorer panel they opened is opened/closed, and relocates theExplorerUienum fromdecentraland/kernel/apis/restricted_actions.prototo a newdecentraland/sdk/components/common/explorer_ui.proto.Round 2 of
openExplorerUi(round 1: #451).Files
proto/decentraland/sdk/components/common/explorer_ui.proto—ExplorerUienum, moved verbatim.proto/decentraland/kernel/apis/restricted_actions.proto— imports the new file;OpenExplorerUiRequest.uinow refers todecentraland.sdk.components.common.ExplorerUi.OpenExplorerUiResultis unchanged and stays put.proto/decentraland/sdk/components/explorer_ui_events_result.proto—PBExplorerUiEventsResult, id 1220.public/sdk-components.proto— oneimport publicline.The component carries
ui, auint32 timestamp(scene tick, matchingPBPointerEventsResult/PBTriggerAreaResult), and aoneof eventwith nested emptyUiOpened/UiClosed. Field numbers 10-19 are reserved for generic window events; future per-panel feature events start at 20.Why move the enum
js-sdk-toolchain's
scripts/rpc-api-generation/index.ts:203hardcodesdecentraland/sdk/components/common/as the only path that collapses a proto type to a single nominal TypeScript identity. With the enum left inkernel/apis, a scene would hold two incomparableExplorerUitypes — one from~system/RestrictedActions, one from@dcl/ecs. TypeScript enums are nominal, soe.ui === ExplorerUi.EU_MAPwould not typecheck and scene authors would need a cast on every comparison.After the move, both generated modules import the same declaration — confirmed empirically against a local codegen run:
There is existing precedent for
restricted_actions.protoimporting fromsdk/components/common/— it already importsavatar_mask.proto.The move and the new component must ship together. A relocation with no component proto importing the enum leaves it ungenerated in
@dcl/ecsand re-declared inapis.d.ts— that would be a regression, not a no-op. Splitting this PR in two is therefore not an option.Compatibility
The relocation is source-breaking but wire-compatible:
1onOpenExplorerUiRequest.ui— unchangedEU_SETTINGS = 0…EU_EVENTS = 6— unchangedOnly the generated type's namespace/location changes.
buf breakingunder this repo's configuredWIRE_JSONpolicy (proto/buf.yaml) passes on this diff; CI runs it as its own step ahead of the check script.unity-explorer needs regeneration only. Both hand-written consumers already import both namespaces and reference the type by its bare name, so the C# namespace change requires no source edits there.
The
validate-compatibilitycheck will be red — expected, and an override is requested./scripts/check-proto-compabitility.shreports exactly one error:The tool is not wrong — it is stricter than the repo's configured
bufpolicy. It compares fully-qualified type names textually and correctly reports a source-level break for consumers of generated code. It isproto-compatibility-tool, pinned at^1.1.2-20220925163655.commit-25bd040, and takes positional arguments only — there is no allowlist or exception flag, so the check cannot be scoped without editing the script. Nothing under.github/or in the script has been touched: the red check is disclosed, not hidden.Override rationale: the break is real but has zero consumers.
openExplorerUimerged 2026-07-28. The latest stable@dcl/sdkis 7.25.0, published 2026-07-27, so it does not contain the API — onlynext/experimentaltarballs do. No creator on the stable channel is affected. That window closes with the next release, which is precisely why the relocation happens now rather than later.Requesting a maintainer override on this single check.
Merge order
protocol (this PR) → publish → repin
@dcl/protocolin js-sdk-toolchain + unity-explorer → test scene.