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

fromSb3: Fix loading component-based pen color blocks; document shadow blocks' fields more carefully #145

Merged
merged 3 commits into from
Jun 17, 2024

Conversation

towerofnix
Copy link
Member

Development:

Hard-codes(!) a special-case fix for the pen_menu_colorParam shadow block; we reviewed all shadow blocks that Leopard supports and would have included like fixes for any similar cases, but this is the only one.

pen_menu_colorParam is different from normal SB3 shadow blocks because its field is named colorParam and does not match the name, COLOR_PARAM, of the input into which it is placed. All other names do match and we can just apply the shadow block's field as an sb-edit input with the same name, but this one needs to be manually transferred from sb3 field colorParam to sb-edit input COLOR_PARAM.

This fixes loading blocks which use this input (there are two in the Pen extension) and makes it so output formats see the COLOR_PARAM input that they're expecting. Our testing:

  • Manually tested with project Set/change pen (color...) menu tests
  • Manually tested with project WebOMatic!
  • Both toLeopard and toSb3 are working, and don't require any changes.
  • Updated snapshot test (which loads with fromSb3 and writes with toLeopard,toSb3, and toScratchblocks)
    • Internal project.json is now pretty-printed w/ JSON.stringify for more convenient git review. The SB3 is very old and saving it from the Scratch editor without making any changes will still produce a flat-out different file (in subtle ways), so the diff was manually reviewed to only commit the code changes. This should make for a clean diff in the snapshot file.

Also improves the clarity of some nearby comments.
Of note, only pen_menu_colorParam requires special behavior due
to a mismatch between the shadow block's field name and the using
blocks' input names.
Essentially just adds the pen extension to the blocks tested.
The internal project.json is pretty-printed with JSON.stringify
for slightly easier git committing (though the sb3 is still
committed as a binary zip file).
@towerofnix towerofnix added bug Something isn't working compatibility Mismatch or currently unsupported language behavior fmt: SB3 Pertains to SB3 format (Scratch 3.0) labels May 30, 2024
@towerofnix towerofnix changed the title Fix loading component-based pen color blocks; document shadow blocks' fields more carefully fromSb3: Fix loading component-based pen color blocks; document shadow blocks' fields more carefully May 30, 2024
@towerofnix
Copy link
Member Author

Have requested a review from @adroitwhiz since this is probably at least of note to you, but all cool if you aren't around to look at this closely at the moment!

Copy link
Collaborator

@adroitwhiz adroitwhiz left a comment

Choose a reason for hiding this comment

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

LGTM! At some point, it may be worth revisiting shadow blocks but this should fix things for now.

@PullJosh PullJosh merged commit bed196c into leopard-js:master Jun 17, 2024
1 check passed
@towerofnix towerofnix deleted the pen-color-param branch June 17, 2024 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compatibility Mismatch or currently unsupported language behavior fmt: SB3 Pertains to SB3 format (Scratch 3.0)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fromSb3: Peculiarity with pen_menu_colorParam as shadow input (etc?)
3 participants