-
Notifications
You must be signed in to change notification settings - Fork 29
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
Add Sprite.dragging ("set drag mode to") + reflect Scratch "click" behavior #201
base: master
Are you sure you want to change the base?
Conversation
That video is FASCINATING. What a cool find. Thanks for putting so much effort into matching Scratch's behavior. Like I said earlier, I'm away for a few days (typing this on my phone) so I can't review until later. |
Thanks @PullJosh! 🥳 It blew us away too. Just such a funny little thing. This pull request is now ready for review. We touched the code up with improved in-file behavior documentation, moved the listeners into separate functions, and gave it a general review for logic smell, so it should be pretty good to review. Please let us know when reviewing if anything is confusing or difficult to wrap your head around. (And no rush, of course!) |
We also (of course) still need to file the sb-edit pull request. Not getting to that tonight 😅 We need to make sure the new property names (mainly Unfortunately, I don't think there's getting around breaking existing projects which have defined custom blocks (i.e. methods of the sprite subclass) called |
That's a good point. Technically this is a breaking change because people are extending |
Yes, bumping the major version is appropriate where Leopard depends on behavior/names that will collide the ways users have already extended their Of course the most ideal approach is to just not collide with those, but since the syntax we desire counts on adding a new property ( In the future we could use symbols as a completely alternate approach, to avoid unexpected collisions: // in Leopard's code
class Sprite {
...
static draggable = Symbol.for('Sprite.draggable');
...
}
// in your code, e.g. generated code
class MySprite extends Sprite {
myScript*() {
...
this[Sprite.draggable] = true;
...
}
} Then "Sprite.draggable" becomes the token, and will almost certainly never cause a collision. If we did this, it would mean changing all Leopard properties to use this sort of syntax. That means changes like this: -this.x += 20;
+this[Sprite.x] += 20; It would enable sprites to name their own variables ...but may or may not be a pain for actually writing code ✨ Obviously, if we used a syntax like this, apart from being a new major version, we'd need a button in the Leopard editor to automatically update existing code to use the new syntax. It's worth putting into perspective that Though, we don't have support for more extensions than just Pen... and with that in mind, maybe a syntax based on the category of blocks would be appropriate...? -this.x += 20;
+this[Motion.x] += 20; The reasoning here is that users have the opportunity to code their own extensions, the same way we code What do you think about that? For now, I don't think there's a trouble with sticking with (The only current inconvenience is that, um, I believe all sprites uniquely import right from the |
I think the more verbose syntax would be way too annoying to use in practice ( It would be neat if we could build a code-mod to help users upgrade Leopard versions automatically. Ideally it would be usable from the CLI but also be integrated directly into the Leopard editor. In the meantime, newly-generated Leopard projects would use the new major version and that alone would be pretty good. |
It probably won't happen that Leopard ever adds a new property that all sprites have again (setting aside compatibility with a hypothetical Scratch 4.0), so I don't know that spending the time to develop an entire dedicated tool is worth it. It would functionally just 1) check if you use We disagree that it's annoying in practice, but we're way more tolerant of this "nonsense" than average LOL - that is why we asked. Do note it's |
Resolves #123.
Adds support for player-style sprite dragging. (Scratch has two drag styles; they're mainly visually different, e.g. so that you can drag a sprite right out of the stage and into your backpack. We only consider player-style dragging relevant for now.)
Draggability is enabled for a sprite by setting
this.draggable = true
, which is analogous to "set drag mode to (draggable)". Sprite initialization properties can also specifydraggable: true
.Behavior is entirely modeled off of Scratch. We match these things:
mousedown
andtouchstart
as "pointer press", andmouseup
andtouchend
as "pointer release". Otherwise terminology is still in terms of the mouse (since it'sthis.input.mouse
, "mouse x", "when this sprite clicked" etc).You should review all the above behavior to confirm it's working as expected. A review looking for missed edge/corner cases would also be appreciated. We're slightly concerned about the lifetime of a clone, but we believe it should at least be completely discarded as soon as the mouse is released (and immediately, if it was actively being dragged).
(*) Original behavior was as follows:
(**) Oh, but it gets dumber. Scratch checks to start a drag every frame (as long as no sprite is currently being dragged). If a draggable sprite enters where your drag started, then that is what gets picked up, regardless where your mouse is now, when you perform a mousemove and you've met the "far enough from where I started" condition. Same behavior if the drag-start was attempted due to the 400ms one-off timeout: doesn't matter where your mouse is now, the attempt is based on where you pressed the mouse down.
This means you can - in Scratch, and Leopard too - start a drag based on where you pressed down the mouse, despite your mouse now being lord knows where-else. Demo below!
scratch-drag-clear.mp4