threerings / tripleplay

Utilities for use in PlayN-based games.
Other
92 stars 32 forks source link

wasRemoved() Element may return true from isShowing() #31

Closed roguenet closed 11 years ago

roguenet commented 11 years ago

Element.isShowing:

public boolean isShowing () {
    Elements<?> parent;
    return isVisible() && ((parent = parent()) != null) && parent.isShowing();
}

The problem is that if an element has wasRemoved() called, and that element's wasRemoved() implementation ends up causing (either directly, or through someone listening on hierarchyChanged) some other code to call isShowing() immediately, it will return true because it has neither had its wasUnparented() method called, nor has its parent finished resolving its own removal, if that's how wasRemoved() was called.

This could probably be solved by changing hierarchyChanged() to a Value so that it retains its state (and then query that state in isShowing()), but I'm not sure what repercussions there might be in this somewhat complicated situation, so I didn't want to just push through the change without putting it up here first.

samskivert commented 11 years ago

Why don't we just call wasUnparented() before we call wasRemoved()? Then by the time wasRemoved() is called on the child it actually will be "was" removed rather than "will be" removed.

roguenet commented 11 years ago

I mainly just wasn't sure if there were any side effects to doing that. This was Jamie's suggestion this morning as well though. I'll get it implemented.

samskivert commented 11 years ago

We will need to change the docs of wasUnparented() as well (say will immediately be followed by wasRemoved instead of preceded).

roguenet commented 11 years ago

Will do