Skip to content
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

Have multiway keep track of the number of keys pressed for each direction #794

Closed
wants to merge 2 commits into from

Conversation

cwgorman
Copy link

@cwgorman cwgorman commented Jul 9, 2014

So if W and UP_ARROW both correspond to the same direction, pressing both at the same time will have no effect, instead of doubling the speed. Resolves issue #793.

…tion. So if W and UP_ARROW both correspond to the same direction, pressing both at the same time will have no effect, instead of doubling the speed.
@kevinsimper
Copy link
Contributor

Hey @cwgorman

You are right, that is not the intended behaviour. That should be fixed!

I looked at the code and have some comments:

You should avoid creating new variables since that will force the garbace collecter to work and we will want to avoid that.
https://github.com/craftyjs/Crafty/pull/794/files#diff-006f862972d60d6e4ffefc1397f44328R711
https://github.com/craftyjs/Crafty/pull/794/files#diff-006f862972d60d6e4ffefc1397f44328R725

Is type conversion also needed here? (Have not runned the code myself, so can not immediately tell)
'' + this._keyDirection[k]] and var dir = '' + this._keyDirection[e.key];

@cwgorman
Copy link
Author

@kevinsimper I've made the changes you've requested. Sorry it took so long; I haven't had much time for personal work lately.

I am curious about the garbage collection though. Is it actually slower than performing the additional lookups? I assumed the JS engines in today's browsers would be smart enough to optimize that.

@Chaosed0
Copy link
Contributor

I ran into this one over the last weekend. Patch looks good to my unexperienced eye - anyone else still looking at this?

@mucaho
Copy link
Contributor

mucaho commented Feb 26, 2015

How will this code perform in the following hypothetical case:
Imagine a space shooter where Q will move the player 1 square left and 1 square up (direction of -90 - 45 == -135), and E will move the player 1 square right and 1 square up (direction of 0 - 45 == -45). Now if Q and E are pressed together, player should just move 1 square up.

Maybe we could split the activeDirections map into activeDirectionsLeft, activeDirectionsRight, activeDirectionsUp and activeDirectionsDown; then increase/decrease the activeDirection__ counters by partitioning the key direction on an incoming key event:

  • (-180 < direction < -90 || 90 < direction < 180 ) => LEFT
  • (-0 < direction < -90 || 0 < direction < 90) => RIGHT
  • (-0 < direction < -180) => UP
  • (0 < direction < 180) => DOWN

This would solve the space shooter example, but still isn't precise enough for non-symmetric keys, but I think even this is overkill.

@mucaho mucaho mentioned this pull request Feb 26, 2015
@mucaho
Copy link
Contributor

mucaho commented Mar 16, 2015

ty cwgorman the proposed changed was integrated alongside other improvements to Multiway in #876

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants