phaser icon indicating copy to clipboard operation
phaser copied to clipboard

Overlap not working as expected.

Open kuabhish opened this issue 3 years ago • 6 comments

Version

  • Phaser Version: 3.53.1
  • Operating system: Mac 11.4
  • Browser: Chrome

Description

Hello @photonstorm I think there is some issue with the overlap function of tiles. For example: in the following example:

house, pub, dummy_region = this.map.createLayer("....", tileset, 0, 0);
this.physics.add.collider(player, [house, pub]); 

this works.. if I set this.map.setCollision(...)

But when I do the same thing for overlap like:

this.physics.add.overlap(player, [dummy_region], () => {
    console.log("overlaping");
});

it is overlapping with the tileset even if it is not overlapping as it is printing this when the player is standing anywhere.

if it worked the same as physics.add.collider it would be great.

kuabhish avatar May 25 '22 18:05 kuabhish

overlap() doesn't consider collision faces at all. If you want it to you can do

this.physics.add.overlap(
  sprite,
  tilemapLayer,
  function onOverlap(sprite, tile) {
    console.log("overlap", tile.x, tile.y);
  },
  function process(sprite, tile) {
    return tile.collides;
  }
);

samme avatar May 25 '22 19:05 samme

this solves it.

kuabhish avatar May 25 '22 19:05 kuabhish

I'm going to reopen this, because when looking at the code earlier I noticed that none of the filterOptions are set - meaning that it will always grab the tiles around the object being checked and iterate through them, testing for collision. If I enable the options in the first call it will skip all of this, potentially saving thousands of redundant tests.

It will also resolve the overlap issue. The downside is that you cannot flag a tile as being 'overlap' or 'collides', it's just always 'collides'. So this would be a breaking change - but, I think it'd be worth doing because tilemap overlap checks are really rare, but collision ones are really common, and it would speed those up no-end.

photonstorm avatar May 25 '22 22:05 photonstorm

@photonstorm So, you are saying you would not allow overlap functionality even with this function ?

overlap() doesn't consider collision faces at all. If you want it to you can do

this.physics.add.overlap(
  sprite,
  tilemapLayer,
  function onOverlap(sprite, tile) {
    console.log("overlap", tile.x, tile.y);
  },
  function process(sprite, tile) {
    return tile.collides;
  }
);

kuabhish avatar May 26 '22 13:05 kuabhish

I thought this was kind of a feature. After setTileIndexCallback() or setTileLocationCallback() you often want to use overlap(), e.g. tile callbacks.

Maybe it could filter only for overlapOnly === false?

samme avatar May 26 '22 15:05 samme

I think there are 2 issues here.

The first, is that in Arcade Physics World it does:

var mapData = GetTilesWithinWorldXY(x, y, w, h, null, tilemapLayer.scene.cameras.main, tilemapLayer.layer);

It then passes this array to collideSpriteVsTilesHandler, which does a load of math and coordinate conversion, before passing each unique tile to TileIntersectsBody and then SeparateTile which in turn does TileCheckX and TileCheckY calls - all of which do nothing but waste CPU if the tile wasn't ever set to actually collide!

We can skip this entire process by just using the filtering options in the initial call to GetTilesWithinWorldXY. If that returns an empty array then it skips doing literally everything else, which is surely what we need.

However, this would be a breaking change for overlap - because basically, that worked on the assumption that all tiles are returned initially, regardless if they collide or not, by that first call to GetTilesWithin and then passes them all to the overlap callback. Which honestly feels wrong. It works in the example you linked @samme because that had just 1 overlapping tile on that layer (the coin), but if there had been 2 overlapping tiles on the layer, say a coin and a tile that kills you, you'd then have to filter which type of tile it was in the overlap callback. Add a third tile and it all gets a lot more complex.

In short, I think this change needs to be made. It will dramatically speed-up general tilemap checks! But I need to think how to handle overlap calls first.

photonstorm avatar May 26 '22 17:05 photonstorm

Any updates on this?

K4T avatar Jan 21 '23 19:01 K4T

This was implemented a while ago and is available in all 3.60 Beta releases since June 2022. I'm happy it's now working as intended and is significantly faster than 3.55 without breaking previous overlap checks, so am closing this issue.

photonstorm avatar Mar 22 '23 17:03 photonstorm