This forum is read only and just serves as an archive. If you have any questions, please post them on github.com/phoboslab/impact

There appears to be a bug in the ImpactJS collision-map code wherein entities may pass through solid tiles in the rightward and downward directions.

`ig.game.collisionMap.trace` is used to check for collisions against the world and returns an object containing the "final resting position" after accounting for such collisions ( .trace documentation).

Here it is being called in the entity update function:
```/* entity.js update function */
var mx = this.vel.x * ig.system.tick;
var my = this.vel.y * ig.system.tick;
var res = ig.game.collisionMap.trace(
this.pos.x, this.pos.y, mx, my, this.size.x, this.size.y
);
```

The bug occurs any time `mx` or `my` is a factor of `ig.game.collisionMap.tilesize`.

Example:

```/*
- Suppose our collision map tilesize is 8.
- Suppose our collision map is 4x4 tiles wide/tall.
- Suppose our collision map is complelely solid, except for tile (0,0).
- Suppose our entity is 8x8 pixels wide/tall.
- Suppose our entity occupies position (0,0) the only walkable tile.
- Suppose our entity has a vel.x of 480 (moving rightward).
- Suppose a perfect 60 frames per second, meaning ig.system.tick is 1/60.
*/

// First a call is made to trace.

/* entity.js update function */
var mx = this.vel.x * ig.system.tick;
//     = 480 * (1/60)
//     = 8;
var my = this.vel.y * ig.system.tick;
//     = 0 * (1/60)
//     = 0;
var res = ig.game.collisionMap.trace(
this.pos.x,  // 0
this.pos.y,  // 0
mx,          // 8
my,          // 0
this.size.x, // 8
this.size.y  // 8
);

// The trace function determines the number of times we will call
// _traceStep.

/* collision-map.js trace function */
var steps = Math.ceil(Math.max(Math.abs(vx), Math.abs(vy)) /this.tilesize);
//          Math.ceil(Math.max(Math.abs(8), Math.abs(0)) / 8);
//          Math.ceil(Math.max(8, 0) / 8);
//          Math.ceil(8 / 8);
//          Math.ceil(1);
//          1;

// Therefore _traceStep will be called 1 time.

// The first thing _traceStep does is update the position assuming no
// collision. (There is code later on that corrects the position if a
// collision is detected.)
res.pos.x += vx; // 0 + 8 == 8
res.pos.y += vy; // 0 + 0 == 0
// So res.pos == { x: 8, y: 0 };

// Now let's handle collision for the x-axis,
// starting by finding a few important values:

var pxOffsetX = (vx > 0 ? width : 0);
//                        ^ of entity
//            = (8 > 0 ? 8 : 0);
//            = 8;
var tileOffsetX = (vx < 0 ? this.tilesize : 0);
//              = (8 < 0 ? 8 : 0);
//              = 0;
var tileX = Math.floor( (res.pos.x + pxOffsetX) / this.tilesize );
//        = Math.floor( (8 + 8) / 8 );
//        = Math.floor( 16 / 8 );
//        = Math.floor( 2 );
//        = 2;

// So pxOffsetX == 8, tileOffsetX == 0, and tileX == 2,

// We've arrived at the bug. It's tileX.
// The player was at (0,0).
// We decided to move rightward.
// We called trace.
// And the first collision tile that will be checked is (2,0)?!
// Seems we skipped tile (1,0).

// Remember also that (0,2) is a solid tile, so a collision
// should occur for that, and it does. The following line is used
// to resolve the position when a collision occurs:
x = res.pos.x = tileX * this.tilesize - pxOffsetX + tileOffsetX;
//            = 2 * 8 - 8 + 0
//            = 16 - 8
//            = 8

// The above line is supposed to update the position such that
// our entity is "hugging" the solid tile, but not overlapping it.
// Instead, we are now positioned at the tile we skipped earlier.
// This means our entity position at the end of the trace call is
// (8,0). In other words, we moved 1 entire tile right, despite
// the fact that the tile is solid.

// TODO: Walk through a normal case and show how it was supposed to work.
// TODO: Explain why this bug only affects rightward and downward movement.
```

Fix:
```/* collision-map.js _traceStep function */

// After:
var tileX = Math.floor( (res.pos.x + pxOffsetX) / this.tilesize );

if( tileX * this.tilesize - pxOffsetX === x + this.tilesize ) {
tileX--;
}

// And...
// After:
var tileY = Math.floor( (res.pos.y + pxOffsetY) / this.tilesize );

if( tileY * this.tilesize - pxOffsetY === y + this.tilesize ) {
tileY--;
}
```
I know you're not done with this post, but this is a great bug report. Keep it coming.
The bug is simple to replicate...

1. Create a player entity:
```ig.module('game.entities.player')
.requires('impact.entity')
.defines(function() {

EntityPlayer = ig.Entity.extend({

speed: null,
animSheet: new ig.AnimationSheet('media/tilesheet.png', 16, 16),

init: function(x, y, settings) {
this.parent(x, y, settings);
this.maxVel.x = this.maxVel.y = 999999;
this.setSpeedSuchAsToTriggerBug();
},

update: function() {
this.setSpeedSuchAsToTriggerBug();
this.handleInputs();
this.parent();
},

setSpeedSuchAsToTriggerBug: function() {
this.speed = ig.game.collisionMap.tilesize / ig.system.tick;
},

handleInputs: function() {
if(ig.input.state('RIGHT') && !ig.input.state('LEFT')) {
this.vel.x = this.speed;
} else if(ig.input.state('LEFT') && !ig.input.state('RIGHT')) {
this.vel.x = -this.speed;
} else {
this.vel.x = 0;
}
if(ig.input.state('DOWN') && !ig.input.state('UP')) {
this.vel.y = this.speed;
} else if(ig.input.state('UP') && !ig.input.state('DOWN')) {
this.vel.y = -this.speed;
} else {
this.vel.y = 0;
}
}
});

});
```

2. Create a level with a collision layer and player entity.

3. Setup the game:
```ig.module('game.main')
.requires(
'game.levels.test',
'impact.game'
)
.defines(function(){"use strict";

var MyGame = ig.Game.extend({

init: function() {

// Bind controls.
ig.input.bind(ig.KEY.UP_ARROW, 'UP');
ig.input.bind(ig.KEY.DOWN_ARROW, 'DOWN');
ig.input.bind(ig.KEY.LEFT_ARROW, 'LEFT');
ig.input.bind(ig.KEY.RIGHT_ARROW, 'RIGHT');

}

});

var scale = 2;
var width = Math.floor(window.innerWidth/scale);
var height = Math.floor(window.innerHeight/scale);
ig.main('#canvas', MyGame, 60, width, height, scale);

});

```

4. Run the game. You can now walk through walls when moving rightward and downward.
Man, that was the best, most comprehensive bug report I've ever read. Thank you Joncom!

I decided to solve the problem a little differently, though. In your example, position (8,0) really is in tile (2,0), so tile (2,0) should be checked, but the trace should not skip over the first tile (1,0).

So I just force an additional trace step, by calculating `steps` a tiny bit higher than previously:

```// Break the trace down into smaller steps if necessary.
// We add a little extra movement (0.1 px) when calculating the number of steps required,
// to force an additional trace step whenever vx or vy is a factor of tilesize. This
// prevents the trace step from skipping through the very first tile.
var steps = Math.ceil((Math.max(Math.abs(vx), Math.abs(vy))+0.1) / this.tilesize);
```

This fix is also in the current dev version in the Git repository.

I'm sorry for the grief this issue must have caused you. Thanks again for the report!
No problem! And good call, your fix is definitely better.
Page 1 of 1
« first « previous next › last »