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

Fix FlxGroup#maxSize #184

Merged

Conversation

IQAndreas
Copy link
Member

In the old behavior, if you set the maxSize property of a FlxGroup, first it destroys all objects in the group that it was not planning on removing, then just drops the rest from the list of members (without even destroying them first) by setting members.length.

public function set maxSize(Size:uint):void
{
    _maxSize = Size;
    if(_marker >= _maxSize)
        _marker = 0;
    if((_maxSize == 0) || (members == null) || (_maxSize >= members.length))
        return;

    //If the max size has shrunk, we need to get rid of some objects
    var basic:FlxBasic;
    var i:uint = _maxSize;
    var l:uint = members.length;
    while(i < l)
    {
        basic = members[i++] as FlxBasic;
        if(basic != null)
            basic.destroy();
    }
    length = members.length = _maxSize;
}

This fix makes sure to only remove the last elements, and only destroys the ones that it removes.

In the old behavior it was destroying elements that it was going to
keep, and just dropping without destroing elements that it wasn't.
@IQAndreas
Copy link
Member Author

If you guys are okay with it, I could easily modify the class to first try to get rid of null elements in the group to shorten the list without removing anything (is there any reason someone would place a null object inside of a FlxGroup, which would cause this addition to interfere with that?).

And perhaps even prioritize getting rid of already destroyed objects first. In fact, I could even go so far as to first remove items before the _marker; that way, older items are removed first. I already know in my head how to do this, but is it an unnecessary extra calculation, or a good feature? Do you see the possibility of any game adjusting the maxSize dynamically while running as some sort of game mechanic?

Also, in FlxGroup, a maxSize of 0 means it allows any number of elements. I'm tempted to default _maxSize to uint.MAX_VALUE and allow a value of 0 to temporarily say "don't allow any children in this group", but this one isn't a big deal at all.

@Dovyski
Copy link
Member

Dovyski commented Oct 30, 2013

I think the code is good as it is, I don't see the need to implement a more selective algorithm.

Also, in FlxGroup, a maxSize of 0 means it allows any number of elements. I'm tempted to default _maxSize to uint.MAX_VALUE and allow a value of 0 to temporarily say "don't allow any children in this group", but this one isn't a big deal at all.

I like the idea, but it will probably break existing code. We can change that in the future. For now, the current code seems good to be merged.

@Dovyski
Copy link
Member

Dovyski commented Nov 10, 2013

I've tested the code and it is working flawlessly. Do you want to make any changes, @IQAndreas , or can I merge it?

@IQAndreas
Copy link
Member Author

Do you want to make any changes, @IQAndreas, or can I merge it?

Nope, no further changes for now. We are good to go.

IQAndreas added a commit that referenced this pull request Nov 10, 2013
@IQAndreas IQAndreas merged commit 4ded6c2 into FlixelCommunity:dev Nov 10, 2013
@IQAndreas IQAndreas mentioned this pull request Nov 10, 2013
5 tasks
@IQAndreas IQAndreas deleted the fix-flxgroup-maxsize branch November 17, 2013 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants