excaliburjs / Excalibur

🎮 Your friendly TypeScript 2D game engine for the web 🗡️
https://excaliburjs.com
BSD 2-Clause "Simplified" License
1.82k stars 189 forks source link

Adjusting contact mtv and/or normal has issues when self is colliderB #3121

Closed mattjennings closed 4 months ago

mattjennings commented 4 months ago

Steps to Reproduce

class Level1 extends ex.Scene {
  onInitialize() {
    const player = new ex.Actor({
      x: 100,
      y: 0,
      width: 100,
      height: 100,
      color: ex.Color.Green,
      collisionType: ex.CollisionType.Active
    })

    player.on('precollision', ev => {
      const {
        contact
      } = ev

      contact.mtv.x = 0
      contact.normal = ex.vec(0, 1)
    })

    const box = new ex.Actor({
      pos: ex.vec(225, 500),
      width: 500,
      height: 100,
      anchor: ex.vec(1, 1),
      collider: ex.Shape.Polygon([
        ex.vec(0, 0),
        ex.vec(-200, -100),
        ex.vec(-200, 0),
      ]),
      collisionType: ex.CollisionType.Fixed
    });

    box.graphics.add(
      new ex.Polygon({
        points: box.collider.get().points,
        color: ex.Color.Gray,
      }),
    )

    this.add(player);
    this.add(box);
  }
}

Run this repro here: https://jsfiddle.net/mattjennings/Lmtdrx3y/7/

The box will correctly stop on the slope. If you swap the lines

    this.add(player);
    this.add(box);

to

    this.add(box);
    this.add(player);

this will cause box to be colliderB in the precollision. When this is the case, the box eventually falls through the slope after about a few seconds.

My running theory is the collider is not properly updated according to mtv, as the collision point seems to fall further and further down until it completely passes the slope collider.

Expected Result

Box should stay on the slope when it is colliderB in precollision

Actual Result

Box eventually falls through the slope

Environment

Current Workaround

Swapping collider A & B around in the precollision handler does seem to fix it... but I am sure there are consequences to doing that 😅

const isColliderA = other === contact.colliderB
if (!isColliderA) {
  const self = contact.colliderB
  contact.colliderA = self
  contact.colliderB = other
}
mattjennings commented 4 months ago

Fairly certain this is due to adjusting contact.normal. Digging into the ArcadeSolver some more, now I see mtv affects the position (which is correct in my repro - the box resolves to where it should be) and normal affects the velocity, so that would explain why it gradually increases until it bypasses the collider completely. Though I'm not seeing anything obvious yet https://github.com/excaliburjs/Excalibur/blob/7a4c82115311f75169fc880150ed06b754a5f365/src/engine/Collision/Solver/ArcadeSolver.ts#L199-L205

mattjennings commented 4 months ago

Ahh, contact.normal also needs to be biased to 1/-1 depending on which is colliderA 🤦‍♂️ my fault. This caused if (bodyB.vel.normalize().dot(normal) < 0) { to be skipped (as it would be 1) and vel would keep increasing.

I think I had spoke with @eonarheim a while ago about adding a way to bias a contact to a collider, so that all of these tricks are done for you. Something like contact.bias(self). That might be the best solution to avoid user error. I'll close this now since it's not a bug.