* Frederic Bouvier -- Thursday 30 June 2005 08:51:
> I am rather on Andy's line here. As I tried to explain to Melchior 
> 'remove' doesn't mean 'destroy'.

We know. It was said a few times in this thread already. removeChild()
just *pretended* to remove something, when it actually only detached it.
If the scope of the returned guarded pointer ended, the whole subtree
basically ended as zombie (depending on the "keep" option). Nobody in
fgfs knew this -- each of the four places that used it, intended to
really remove the whole subtree. Only fgsd, for which it was written,
knew the secret. (Don't expect all people to dive into the sg code!).
There was no way to really remove a subtree -- the only function that
pretended it would, failed at it badly. All the consequence of a poorly
chosen name.

> it was copied in the _removedChild vector before 
> returning, hence incrementing the counter. If you don't care having 
> aliases pointing to garbage ( something David was against ), you would 
> just have to dump the _removedChild vector and, if the 
> SGPropertyNode_ptr returned is not used, the tree should be destroyed. 
> Two functions shouldn't be necessary.

It would have to do this recursively. Because neither SGPropertyNode_ptr
nor SGPropertryNode do AFAIK destroy their children on destruction. (And I
don't know if this would be desirable.) So there *had* another function to be
written. Andy had suggested to remove the whole _removed_children thing[1].
So what is it now? Just pretend everything was well, when it wasn't? The
old "system" was misleading and broken, and we discuss if the new is
broken, too. Fine. Then fix either.

But whatever the decision is: the current method doesn't mess with the
counter. It doesn't increase it, and it doesn't decrease it[2], nor does
it change existing conditional behavior of other functions based on the
counter. Everything is as it used to be -- the zombie thing still available
as detachChild(). We are in fact discussing if the newly added removeChild
is allowed to make decisions based on the refcounter. That's all. If it may
or may not refuse to remove nodes from the main tree if they are still in
use, and referenced by anyone else than just the parent. I thought it would
be a good idea to do it like that, but I don't care much if it's changed
to not consider the refcounter at all. I just don't want to see the zombie
generator used under the misleading name "removeChild()" again.


[1] http://baron.flightgear.org/pipermail/flightgear-devel/2005-May/036227.html

[2] OK, alias and unalias do that now, but could be changed to make use
    of a SGPropertyNode_ptr. Fact is that they need to affect the refcounter

Flightgear-devel mailing list

Reply via email to