With regard to ISceneNode::remove()

[Mel]

I have taken a peek to the ISceneNode method remove, and seems that, besides telling the parent to delete from its children list, and dropping it, if there is no parent, it does nothing at all.

Orphan nodes have no other way to delete themselves? deleting manually the pointer and dropping the nodes is something not recomendable, and orphan nodes won't remove even if they are told to do so, or if they are added to the deletion queue, or if a deletion animator is added to them. How should these nodes be deleted when they are no longer needed?

[Abraxas)]

Are you sure deleting the parent won't set the value of parent to 0 (null) which corresponds to the root scene node?

[Mel]

An orphan node has no parent. The remove method searches in the paren'ts children list if the node is there, if it is there, it is removed from the paren'ts list and dropped, and so, if there is no extra reference, deleted, But if there is no parent (it is NULL), it does nothing.

Code: Select all

 
//! Removes this scene node from the scene
/** If no other grab exists for this node, it will be deleted.
*/
    virtual void remove()
    {
    if (Parent)
    Parent->removeChild(this);
    }
 

The root Scene Node is another scene node after all (The scene manager, by the way...), and so, its pointer can't null.

But scene nodes can have NULL parents, so, if the remove method doesn't find a parent, won't do anything at all. How to delete these nodes? directly? dropping them?

[Abraxas)]

Maybe take a look at removeChild(), does this reset the parent of an orphan?

[Mel]

It searches in the child list of the parent

Code: Select all

 
    //! Removes a child from this scene node.
    /** If found in the children list, the child pointer is also
    dropped and might be deleted if no other grab exists.
    \param child A pointer to the child which shall be removed.
    \return True if the child was removed, and false if not,
    e.g. because it couldn't be found in the children list. */
    virtual bool removeChild(ISceneNode* child)
    {
    ISceneNodeList::Iterator it = Children.begin();
    for (; it != Children.end(); ++it)
    if ((*it) == child)
    {
    (*it)->Parent = 0;
    (*it)->drop();
    Children.erase(it);
    return true;
    }
    
    _IRR_IMPLEMENT_MANAGED_MARSHALLING_BUGFIX;
    return false;
    }
 

But it never reaches this point if the scene node has no parent, from this i asume that dropping an orphan node is enough, but scene nodes shouldn't ever be dropped, or does this works somehow else?

[Abraxas)]

I think I solved it.

If I'm right, orphans cannot be actually created.

Lets say you have node A, child of root.

Then node B, child of node A.

You remove() node A, which would leave B an orphan BUT:

Code: Select all

    (*it)->drop();
    Children.erase(it);

It calls Children.erase which will kill B also.

So removing a parent also murders the whole family.

[CuteAlien]

ISceneNode::remove() is about removing it from the scenegraph - it's not about memory. It only drops the grab() which the scenegraph itself keeps. And as the scenegraph in Irrlicht is a tree with the scenemanager as root you can't have a node without parent in the scenegraph. The moment the parent is 0 it means it's removed from the scenegraph already (aka - it won't be rendered anymore anyway).

If you keep nodes around yourself outside the scenemanager's control then you have to do your own grab() and drop() the same way the scenemanger does it now.

edit: Think of it like this: ISceneNode::remove() should probably rather be ISceneManager::removeNode(ISceneNode*). At least that's how I would probably write it when starting the engine new from scratch.

[hybrid]

Though orphans might be created with custom scene nodes, as we didn't integrate some hidden, always called helper functions. These could automatically integrate it into the scene graph. Right now, custom scene nodes might override some parts and create special situations which we didn't care for.

[Mel]

So, orphan nodes should be avoided then?

[CuteAlien]

Well, not sure what you're doing. The thing is orphan nodes are not rendered as they are not part of the scenegraph. If you do for example your own render-calls or want to keep them around for other reasons there is nothing speaking against them. Just that you don't need remove() them then - as they are already removed from the scenegraph (that's basically what remove does). ISceneNode::remove() is only about the scenegraph - the complement to addSceneNode. The only reason it even contains a drop() as well is because the scengraph adds a reference when the node is added to it.

[Mel]

I was messing with my own custom nodes indeed. And found that whenever i tried to remove my custom nodes, when they were orphan, they never got actually deleted, if the remove method is just to keep the structure of the scene graph, then, they can be safely dropped, right?

[CuteAlien]

You can safely drop if you don't mess up the reference count :-) I really can't tell without knowing exactly what you did if it's safe in your case. If you did grab() it then you can drop() it. If you created the node yourself (for example with new) then you can also drop() it.
If you used something like yourSceneNode->setParent(0) - then the node is already deleted unless you did grab() it first.

[Mel]

I created the node with a NULL parent,

Code: Select all

 
sparkSceneNode* spark = new sparkSceneNode(smgr,0,getNewID(),cfg);
 
//this is the constructor:
sparkSceneNode::sparkSceneNode(scene::ISceneManager* smgr,scene::ISceneNode* parent,s32 ID,structSparkConfiguration& cfg)
:scene::ISceneNode(smgr,parent,id)
{
configuration = cfg;
}
 

i manage the node with my own structures, so when i no longer need it, i call "remove()", i never grab it, and i don't set it to any parent, i thought that remove sort of dropped it on its own.

[CuteAlien]

When you create objects in Irrlicht with new or with a function starting witch the word create then you should call drop() at the end. So in your case it's not in the scenegraph, so no need to call remove(). But you have added a reference counter with new so you need to call drop().

[Mel]

Fine then. Dropping scene nodes from now on...

It could be added to the remove method a drop() call if there was no parent. This way, the deletion of scene nodes could be unified somehow. i.e. all the scene nodes will safely delete using this remove method wether it has a parent or not... though i suspect that the remove method is used for more than deleting the nodes.