FlixelCommunity / flixel

Community fork of Adam “Atomic” Saltsman's popular game engine Flixel. Distilled from a variety of Flash games he worked on over the last couple years, including Gravity Hook, Fathom and Canabalt, its primary function is to provide some useful base classes that you can extend to make your own game objects.
http://flixelcommunity.org/
Other
84 stars 17 forks source link

Fix `FlxGroup#maxSize` #184

Closed IQAndreas closed 10 years ago

IQAndreas commented 10 years ago

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.

IQAndreas commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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

IQAndreas commented 10 years ago

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

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