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

FlxGroup.forEachOfType recursive part not working #1876

Closed
sano98 opened this issue Jun 26, 2016 · 2 comments

Comments

Projects
None yet
3 participants
@sano98
Copy link

commented Jun 26, 2016

  • 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

This comment has been minimized.

Copy link
Member

commented Jun 26, 2016

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.

@Gama11 Gama11 added the Bug label Jun 26, 2016

@sano98

This comment has been minimized.

Copy link
Author

commented Jun 26, 2016

@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);
            }
        }

MSGhero added a commit to MSGhero/flixel that referenced this issue Jul 2, 2016

Fix for HaxeFlixel#1876
Condition wasn't allowing groups to be traversed

MSGhero added a commit to MSGhero/flixel that referenced this issue Jul 3, 2016

@Gama11 Gama11 closed this in #1881 Jul 10, 2016

Gama11 added a commit that referenced this issue Jul 10, 2016

Aurel300 added a commit to larsiusprime/haxeflixel that referenced this issue Apr 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.