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

Double jump bug #91

Closed
apaatsio opened this issue Sep 3, 2020 · 3 comments
Closed

Double jump bug #91

apaatsio opened this issue Sep 3, 2020 · 3 comments
Labels
bug Something isn't working

Comments

@apaatsio
Copy link
Contributor

apaatsio commented Sep 3, 2020

It's possible to perform a "double jump". It seems that sometimes when you press the Up key right at the peak of the first jump, it performs a second jump. I'm sure this is not the desired behavior. This happens maybe about 1 out of 20 times when I try to deliberately do it.

double_jump

Excalibur: 0.24.4
sample-platformer: 717c7a1

@eonarheim
Copy link
Member

@apaatsio Good find! Double jumps definitely not intentional.

Must be something odd about https://github.com/excaliburjs/sample-platformer/blob/master/src/bot.ts#L59 allow the jump to be registered https://github.com/excaliburjs/sample-platformer/blob/master/src/bot.ts#L99

@eonarheim eonarheim added the bug Something isn't working label Sep 3, 2020
@eonarheim
Copy link
Member

eonarheim commented Sep 4, 2020

Okay, this is a subtle issue but I believe this is because the collision resolution runs after actor update.

Immediately after the jump the actor update is complete and the jump impulse won't be applied to position until the next frame, so still colliding.
image

In the same frame, collision is processed and the actor geometry is still intersecting so onGround is set back to true, making a double jump possible in the very next frame.
image

Potential solutions:

  • Switch to onPreUpdate to ensure impulse is processed the same frame as collisions
  • Shift the actor pos.y up to keep from re-colliding with the floor the same frame as the jump

@eonarheim
Copy link
Member

@apaatsio I switched to onPreUpdate seems to do the right thing, thanks for the find!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants