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

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

Open
adroitwhiz opened this issue Mar 6, 2023 · 1 comment
Labels
API / interface Relevant to object structures and interfaces beyond serialization discussion Looking for feedback and input

Comments

@adroitwhiz
Copy link
Collaborator

adroitwhiz commented Mar 6, 2023

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.

@towerofnix
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API / interface Relevant to object structures and interfaces beyond serialization discussion Looking for feedback and input
Projects
None yet
Development

No branches or pull requests

2 participants