Scene::RemoveActor(Actor* pActor) Bug?

May 10, 2013 at 9:13 PM
Edited May 11, 2013 at 4:14 PM
Am I wrong or could it be that the while loop will never leave.
auto it = m_vActors.begin();

while ( it != m_vActors.end() ) {
    if ( *it == pActor ) it = m_vActors.erase( it );
}
full mehod:
void Scene::RemoveActor( Actor* pActor )
{
    // First we iterate through the list of actors and remove any instances of 
    // this actor. 

    auto it = m_vActors.begin();

    while ( it != m_vActors.end() ) {
        if ( *it == pActor ) it = m_vActors.erase( it );
    }

    // Now we remove the actor's node from the scene's root node, if it is there.
    // In general, an actor should always be attached directly to the scene's 
    // root, but there may be cases where an actor is attached to another actor.
    // To accommodate both situations, we do the removal by accessing the actor's
    // node's parent.  This ensures that the actor is completely removed from the
    // scene, and can safely be added back into the scene at a later time.

    Node3D* pParent = static_cast<Node3D*>( pActor->GetNode()->GetParent() );
    if ( pParent ) pParent->DetachChild( pActor->GetNode() );
}
Coordinator
May 14, 2013 at 2:47 AM
I am fairly certain that you are correct - in the case that the first (or any for that matter) actor doesn't match, then the iterator doesn't advance. I'll have to add an else statement and use that to increment the iterator when there is no match.

Thanks for that - it would be a potentially tricky issue to track down, and you saved some great amount of debugging :)
May 14, 2013 at 5:37 AM
Edited May 14, 2013 at 5:38 AM
"I'll have to add an else statement and use that to increment the iterator when there is no match." correct!

Furthermore, I saw that there was somewhere else the same problem again. Dont know where it was now.
Coordinator
May 18, 2013 at 1:52 PM
I updated this and the build should be committed this weekend (either today or tomorrow). If you find that other instance, please let me know where it is! I looked around but didn't find any similar constructs...

Thanks for the tip!