sb-edit icon indicating copy to clipboard operation
sb-edit copied to clipboard

Block inputs' types don't appear to ever account for reporter blocks being plugged in

Open adroitwhiz opened this issue 1 year ago • 1 comments

I really hope I'm just missing something here, because this seems to undermine the entire type-safety of the library.

Right now, it seems that the Block type is set up such that a block's inputs are always assumed to hold literal values and never any reporter blocks plugged into them. For instance, the operators_add block is always assumed to have two BlockInput.Number inputs, each of which holds a numeric literal. The possibility that these inputs may also be occupied by other (reporter) blocks doesn't seem to be considered.

So far, there are a couple reasons that this has managed to sneak through the API:

  • All serialization code written so far seems to have a generic "serialize inputs" function that treats all inputs as BlockInput.Any, meaning that it handles the case where those inputs are blocks.
  • The Block type is a union of KnownBlock and UnknownBlock. Since UnknownBlock is actually just a supertype of KnownBlock where the opcode can be any string (including redefining known opcodes) and all inputs are BlockInput.Any, all code which uses the Block type cannot make any assumptions about the "shape" of the block's inputs based on its opcode, and must handle all cases.

Even so, I've already found a case where serialization errors out as a result of incorrectly narrowing blocks' inputs based on opcodes: the "change pen (param) by" block cannot be converted if a reporter block is plugged into the menu: image image

There are probably many more instances of this throughout the codebase. I think there needs to be a major rework of the Block class in order to fix this soundness hole, especially if we want to use TypeScript to its full potential. We may want to consider separating inputs and fields the same way Scratch does, so that we can tell which inputs may and may not have blocks plugged into them.

adroitwhiz avatar Mar 06 '23 03:03 adroitwhiz

I'd say you're (unfortunately!) right in your assessment here — I ran into a number of issues which were flavored by this when reworking toLeopard.ts recently.

We may want to consider separating inputs and fields the same way Scratch does, so that we can tell which inputs may and may not have blocks plugged into them.

I'm feeling this. Since Scratch 3.0 is the base for KnownBlock, we should be able to accurately and completely reflect information about those blocks in sb-edit — including the difference between inputs and fields.

Since we're investigating block inputs here, it may be worth recognizing that Scratch actually keeps a record of the manually entered value of an input "behind" a reporter occupying that input slot. For sb3 -> sb3 transparency, we may want to extend the input (not field) class to keep track of that value.

towerofnix avatar Mar 07 '23 21:03 towerofnix