Gamua / Starling-Framework

The Cross Platform Game Engine
http://www.starling-framework.org
Other
2.87k stars 826 forks source link

Method removeChild():DisplayObject never returns null #777

Closed b005t3r closed 8 years ago

b005t3r commented 9 years ago

I'd expect removeChild() to return null if the given child was not removed (because it's not a child of the given parent), so I could, for example, use it to check if the parent has changed as a result of this call.

Currently, it always returns the object passed as child, which is not really that useful.

neolit123 commented 9 years ago

hey, b005t3r.

i'm sure you know that what Starling has is compatible with the Flash API and changing it could break for some users. but i do agree, what you suggest makes sense, because the caller already has a reference to the DisplayObject passed to removeChild()...and i don't think there are many users that grab the result from removeChild() as it's a known instance. in the Flash API the removeChild() return is such so that it's similar to the removeChildAt().

possibly a viable change.

contains() also exists, which BTW i think can be simplified in the current source from:

/** Determines if a certain object is a child of the container (recursively). */
        public function contains(child:DisplayObject):Boolean
        {
            while (child)
            {
                if (child == this) return true;
                else child = child.parent;
            }
            return false;
        }

to:

public function contains(child:DisplayObject):Boolean
{
    return child && child.parent == this;
}
PrimaryFeather commented 9 years ago

Makes sense! I agree that this return value doesn't have much worth currently; I just did it that way because the Flash API does it like that. So it makes sense to change it to something more useful. I'll think about it!

@neolit123: the contains method not only checks if the object is a child of the container, but also if it's a grand-child, etc. That way, you can quickly check if an object is "underneath" another in the display tree. Thus the loop. :wink:

PrimaryFeather commented 9 years ago

BTW, any comments on the removeChild change are welcome! What do others think — good idea, bad idea? Thanks in advance!

b005t3r commented 9 years ago

I have no idea how old Flash API works, obviously, but what I suggested is based on how a very robust JAVA Collection framework works - all methods like add(), remove() there return a value which clearly indicates if a given collection has changed in the process, which is very useful for e.g. refreshing views for which the collection was a model.

So that's my vote for changing the way it works currently to returning null if the DisplayObjectContainer did not change :)

neolit123 commented 9 years ago

the contains method not only checks if the object is a child of the container, but also if it's a grand-child, etc. That way, you can quickly check if an object is "underneath" another in the display tree. Thus the loop.

i see, makes perfect sense.

PrimaryFeather commented 9 years ago

Thanks, guys! I'm leaning towards making that change in the 2.0 branch. =)

PrimaryFeather commented 8 years ago

I must admit I totally forgot about that ...

@joshtynjala What do you think about this? Would it help / be a problem for Feathers if I changed this?

joshtynjala commented 8 years ago

I don't think I use the return value from removeChild() anywhere in Feathers. I don't think it would break anything.

PrimaryFeather commented 8 years ago

Closed with 1f08c5af7af6ddf193ed74495a8e66acf2ce528f.