Node3D::DetachChild Bug

Jul 19, 2013 at 7:19 AM
Edited Jul 19, 2013 at 7:20 AM
Your version:
void Node3D::DetachChild( Entity3D* Child )
{
    for ( auto pChild : m_vChildren )
    {
        if ( pChild == Child )
        {
            pChild->DetachParent();
            pChild = nullptr;
        }
    }
}
The corrected version:
void Node3D::DetachChild( Entity3D* Child )
{
    for (int i = 0; i < m_vChildren.size(); i++ ) //for ( auto pChild : m_vChildren )
    {
        if ( m_vChildren[i] == Child )
        {
            m_vChildren[i]->DetachParent();
            m_vChildren[i] = nullptr;
        }
    }
}
Problem is here that pChild = nullptr; not overrides the object inside the std::vector, because for ( auto pChild : m_vChildren ) makes copy by value. But m_vChildren[i] = nullptr; works.

Correct me if im wrong.
Coordinator
Jul 19, 2013 at 12:21 PM
I'm pretty sure you are correct - I should be using a reference to the child, rather than a copy of it. Can you try to add a '&' after 'auto' in the first example and see if that takes care of the issue?
Jul 19, 2013 at 4:11 PM
Ive tested it. Works well.
void Node3D::DetachChild( Entity3D* Child )
{
    for ( auto& pChild : m_vChildren )
    {
        if ( pChild == Child )
        {
            pChild->DetachParent();
            pChild = nullptr;
        }
    }
}
Jul 23, 2013 at 11:52 PM
Edited Jul 23, 2013 at 11:54 PM
Attaching a child the same:

Should be then:
void Node3D::AttachChild( Entity3D* Child )
{
    // Check for open spots in the vector
    for ( auto& pChild : m_vChildren )
    {
        if ( pChild == nullptr )
        {
            pChild = Child;
            Child->AttachParent( this );
            return;
        }
    }

    // If no open spots then add a new one
    m_vChildren.push_back( Child );
    Child->AttachParent( this );
}
or the nullptr in m_vChildren will not be filled with an object.
Coordinator
Jul 29, 2013 at 1:01 AM
These both should be fixed in the latest commit. Thanks for posting them!