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

[Spritelab] Add Mini Toolbox to event blocks, behind experiment #34603

Merged
merged 5 commits into from May 6, 2020

Conversation

ajpal
Copy link
Contributor

@ajpal ajpal commented May 4, 2020

Experiment flag: miniToolbox

Done:

  • miniToolboxBlocks as a new option in the block config
  • generated code changes to make everything work
  • subject/object pointer blocks

To Do:

  • set up shadowing for subject and object blocks
  • make the blocks within the mini toolbox shadow the costume
  • add the word "sprite" on the block when there's no costume to show
  • Make it configurable on levelbuilder whether or not to show the minitoolbox

image

image

Without experiment:
image

@ajpal ajpal added the star-labs label May 4, 2020
@ajpal ajpal marked this pull request as ready for review May 4, 2020 23:18
@ajpal ajpal changed the title Mini Toolbox [Spritelab] Add Mini Toolbox to event blocks, behind experiment May 4, 2020
});
miniToolboxXml += '\n</xml>';
this.tray = false;
Blockly.bindEvent_(toggle.fieldGroup_, 'mousedown', this, () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: Mind leaving a quick comment explaining what's going on here? It makes sense next to the visual, but is less obvious when reading the code directly.

case 'gamelab_clickedSpritePointer':
return '{id: extraArgs.sprite}';
case 'gamelab_subjectSpritePointer':
return '{id: extraArgs.sprite}';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For PR documentation: When I first read this, I was expecting to see three different IDs (i.e. extraArgs.sprite is here twice). Why is the .sprite ID reused?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we detect an event, we call back into the interpreted code, passing in any relevant info in the extraArgs parameter. For click events, there's just one sprite, so the shape of that object is {sprite: <some id>}. For touch events, there are two sprites, so we have {sprite: <some id>, target: <some id>}
The generated code for the clicked/subject/object pointer blocks just need to evaluate to the appropriate sprite, referenced by its id, so the generated code should be {id: <some id>}. In the case of click events, the relevant id is passed in as extraArgs.sprite and in the case of touch events, the ids are extraArgs.sprite and extraArgs.target

Copy link
Contributor

@jmkulwik jmkulwik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super exciting that this is happening! I had one quick documentation ask, if you don't mind (and a nit). Besides that, good to go :)

@ajpal ajpal merged commit 83e7cff into staging May 6, 2020
@ajpal ajpal deleted the apr30-mini-toolbox branch May 6, 2020 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants