Broadcast Inputs and Implicit Message Deletion

[kchadha]

Resolves

Resolves #1268
Resolves #1270
Resolves #1290
Resolves #1304

Proposed Changes

Broadcast blocks (namely broadcast and broadcast and wait) now have pluggable inputs.
New messages can once again be created from the flyout (resolving the issue in #1270).
New messages created from a block in the flyout are created as variables in the flyout's target workspace, thus appearing in all broadcast block dropdown menus.

A message is created when either:

1. the broadcast block dropdown menu is clicked for the first time OR
2. any broadcast block with the message selected in its dropdown field is dragged out of the flyout and created on the workspace (this is not new functionality) OR
3. the message is explicitly created using the New message option in any of the broadcast block dropdowns.

A message is deleted using the implicit deletion behavior from SB2. Most of this functionality is implemented in scratch-vm. A message is deleted (disappears from dropdowns) if all blocks referencing that message are removed from the VM. This deletion behavior updates every time the workspace is updated (e.g. switching sprites or switching between the blocks tab and the costumes/sounds tabs). When the workspace is updated, the broadcast blocks in the flyout update their fields according to SB2 behavior -- the broadcast message that comes first in sorted order will appear selected in the field of each of the broadcast blocks in the flyout. The sorting behavior is the same as that used to sort the options in the dropdown menu (alphanumeric sorting by message name).

The assertion that shadows cannot have variables is no longer true, so the assertion statement has been removed.

Diversions from SB2

SB2 projects that have empty string message names will be converted to a fresh name message#.
In SB2, plugging in an input, e.g. ((1) + (1)), into a broadcast [foo] or broadcast [foo] and wait block, and then taking that input back out will result in removing the old message name foo and replacing it with the empty string. In the proposed changes, foo will remain in the message dropdowns, as well as in the shadow behind the plugged in input.

The reason for these divergences are to avoid the potential for buggy behavior when introducing this round-about way of creating empty string message names when they are otherwise disallowed from explicit creation.

If we decide that we would like to maintain this SB2 behavior, that change can be made.

Reason for Changes

To maintain compatibility with Scratch 2.0 which allows blocks to be plugged into the broadcast block inputs, and has the implicit deletion behavior.

Test Coverage

Existing tests pass.

Additional Notes

This PR depends on corresponding PRs in scratch-vm and scratch-gui:
LLK/scratch-gui#1033
LLK/scratch-vm#860

These three PRs can be tested together here.

[rachel-fenichel]

No variables in shadows: read this thread.

[kchadha]

@rachel-fenichel, thanks for the pointer to the thread. That makes a lot of sense. I guess this should also factor into the discussion for #1290. If we explicitly disallow deletion/renaming for broadcast messages (not saying this is what will necessarily happen), would this then be a non-issue? e.g. Is deletion/renaming the only consideration we should make when deciding to have a field_variable in a shadow?

[rachel-fenichel]

I think deletion/renaming is the only concern, but it includes implicit deletion/renaming.

Variable fields may now be better at creating variables at the right time, so maybe it's okay. I would experiment with some of the workflows from that thread to see if anything ends up feeling too weird.

[kchadha]

Right now I have no deletion of broadcast messages implemented at all (explicit or implicit), so testing that out will probably have to wait until we make a decision about what deletion behavior should be.

[kchadha]

@paulkaplan Does it make sense to change this and corresponding PR's mentioned above to be against develop instead of these new separate branches ('broadcast-messages' branch)?

My original intuition for the separate branches was so that they would be a staging area for all of the new broadcast block changes (including merging in Rachel's changes from upstream blockly). I think this might not make sense anymore, since merging in Rachel's work will be a pretty substantial change/refactor anyways, and it's not planned for our current milestone.

[paulkaplan]

LG

[rachel-fenichel]

Did you check for problems related to variables in shadows?

[kchadha]

@rachel-fenichel I believe we don't have any issues with variables in shadows right now. Variables in shadows aren't implicitly deleted unless all blocks (including obscured shadows) with a particular variable are deleted.

This is not the same behavior as in scratch 2 where a block covering a shadow deletes the underlying variable in the shadow field.

We decided to avoid this in Scratch 3 (unless an issue with it comes up) because in Scratch 2, this is how the empty string names are created, and we wanted to avoid those in Scratch 3, if possible.