Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Load smart item asset path #1062

Merged
merged 3 commits into from
Feb 6, 2025
Merged

fix: Load smart item asset path #1062

merged 3 commits into from
Feb 6, 2025

Conversation

cyaiox
Copy link
Collaborator

@cyaiox cyaiox commented Feb 6, 2025

Asset Loading Race Condition Fix

Context and Problem Statement

When dragging and dropping assets (particularly NFTs and smart items) into the canvas, there was a race condition where the asset would fail to load on the first attempt but succeed on subsequent attempts. This occurred because the asset was requested before it was fully downloaded and available in the filesystem.

Solution

Implemented a retry mechanism in the SceneContext.getFile() method to handle cases where an asset is temporarily unavailable. This provides a more robust solution without introducing unnecessary complexity.

Key changes:

  • Added retry logic to getFile() with a maximum of 3 attempts
  • Added 500ms delay between retries to allow for asset availability
  • Improved error logging for debugging purposes

Testing

  • Tested dragging and dropping NFTs - works on the first attempt
  • Tested dragging and dropping Video Player - works on the first attempt
  • Tested dragging and dropping smart items - works on the first attempt
  • Verified retry mechanism works with console logging
  • Confirmed no regressions in other asset loading scenarios

Impact

This change improves the reliability of asset loading in the Inspector, particularly for NFTs and smart items. Users will no longer experience failed first attempts when dragging and dropping these assets into the canvas.

Closes: decentraland/creator-hub#346

Copy link
Contributor

github-actions bot commented Feb 6, 2025

Test this pull request

  • The @dcl/sdk package can be tested in scenes by running

    npm install "https://sdk-team-cdn.decentraland.org/@dcl/js-sdk-toolchain/branch/fix/smart-item-asset-path/dcl-sdk-7.7.4-13181398394.commit-d278f8d.tgz"
  • To test with npx init

    export SDK_COMMANDS="https://sdk-team-cdn.decentraland.org/@dcl/js-sdk-toolchain/branch/fix/smart-item-asset-path/dcl-sdk-commands-7.7.4-13181398394.commit-d278f8d.tgz"
    npx $SDK_COMMANDS init
  • The @dcl/inspector package can be tested by visiting this url

    • Or by installing it via NPM
    npm install "https://sdk-team-cdn.decentraland.org/@dcl/js-sdk-toolchain/branch/fix/smart-item-asset-path/@dcl/inspector/dcl-inspector-7.7.4-13181398394.commit-d278f8d.tgz"
  • The /changerealm command to test test in-world

    /changerealm https://sdk-team-cdn.decentraland.org/ipfs/fix/smart-item-asset-path-e2e
    
  • You can preview this build entering:
    https://playground.decentraland.org/?sdk-branch=fix/smart-item-asset-path

Copy link

cloudflare-workers-and-pages bot commented Feb 6, 2025

Deploying js-sdk-toolchain with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8f278e1
Status: ✅  Deploy successful!
Preview URL: https://a0a9f9de.js-sdk-toolchain.pages.dev
Branch Preview URL: https://fix-smart-item-asset-path.js-sdk-toolchain.pages.dev

View logs

@cyaiox cyaiox enabled auto-merge (squash) February 6, 2025 14:57
@cyaiox cyaiox merged commit 2b88243 into main Feb 6, 2025
8 checks passed
@cyaiox cyaiox deleted the fix/smart-item-asset-path branch February 6, 2025 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NFT Smart Item lacks model/graphic when added
2 participants