feat: added PBExplorerItemPurchaseResult and iap open event - #464
feat: added PBExplorerItemPurchaseResult and iap open event#464davidejensen wants to merge 8 commits into
Conversation
Test this pull request
|
davidejensen
changed the title
popuz
left a comment
popuz
left a comment
There was a problem hiding this comment.
Overall this is the extension model working as designed, nice: EU_ITEM_PURCHASE is the cheap "new panel = new enum value" path, oneof params materializes the extension point exactly as its placeholder comment described, and the separate outcome component follows the recipe the 1220 extension-point comment prescribes ("an event that belongs to one panel alone … gets its own result component"). Two design-level questions I'd like settled before this merges, since scene code will be written against the answers:
1. What does WAS_ALREADY_OPEN mean once modals join the namespace? The comments were relaxed from "fullscreen explorer panels" to "explorer UI panels", which implies the purchase modal is not fullscreen. The one-open-panel invariant is what gives WAS_ALREADY_OPEN its meaning, and consumers rely on it (the SDK-side wait-for-close helper in decentraland/js-sdk-toolchain#1543 assumes close events are delivered only to the scene whose call opened the panel). Please define explicitly — and record in the proto comments: does an open fullscreen panel (say the map) make EU_ITEM_PURCHASE return WAS_ALREADY_OPEN? Does an open purchase modal block opening the map? One shared slot, or fullscreen panels and modals as two independent slots?
2. Purchase results correlate only by urn. Two flows for the same urn close together (double-click, scene retry) produce indistinguishable results. Suggested additive fix while the wire is young: a scene-supplied string correlation_id in PurchaseParams, echoed back in PBExplorerItemPurchaseResult. One optional field on each side, fully backward compatible, and it turns the SDK-side "await the purchase outcome" from a FIFO heuristic into an exact match. We hit exactly this ambiguity class with 1220, where the absence of a session id forces heuristics; here it is still cheap to avoid.
Inline nits on the specific lines.
| EU_PLACES = 5; | ||
| EU_EVENTS = 6; | ||
| // Opens the item-purchase confirmation modal for the urn in PurchaseParams. | ||
| // UiOpened/UiClosed are emitted in PBExplorerItemPurchaseResult like for every other value; |
There was a problem hiding this comment.
"UiOpened/UiClosed are emitted in PBExplorerItemPurchaseResult" — typo for PBExplorerUiEventsResult? UiOpened/UiClosed live in 1220. If this is not a typo, the lifecycle of this one panel would be split across two channels — please clarify which component carries opened/closed for EU_ITEM_PURCHASE. (Also a stray blank line with trailing whitespace slipped in below the enum value.)
Adds an optional request_id to OpenExplorerUiRequest, echoed back in PBExplorerUiEventsResult (1220) and PBExplorerItemPurchaseResult (1221). Today a scene cannot tell which events belong to which openExplorerUi call. The explorer knows the pairing -- opened and closed are emitted from a single await scope of one call -- but it is lost at the wire boundary, so SDK helpers have to guess it from timestamp windows and per-panel FIFO matching. The scene sets the id before sending the request, so events that arrive before the RPC response resolves are still correlatable. 0 is the proto3 default and means uncorrelated, which keeps the change additive for clients that do not implement it. This is not the transaction_id deferred earlier in review: that one is about purchase semantics, this one is about binding a call to its own event stream.
A proto3 scalar without `optional` generates a required TypeScript field,
so `openExplorerUi({ ui })` would stop compiling for every existing caller
-- including scenes already on @dcl/sdk@next. Explicit presence keeps the
change additive and lets the SDK omit the field instead of sending a
sentinel.
The result components keep the plain uint32: scenes only read those, and 0
reads better there than an undefined check at every access.
Co-Authored-By: Claude <noreply@anthropic.com>
EU_ITEM_PURCHASE is a popup, not a fullscreen panel, so it can be shown on top of an already open one. WAS_ALREADY_OPEN now means the requested panel is open rather than any panel, which is what makes concurrent sessions expressible -- and what makes request_id load bearing instead of a nicety. Also points the purchase outcome at 1221; 1220 is the component the line above it describes. Co-Authored-By: Claude <noreply@anthropic.com>
The two look redundant when read from the caller's side, which invites removing one of them. They are not: the response is point to point, the component is a scene wide stream. Co-Authored-By: Claude <noreply@anthropic.com>
3 participants
Extend OpenExplorerUi to support in-scene item purchases
Adds support for opening the item-purchase confirmation modal from scene code, reusing the existing OpenExplorerUi restricted action instead of introducing a separate RPC (as was previously attempted in #462).
Changes:
Design decisions:
- The purchase outcome is delivered asynchronously via the new CRDT component rather than synchronously in the RPC response. This avoids blocking the restricted action call for the entire purchase modal lifecycle and is consistent with how OpenExplorerUi works for other panels.
- Failed is intentionally coarse (no error sub-types) to prevent wallet-balance probing via differential error analysis.
- Status variants use empty messages inside a oneof rather than an enum, allowing future per-status metadata (e.g. transaction hash on Purchased) without breaking the wire format.
- The client resolves pricing independently from the URN; the scene never handles prices or wallet details.