keyIsDown function

[limikael]

Please see this as the start of a discussion rather than something ready to integrate...

Anyway, the idea with this is to be able to check if a key is currently pressed, in order to get a game like behaviour where it is possible to press for two keys and be able to move a sprite diagonally.

check the example in keyboard/gamestyle.js

Two issues that I see currently:

• What should the name be? The best would be keyIsPressed but that is already busy. I chose keyIsDown in lack of better ideas but not 100% satisfied with it.
• It needs some cross browser/version/os compatibility testing and possible work arounds. It currently works in the latest versions of Chrome and Firefox on my Mac but I noticed it not working in some version of Firefox on some version of Linux. So a bit of testing needed...

Let me know what you think!

[zaerl]

I like this approach. I keep track of key pressed using keyPressed()/keyReleased(). As an addendum what about a sketch variable like keys[]?

[limikael]

@zaerl Glad you like it! About keys[], it sort of already exists as the private variable p5.prototype._downKeys, I guess it could be exposed if need be. But what would be the purpose? IMO it looks cleaner like this:

    if (keyIsDown(LEFT_ARROW)) {
        // do something.
    }

Than like this:

    if (keys.indexOf(LEFT_ARROW)>=0) {
        // do something.
    }

In order to check if a key is pressed. But maybe there are other use cases that would be solved by exposing keys[]? If so, which for example?

[zaerl]

I see a keys[] variable as a supplement to the keyIsDown call, not a substitute. If you expose all the keys you can iterate them during a draw call for example.

[limikael]

Ok I see... Makes sense... Anyone else who ha a chance to look at it?

[c3c1l1a]

I have tested this out on the firefox version(32.0) am using and it seems to work fine.

[GoToLoop]

@limikael, what you're proposing already exists across Processing frameworks.
And particularly in P5.JS in the following format: if (keyIsPressed & keyCode == LEFT_ARROW) {}

[zaerl]

Unfortunately keyCode contains the last keyboard key pressed, you can not keep track of more than one key pressed at the same time. You can only track them with keyPressed()/keyReleased().

[limikael]

@GoToLoop, yes as @zaerl says, it is only possible to track one key at a time using that method. Check this example:

https://github.com/limikael/p5.js/blob/master/examples/keyboard/gamestyle.js

And notice that it is possible to move the object diagonally by pressing two arrow keys at the same time. This is something you would expect if you are creating a game.

[GoToLoop]

You can only track them with keyPressed()/keyReleased().

That's the "traditional" way of keeping track of multiple keys across Processing frameworks! 👐

[GoToLoop]

This is something you would expect if you are creating a game.

Indeed it's much easier that way. Although it's delegated to the framework the task to keep track which keys are currently pressed! Dunno how much that would slow the framework. Perhaps nothing much!?

[GoToLoop]

And remember that Processing isn't exactly a game framework, but for graphics artists! 😎

[zaerl]

I think that this is a nice addition that does not slow down the entire thing 😄

[GoToLoop]

Although that'd abstract away the formal keyPressed()/keyReleased() way of doing that, it's undeniable how much more intuitive & easier & lazier it gets! 😜
Although it's more game focused, it's got some educational value! Who knows KhanAcademy.org/cs would swap ProcessingJS in favor of P5.JS once it acquires 3D capability? :squirrel:

[limikael]

Thanks for the comments!

Dunno how much that would slow the framework. Perhaps nothing much!?

Maintaining the list of currently pressed keys is done by keeping them in an array, which rarely contains more than 2 entries, so I would argue nothing much.

And remember that Processing isn't exactly a game framework, but for graphics artists!

As far as I understand, it is a framework for graphics artists who wants to learn programming. Wouldn't it then be in their interest to show a welcoming attitude towards other programmers? If the community around p5.js would grow to also include programmers who has an interest in game design, then the shared knowledge base of the community would grow, and this would be in everyone's interest I think.

[GoToLoop]

checkes -> checks. Remove extra 2nd vowel e...
behaivour -> behaviour or behavior. That is, swap mistyped vowel i to after consonant v. ^_^
simoultaneously -> simultaneously. That is, just erase the extra mistyped 1st o vowel! HEHE

[GoToLoop]

I've got a modernized & radical propose: Instead of a regular Array, why not a more specialized structure like Set? https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set

We only need 3 of its methods: add(), delete() & has(). And all of those 3 methods are already implemented in all 5 major browsers! So they're safe to use right now! 😺

• add() -> https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/add
• delete() -> https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/delete
• has() -> https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/has

Take a look at how more concise and easier your keyIsDown() gets after Set replaces Array: 😇

#L3003 @ lib/p5.js & #L18 @ src/input/keyboard.js:

p5.prototype._downKeys = []; becomes: p5.prototype._downKeys = new Set;

#L3012 @ p5.prototype.onkeydown = function (e) {} & #L123 @ src/input/keyboard.js

if (this._downKeys.indexOf(e.which) < 0) this._downKeys.push(e.which);
simply becomes: this._downKeys.add(e.which);

#L3032 @ p5.prototype.onkeyup = function (e) {} & #L170 @ src/input/keyboard.js

if (this._downKeys.indexOf(e.which) >= 0) this._downKeys.splice(this._downKeys.indexOf(e.which), 1);
likewise just: this._downKeys.delete(e.which);

#L3059 @ p5.prototype.keyIsDown = function (code) {} & #L269 @ src/input/keyboard.js

return this._downKeys.indexOf(code) >= 0;
a little shorter: return this._downKeys.has(code);

[GoToLoop]

Only thing that still puzzles me is why _downKeys is a property of prototype.
AFAIK, it's equivalent to Java's static keyword. And that means the very same _downKeys is probably gonna be shared among p5 instances rather than have its own instance, isn't it? 😰

[hamoid]

I haven't been following carefully, so I might have missed something, but why an Array or a Set, and not just an object? Is it slower? It looks more readable to me:

p5.prototype._downKeys = {}; // create
this._downKeys[e.which] = true; // add
delete this._downKeys[e.which]; // remove
return this._downKeys[code] === true; // read

@GoToLoop Isn't it the other way around? Without prototype it would be a static, with prototype each p5 object has it's own set of _downKeys. Or I'm too sleepy :)

[GoToLoop]

Without prototype it would be a static...

In Java, objects contain instance variable fields only. Rest of the members stay in their original class!
AFAIK, JS defines "class" static members in its prototype. Instance members go to this instead.

[GoToLoop]

It'd be a complete waste of RAM if methods/functions weren't static!
Let's say we create 1000 instances of p5.Vector. Imagine if all those methods would instantiate along!
Only instance variables should be instantiated! Methods, constants and shared fields should have 1 copy only and stay inside their original "class". In JS terms, the latter members should go to prototype!

P.S.: I still prefer Set over Object or Array. Although Object isn't that bad! 😉

[zaerl]

I think that explaining how javascript prototypal inheritance model works is far beyond the scope of this discussion.

Please just close this issue and merge the commit if you think that it's a good addition to the API.

[limikael]

Ok fixed some spelling mistakes... It would make sense to use Set here, but I think if we did we would risk missing some browser compatibility, since it is an experimental feature. Object would work and would probably be the fastest, but in the name of "make it work then make it better" I think we can stick with the array for now... If someone wants to create a pull request to my pull request I'll consider it though...

What about the original questions?

• Is the name keyIsDown good?
• Did anyone get a chance to test for browser compatibility? What sort of compatibility do we need?

[GoToLoop]

... to use Set here, but I think if we did we would risk missing some browser compatibility,...

That's true for the time being, but as I've mentioned already, Set is well-established now!
Take a look at "Browser compatibility" table at the end of its reference page:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set

But for the time being, just to give some more months for Chrome/Chromium version 38 to be more wide-spread, I'd go w/ @hamoid's idea of Object!

Is the name keyIsDown() good?

It is too! We've already got keyIsPressed: http://p5js.org/reference/#/p5/keyIsPressed
But the latter is a variable, not a function. Perhaps a function should have verb "is" before the subject "key", as in isKeyDown()? What does every1 else think about it? 😁

[lmccart]

hi @zaerl and all, I will review and merge if it makes sense next week after the holiday

[lmccart]

thanks everyone for the thoughtful discussion. @limikael one question, does _downKeys need to be part of the p5 prototype? since it's only accessed within the keyboard module, couldn't it just be an internal variable var downkeys = []?

I think the name sounds good, but would keyDown() make more sense? leaving the names with "Is" in them to be variables (keyIsPressed, mouseIsPressed)?

[limikael]

You are right @lmccart, there is no reason for _downKeys to be part of the prototype. I was going to change it and create a new pull request but saw that you had already changed it... :)

Hm, I don't know if keyDown would make more sense actually... It is true that names with "Is" in is only used for variable names currently, but it is only those two, so I don't know if that actually means that it feels like a "convention" for the users of p5.js. It is not really a convention in other libraries either, the closest that comes to mind is java: http://www.cwu.edu/~gellenbe/javastyle/method.html but it says nothing about "is" being reserved specifically for variables.

So I think keyIsDown sounds better than keyDown, also because it differentiates the function from keyPressed which is a callback. So maybe keyIsDown is the best actually...

[lmccart]

yes I was thinking about this again last night and I agree completely with your name reasoning, let's leave as is. thank you for adding this functionality to the library!! sorry it took so long to get it all merged in.

[limikael]

Ok! Thank you for giving me the opportunity to participate! :)
Den 4 dec 2014 14:09 skrev "Lauren McCarthy" notifications@github.com:

yes I was thinking about this again last night and I agree completely with
your name reasoning, let's leave as is. thank you for adding this
functionality to the library!! sorry it took so long to get it all merged
in.

—
Reply to this email directly or view it on GitHub
https://github.com/lmccart/p5.js/pull/411#issuecomment-65622891.

[darbicus]

I took a look at this issue a while back and I always had problems with the tab keys forcing all of the other keys to lock as being "down" after the focus was lost on the page while the keys were being pressed. So I wrote up a little fiddle that solved it. I'm not sure if you guys took this issue into consideration or already accounted for it. but heres the fiddle: http://jsfiddle.net/Darbicus/jfh8jgy8/

[hamoid]

Good ideas to empty the container on blur, and to store the key press
timestamps in case they're useful in your program.

It's something minor, as people can not press the keys thousands of
times per second, but for performance it might be nice to have a fixed
size array to store the keys that are down, and avoid 'delete'. They
would contain 0 if not pressed, or a timestamp if pressed.

Then, to test if the key is down one can check if the value is not 0.
And if you care about when the key was pressed, that information is also
available. It does not have to be exposed to the user, as p5js is
supposed to stay simple. But I think it would be nice if the timestamp
is there in case someone wants that info.

[GoToLoop]

... but for performance it might be nice to have a fixed size array to store the keys that are down, and avoid delete.

Dunno whether you've noticed it, but internal implementation changed from Array to Object as you advised us so: https://github.com/lmccart/p5.js/pull/444.
However, w/ a tiny difference, instead of delete downKeys[e.which];,
I've gone w/ downKeys[e.which] = false; ---> https://github.com/lmccart/p5.js/pull/445.
It's not fixed size, but downKeys never deletes already pushed keys. Only grows up! :D

[limikael]

@darbicus, it makes perfect sense and it is not something we have accounted for so far... it would be awesome to have that added, in my opinion... have you thought about adding it and creating i pull request?

[hamoid]

@GoToLoop great, I read objects were used, but didn't notice the =false part.
@darbicus The blur could be also used to optionally pause the animation. Is that implemented? processing.js offers that option, to avoid maxing the cpu when you are looking at other web pages.

[darbicus]

@hamoid yeah that's a good idea. I'm not sure if it is in p5.js because I didn't find anything like that in the code. I only saw a random event Listener for 'focus' and 'blur' that set the property focused to true or false.

I would add the functionality right now, but since you said "optionally." I believe we would have to discuss what the keyword that enabled this feature. animateOnFocus? or pauseAnimationOnBlur? I don't like either because they are too long. Is there a pause animation function? because then users could implement this functionality themselves if they wanted just by adding addEventLIstener('blur',function(){p5.pause();});

I pulled it to your fork limikael because it is your function and I haven't tested it to make sure the blur event even registers. But assuming they will it should work fine.

[lmccart]

yes, this is a great idea! there's nothing like this in there yet.

[hamoid]

Personally I would pause on blur by default... Imagine how many
gigawatts of power we could save when p5js takes over the world ;)

Probably this discussion should move to a new thread...