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

Fixing GIF ordering issues with files with a large number of frames #9774

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from

Conversation

alx-zhu
Copy link

@alx-zhu alx-zhu commented Dec 8, 2024

Resolves

Resolves #5875

Before: Out of order GIFs after costume upload:

Screen.Recording.2024-12-08.at.4.18.34.PM.mov

After: In order GIF after costume upload:

Screen.Recording.2024-12-08.at.4.20.44.PM.mov

Proposed Changes

Modify the function handleNewCostume in src/containers/costume-tab.jsx to send costumes sequentially to the Scratch VM using async await syntax to ensure a costume is received by the VM before moving onto the next.

Reason for Changes

  1. Large GIF frames were being added out of order when uploaded as costumes because they were being sent in parallel to the Scratch VM. Confirmed that the frames are in order when processed in src/lib/gif-decoder.js, src/lib/file-uploader.js, AND src/containers/costume-tab.jsx, but were received out of order by scratch-vm/src/virtual-machine.js in the addCostume function.

  2. Updated to modern async await syntax to handle Promises, which is the most up-to-date method of managing Promises in JS.

Test Coverage

Before:

Unit tests:
image

Costume test:
image
image

After:

Unit Tests
image

Costume tests:
image
image

Browser Coverage

Check the OS/browser combinations tested (At least 2)

Mac

  • Chrome
  • Firefox
  • Safari

Windows

  • Chrome
  • Firefox
  • Edge

Chromebook

  • Chrome

iPad

  • Safari

Android Tablet

  • Chrome

Copy link

github-actions bot commented Dec 8, 2024

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@alx-zhu
Copy link
Author

alx-zhu commented Dec 8, 2024

I have read the CLA Document and I hereby sign the CLA

2 similar comments
@sherryzhuge1
Copy link

I have read the CLA Document and I hereby sign the CLA

@HeidiTao
Copy link

HeidiTao commented Dec 8, 2024

I have read the CLA Document and I hereby sign the CLA

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.

Uploading gifs as costumes imports images in wrong order
3 participants