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

FlxTilemap.overlapsWithCallback calls callback when no overlapping #37

Closed
FlixelCommunityBot opened this issue Sep 13, 2012 · 16 comments
Closed
Assignees
Milestone

Comments

@FlixelCommunityBot
Copy link

Issue #194 by: imagame

The tile's registered callback is been calling when there is no overlapping, for tiles that not allow collisions.
The tile callback should be called only if there is overlapping, not in all cases (line 977)

For tiles that allow collisions it is correct.

@ghost ghost assigned Dovyski Jan 25, 2013
@IQAndreas
Copy link
Member

@Dovyski, you assigned this issue to yourself. Are you still working on it, or should I "open it up to the public"?

@Dovyski
Copy link
Member

Dovyski commented Feb 27, 2013

I am still working on it. I've committed some code to the Sandbox to test it, but have no time to continue. Does anyone want to continue from where I left or should I keep going?

@IQAndreas
Copy link
Member

Does anyone want to continue from where I left or should I keep going?

It's entirely up to you. If you have found something and are on the right track, go ahead and finish when you get the time; we aren't in a hurry. But if you feel stuck, perhaps a second set of eyes could help.

@Dovyski
Copy link
Member

Dovyski commented Feb 28, 2013

Ok, I will keep working on it. I think it's not far from over.

@Dovyski
Copy link
Member

Dovyski commented Mar 1, 2013

I've just committed some tilemap collision tests to FlixelSandbox. I could not reproduce this bug, FlxTilemap.overlapsWithCallback() seems to be working as expected.

It's important to note that FlxTilemap.overlapsWithCallback() checks all tiles that allow collision, so if the object is colliding with any of them (e.g. the floor), the callback will be invoked.

@IQAndreas
Copy link
Member

It's been a year since @imagame opened the issue, so he was definitely using Flixel 2.55, not a previous version. But he has probably found a solution or just moved on.

It's important to note that FlxTilemap.overlapsWithCallback() checks all tiles that allow collision, so if the object is colliding with any of them (e.g. the floor), the callback will be invoked.

That makes sense. Shall we close this issue for now, but double check with imagame in the original issue? Perhaps he still has the old code laying around and we can see if this was the case or not.

@Dovyski
Copy link
Member

Dovyski commented Mar 1, 2013

I think he found a solution too :)

I will close this issue here then. I already asked @imagame about the code in the original issue. If we get something, we can re-open this issue.

@Dovyski Dovyski closed this as completed Mar 1, 2013
@imagame
Copy link

imagame commented Mar 1, 2013

Original code (while loop starting in line 945 in overlapsWithCallback function):

while(row < selectionHeight)
{
    column = selectionX;
    while(column < selectionWidth)
    {
        overlapFound = false;
        tile = _tileObjects[_data[rowStart+column]] as FlxTile;
        if(tile.allowCollisions)
        {
            tile.x = X+column*_tileWidth;
            tile.y = Y+row*_tileHeight;
            tile.last.x = tile.x - deltaX;
            tile.last.y = tile.y - deltaY;
            if(Callback != null)
            {
                if(FlipCallbackParams)
                    overlapFound = Callback(Object,tile);
                else
                    overlapFound = Callback(tile,Object);
            }
            else
                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(overlapFound)
            {
                if((tile.callback != null) && ((tile.filter == null) || (Object is tile.filter)))
                {
                    tile.mapIndex = rowStart+column;
                    tile.callback(tile,Object);
                }
                results = true;
            }
        }
        else if((tile.callback != null) && ((tile.filter == null) || (Object is tile.filter)))
        {
            tile.mapIndex = rowStart+column;
            tile.callback(tile,Object);
        }
        column++;
    }
    rowStart += widthInTiles;
    row++;
}

And now same loop with my fix replacing 'else' code in line 977, to avoid calling the tile callback in all situations (only has to be called if there is overlapping !):

while(row < selectionHeight)
{
    column = selectionX;
    while(column < selectionWidth)
    {
        overlapFound = false;
        tile = _tileObjects[_data[rowStart+column]] as FlxTile;
        if(tile.allowCollisions)
        {
            tile.x = X+column*_tileWidth;
            tile.y = Y+row*_tileHeight;
            tile.last.x = tile.x - deltaX;
            tile.last.y = tile.y - deltaY;
            if(Callback != null)
            {
                if(FlipCallbackParams)
                    overlapFound = Callback(Object,tile);
                else
                    overlapFound = Callback(tile,Object);
            }
            else
                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(overlapFound)
            {
                if((tile.callback != null) && ((tile.filter == null) || (Object is tile.filter)))
                {
                    tile.mapIndex = rowStart+column;
                    tile.callback(tile,Object);
                }
                results = true;
            }
        }
        else
        //@imagame replace original else condition, to avoid calling callback if no overlap found
        {
            if (tile.callback != null)
            {
                tile.x = X+column*_tileWidth;
                tile.y = Y+row*_tileHeight; 
                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(overlapFound)
                {
                    if((tile.callback != null) && ((tile.filter == null) || (Object is tile.filter)))
                    {
                        tile.mapIndex = rowStart+column;
                        tile.callback(tile, Object);
                    }
                }
            }
        }
        /* @imagame replaced else
        else if((tile.callback != null) && ((tile.filter == null) || (Object is tile.filter)))
        {
            tile.mapIndex = rowStart+column;
            tile.callback(tile,Object);
        }*/
        column++;
    }
    rowStart += widthInTiles;
    row++;
}

@Dovyski Dovyski reopened this Mar 2, 2013
@Dovyski
Copy link
Member

Dovyski commented Mar 4, 2013

I've read the whole FlxTilemap.overlapsWithCallback() documentation and spent a huge amount of time understanding its code:

/**
 * Checks if the Object overlaps any tiles with any collision flags set,
 * and calls the specified callback function (if there is one).
 * Also calls the tile's registered callback if the filter matches.
 * 
 * @param   Object              The <code>FlxObject</code> you are checking for overlaps against.
 * @param   Callback            An optional function that takes the form "myCallback(Object1:FlxObject,Object2:FlxObject)", where Object1 is a FlxTile object, and Object2 is the object passed in in the first parameter of this method.
 * @param   FlipCallbackParams  Used to preserve A-B list ordering from FlxObject.separate() - returns the FlxTile object as the second parameter instead.
 * @param   Position            Optional, specify a custom position for the tilemap (useful for overlapsAt()-type funcitonality).
 * 
 * @return  Whether there were overlaps, or if a callback was specified, whatever the return value of the callback was.
 */
public function overlapsWithCallback(Object:FlxObject,Callback:Function=null,FlipCallbackParams:Boolean=false,Position:FlxPoint=null):Boolean

Here are my thoughts so far according to what I've understood from the method code:

  • The Callback param is a function that takes the form myCallback(Object1:FlxObject,Object2:FlxObject):Boolean (pay attention to the return type). It is used to check if a tile and the Object param are overlapping. I assume it is there to allow custom overlapping logic. As a consequence if that callback returns false, Flixel assumes there was no overlap between Object and the tile being analyzed.
  • If Callback == null then Flixel will check the overlapping based on width/height and x/y values of both the Object and the tile being analyzed.
  • If Flixel detects an overlap AND the tile being analyzed has a callback method (set with setTileProperties()) AND Object is listed in the tile's filter property (also set with setTileProperties()), then the tile's callback method will be invoked.
  • Callback will be invoked to check for overlapping only if the tile being analyzed allows collision (which can be set using setTileProperties().
  • If an overlap is found or not, overlapsWithCallback() will aways invoke the tile's callback method (which is not the Callback param) if it is not null AND Object is listed in the tile's filter property.

Even though this is what I've understood from the code, I am not able to reproduce that behavior. I am running some tests. As soon as I have some news, I will let you guys know.

@Dovyski
Copy link
Member

Dovyski commented Mar 4, 2013

After some time in the deep catacombs of Flixel, I've found an answer :D

All I said in my previous post is correct and accurate. Flixel will invoke the tile's callback every time it detects an overlap, no matter if the tile allows collision or not. I could not reproduce @imagame bug: Flixel invoked the tile's callback only when an overlap was found (I've pushed to the Sandbox the code I used).

@imagame Could you please provide us with a snippet of your game's code?

@imagame
Copy link

imagame commented Mar 5, 2013

I'll try to recreate it, once i review the code of my game (more than a
year since i wrote it). Give me some time and i'll provide you with an
example. Anyway i would not want you to waste your time.

@Dovyski
Copy link
Member

Dovyski commented Mar 5, 2013

Ok, I will wait for your code. Don't worry about the time, it's not wasted :)

@imagame
Copy link

imagame commented Mar 6, 2013

I've reproduced the "overlapping issue behaviour" using your FlixelSandbox
project, playing with TestTileMap.as

As it is now TestTileMap.as when the player collides with Tile 3 (the
yellow one) it starts blinking. As you can see it continues blinking when
the player is positioned just in the left of the yellow tile but without
overlapping it. Same happens when the player is located over (on top of)
the tile but not overlapping (I changed the code removing gravity so you
can freely move the player and this way is better to check that the player
blinks but not overlaps the tile). Only when the player is at the right of
the yellow tile (or down it if you locate another tile-3 in the mid of the
map) the blink is started in a pixel perfect overlapping.
If you change the player size (vertical and/or horizontal) you will
experience different overlaps situations: if the player is the same size of
the tile (8x8 pixels) there is no problem (the overlap is perfect, well
almost perfect, 1 pixel on the left and 1 on the top is also considere
overlaped but the same occurs if we fix the FlxTilemap code). If the player
is double the size of the tile there is also no problem. Same happens if
the player has a multiple tile size (N-size the tile). But in case any of
both player dimensions (with or height) is not the same (or multiple) then
you can appreciate that the player blinks when there is no overlapping.
This effect it also appears if you define tile sizes much bigger than
player size.
The reason of that (as far as I've check) is the callback is always been
called (when the tile does not allow collisions) not attending to player
size so calling the callback even if there is not a pixel-perfect
player-tile overlap.
If you modify FlxTileMap entering the fix I explain in my past mail, and
you repeat the play with the yellow tile, you will check that the player
only blinks when there is a real overlap, no matter if it is located on the
left, the right or on top/down the yellow tile, and no matter the size of
the player. Is a pixel-perfect overlap (excep 1 pixel line on top and 1on
the lef, not sure why), that I think is what is expected.

I'm not sure if it could be considered and issue or not. Certainly for my
game (a cenital view game where the player must avoid the water --tiles
with callback-- it was necessary to have a pixel perfect collision check
wherever the player was located)

Hope this comments help, as you can see your code is valid to test it. Only
change is setting player1.acceleration.y to 0, and enabling up and down
keys to let the player move vertically (instead of jumping). In this
scenario is easier to make some test with different player sizes and
entering different type-3 tiles to experimen with distinct conditions.

2013/3/5 Fernando Bevilacqua notifications@github.com

Ok, I will wait for your code. Don't worry about the time, it's not wasted
:)


Reply to this email directly or view it on GitHubhttps://github.com//issues/37#issuecomment-14458745
.

@Dovyski
Copy link
Member

Dovyski commented Apr 26, 2013

Finally I got some time to test your case and I've confirmed the bug. I will investigate it even further.

@Dovyski
Copy link
Member

Dovyski commented Apr 26, 2013

I've fixed the bug using @imagame 's solution (I just removed a few duplicated code blocks). I tested it using the sandbox and it seems to be working.

If anybody wants to test it further, use this Flixel branch which contains the fix.

Dovyski added a commit that referenced this issue Aug 20, 2013
…overlapping.

The overlapping check performed for tiles with allowCollision = false ignored the object widht/height, so the tile callback would fire even when the Object was placed close to, but not overlapping, the tile.
Dovyski added a commit that referenced this issue Aug 20, 2013
Fix issue #37 FlxTilemap.overlapsWithCallback calls callback when no overlapping
@Dovyski
Copy link
Member

Dovyski commented Aug 20, 2013

Fixed here 99447a0.

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

No branches or pull requests

4 participants