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

auto install & import asset-packs for editor scenes #909

Merged
merged 3 commits into from
Mar 4, 2024

Conversation

gonpombo8
Copy link
Contributor

@gonpombo8 gonpombo8 commented Mar 1, 2024

With this PR is not mandatory to import and install the @dcl/asset-pack lib for the editor scenes.
The sdk-commands does all this at build-time in the background using the asset-pack version that the inspector used to build the scene.
So this way we don't have mismatched of @dcl/asset-packs version. (The inspector already has the asset-packs lib, and the scene could have another version that could lead into mysterious bugs )

Before (game.ts scene):

import { engine } from '@dcl/sdk/ecs'
import { initAssetPacks } from '@dcl/asset-packs/dist/scene-entrypoint'

initAssetPacks(engine)

function main() {}

Now

import { engine } from '@dcl/sdk/ecs'
function main() {}

@gonpombo8 gonpombo8 force-pushed the feat/asset-packs-auto-import branch from c2ccd00 to 3cc3e59 Compare March 1, 2024 18:45
Copy link

cloudflare-workers-and-pages bot commented Mar 1, 2024

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: d82757c
Status: ✅  Deploy successful!
Preview URL: https://4f466ac5.js-sdk-toolchain.pages.dev
Branch Preview URL: https://feat-asset-packs-auto-import.js-sdk-toolchain.pages.dev

View logs

Copy link
Contributor

github-actions bot commented Mar 1, 2024

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/feat/asset-packs-auto-import/dcl-sdk-7.4.8-8140805204.commit-b25463a.tgz"
  • To test with npx init

    export SDK_COMMANDS="https://sdk-team-cdn.decentraland.org/@dcl/js-sdk-toolchain/branch/feat/asset-packs-auto-import/dcl-sdk-commands-7.4.8-8140805204.commit-b25463a.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/feat/asset-packs-auto-import/@dcl/inspector/dcl-inspector-7.4.8-8140805204.commit-b25463a.tgz"
  • The /changerealm command to test test in-world

    /changerealm https://sdk-team-cdn.decentraland.org/ipfs/feat/asset-packs-auto-import-e2e
    
  • You can preview this build entering:
    https://playground.decentraland.org/?sdk-branch=feat/asset-packs-auto-import

@@ -2,7 +2,7 @@
"name": "@dcl/inspector",
"version": "0.1.0",
"dependencies": {
"@dcl/asset-packs": "^1.12.2",
"@dcl/asset-packs": "^1.12.3-20240301140405.commit-8aef784",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: make a asset-packs release and use that stable versioin

import { initAssetPacks } from '@dcl/asset-packs/dist/scene-entrypoint'
initAssetPacks(engine)

// TODO: do we need to do this on runtime ?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move this to another issue

Copy link

codecov bot commented Mar 1, 2024

Codecov Report

Attention: Patch coverage is 40.00000% with 27 lines in your changes are missing coverage. Please review.

Project coverage is 67.05%. Comparing base (8dbeee0) to head (d82757c).

Files Patch % Lines
...@dcl/sdk-commands/src/logic/project-validations.ts 23.80% 15 Missing and 1 partial ⚠️
packages/@dcl/sdk-commands/src/logic/config.ts 33.33% 8 Missing ⚠️
packages/@dcl/sdk-commands/src/logic/bundle.ts 60.00% 2 Missing ⚠️
...ages/@dcl/sdk-commands/src/commands/build/index.ts 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #909      +/-   ##
==========================================
- Coverage   67.13%   67.05%   -0.09%     
==========================================
  Files         529      529              
  Lines       16676    16710      +34     
  Branches     2132     2145      +13     
==========================================
+ Hits        11196    11205       +9     
- Misses       5143     5167      +24     
- Partials      337      338       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gonpombo8 gonpombo8 merged commit 65ac835 into main Mar 4, 2024
9 of 10 checks passed
@gonpombo8 gonpombo8 deleted the feat/asset-packs-auto-import branch March 4, 2024 13:29
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.

3 participants