HaxeFlixel / flixel-ui

GUI library for HaxeFlixel
171 stars 63 forks source link

Properly destroy `FlxUIList` #254

Closed Starmapo closed 1 year ago

Starmapo commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

thanks