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

Properly destroy FlxUIList #254

Merged
merged 1 commit into from Oct 16, 2023
Merged

Conversation

Starmapo
Copy link
Contributor

Fixes prevButton and nextButton not being destroyed if they're not added inside FlxUIList.
This also makes the button offset points put back to the pool using FlxDestroyUtil, preventing a crash if they're both already null.

@Geokureli
Copy link
Member

I'm completely unfamiliar with this class is there an example of how it's typically used so I can poke around and learn more?

@Starmapo
Copy link
Contributor Author

It's used for FlxUIRadioGroup to display each option (line 136). I'm using radio groups in my project and I found out that the graphics FlxUIList loads for the "previous" and "next" buttons (line 114, line 150, and the button labels) aren't cleared properly. I've never seen these in action before, but it seems like it's used for when a specific height is set (line 304) and there's not enough space to fit list items.

From what I can tell, the buttons are only added inside the list group when they're needed (line 341), in which case the buttons would be destroyed properly. If they're not needed however, then they're absent from the group (line 262), and so when the list is destroyed, the buttons are excluded from being destroyed, also meaning that the graphics will stay at a use count of 1 and won't be destroyed automatically.

This should work to test it out:

import flixel.addons.ui.FlxUIRadioGroup;

class PlayState extends flixel.FlxState
{
	override function create()
	{
		final labels = ["Option 1", "Option 2", "Option 3"];
		final radioGroup = new FlxUIRadioGroup(0, 0, labels, labels);
		radioGroup.destroy(); // radio group was destroyed, but list buttons weren't. Their graphics are still present in the cache
		
		super.create();
	}
}

@Geokureli Geokureli merged commit edb5092 into HaxeFlixel:dev Oct 16, 2023
15 checks passed
@Geokureli
Copy link
Member

thanks

Geokureli added a commit that referenced this pull request Dec 22, 2023
@Geokureli Geokureli added this to the 2.6.0 milestone Mar 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants