Add glide (number) secs to [dropdown] block #662
Conversation
|
@ericrosenbaum seeing as you were requested for review, any comments? /cc @thisandagain |
|
We should consider refactoring to reduce duplication of the code (which could be a source of bugs later). So instead of copying and pasting the code from goto and glide, we should make two new helper functions, one for target selection and one for gliding, that can be called by the other blocks. What do you think? |
|
Also note that to update the menu with the names of the sprites, we'll need to add a function in scratch-gui like this. |
|
Ah, I see. Will change this tomorrow :D |
|
@ericrosenbaum Note: I made |
|
Hmm, I knew something was wrong but for some reason I can’t see Travis logs. Will look into it when I can. |
src/blocks/scratch3_motion.js
Outdated
| if (!goToTarget) return; | ||
| targetX = goToTarget.x; | ||
| targetY = goToTarget.y; | ||
| } | ||
| util.target.setXY(targetX, targetY); | ||
| return (targetX, targetY); |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
Hmm, I was wondering if that would be a problem. I don’t really know how JavaScript global/locals work though so a little help would be appreciated (assuming I don’t research it first). |
src/blocks/scratch3_motion.js
Outdated
|
|
||
| goTo (args, util) { | ||
| let targetXY = getTarget(args.TO, util); | ||
| util.target.setXY(targetXY[0], targetXY[1]); |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
Ah, “getTarget” is not defined - I’ll really need some global/local help here. |
src/blocks/scratch3_motion.js
Outdated
| } | ||
|
|
||
| goTo (args, util) { | ||
| const targetXY = getTarget(args.TO, util); |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
@towerofnix Finally, Travis is happy :) Final question is, I guess: |
src/blocks/scratch3_motion.js
Outdated
| @@ -51,24 +52,31 @@ class Scratch3MotionBlocks { | |||
| util.target.setXY(x, y); | |||
| } | |||
|
|
|||
| goTo (args, util) { | |||
| getTarget (TO, util) { | |||
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
Rename function getTarget to getTargetXY Rename parameter TO to targetName
|
@rschamp Done! Travis is still happy so that's a plus :) Merge when you will. |
Resolves
scratchfoundation/scratch-gui#598
Proposed Changes
Adds the block to
motion.js, basically copying target selection from go to and copying the glide from glide.Comes with scratchfoundation/scratch-blocks#1046 and scratchfoundation/scratch-gui#634