FlxMouseEventManager.removeAll() fails to completely unregister even number of objects

[SamBumgardner]

• Flixel version: 4.1.0
• OpenFL version: 3.6.1
• Lime version: 2.9.1
• Affected targets: at least Neko, HTML5, and flash, although I'm guessing it happens on the rest as well.

---

Code snippet reproducing the issue:

package;

import flixel.FlxG;
import flixel.FlxSprite;
import flixel.FlxState;
import flixel.input.mouse.FlxMouseEventManager;

class MenuState extends FlxState
{
    override public function create():Void
    {
        super.create();

        var spr1 = new FlxSprite(100, 200);
        var spr2 = new FlxSprite(200, 200);
        var spr3 = new FlxSprite(300, 200);

        FlxMouseEventManager.init();
        FlxMouseEventManager.add(spr1, null, function(button:FlxSprite){FlxG.switchState(new PlayState()); }, 
                                       null, function(button:FlxSprite){ button.velocity.set(0, 0); });
        FlxMouseEventManager.add(spr2, null, function(button:FlxSprite){FlxG.switchState(new PlayState()); }, 
                                       null, function(button:FlxSprite){ button.velocity.set(0, 0); });
        //FlxMouseEventManager.add(spr3);

        add(spr1);  
        add(spr2);
    }

    override public function switchTo(nextState:FlxState):Bool
    {
        FlxMouseEventManager.removeAll();
        return super.switchTo(nextState);
    }
}

---

Observed behavior:

I registered a pair of FlxSprites to the FlxMouseEventManager, setting its mouseUp function to switch between states and its mouseOut function to call one of the sprite's variable's functions. I also overrode the starting state's switchTo to call FlxMouseEventManager's removeAll function, so the buttons should be removed from FlxMouseEventManager whenever the state switch occurs.

When the game switches between states, it properly removes the second FlxSprite, but it fails to remove the first. If the second sprite was clicked, everything's fine. If the first sprite was clicked, however, its mouseOut function is called after the state switch finishes. Since the registered mouseOut function interacts with one of the sprite's variables that was set to null during the state change, the game crashes.

As far as I can tell, this always happens if an even number of objects are registered to the FlxMouseEventManager.

If you have an odd number of objects registered to the FlxMouseEventManager, no problem occurs. You can try it by uncommenting the line FlxMouseEventManager.add(spr3);

Expected behavior:

I expected that all registered objects would be removed from FlxMouseEventManager by removeAll(), regardless of whether number of objects is even or odd.

Side note: I did find a workaround. By removing the FlxMouseEventManager plugin from FlxG and then calling its init(), I could force the FlxMouseEventManager to clear itself completely. It may also be possible that removing instances one by one works, but I haven't tried it yet.

[MSGhero]

The issue internally is that removeAll is iterating over the array while also removing from it. I can fix it later today or tomorrow if no one else gets to it.

[SamBumgardner]

Cool, thanks!

[Gama11]

@MSGhero That explanation doesn't make sense to me. A new array is assigned to _registeredObjects at the end of removeAll(), so its length will always be 0 at the end of it no matter what.

[MSGhero]

Remove() does more than just removing from the array, and it's that behavior that gets skipped over. Right?

If not, this at least fixes the memory that isn't cleaned up when "removing while iterating" skips elements.

[Gama11]

It just removes and nulls the members (and it's debatable whether that really helps garbage collection).

[MSGhero]

I guess this should actually be tested. I wrote that fix really quickly between classes but haven't had a chance to try it out.

[MSGhero]

@SamBumgardner can you try the fix in #1990 and see if that resolves your issues?

It needs to be fixed regardless, but I want to know if this is the actual issue you were having.

[SamBumgardner]

Sure, I'll check it out.

[MSGhero]

It only took 20 days for me to find some free time haha...
Ha...
...

[SamBumgardner]

No worries!

It looks to me like the issue is fixed. The code snippet that I posted with the issue is fine now, and my larger project isn't running into any problems either.

Thanks a bunch for fixing it!

[MSGhero]

@Gama11 closed by #2005?

[Gama11]

I want to look into this again. It shouldn't be necessary to call removeAll() manually.

[Gama11]

Yeah, it seems the issue persists if you get rid of the removeAll() call.