-
Notifications
You must be signed in to change notification settings - Fork 28
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
Extract control flow ("triggers") logic #197
Comments
Judging by your code and discussion, I'm reading three assumed "moments" of responsibility, each of which has some contextual nuances:
These are all fundamentally present in your proposal, and carry inherent responsibilities, currently:
The steps only depend on access to the details I've described. I.e, the code dispatching an event has to know the name of the event it wants to dispatch, as well as any data to send alongside it, and have a reference to a specific target / set of targets, if desired. But it doesn't need to interview those targets for a list of the targets' handlers, or about the meta state of those handlers. A key moment that does not exist in your proposal:
That is, you don't indicate that any event name is ever, inherent to that event name, associated with different behavior. You don't suggest that a listener for I like that, I think! Will probably touch on this some more while responding to your points. An "extra" moment that you are also disregarding, but is actually present in DOM listeners:
DOM provides You've proposed dodging this altogether by simply letting
Relevant reference code to this point: myHandler.dispatchEvent("keyUp", { key: "a" }, { duplicate: "ignore" });
// If there is already a "keyUp" event running, ignore this newly-dispatched event.
// ^ This isn't quite good enough. In Scratch, key press events for the same key
// will be ignored but key press events for two different keys can run simultaneously.
// Is there a nice-but-still-generic way to represent that behavior?
// I don't have a good answer. I want to point out that there is no such thing in Scratch (excluding extremely obscure bugs and in-editor cases) as two copies of the same script running at once. There's no such thing as running a "duplicate" of the handler for the event. Instead, event handlers (scripts with hat blocks) either can or cannot be interrupted and restarted. So I'm going to rewrite this dispatch like this instead, for clarity: myHandler.dispatchEvent("keyUp", { key: "a" }, { interrupt: "skip" });
// possible "interrupt" property options:
// - "restart" (discard current evaluation state, start over)
// - "skip" (retain current evaluation state, drop/discard event)
// - "queue" (retain current evaluation state, start it again afterwards)
// [there's no such thing as "queue" in real Scratch, just providing
// as an example of possible behavior here] We still need to address deciding which handlers to restart, though. Of course, at the end of the day, the problem is that listeners aren't describing what they are doing and so we, Leopard (at the moment "when an event listener is about to be activated") have no idea whether this listener is suitable for restarting or not. By ruling out the moment "when a listener is evaluated" (no Let's think about that third step, the current one we're hypothetically evaluating, in more detail.
OK. Let's think about Scratch scripts now.
Scratch has hat blocks that don't actually provide context to the scripts beneath them. Really, all a hat block is responsible for is telling the runtime information about this "event handler" (the blocks beneath it) — giving enough information for the runtime to decide whether or not to run the script! I think the example above is actually a useful one to try out in Scratch, yourself. Go put those blocks together! Did you see that when you pressed any key, it started the "pop" script, when you followed that up by pressing some other key, it didn't start the script all over again? Even though an event with different "details" is what caused the hat block to trigger the first time, you still had to wait for that response to finish before any new events you sent would go through. OK, OK, secretly there are actually two ways forward here. I'm going to reel back a little because the first one is so simple it might sting a little, and it'll be nice to have out of the way, first: If you want to, otherwise unconditionally, not interrupt a running handler, the only information needed is whether or not the handler is currently running, and we don't need to change anything to be able to observe that. All we need to do is keep track of whether or not each handler (for the relevant event name) is currently running. If a The second approach is to go for something perhaps a little bit closer to Scratch's blocks. You might represent the scripts above this way, in Leopard: this.addEventListener("keyPressed", { key: "a" }, function*() {
yield* this.sayAndWait("A is for: I'm *A* happy camper!", 4);
});
this.addEventListener("keyPressed", function*() {
yield* this.startSound(this.sounds.pop /*or whatever */);
yield* this.thinkAndWait("I have no idea what key was pressed... Oh me oh my...", 3);
}); Say you press the A key, now.
Well, this way you still need to keep track of which scripts are running, because otherwise "interrupting" doesn't even mean anything... I wanted to bring this up as an alternate syntax of dealing with event listeners, but I don't like it for a couple reasons:
I think the simpler solution (necessary anyway for reflecting Scratch behavior) is better: just know which ones are running, and avoid interrupting those if the dispatch was provided
You know, it didn't click until right now that clones don't re-run the But yes, as long as the listeners are never arrow functions, they can be rebound and it shouldn't be very hard to have whatever part of code is actually responsible for evaluating those handlers, to call them bound to the applicable clone, rather than the applicable sprite. Note that you can't bind a function twice: f = function() { return this }
obj1 = {a: 'b'}
obj2 = {c: 'd'}
console.log(f.bind(obj1).call(obj2)) // {a: 'b'}
console.log(f.call(obj2)) // {c: 'd'} So we would have to never literally use
Hmm. I mean, there are actually very few events that have to do with the current sprite. Those ones are simply meaningless if you take them out of the context of that sprite. Does "when this sprite click (but psst I mean anywhere in the project)" mean "when any sprite is clicked" or does it mean "when any sprite or the stage is clicked" or does it mean "when the got dang project is clicked"? When-key-pressed isn't dispatched on individual targets, it's dispatched on the project and then the event is disseminated to targets. No target "owns" the event, it's just responding like a "when I receive" block. (Fun fact: in super early versions of Scratch, the green flag was literally coded as a broadcast!) The problem is that this is backwards to how JavaScript treats propagation. In JavaScript, if you want to detect a key anywhere on the page, you have to add that event right on the document body or the window. You can't add it onto yourself (some random element), because that element won't receive the event unless it's focused! In Leopard we are adding events right onto ourselves, and the project is disseminating the events to us. It's bubbling down instead of up. It's still bubbling, so it's a similar behavior, but um... the direction is more like an anvil than a bubble, you know... I don't know if there is a good way to get around this. In JavaScript if you have a component that does not live forever but does listen to things outside of it, it needs both a constructor and a tear-down function, and JavaScript literally just doesn't support this by default - you or a framework takes responsibility for calling that teardown function. As they say, there is a bit of ✨ magic ✨ that Leopard should probably be taking care of for anyone coming from Scratch, but it's inevitably going to be a) hacky and fragile or b) inconsistent with "normal" JavaScript behavior. I'm inclined towards the latter of those two evils. Accordingly, I think we should avoid using terminology that is carefully tied to typical event behavior of JavaScript, because we just can't reflect it perfectly if we want to take care of responsibility that wouldn't typically be ours. IMO adding event handlers on
I don't think binding is a problem at all! Like we discussed above, dissemination is down into sprites. Whatever is running the receivers understands which target owns the receiver, because the target added the receiver to itself. this.addEventListener("myCoolEvent", this.mySpriteMethod);
project.dispatchEvent("myCoolEvent", {foo: "bar"});
// this runs myCoolEvent handlers on all targets by iterating over
// those targets, and of course it knows which target it's
// currently iterated onto, and runs the handler bound to
// that target! IMO all events dispatched onto project are disseminated onto all targets. That is just what Target-specific events are as simple as
IMO this is the wrong approach. You couldn't possibly have two receivers for different broadcasts running at the same time unless you exposed information about the event handler itself, which I described being against earlier. I don't think broadcast dissemination has any special problem though. You just add the event listener onto yourself, dispatch on the project, project disseminates event to all targets, all target handlers are run with appropriate "this" binding. |
Sorry most of that rambling is probably pretty poorly structured LOL. Here's a TL;DR! Overall I love the proposal. It's slick and clean and looks both easy and fun to use. It's related to JavaScript code patterns instead of being soaked in Leopard-isms — something we've agreed, with consensus, is a really good thing, before! #177 (comment) We definitely don't claim to hold the same opinions as whatever we wrote back then though LOL. Not that they wouldn't have any merit - it's just that, in general, avoiding code particular to Leoaprd and using language that's closer to JavaScript is mostly a good idea. However, we don't want to go overboard with "looking like" normal JavaScript. Because, normal JavaScript isn't coded the way that Scratch is — constructors and teardowns just for dealing with event listeners are normal in JS and total nonsense in Scratch (and maybe other game-oriented programming languages, too!). If we are going to hand-wave certain idiosyncrasies of JavaScript away, we need to be careful in what JS behavior is tied to those, and not act like we're representing those, either. This is why I'm in favor of a more simplistic approach where dispatching on the project always disseminates the event to all targets, and we just use alternate event names for events, like We wrote a lot of technical details but in general aren't too worried about running into technical issues - not with clones, not with broadcasts, not any of that. The level of simplicity actually puts Leopard projects in an easier place to get implemented without a ton of pain! I think it aligns well with Scratch and that basically all of the ✨ magic ✨ that Leopard wants to be responsible for - it can be responsible for, without much difficulty. As long as code interfaces with Leopard the way Leopard needs: it's impossible to restart promises so please use generators if you want interruptible async, it's impossible to rebind an already-bound function so please don't use arrow functions, just pass in a normal non-bound function, perhaps defined on your |
I was struggling to keep track of all these details in my head, so I created a proof of concept event listener system. https://codepen.io/PullJosh/pen/eYazxax?editors=0010 It supports event listener callbacks that are functions, async functions, generators, and async generators. When you dispatch an event, you can choose to restart currently-running iterators or skip those that are currently running (using I also created a boolean option called I tried to set up the demo environment to be as easy to play with as possible. Please give it a try and check if the behavior all seems correct. (I'll be honest–the code is kinda ugly. But I think the behavior might be correct.) |
By the way, I've always been a reckless driver when it comes to testing code, but this little bit of logic in the CodePen above feels like precisely the kind of code that would benefit most from automated unit testing. It feels like a good sign that extracting the code out like this puts it in prime position to be easily testable, independent of the rest of Leopard's behavior. |
Wow, this is impressive. We admittedly jumped into the example before reading your comments above properly through. I was expecting to ask if you were being aware of Scratch not actually executing scripts as soon as the corresponding event occurs (i.e, "priming" hat blocks to be executed during the next frame), but then I saw you have an "Immediate mode" checkbox and it's on by default (for the demo). Nice! "Immediate mode", I believe, doesn't exist in Scratch except for custom blocks, which aren't internally handled the same way (but do exhibit running a different script instantaneously, like "immediate mode" does). My impression is that with "immediate mode" enabled all the behavior lines up with what I'd expect, which is awesome. But I don't have a "canon" behavior to compare "immediate mode" against, so for the sake of review I'll be focusing on behavior with "immediate mode" disabled. We'd encourage you to make a clone/copy of this pen before looking into fixes, so that the original is kept around and you/we can compare the implementations. Table of cases:
Working (Case A): Two same-frame clicks of
Not working (Case B): Following
Not working (Case C): Two same-frame clicks of
I'd expect it to be just the same as clicking Sprite1 once, because there isn't an active script to restart during frame 1. (I've only "primed" it for executing on the next frame, I have not begun execution.) Not working (Case D): Following
This is exactly the same behavior as Case B. Working (Case E): Following
Working (Case F): Following
This is exactly the same behavior as Case E. Working (Case G):
Working (Case H):
This is exactly the same behavior as Case G.
Totally! Hopefully following through these manual tests gives you an impression of what behavior we'd expect. Totally encouraged to ask if you're confused or wondering about anything. Automated tests will definitely assist making sure a working implementation keeps working. It looks like there are more examples coded to check out ( Thanks a bunch for putting this together!! We haven't taken a look at the code either, except for the example Leopard project (definition for |
Adding a quick flag in the sand here to point out that decorators are not currently supported in JavaScript but they have been in TypeScript for years. If decorators ever come to JS it would be lovely to have. class MySprite extends Sprite {
constructor() {
// ...
// No longer needed!
// this.addEventListener("greenFlag", this.whenGreenFlagClicked);
}
// Yay for decorators 🎉
@greenFlag
*whenGreenFlagClicked() {
// ...
}
} (@towerofnix I'll investigate your response more when I can) |
They're workin' on it! Stage 3, so waiting on progress from implementers (which is always moving, slower or quicker). https://github.com/tc39/proposal-decorators |
In discussions about integrating with Patch (#195), it seems to be clear that we can improve the design of Leopard to make it more extensible and valuable for users if we extract out what we currently call the "trigger" system into a separate, cleaner, more general-purpose control flow library/package/module/something so that users of the Leopard library can choose whether or not they want to use it. (Or, one might want to mimic Scratch's control flow without using everything else that Leopard provides.)
Additionally, as I've spent more time editing Leopard projects by hand, I have found myself annoyed with the triggers system. The way we currently set up a triggers array in the sprite constructor...
...is very functional and reasonably concise (although we could definitely make it more concise). But it is hyper-specific to Leopard and is not really similar to the way that other JavaScript code generally handles events.
I think a more similar-to-js API would be to have some kind of "event listener" system analogous to the DOM
element.addEventListener("click", handler)
API. I see a few main distinctions that would set our API apart:addEventListener
setup.this
. The current triggers system is very careful to make sure that whatever method you pass is bound to the sprite so thatthis.whatever
works as expected from within the called method. The DOMaddEventListener
API does this differently, usingevent.target
instead, but I definitely think I prefer thethis
binding for Leopard's purposes.If we're extracting this event/control flow logic into its own module (or something like that), I think it makes sense to have a generic API that can be used outside of the context of OOP (where events are attached to a particular sprite instance and so on), but that can be easily plugged into the Sprite/Stage/Project class system as needed.
Here's a proposal for the generic API:
This generic API could then be integrated into a Leopard project like so:
There are a couple of problems and open questions with this approach:
EventEmitter
can be given a sprite instance to bind to, it won't be too bad.Project
and you can listen to events on the whole project if you want?this.project.addEventListener("myCoolEvent", mySpriteMethod)
and you send out a broadcast by doingthis.project.dispatchEvent("myCoolEvent")
, but then you have to be careful to make sure that the listener method is somehow being bound to the correct sprite–because by default it would be bound to the project. Or, alternatively, do we have a dedicated "broadcast" method that just provides the broadcast name to the listener?The text was updated successfully, but these errors were encountered: