Window display() and removing elements of a vector array

[OrreBorre]

Hi! I don't know if there is a similar post in here already, there were just too many posts to go through them all.

I have this strange problem. I am working on a beginner project in a game development course.

I am doing colliders and collisions.

What I want to do is I want to remove one specific collider in case it is colliding. I have made a crude collider identification system using ints. 1 means it is a player and 2 means it is an enemy. (I am aware of the inconvenience of this system, I will change that to enums as soon as I get it working. the problem is not the identification system.)

The problem is that when these two, player and enemy, are colliding I want to remove the enemy collider. I have a function that iterates trough my vector via their index and if the current element's collider is an enemy, I delete the collider, set it to nullptr, swap this element with the last one and run pop_back(). I have debugged and it works, I have doublechecked to track the collider in it's vector.

I have drawn rectangles for each of these colliders, the one I delete only synchronize its position to its rectangle (sf::RectangeShape() ) if (collider != nullptr).

When it's time for sf::renderwindow::display() it crashes. All the other collisions work like I want them to, meaning the displayfunction works.

The remove function is only called if player and enemy have collided. this turns a bool to true. what am I missing. I thought this is relevant because I use SFML and it crashes (from what I can tell when everything is supposed to display)

All I get for an error message is <the program> has triggered a breakpoint (not one of mine) I use VS Ultimate 2012

code:

void CollisionManager::RemoveCollider()
{
        int count;
        count = 0;
        int index;
        for (int i = 0; i < m_axRectColliders.size(); i++)
        {
                if (m_axRectColliders[i]->GetEnemy() == 2)
                {
                        delete m_axRectColliders[i];
                        m_axRectColliders[i] = nullptr;
                        index = count;
                }
                else
                {count++;}
        }
        std::swap(m_axRectColliders[index], m_axRectColliders.back());
        m_axRectColliders.pop_back();
}

only updating if it is not nullptr m_rect2 is the collider, m_rectShape2 is its shape

if (m_rect2 != nullptr)
        {
                m_rect2->SetPositionX(m_rectShape2.getPosition().x);
                m_rect2->SetPositionY(m_rectShape2.getPosition().y);
        }

the update takes place before the removal:

mgr->CheckCollisionRectVsRect();
        m_rectShape.setPosition(m_rect->GetPosition());
        m_rectShape2.setPosition(m_rect2->GetPosition());
        if (mgr->GetPlayerVsEnemy())
        {
                mgr->RemoveCollider();
                mgr->SetPlayerVsEnemy(false);
        }

I set the playershape(m_rectshape) to it's collider's position and the enemy's the same. this have no effect on my problem because it happens before I call RemoveCollider() and I have tried commenting the position updates away with no result. Why I do this is so that I can push them away with each other. I played a little with the collisions before this.

And here's where the program crashes:

void GameState::Draw()
{






        m_pWindow->clear(sf::Color(0x11,0x22,0x33,0xff));
        m_pWindow->draw(m_shape);
        m_pWindow->draw(m_shape2);
        m_pWindow->draw(m_rectShape);
        m_pWindow->draw(m_shape3);
       
        m_pWindow->draw(m_rectShape2);
        m_pWindow->draw(m_rectShape3);
        m_pWindow->display();
}

at the end of the scope.

I can't think of anything else I have tried different things I am sure it is something about how I remove it but I just don't know what...

[Nexus]

Hi! I don't know if there is a similar post in here already, there were just too many posts to go through them all.

That's why people usually search for things

Your loop looks strange. Why don't you break if a collision is found, or swap immediately instead of once at the end? Also, int is the wrong type, you should use std::size_t for indices. And I'll never understand why people write code like

int count;
count = 0;

if they can initialize the variable directly

In general, your approach of element removal looks very complicated to me, because you mix the swap-and-pop_back idiom with manual memory management, null pointer states and other logic. I would do it as follows:

• Store all elements as plain objects (std::vector<Collider>) or if it's really necessary smart pointers (std::vector<std::unique_ptr<Collider>>), but definitely not raw pointers that require manual memory management.
  
• Perform collision between all objects, set a flag for the ones who die during the collision.
• Use std::remove_if() and std::vector::erase() to remove all dead colliders at once.

[OrreBorre]

Why don't you break if a collision is found, or swap immediately instead of once at the end?

It is not a collision detection loop I try to find the collider to remove after a collision have already occurred.

But I suppose I could have swapped immediately instead.

well lol It sure looks awful now that you've mentioned it about the count variable but I guess I wasn't focusing on trivialities such as that, but to get this to work before I have pulled all my hair from my head

But thanks for the help I will try the remove_if and give the erase another go

[OrreBorre]

or did you mean I could remove it in the collision detection loop btw??

[Nexus]

No. As stated, set a flag when colliding, and remove the marked elements in a separate loop afterwards.

[OrreBorre]

Hi thank you for the advice. I managed to solve the problem. I don't know if it is customary to update a solved problem or just annoying, please tell me if it is.

Any way, the main problem was that I used the actual colliders in the position update instead of updating via the vector array! I am suspecting that lead to confusion for the compiler and that when you push something into the vector you make a copy of it. any way it works now.

The reason to why I assigned it nullptr was to not have a dangling pointer.. I'll have to research the smart pointers, they seem convenient.

Awesome and fast help. thanks again.

[Nexus]

It's appreciated if you update solved problems, so that future readers can benefit from it