From 8bdbabd55a638e767820aa435042b08180219096 Mon Sep 17 00:00:00 2001 From: Vincent Beers Date: Wed, 15 Apr 2020 14:24:08 +0200 Subject: [PATCH] [#1418] Fix freeze when actor width or height is 0 and tilemap is shown (#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 --- CHANGELOG.md | 1 + src/engine/TileMap.ts | 7 +++++-- src/spec/TileMapSpec.ts | 38 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da5fbc97c..67cd2c888 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)) diff --git a/src/engine/TileMap.ts b/src/engine/TileMap.ts index 8c2e20e91..7f4b01e45 100644 --- a/src/engine/TileMap.ts +++ b/src/engine/TileMap.ts @@ -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)) { @@ -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); }); diff --git a/src/spec/TileMapSpec.ts b/src/spec/TileMapSpec.ts index 5ab46e29a..978a4da70 100644 --- a/src/spec/TileMapSpec.ts +++ b/src/spec/TileMapSpec.ts @@ -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)); }); @@ -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)); }); @@ -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(); + }); + }); });