Skip to content

Conversation

DetectiveBaldi
Copy link
Contributor

@DetectiveBaldi DetectiveBaldi commented Oct 4, 2025

Prior to removing this check, killing a group object, and then updating the group position caused some strange behavior, which can be recreated through this sample:

package;

import flixel.FlxG;
import flixel.util.FlxColor;
import flixel.FlxSprite;
import flixel.group.FlxSpriteGroup;
import flixel.FlxState;

class PlayState extends FlxState
{
	var sprite:FlxSprite;

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

		FlxG.camera.bgColor = FlxColor.WHITE;

		var group:FlxSpriteGroup = new FlxSpriteGroup();

		group.setPosition(64, 64);

		add(group);

		sprite = new FlxSprite();

		sprite.makeGraphic(64, 64, FlxColor.RED);

		// Commenting this line fixes issue
		sprite.kill();

		group.add(sprite);

		// Despite updating group position, sprite stays at 64x64y
		group.setPosition(128, 128);
	}

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

        // To view displacement
		if (FlxG.keys.justPressed.SPACE)
			sprite.revive();
	}
}

At first glance this check makes sense, don't transform objects that don't exist. But I feel like this check falls under the category of premature optimization and it caused me a really aggravating hour long debug session trying to figure out why my sprite group objects weren't updating positions.

I'm not sure if this will be accepted considering the current status and seemingly pending removal of FlxSpriteGroup, but I'd rather not migrate all my code to use some extension or alternative class and I feel like it makes sense to just push it here

@DetectiveBaldi DetectiveBaldi changed the title Remove exists check when calling `FlxSpriteGroup. Remove exists check when calling FlxSpriteGroup.multiTransformChildren Oct 4, 2025
@Geokureli
Copy link
Member

Geokureli commented Oct 4, 2025

It makes 1000% sense to make this change but we just can't change fundamental behavior like this. This is a breaking change so you'll need to extend the class and override this behavior yourself. Eventually, the plan is to change how the draw tree works, and make spriteGroups/spriteContainers obsolete in favor of some new version.

Even if we do approve this change there's no good way to broadcast this change to people who are expecting the old behavior

Side note: you shouldn't ever use FlxSpriteGroup, use FlxSpriteContainers.

Edit: here's what I typically do in my games:
https://gist.github.com/Geokureli/ac82881883b8862530796f0b29200c17

@DetectiveBaldi
Copy link
Contributor Author

It makes 1000% sense to make this change but we just can't change fundamental behavior like this. This is a breaking change so you'll need to extend the class and override this behavior yourself. Eventually, the plan is to change how the draw tree works, and make spriteGroups/spriteContainers obsolete in favor of some new version.

Even if we do approve this change there's no good way to broadcast this change to people who are expecting the old behavior

Side note: you shouldn't ever use FlxSpriteGroup, use FlxSpriteContainers.

Edit: here's what I typically do in my games: https://gist.github.com/Geokureli/ac82881883b8862530796f0b29200c17

Yeah, I can see how it would be almost impossible to communicate this change to developers.

Thank you for the git gist, I can most likely work with something like this for the time being

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.

2 participants