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/gif ordering: Fixing frame ordering issue for uploading costumes using GIFs. #1

Merged
merged 6 commits into from
Dec 8, 2024

Conversation

alx-zhu
Copy link
Owner

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

Resolves

Resolves scratchfoundation#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

@alx-zhu alx-zhu self-assigned this Dec 8, 2024
@alx-zhu alx-zhu added the bug Something isn't working label Dec 8, 2024
Copy link
Collaborator

@sherryzhuge1 sherryzhuge1 left a comment

Choose a reason for hiding this comment

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

Looks good

@sherryzhuge1 sherryzhuge1 merged commit be6883a into develop Dec 8, 2024
@alx-zhu alx-zhu changed the title Fix/gif ordering Fix/gif ordering: Fixing frame ordering issue for uploading costumes using GIFs. Dec 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uploading gifs as costumes imports images in wrong order
2 participants