FlxGroup.forEachOfType recursive part not working

[sano98]

• Flixel version: 4.1.0
• OpenFL version: ?.?.?
• Lime version: ?.?.?
• Affected targets:

---

Code snippet reproducing the issue:

package;

import flixel.FlxState;

class PlayState extends FlxState
{
    override public function create():Void
    {
        var mastergroup:FlxGroup = new FlxGroup();
        var subgroup:FlxGroup = new FlxGroup();

        var testsprite:FlxSprite = new FlxSprite();
        var testsprite2:FlxSprite = new FlxSprite();
        var testsprite3:FlxSprite = new FlxSprite();
        var testsprite4:FlxSprite = new FlxSprite();

        mastergroup.add(testsprite);
        mastergroup.add(testsprite2);

        mastergroup.add(subgroup);
        subgroup.add(testsprite3);
        subgroup.add(testsprite4);

        mastergroup.forEachOfType(FlxSprite, this.traceSprite, true);
    }

        public function traceSprite(MySprite:FlxSprite):Void
       {
              trace(MySprite);
       }

---

Observed behavior:
When setting the recursive flag to true, subgroups of the respective group will still not be searched, despite what the function descriptiuon says. Therefore, only the testsprites 1 and 2 will be traced, the ones in the subgroup will be ignored.

Expected behavior:
Obviously, with Recursive set to true, one would expect 4 traces.

The Problem

if (basic != null &&  Std.is(basic, ObjectClass))

This line will obviously not be true for a FlxGroup (as long as we are looking for FlxSprite or Strings or anything else not being a FlxGroup)

Suggested Solution

/**
     * Applies a function to all members of type Class<K>
     * 
     * @param   ObjectClass   A class that objects will be checked against before Function is applied, ex: FlxSprite
     * @param   Function      A function that modifies one element at a time
     * @param   Recurse       Whether or not to apply the function to members of subgroups as well 
     */
    public function forEachOfType<K>(ObjectClass:Class<K>, Function:K->Void, Recurse:Bool = false)
    {
        var i:Int = 0;
        var basic:FlxBasic = null;

        while (i < length)
        {
            basic = members[i++];

            if (basic != null && Recurse && Std.is(basic, FlxGroup))
            {
                var group = resolveGroup(basic);
                if (group != null)
                {
                    group.forEachOfType(ObjectClass, cast Function, Recurse);
                }
            }


            if (basic != null &&  Std.is(basic, ObjectClass))
            {
                Function(cast basic);
            }
        }
    }

[MSGhero]

I've never used this before, but isn't your proposal excluding FlxSpriteGroup? The point of resolveGroup is to gain access to members via casting to a typed group, so I don't think the Std.is is needed there at all. If the FlxBasic isn't a group of some kind, the resolve will fail and nothing will happen anyway.

[sano98]

@MSGhero: Yeah, you have two good points there. I wasn't aware that there is a class like FlxSpriteGroup, showing "group like behaviour" while not being a child class of FlxGroup.

And the check whether it is a group will, like you said, be done during the resolveGroup; the return values of nullwill be ignored.

Nevertheless, the current implementation is not working.
Guess this one should do the trick:

public function forEachOfType<K>(ObjectClass:Class<K>, Function:K->Void, Recurse:Bool = false)
    {
        var i:Int = 0;
        var basic:FlxBasic = null;

        while (i < length)
        {
            basic = members[i++];

            if (Recurse)
            {
                var group = resolveGroup(basic);
                if (group != null)
                {
                    group.forEachOfType(ObjectClass, cast Function, Recurse);
                }
            }

            if (basic != null && Std.is(basic, ObjectClass))
            {
                Function(cast basic);
            }
        }