2

I'm currently working on a game as a project for my University. It's being made in C++ with SDL2.

I have a vector that holds pointers of the class Enemies, which is an abstract parent class of the Plant class. In the constructor of my enemy manager, I am pushing back a pointer to a Plant object into the enemies vector.

m_pEnemies.push_back(new Plant(projectileManager));

At some point, the plant comes in contact with a projectile (the detection works fine and all), and I run this piece of code in order to remove it from the vector:

Enemies* temp{ m_pEnemies[i] };
m_pEnemies[i] = m_pEnemies.back();
m_pEnemies.back() = temp;
delete m_pEnemies.back();
m_pEnemies.pop_back();

The enemy is destroyed during the game, and I am getting no run-time errors for illegal memory accessing, but there are memory leaks. When I placed a breakpoint, it showed that the destructor of the Plant object does not get called. It should be when I pop the back of the vector, but for some reason it doesn't.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
Alex
  • 75
  • 7
  • 8
    Is your base class destructor virtual? Also just as general advice: Don't use raw pointers but smart pointers (`std::unique_ptr` or `std::shared_ptr`). Ideally you don't call `new` and `delete`. – Philipp Jun 15 '22 at 22:17
  • 2
    Maybe this will help you https://stackoverflow.com/questions/461203/when-to-use-virtual-destructors – Jane Doe Jun 15 '22 at 22:18
  • 2
    Obligatory reminder that manually calling new/delete is unsafe, and should be done for practice only (or when writing custom containers or smart pointers). Prefer storing `std::unique_ptr`s in the vector. – HolyBlackCat Jun 15 '22 at 22:18
  • 1
    Look at [The Rule of 3/5/0](https://en.cppreference.com/w/cpp/language/rule_of_three). Then look at "auto_ptr" (deprecated as of C++11) and "unique_ptr": https://www.geeksforgeeks.org/auto_ptr-unique_ptr-shared_ptr-weak_ptr-2/. And consider a different approach (that doesn't rely on "new"). – paulsm4 Jun 15 '22 at 22:24
  • Need to provide more details for anyone to provide an accurate answer. The above comments probably pretty much tell you what you need to do, though. – Den-Jason Jun 15 '22 at 22:36
  • 1
    Also..... you should remove the item from `m_pEnemies` before deleting it; if that's shared in a multithreaded environment then you're asking for potential use-after-free (when you're deleting it whilst it's still a member of a collection). – Den-Jason Jun 15 '22 at 22:39
  • FYI, you don't need to move the enemy being `delete`d to the back of the vector just to pop it: `auto* temp = m_pEnemies[i]; m_pEnemies[i] = m_pEnemies.back(); m_pEnemies.pop_back(); delete temp;` Or, you can simply remove the enemy directly: `auto *temp = m_pEnemies[i]; m_pEnemies.erase(m_pEnemies.begin()+i); delete temp;` Alternatively: `auto it = m_pEnemies.begin()+i; auto *temp = *it; m_pEnemies.erase(it); delete temp;` In any case, if you use `unique_ptr`, you won't need the `delete` anymore, thus simplifying the code even further. – Remy Lebeau Jun 15 '22 at 22:40
  • 1
    @paulsm4 `auto_ptr` can't be stored in a `vector`. That is one of the reasons why `unique_ptr` and move sementics were invented in the first place. – Remy Lebeau Jun 15 '22 at 22:41
  • 1
    @paulsm4 `auto_ptr` isn't just deprecated, it has been removed from the language. There is really no need to mention it except in a historical context as it is more likely to lead to confusion than anything else. – François Andrieux Jun 15 '22 at 22:50
  • 1
    @paulsm4 -- Geeksforgeeks is not a reputable site to learn C++ from. – PaulMcKenzie Jun 15 '22 at 22:50

1 Answers1

1

I got the answer from Philipp's comment!

I had forgotten about virtual destructors, thanks for that!

And for everyone that is mentioning "smart pointers", we haven't covered them in our course so I am not allowed to use them :/.

Alex
  • 75
  • 7
  • 1
    Fine. Next time, please post a [mre], it makes it so much easier to help you. – Paul Sanders Jun 15 '22 at 23:01
  • *we're not allowed to use them*. Got to love education that stifles any initiative on the part of the pupil. – john Jun 16 '22 at 06:07
  • @john While I do agree, they do need to grade us based on what we have learned during the course so if I start using things that they never taught us and I use them wrong should they remove points for that? I don't know it makes sense to me that we aren't allowed to use certain things we haven't covered. – Alex Jun 16 '22 at 14:30
  • @Alex It a fair point but can they not also grade you on things like flair and initiative? But I can see that restricting you to certain topics makes things easier for the teachers. – john Jun 16 '22 at 14:49