Skip to content

Add FlxG.collision and FlxColliders #3431

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

Draft
wants to merge 8 commits into
base: dev
Choose a base branch
from

Conversation

Geokureli
Copy link
Member

@Geokureli Geokureli commented May 8, 2025

#3430

While FlxG.collision is a good thing to have and helpful in general, the main purpose of this PR is to better handle collisions described in #3418

Still isn't perfect, in the EZ platformer demo when you jump up against a vertical wall you'll hit the bottom of a tile, rather than slide up it. One way to fix this is to check the object's deltaY and check tiles in reverse when going up, that should fix this, but I'm wondering if this any less ad-hoc and confusing than #3418.

A couple other features in this PR, are:

  • Overlap and collide don't use Dynamic, and have type params
  • Can compute overlap without setting touching flags
  • Paves the way to discerning a "collision overlap" with a simpler, normal "overlap" which ignores objects' last field
  • FlxCollision: Static extension methods obj.getDeltaRect() and objA.overlapsDelta(objectB)

TODO:

  • Check tiles in reverse when moving up
  • Come up with a shorter/better name than FlxG.collision.collides/overlaps
  • Test the new overlap and collide with type params
  • See if anything from FlxCollision should go in FlxG.collision
  • Add new separator method for non-orthogonal overlaps, I.E.: reflect colliders' velocity along the normal of the collision overlap, rather than reflecting X and Y axis individually
  • Fix CI
  • Add circle collisions
  • Fix bugs and features in test state
    • implement FlxTilemapExt's slopeSlowDownFactor feature
    • Fix finicky collisions (example image below)
Screenshot 2025-05-29 at 11 16 25 AM

@Geokureli Geokureli mentioned this pull request May 8, 2025
@Geokureli
Copy link
Member Author

The reverse iteration worked for EZPlatformer, but I noticed that FlxTilemapExt doesn't work with FlxG.collision.collides, because it uses objectOverlapsTiles, it should either use forEachCollidingTile, or I should make objectCollidesTiles and use that. I should note that I think FlxTilemapExt is outdated and it's not extremely important to make it work with the new system unless FlxG.collides/overlaps is deprecated before a replacement is made for FlxTilemapExt

Comment on lines +41 to +44
overload public inline extern function overlap<TA:FlxObject, TB:FlxObject>(a, ?b, ?notify:(TA, TB)->Void, ?process:(TA, TB)->Bool)
{
return overlapHelper(a, b, notify, process);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a good change. In most of my uses of the current way, I immediately cast them to the object types I know them to be.

Copy link
Member Author

@Geokureli Geokureli May 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've also been meaning to make a macro that would decide these types based on the types of args passed in

I.e.: FlxTypedGroup<Player> and FlxTypedGroup<FlxTypedSpriteGroup<Coin>> would be overlap<Player, Coin>

Edit: i may be able to accomplish this via overloading, come to think

Comment on lines 179 to 188
if (abs(overlapX) < abs(overlapY))
{
updateTouchingFlagsXHelper(object1, object2);
separateXHelper(object1, object2, overlapX);
}
else
{
updateTouchingFlagsYHelper(object1, object2);
separateYHelper(object1, object2, overlapY);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is effectively just a "poor man's" SAT implementation, as it's not checking the projection axes of the shapes involved, and instead just implicitly looks at x and y projections). I haven't convinced myself it's better than the existing "first X, then Y" process other than this avoids my reuse of the objects' last to make it work correctly.

The main benefit of this to me feels like it may open up flixel to (one day) support non rectangular shapes. Not sure if there's desire to support n-gon shapes. Circles don't strictly need something like SAT to work.
The downside to this is now internal seems between shapes will cause snagging in a somewhat unpredictable manner as it depends on the dimensions of the overlap with the seam.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you link me to an explanation of SAT? It's hard to google this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the tutorial/guide I used back when I implemented this long ago:
https://www.metanetsoftware.com/technique/tutorialA.html
https://www.metanetsoftware.com/technique/tutorialB.html

Unfortunately, all the flash demos are dead, but it did a really good explanation for my brain.

The gist of SAT is that you project two collision shapes onto each the shapes' normals in isolation. The smallest overlapping axis is the one you move the shapes along. Incoming attempt at oversimplifying it just to give a starting point:

SATDemoAttempt

  1. First image shows two shapes we've identified as needing resolution (I'm glossing over the broad phase) and all of the normals of said shapes. Of which there are 4, as direction doesn't matter (2 from the square, and 2 from the triangle as it's bottom edge is not unique)
  2. Project both shapes along all 4 of these axes and find the one of shortest overlap
  3. Resolve the shapes along that axis of shortest overlap.

This works for convex polygons.

I had commented that that flixel does a "poor man's" version of this as it just assumes we are only needing to "project" along the X and Y axes. So the idea is there, but there is no consideration for more complicated shapes other than axis-aligned rectangles.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you can install the Ruffle extension from the chrome store and it will let you view the embedded flash player elements. Highly recommend as it they are a bunch of interactive demos to let you play with the concepts they talk about.

* @param process Called on every object that overlaps another, determines whether to call the `notify`.
* @return Whether any overlaps were detected.
*/
overload public inline extern function overlap<TA:FlxObject, TB:FlxObject>(a, ?b, ?notify:(TA, TB)->Void, ?process:(TA, TB)->Bool)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this change as I almost always write the first two lines of my notify/process func as

var player:Player = cast a;
var other:Enemy = cast e;

Will this blow up if someone collides/overlaps a group that has the wrong types in it? More concretely if they do something like overlap<Player, Coin>(player, allCollidableObjectsInGame, (p, c) -> { ... });

I am not in a spot where I can just run the code, but wanted to ask while it's on my mind so I don't lose the thought.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still need some testing, but i dont think it will any worse than using wrong types with the current system, which just uses Dynamics. One benefit of this is that it ensures the notify and process use the same types

@Geokureli Geokureli marked this pull request as draft May 9, 2025 13:12
@Geokureli
Copy link
Member Author

Geokureli commented May 19, 2025

Made a ton of changes. Now object.computeCollisionOverlap returns how the objects should be separated. Previously, it returned how they should be separated on each axis, and the caller would ignore the larger one. This should be better for non-AABB objects, thanks for the tip @MondayHopscotch, I loved those metanet tutorials but completely forgot they existed.

I also started work on making slope tools for tilemaps, it all works somewhat well, except in my new version of FlxTIlemapExt glue-down is not great, and i haven't implemented slow down when going up slopes. I also made FlxTIlemapExt more compatible with FlxG.collision.collide

I'm considering making an ICollider interface, and change all refs to FlxObject to IColliders (but only in new code), this way you can make custom colliders without having to extend FlxObject, this is in line with this goal. colliders would only need a getBounds(resultRect) and a computeCollisionOverlap The issue here is that FlxQuadTrees would need to be built from the ground up. Though, FlxQuadTrees are in need of an update so maybe having another class that will one day replace the existing one is an option (a very ugly option). The simple option is to just use FlxObjects, but then changing all this later is a huge pain

Also: we still separate each axis assuming they hit at an orthogonal wall, we should add a specific 2d separate that assumes the resulting overlap is the normal of the collision

@Geokureli
Copy link
Member Author

Geokureli commented May 29, 2025

Massive changes, added FlxColliders, which will be the control center for all physics-related things. I also learned that FlxQuadTrees have terrible performance due to limitations from the flixel version

This includes various features/changes I've been wanting to do, in a (seemingly) backwards compatible way, namely:
#2716 - Expand FlxObject colliders (CIRCLES!)
#2704 - add linearDrag, maxLinearVelocity and drag modes
#2891 - stop using static fields for everything in quadtrees

New todos added

@Geokureli Geokureli changed the title Add FlxG.collision Add FlxG.collision and FlxColliders Jun 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants