Skip to content

Add FlxG.centerGraphic() and camera.centerGraphic() #3329

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

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

Conversation

richTrash21
Copy link
Contributor

@richTrash21 richTrash21 commented Jan 3, 2025

Closes #3309

Another attempt at #3310 based on Geokureli's suggestion.

Adds FlxG.centerGraphic() and camera.centerGraphic() for centering sprites by their graphic size and FlxG.centerHitbox() and camera.centerHitbox() for centering objects by their hitbox size.

To Do

  • Fix inconsistent behaviour of FlxG.centerGraphic() and camera.centerGraphic() on sprites with pixelPerfectPosition or pixelPerfectRender flags set to true. Edit: find another method.
  • Fix unit tests, FlxGTest.testCenterGraphic() and FlxCameraTest.testCenterGraphic() are failing on rotated sprites for seemingly no reason.

@NeeEoo
Copy link
Contributor

NeeEoo commented Jan 3, 2025

I'm not really sure if reflection in c++ works when using generics.

Because both FlxG.random.shuffle and FlxG.random.getObject both don't exist in reflection due to generics

@richTrash21
Copy link
Contributor Author

I'm not really sure if reflection in c++ works when using generics.

Because both FlxG.random.shuffle and FlxG.random.getObject both don't exist in reflection due to generics

As far as i know the reason why FlxG.random.shuffle and FlxG.random.getObject doesn't exist in reflection is due to @:generic metadata.

I've also made a small test to confirm that:

class Main {
	static function main() {
		var stringField:String;
		var intField:Int;
		var floatField:Float;

		stringField = test1("some string");
		intField = test1(30);
		floatField = test1(1.0034);

		stringField = test2("some string");
		intField = test2(30);
		floatField = test2(1.0034);

		trace(Reflect.getProperty(Main, "test1")); // returns test1 function
		trace(Reflect.getProperty(Main, "test2")); // returns null
	}

	static function test1<T>(v:T):T {
		trace("test generic func #1 (" + Std.string(v) + ")");
		return v;
	}

	@:generic
	static function test2<T>(v:T):T {
		trace("test generic func #2 (" + Std.string(v) + ")");
		return v;
	}
}

@NeeEoo
Copy link
Contributor

NeeEoo commented Jan 3, 2025

Damn, i wonder why haxe wouldn't generate a base one for reflection purposes

Copy link
Member

@Geokureli Geokureli left a comment

Choose a reason for hiding this comment

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

Note that the actual graphic width of a sprite depends on sprite.getGraphicBounds and sprite.getGraphicMidpoint(), which is the world coordinates of the graphic, and accounts for origin, angle, offset, pixelPerfectPosition and any future features or features in extending classes.

Also note: there is sprite.getScreenBounds, which accounts for scrollFactor and camera.zoom

We should use those (and be sure to put any rect/point back into the pool), or if you think it's beneficial we can add a sprite.getScreenMidpoint()

I thought about maybe having getGraphicWidth, getScreenWidth and the like, and having the existing methods use that, but truthfully, it might be a hassle, and even a little less performant. I'm leaning against that, but let me know what you think

@richTrash21 richTrash21 marked this pull request as draft February 21, 2025 11:06
@richTrash21 richTrash21 marked this pull request as ready for review April 8, 2025 22:58
Copy link
Member

@Geokureli Geokureli left a comment

Choose a reason for hiding this comment

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

  1. center should be centerGraphic
  2. I don't see a case where object.x and object.getHitbox().x will ever be different (by definition)
  3. Avoid setting sprite.pixelPerfectPosition and the like
  4. unit tests should use hard coded values

I'm gonna be focusing on getting 6.1.0 out as soon as I have time, so I put this on 6.2.0. consider that set in stone enough to edit the @since tags

Comment on lines 1887 to 1890
final pixelPerfectPosition = sprite.pixelPerfectPosition;
final pixelPerfectRender = sprite.pixelPerfectRender;
sprite.pixelPerfectPosition = false;
@:bypassAccessor sprite.pixelPerfectRender = false;
Copy link
Member

@Geokureli Geokureli Apr 9, 2025

Choose a reason for hiding this comment

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

I think we should make a new helper that allows us to get the unrounded value, or a new arg in the existing one that allows us to ignore this field. this is not the first case where pxPerfPos has been a hassle

Copy link
Member

Choose a reason for hiding this comment

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

Now that I think about this, maybe this should NOT attempt to avoid pixelPerfectRender and position, perhaps the unit test was failing because it should have accounted for this? Can you elaborate on why it failed and why centering should not honor pixelperfectRendering?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unit tests were actually fine, the problem was discovered in a test project, where grapthic is centered every frame. pixelPerfectPosition = true or pixelPerfectRender = true led to shaking, so my workaround was to temporarily disable them, as i don't see a reason to add another function to get graphic bounds.

For now i removed this workaround. Adding another argument to functions will work, but will cause problems with function override. Should this be a concern?

My test project:

package;

import flixel.FlxG;
import flixel.FlxSprite;
import flixel.util.FlxAxes;

class PlayState extends flixel.FlxState
{
	var graphic:FlxSprite;
	var centerMode = FlxAxes.XY;
	var rorate = false;

	override public function create()
	{
		super.create();

		FlxG.cameras.bgColor = flixel.util.FlxColor.GRAY;

		graphic = new FlxSprite().makeGraphic(20, 20);
		graphic.scrollFactor.set(2, 2);
		graphic.origin.set(20, 20);
		graphic.offset.set(200, 200);
		graphic.scale.set(2, 4);
		add(graphic);

		FlxG.watch.add(FlxG.camera, "scroll", "camera");
		FlxG.watch.add(this, "graphic", "graphic");
		FlxG.watch.add(graphic, "pixelPerfectPosition", "pixelPerfectPosition");
		FlxG.watch.add(graphic, "pixelPerfectRender", "pixelPerfectRender");
	}

	override public function update(elapsed:Float)
	{
		super.update(elapsed);

		var mult = 10.0;
		if (FlxG.keys.pressed.SHIFT)
			mult *= 10;

		if (FlxG.keys.pressed.A)
			FlxG.camera.scroll.x -= mult * elapsed;
		if (FlxG.keys.pressed.D)
			FlxG.camera.scroll.x += mult * elapsed;

		if (FlxG.keys.pressed.W)
			FlxG.camera.scroll.y -= mult * elapsed;
		if (FlxG.keys.pressed.S)
			FlxG.camera.scroll.y += mult * elapsed;

		mult /= 100;
		if (FlxG.keys.pressed.Q)
			FlxG.camera.zoom += mult * elapsed;
		if (FlxG.keys.pressed.E)
			FlxG.camera.zoom -= mult * elapsed;

		if (FlxG.keys.justPressed.Z)
			centerMode = FlxAxes.fromBools(!centerMode.x, centerMode.y);
		if (FlxG.keys.justPressed.X)
			centerMode = FlxAxes.fromBools(centerMode.x, !centerMode.y);

		if (FlxG.keys.justPressed.C)
			graphic.pixelPerfectPosition = !graphic.pixelPerfectPosition;
		if (FlxG.keys.justPressed.V)
			graphic.pixelPerfectRender = !graphic.pixelPerfectRender;

		if (FlxG.keys.justPressed.R)
			rorate = !rorate;
		if (rorate)
			graphic.angle += elapsed;

		FlxG.camera.centerGraphic(graphic, centerMode);
	}
}

Copy link
Member

Choose a reason for hiding this comment

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

this is an example to reproduce the shaking effect?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Shaking appears when camera is moved or sprite is rotated.
Side note, might also be the case for any transformation.

@richTrash21 richTrash21 changed the title Add FlxG.center() and camera.center() Add FlxG.centerGraphic() and camera.centerGraphic() Apr 10, 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.

remove screenCenter
3 participants