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 undefined sb3Block.next #146

Merged
merged 2 commits into from
Jun 17, 2024

Conversation

towerofnix
Copy link
Member

Development:

See #143 for some technical discussion.

Best to view the commits separately, as they are independent changes, though they affect the same area of code.

  • Makes undefined (missing) next on blocks in a project.json not crash fromSb3. This may be a omission by a minifier used in the demo project, rather than something Scratch itself will ever serialize, though Scratch handles deserializing undefined next just fine, so sb-edit should too.
  • Improves some code clarity. blockWithNext is sometimes called without its parentId argument so we just make it parentId?: string instead of parentId: string | undefined. And getBlockScript, previously called once for each script in a target, is now called just once for each target, because getBlockScript does not care which script you're about to get, and is structured to make all scripts available. (It's a currying function, so now we use it like one!)

Tested manually with the project shared in #141, WebOMatic!.

Also cleans up the parameters for blockWithNext.
getBlockScript returns a function which is useful for getting any
top-level script, and getBlockScript only operates on the total
list of the target's blocks, which doesn't change dependant on the
script you are trying to access. So use it like the currying
function it presents itself as and don't re-run getBlockScript
for every script in the target.
@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 requested a review from PullJosh May 30, 2024 16:58
Copy link
Collaborator

@PullJosh PullJosh left a comment

Choose a reason for hiding this comment

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

These code changes look sensible. CI says it compiles and passes all tests, but I'm not at a computer so I haven't been able to personally test the new behavior yet.

Copy link
Collaborator

@PullJosh PullJosh left a comment

Choose a reason for hiding this comment

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

Tested and looks good 👍 Thank you!

@PullJosh PullJosh merged commit 9b0f978 into leopard-js:master Jun 17, 2024
1 check passed
@towerofnix towerofnix deleted the block-with-next branch June 17, 2024 20:49
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: Peculiarities in blockWithNext
2 participants