feat: add deep link param to force open backpack on client open#9398
feat: add deep link param to force open backpack on client open#9398davidejensen wants to merge 4 commits into
Conversation
|
Windows and Mac build successful in Unity Cloud! You can find a link to the downloadable artifact below. |
decentraland-bot
left a comment
There was a problem hiding this comment.
PR Review: feat: add deep link param to force open backpack on client open
STEP 2 — Root-cause check
PASS. New feature — adds a force-open-backpack deep link parameter so the shop's "try in world" flow can open the backpack immediately on landing. No symptom masking or workarounds.
STEP 3 — Design & integration
Owner search:
| New logic | Natural owner | Verdict |
|---|---|---|
| Launch-arg → open backpack | ExplorePanelPlugin — already owns keyboard-shortcut → open backpack (lines 668-669) |
✅ Correct home |
| Runtime deep link → open backpack | DeepLinkHandle — already owns position/realm/community deep link routing |
✅ Correct home |
| Constructor wiring | DynamicWorldContainer — single composition-root instantiation point |
✅ Acceptable |
Observation (P2): ExplorePanelPlugin.ForceOpenBackpackOnLandingAsync and DeepLinkHandle.OpenBackpackWhenLandedAsync have identical method bodies — both await LoadingStatus.LoadingStage.Completed then call mvcManager.ShowAsync(…Backpack…). While the two trigger paths (CLI arg vs runtime deep link) are legitimately distinct, the duplicated implementation is prone to maintenance drift. Consider extracting the shared wait-and-show logic into a small static helper accessible to both call sites (e.g. a BackpackLauncher.OpenOnLoadingCompleteAsync(ILoadingStatus, IMVCManager, CancellationToken) method).
Teardown / consumption trace:
ForceOpenBackpackOnLandingAsync— uses thectfromInitializeAsync; cancelled on plugin dispose. No subscriptions to leak. ✅OpenBackpackWhenLandedAsync— usesthis.tokenfrom the constructor; cancelled whenDynamicWorldContainer'sCancellationTokenfires. No subscriptions to leak. ✅
Constructor widening note: DeepLinkHandle now takes IMVCManager and ILoadingStatus. This follows the existing pattern (cf. CommunityDataService which also triggers UI), and there is a single instantiation point. Acceptable.
PASS.
STEP 4 — Member audit
| Member | Scope | Consumers | Verdict |
|---|---|---|---|
AppArgsFlags.FORCE_OPEN_BACKPACK |
public const | ExplorePanelPlugin, DeepLinkHandle |
✅ |
DeepLinkHandle ctor (widened) |
public | 1 (DynamicWorldContainer) |
✅ |
ForceOpenBackpackOnLandingAsync |
private | 1 (inline call) | ✅ |
OpenBackpackWhenLandedAsync |
private | 1 (inline call) | ✅ |
No single-use public members, no re-derived accessors, no absent-≠-false issues.
STEP 5 — Line-level findings
See inline comments. One P2 formatting issue identified.
STEP 6 — Complexity
SIMPLE. Four files, ~45 lines of additions. Straightforward flag-check → async UI panel show. No ECS system changes, no structural component operations, no complex async flows, no infrastructure modifications.
STEP 7 — QA assessment
QA_REQUIRED: YES. Changes affect runtime behavior — the backpack panel auto-opens on load when the flag is present. Must verify:
- Normal launch (no flag) — no regressions
- Launch with
--force-open-backpack— backpack opens after loading completes - Runtime deep link with
force-open-backpackparam — backpack opens after loading completes
STEP 8 — Non-blocking warnings
None. No Main.unity scene changes.
Security review
No security issues found. The feature adds a boolean flag presence check that triggers a UI navigation action. No injection surface, no auth changes, no privilege escalation, no data exposure.
REVIEW_RESULT: PASS ✅
COMPLEXITY: SIMPLE
COMPLEXITY_REASON: Adds a boolean deep link flag with straightforward flag-check → async UI panel show; no ECS, structural, or infrastructure changes.
QA_REQUIRED: YES
Reviewed by Jarvis 🤖 · Requested by decentraland-bot via GitHub
|
Warnings not reduced: 19087 => 19157 — remove at least one warning to merge. |
DafGreco
left a comment
There was a problem hiding this comment.
✔️ PR reviewed and approved by QA on both platforms following instructions playing both happy and un-happy path
Regressions for this ticket had been performed in order to verify that the normal flow is working as expected:
- [ ✔️ ] Backpack and wearables in world
- [ ✔️ ] Emotes in world and in backpack
- [✔️ ] Teleport with map/coordinates/Jump In
- [✔️ ] Chat and multiplayer
- [ ✔️ ] Profile card
Evidence with params and without
Pull Request Description
What does this PR change?
Fix #9379
This PR adds a new parameter that can be passed to the client to force open the backpack as soon as a user loads in world.
Main usage of this will be with the new shop, users will be able to click a "try in world" button for wearables/emotes, with this new param they will immediately see the backpack as soon as they load in
Test Instructions
Test Steps
_
Additional Testing Notes
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.