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

Precollision event sometimes has the incorrect side #2473

Closed
mattjennings opened this issue Aug 28, 2022 · 4 comments · Fixed by #2474
Closed

Precollision event sometimes has the incorrect side #2473

mattjennings opened this issue Aug 28, 2022 · 4 comments · Fixed by #2474

Comments

@mattjennings
Copy link
Contributor

Steps to Reproduce

https://codesandbox.io/s/compassionate-feynman-tqx8ok?file=/index.js

I'm noticing some odd behaviour in which collision with a wall on the right side sometimes returns a collision on the left. In the example above, use the arrow keys to walk into the wall and look at the console logs. The collisions are tracked by pushing the ev.side to an array during the precollision event. When walking into the wall, it will correctly have a collision on the right side. But if you stop moving, it will have collisions on the left side.

CleanShot.2022-08-28.at.17.02.30.mp4

Expected Result

Only bottom/right side should come through in precollision events

Actual Result

Left side comes through

Environment

  • browsers and versions: Chrome 104
  • operating system: macOS 12.5
  • Excalibur versions: 0.27
  • (anything else that may be relevant)

Current Workaround

This does not happen if you use an odd-numbered width player (say, 39px) and round the player's position in the postcollision event. (I have however noticed other odd behaviour when doing this, which I will make another issue for)

 constructor() {
    super({
      name: 'player',
      x: 200,
      y: 200,
      width: 39,
      height: 40,
      color: ex.Color.Red,
      collisionType: ex.CollisionType.Active,
    });

    this.on("precollision", (ev) => this.onPreCollision(ev));
    this.on("postcollision", (ev) => this.onPostCollision(ev));
  }

  onPostCollision(ev) {
		if (ev.side === ex.Side.Left) {
			this.pos.x = Math.floor(this.pos.x)
		} else if (ev.side === ex.Side.Right) {
			this.pos.x = Math.ceil(this.pos.x)
		}
	}
@mattjennings
Copy link
Contributor Author

re: workaround leading to another issue, I'm not really sure it is an issue, so i'll just follow it up here.

If you have an odd-width collider and an anchor of (0.5,0.5), when you collide with a wall your collision is going to be resolved to a half-pixel. i.e 295.5 instead of 295. This makes sense. But if you round that position in the postcollision event, you are probably going to get unexpected results (for a collision on the right, you'll either move into the wall if you round up, or away from the wall if you round down). So I think the answer there is to not use an odd-width collider... but then that leads to this issue.

I'm not sure if you have any thoughts on that. I just happened to be using an odd-width collider in my game when I came across other issues, and switching to an even-width one lead to this.

@eonarheim
Copy link
Member

@mattjennings thanks for the issue!

This is certainly an interesting issue, on first brush I'm wondering if the frame after resolution phase has an infinitesimal velocity towards the left, causing the next precollision to detect a possible collision with the left edge?

At minimum I think some floating point arithmetic edge case might explain the different behavior oddness vs evenness that you noticed

@eonarheim
Copy link
Member

It looks like the (-0, -0) mtv vector is treated as the left side, I think this might be the source of the issue.

image

image

I think maybe filtering out 0 length penetrating collisions may resolve the issue?

@mattjennings
Copy link
Contributor Author

Ah yes, filtering out -0 and 0 length collisions seems to do the trick!

eonarheim added a commit that referenced this issue Sep 5, 2022
Closes #2473

This PR fixes an issue where a degenerate 0 sized collision causes issues with the precollision event emit. Previously an erroneous left precollision would be emitted when the minimum translation vector (mtv) was `(-0, -0)`

<img width="299" alt="image" src="https://user-images.githubusercontent.com/612071/187574499-96889716-ba1a-418e-995e-b7673ce643e2.png">


## Changes:

- Filters out 0 sized contacts in precollision to guard against a degenerate side calculation
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 a pull request may close this issue.

2 participants