Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use 'overlapFound' in boolean expression for Callback in FlxTilemap.overlapsWithCallback #1381

Open
overgrown opened this issue Nov 22, 2014 · 2 comments

Comments

@overgrown
Copy link

overgrown commented Nov 22, 2014

Suggestion

This is a small suggestion to the function overlapsWithCallback() in FlxTilemap that I'm 90% sure is how it was intended to behave. Change the line:

if (tile.allowCollisions != FlxObject.NONE)

to

if (overlapFound && tile.allowCollisions != FlxObject.NONE)

Reasoning

In FlxTilemap, the function overlapsWithCallback() computes if there is an overlap then immediately does the callback:

overlapFound = ((Object.x + Object.width) > tile.x)  && (Object.x < (tile.x + tile.width)) && 
               ((Object.y + Object.height) > tile.y) && (Object.y < (tile.y + tile.height));

if (tile.allowCollisions != FlxObject.NONE)

The problem here is that it proceeds to call Callback for every tile with any collision flag set, regardless if the Object overlaps with the current tile or not. This is bad since you'll be calling the callback many times on tiles that the object does not overlap / collide with.

Instead, since it just computed if there is an overlap, and you'd only want to do Callback if there is, the line above should be

if (overlapFound && tile.allowCollisions != FlxObject.NONE)

@overgrown
Copy link
Author

Another Suggestion

An improvement to the same function is to move the calculation of tile.mapindex up with the other code tile data code. Here's what it should look like (the last line):

            tile = _tileObjects[dataIndex];
            tile.width = _scaledTileWidth;
            tile.height = _scaledTileHeight;
            tile.x = X + column * tile.width;
            tile.y = Y + row * tile.height;
            tile.last.x = tile.x - deltaX;
            tile.last.y = tile.y - deltaY;
            tile.mapIndex = rowStart + column;

Without this, when Callback is called, mapIndex is zero so there is no way to know which tile triggered the collision callback. Again, this line is already in the code, but it is lower down when the tile's own callback is called. It should be here so mapIndex is available to both Callback and the tile's callback.

@Gama11
Copy link
Member

Gama11 commented Mar 27, 2015

After a bit of digging I found that this code used to look different in AS3 Flixel and was originally changed here by @Beeblerox. That commit seems to have been inspired by this one from @Dovyski in the FlixelCommunity fork (with discussion in FlixelCommunity/flixel#37).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants