Skip to content

Commit

Permalink
[#1418] Fix freeze when actor width or height is 0 and tilemap is sho…
Browse files Browse the repository at this point in the history
…wn (#1491)

Closes #1418 

TileMapImpl.collides() got stuck infinitely on the `trace points for overlap` loop when the actor's size is zero.

## Changes:

- Check size of actor before deciding whether to run the collision logic
- Return null when actor width or height is 0

Co-authored-by: Vincent Beers <vincent.beers@sogeti.com>
  • Loading branch information
DaVince and Vincent Beers authored Apr 15, 2020
1 parent ea301f7 commit 8bdbabd
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).

### Fixed

- Fixed Excalibur crashing when displaying both a tilemap and a zero-size actor ([#1418](https://github.com/excaliburjs/Excalibur/issues/1418))
- Fixed animation flipping behavior ([#1172](https://github.com/excaliburjs/Excalibur/issues/1172))
- Fixed actors being drawn when their opacity is 0 ([#875](https://github.com/excaliburjs/Excalibur/issues/875))
- Fixed iframe event handling, excalibur will respond to keyboard events from the top window ([#1294](https://github.com/excaliburjs/Excalibur/issues/1294))
Expand Down
7 changes: 5 additions & 2 deletions src/engine/TileMap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ export class TileMapImpl extends Class {
const height = actor.pos.y + actor.height;
const actorBounds = actor.body.collider.bounds;
const overlaps: Vector[] = [];
if (actor.width <= 0 || actor.height <= 0) {
return null;
}
// trace points for overlap
for (let x = actorBounds.left; x <= width; x += Math.min(actor.width / 2, this.cellWidth / 2)) {
for (let y = actorBounds.top; y <= height; y += Math.min(actor.height / 2, this.cellHeight / 2)) {
Expand Down Expand Up @@ -232,10 +235,10 @@ export class TileMapImpl extends Class {
const solid = Color.Red;
solid.a = 0.3;
this.data
.filter(function(cell) {
.filter(function (cell) {
return cell.solid;
})
.forEach(function(cell) {
.forEach(function (cell) {
ctx.fillStyle = solid.toString();
ctx.fillRect(cell.x, cell.y, cell.width, cell.height);
});
Expand Down
38 changes: 36 additions & 2 deletions src/spec/TileMapSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe('A TileMap', () => {
});
const spriteTiles = new ex.SpriteSheet(texture, 1, 1, 64, 48);
tm.registerSpriteSheet('default', spriteTiles);
tm.data.forEach(function(cell: ex.Cell) {
tm.data.forEach(function (cell: ex.Cell) {
cell.solid = true;
cell.pushSprite(new ex.TileSprite('default', 0));
});
Expand All @@ -80,7 +80,7 @@ describe('A TileMap', () => {
});
const spriteTiles = new ex.SpriteSheet(texture, 1, 1, 64, 48);
tm.registerSpriteSheet('default', spriteTiles);
tm.data.forEach(function(cell: ex.Cell) {
tm.data.forEach(function (cell: ex.Cell) {
cell.solid = true;
cell.pushSprite(new ex.TileSprite('default', 0));
});
Expand All @@ -94,4 +94,38 @@ describe('A TileMap', () => {
});
});
});

describe('with an actor', () => {
let tm: ex.TileMap;
beforeEach(() => {
tm = new ex.TileMap({
x: 0,
y: 0,
cellWidth: 64,
cellHeight: 48,
rows: 10,
cols: 10
});
tm.data.forEach(function (cell: ex.Cell) {
cell.solid = true;
});
});

it('should collide when the actor is on a solid cell', () => {
const actor = new ex.Actor(0, 0, 20, 20);

const collision = tm.collides(actor);

expect(collision).not.toBeNull();
expect(collision).toBeTruthy();
});

it('should not collide when the actor has zero size dimensions', () => {
const actor = new ex.Actor(0, 0, 0, 0);

const collision = tm.collides(actor);

expect(collision).toBeNull();
});
});
});

0 comments on commit 8bdbabd

Please sign in to comment.