leopard-js / sb-edit

Javascript library for manipulating Scratch project files
MIT License
55 stars 15 forks source link

fromSb3: Peculiarities in blockWithNext #143

Closed towerofnix closed 5 months ago

towerofnix commented 6 months ago

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 commented 6 months ago

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)