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

feat: asset bundle fallback #3008

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
Open

feat: asset bundle fallback #3008

wants to merge 8 commits into from

Conversation

dalkia
Copy link
Collaborator

@dalkia dalkia commented Dec 13, 2024

What does this PR change?

  • Makes the scene definition to be fetched from the AB registry instead of the Catalyst Active Endpoints
  • Behind feature flag

How to test the changes?

  1. The first test phase should be done in zone.
  2. Create a scene using the builder and deploy it
  3. Open the client using --dclenv zone
  4. Go to you scene, you should see it
  5. Using the builder, create a broken state. IE: delete all images so the ABConverter fails. Deploy it
  6. Go back to the client. You should still be able to see it

Our Code Review Standards

https://github.com/decentraland/unity-renderer/blob/master/docs/code-review-standards.md

@dalkia dalkia self-assigned this Dec 13, 2024
@dalkia dalkia force-pushed the feat/asset-bundle-fallback branch from 7ecb4ca to 0328c41 Compare December 16, 2024 18:07
@dalkia dalkia added the force-build Used to trigger a build on draft PR label Dec 17, 2024
@dalkia dalkia marked this pull request as ready for review December 18, 2024 13:33
@Ludmilafantaniella Ludmilafantaniella requested review from Ludmilafantaniella and removed request for DafGreco December 18, 2024 13:57
Copy link

@Ludmilafantaniella Ludmilafantaniella left a comment

Choose a reason for hiding this comment

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

Here’s what we validated during the process:

  1. Fall-back to the most recent outdated scene bundle: ✅ Passed successfully.
  2. Correct bundle retrieval for updated scenes: ✅ Also passed without issues.
  3. Validate local cache removal for non-active scenes: ❌ This point failed because the reload functionality hasn’t been implemented yet, and Juani confirmed it won’t be ready in time.

Additional Notes:

  • The bug we encountered was related to the back-end and didn’t affect our testing results. 182 (dapps)
  • We couldn’t test the unpublish flow, meaning the modification of scene pointers (shrink or expand) still needs to be validated.
Untitled.video.-.Made.with.Clipchamp.mp4

Ludmilafantaniella

This comment was marked as duplicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
force-build Used to trigger a build on draft PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants