Add tests for input methods. #42
Conversation
|
|
||
| function mouseup(which) { | ||
| myp5._setProperty('mouseButton', which); | ||
| myp5._setProperty('mouseIsPressed', false); |
There was a problem hiding this comment.
Since this is using internal p5 methods, it may break when we upgrade the version of p5.js in the repository. Same goes for the use of _onkeydown and _onkeyup a few lines down.
|
I knew about this. I definitely want to fix it so keyDown and keyWentDown If we had a release process and semver I would bundle this with other |
| mousedown(LEFT); | ||
| nextFrame(); | ||
|
|
||
| expect(myp5.mouseWentDown(LEFT)).to.be.true; |
There was a problem hiding this comment.
I feel like for this test to cover the behavior of mouseWentDown properly it needs to run a third frame (as the mouseDown test does above) and show that mouseWentDown goes back to false.
There was a problem hiding this comment.
Good idea, just added that in d5cc6fb.
|
Ok, it looks like there are enough concerns about the fix for the bug that I'm just spinning it off into #43 and we can deal with it later! |
| expect(myp5.mouseWentDown(LEFT)).to.be.true; | ||
|
|
||
| mouseup(LEFT); | ||
| nextFrame(); |
There was a problem hiding this comment.
This isn't quite what I meant. With the expected behavior of mouseWentDown the mouseUp(LEFT) call here shouldn't be necessary for mouseWentDown(LEFT) to be false on the next frame. I was hoping the tests would illustrate this behavior:
| Frame | Event | mouseWentDown | mouseDown |
|---|---|---|---|
| 1 | up | false | false |
| 2 | down | true | false |
| 3 | -- | false | true |
There was a problem hiding this comment.
Ahh yes, of course! Sorry about that, I think I just fixed this.
Specifically, addresses the comment here: https://github.com/molleindustria/p5.play/pull/42#discussion_r54362451
|
Looks good! |
|
Woot thanks! |
This adds tests for the input methods in p5.play. I actually used it to ensure that #28 didn't cause regressions. 😁
However, I think I might have also uncovered a bug. The documentation for
keyDownstates:However, the implementation is checking to see if the internal key state is specifically equal to
KEY_IS_DOWN. The problem is that the internal keystate might actually beKEY_WENT_DOWN, in which case the method will retrunfalsebutkeyIsDown()will betrue!It's understandable that no one has noticed this, because it just means that the result of
keyDownwill be inconsistent withkeyIsDownfor only the frame in which the key went from up to down.A parallel problem exists with
mouseDown/mouseIsDown.@islemaster is this something you think we should change? And does the test suite look OK overall?