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: Peculiarities in blockWithNext #143

Closed
towerofnix opened this issue May 30, 2024 · 1 comment · Fixed by #146
Closed

fromSb3: Peculiarities in blockWithNext #143

towerofnix opened this issue May 30, 2024 · 1 comment · Fixed by #146
Assignees
Labels
bug Something isn't working compatibility Mismatch or currently unsupported language behavior fmt: SB3 Pertains to SB3 format (Scratch 3.0)

Comments

@towerofnix
Copy link
Member

towerofnix commented May 30, 2024

Investigating #141.

A simple change in blockWithNext to help debug it:

 if (sb3Block.next !== null) {
+  console.log(block.opcode, '->', sb3Block.next);
   next = blockWithNext(sb3Block.next, blockId);
 }

Last few messages leading up to error are:

data_deletealloflist -> c:
data_deletealloflist -> F
procedures_call -> %
procedures_call -> mk
argument_reporter_string_number -> undefined

This project doesn't appear to be generated from a normal Scratch editor, i.e. those are extremely short IDs compared to a normal project:

_whenflagclicked -> qSAt|FhG1SUUsVRn{iDY
procedures_definition -> :r0{L0I?SV?{-Hk.a(.D
event_whenflagclicked -> qLuxW0/!F{U*KyZ0T6tJ

So it's quite possible that the sb3Block.next === undefined instead of null like we're checking for is an exemption the minifier or project saver this project was made with took. (Unmodified scratch-vm has no problem loading this.)

Still, it seems like this is something we are more or less trying to detect but... not treating appropriately...? Check out a little bit earlier in blockWithNext:

const block = new BlockBase({
  opcode: sb3Block.opcode,
  inputs: getBlockInputs(sb3Block, blockId),
  id: blockId,
  parent: parentId,
  next: sb3Block.next ?? undefined
}) as Block;

What's with that ?? undefined, if we're going to be comparing it against null immediately thereafter? Shouldn't it be ?? null?

@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 self-assigned this May 30, 2024
@towerofnix
Copy link
Member Author

OK, no, that was a misread of the code. sb3Block.next !== block.next and ?? undefined is to coalesce to the parameter interface for BlockBase. We need to represent sb3's Block.next, as string | null | undefined (even though Scratch probably doesn't generate next: undefined). Probably Block.parent also for safety. (Or next?: string | null etc)

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 a pull request may close this issue.

1 participant