Skip to content
New issue

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

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

Already on GitHub? # to your account

Deprecate screenCenter(), add gameCenter() and viewCenter() #3310

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

richTrash21
Copy link
Contributor

Closes #3309

@richTrash21 richTrash21 changed the title Deprecated screenCenter(), add gameCenter() and viewCenter() Deprecate screenCenter(), add gameCenter() and viewCenter() Dec 11, 2024
@Geokureli Geokureli added this to the Next Minor Version milestone Dec 11, 2024
@Geokureli
Copy link
Member

this is all good but I'm not gonna add this to the 5.9.0 release as we need to update all the documentation, demos and snippets, first

@Geokureli
Copy link
Member

Geokureli commented Dec 11, 2024

Actually we need to also update the unit tests, first.
https://github.com/HaxeFlixel/flixel/blob/dev/tests/unit/src/flixel/FlxObjectTest.hx#L344-L371

Should also add a separate test for viewCenter

@richTrash21
Copy link
Contributor Author

this is all good but I'm not gonna add this to the 5.9.0 release as we need to update all the documentation, demos and snippets, first

oh ok, so i guess it will be merged in 6.0.0?

@Geokureli Geokureli modified the milestones: Next Minor Version, 5.9.0 Dec 11, 2024
@Geokureli
Copy link
Member

changed my mind, I need to edit a bunch of demos for 5.9.0 already, so this is now on 5.9.0

@Geokureli
Copy link
Member

Geokureli commented Dec 11, 2024

Sorry, changed my mind again, I'm not sure I'm happy with these names, and I think there's other issues, involved here.
gameCenter isn't very intuitive, and I feel like viewCenter should use the graphical midpoint, not the hitbox midpoint, or maybe there should be two, and maybe this is also true for gameCenter?

I'm leaning now, towards FlxG.center(sprite) and FlxG.centerHitBox(obj) and maybe myCamera.center(sprite) or something. I may shelf this for a bit and think about it.

For now, screenCenter is used a ton, and deprecating it at the last second is gonna impact a lot of projects, when most people are using it just to center something on a ui screen, and there's already plenty of deprecation in 5.9.0. So this will either go to 5.10.0 or 6.0. in the future we're planning on having colliders and images be a component of FlxObjects and FlxSprites, so maybe it makes sense to do camera.center(sprite.image) or FlxG.center(obj.collider) when that happens

@Geokureli Geokureli modified the milestones: 5.9.0, Next Minor Version Dec 11, 2024
@NeeEoo
Copy link
Contributor

NeeEoo commented Dec 12, 2024

I would also kinda like a sprite.centerTo(otherSprite) or different function names

to center a sprite on top of another sprite

@Geokureli
Copy link
Member

Geokureli commented Dec 12, 2024

I would also kinda like a sprite.centerTo(otherSprite) or different function names

to center a sprite on top of another sprite

I've been thinking about that too, but that might be better as a static extension, and it has the same issue of confusing collider width/height with graphic width/height

For now I recommend doing the static extension yourself

import flixel.FlxObject;
import flixel.utils.FlxAxes;

class AlignTools
{
    static inline function centerOn<T:FlxObject>(objA:T, objB:FlxObject, axes:FlxAxes):T
    {
        if (axes.x) objA.x = objB.x + (objB.width - objA.width) / 2;
        if (axes.y) objA.y = objB.y + (objB.height - objA.height) / 2;
        return objA;
    }
}

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

Successfully merging this pull request may close these issues.

remove screenCenter
3 participants