Jump to content
  • Advertisement
Sign in to follow this  
Alian Vesuf

C++ SDL - Deleting dynamically allocated objects

This topic is 2084 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hello GDNet. I am studying the SDL library and while doing so i find myself in troubles back in the C++ basics. 
The question is : If you delete an object that is dynamically allocated, and then try it again does it cause problems or leaks ?

 

The issue is as follows:

I have a Sprite class which basically deals with loading sprites and drawing them onto the screen.

I have Game class with functions like Init() Draw() Update() Quit().

I dynamically create new sprites in the Game class for each image i want to draw, and then i delete objects in the Quit() function.

 

In the main game loop i experimented with some things. I attached a key event to delete specific sprite object which actually removes the

sprite from the screen and clears the allocated memory. Note that i did that in the game loop. And in the Quit function i also delete

that object. Is this ok ? It does not seem to crash the program or anything but what happens behind the scenes ?

This can be an issue when the Player kills an Enemy and i free that specific object, and then back in the Quit function i try to 
free the garbage again.

Here is some code for reference

game.cpp

 Sprite Background = new Sprite();

Game::Initialize()
{
    Background->LoadSprite("bg.bmp");
    /........../
}

Game::Loop()
{
    while(Running){
      while(PollEvent){
         if(KEY == "x"){
            delete Background;
           Background = NULL;
        }
     }
   }
}

Game::Quit()
{
    //Background once deleted and now again ?
    delete Background;
}

Share this post


Link to post
Share on other sites
Advertisement

If you try to call delete on a NULL pointer, it will be ignored.

There's no necessity of a null check like:

if (pointer)
    delete pointer;

Even less:

delete pointer; //Free allocated memory.
pointer = NULL;
delete pointer; //Just to make sure.

That second delete in your code is doing nothing, but it detracts from your code's readability, potentially making it confuse to anyone reading it (even you).

Even if it does no harm directly, it is doing no good either, and I would advise against this.

Always prefer to make clear code and it will make your life way easier when a nasty bug appear!

Edited by dejaime

Share this post


Link to post
Share on other sites

I am confused here. Basically i delete all objects in the Quit function. But if an enemy object gets killed in the main loop where game logic happens i free the texture and delete the object. Also you never know if the enemy will be killed in the main loop so you need to take care of it in the end function also.

 

So the best practice is to assign NULL to pointers that don't get to play anymore ( if they ) and delete them in the end just to make sure or if they survived game logic ?

Edited by aliasc

Share this post


Link to post
Share on other sites
Don't do that.
 
Double-delete is a very serious bug.

Sorry about that, fixed the code with what I originally meant to write.

I'm just too used to my overloaded delete that nullifies it for me; my bad.

 

 

@ Back on topic,

Under normal circumstances, you delete any objects when you know for sure they won't be reused.
Sometimes that happens during the gameplay, and sometimes at the end.
 
Still, you can avoid going around calling delete, by, for example, calling a function to make unused objects delete themselves.
 

#include <algorithms>
#include <vector>

bool exit;

class AwesomeGameObj {
    private:
        static std::vector<AwesomeGameObj*> MasterVector;
        bool isUseless;
    public:
        AwesomeGameObj () {
            /*Initialize everything*/
            isUseless = false;
            MasterVector.pushback(this);
        }
        
        ~AwesomeGameObj () {
            /*Do the cleanup*/
            MasterVector.erase(/*this object*/);
        }
        
        void de_init () {
            while ( !MasterVector.empty() ) {
                delete MasterVector.back();
                MasterVector.pop_back();
            }
        }
        
        bool useless () const {return isUseless;}
        
        static void clean_up () {
            for (/*every object in MasterVector*/)
                if (object->useless()) {
                    vec.erase(/*object*/);
                    delete /*object*/;
                }
        }
        
        static void clean_up (std::vector<AwesomeGameObj*> vec*) {
            for (/*every object in vec*/)
                if ( object->useless() ) {
                    vec->erase(/*object*/);
                    delete /*object*/;
                }
        }
        
        void update () {
            /*do anything it needs to do*/
            if (/*something very very sad happened*/)
                isUseless = true;
        }

};

void quit () {
    AwesomeGameObj::de_init();
    /*do it for any other object types that needs this clean up here*/
}

void init_all () {
    /*init anything*/
}

int main()
{
    //Game Objects in general
    std::vector<AwesomeGameObj*> gameObjects; 
    
    //Fix don't-delete-while-running game objects.
    std::vector<AwesomeGameObj*> miscUseObjects;
    
    init_all();
    
    //Initialize some special objects
    while (/*needs misc objects*/)
        miscUseObjects.push_back (new AwesomeGameObj);
    
    //Main loop
    do {
        while (/*needs new objects*/) {
            gameObjects.push_back (new AwesomeGameObj);
        }
        
        for (/*all gameObjects and miscUseObjects*/) {
            object.update();
        }
        
        if (/*you want to free memory*/)
            AwesomeGameObj::clean_up(gameObjects);
        
    } while (!exit);
    
    //We'll most probably have objects remaining;
    quit();
    
    return 0;
}

As you can see in this example pseudo code, I avoid to call delete on the main and quit functions. I do this by passing on the responsibility of doing so to the object's own class, and all I have to do is tell it to clean up its memory, despite how many objects I need to free and where they are, where they were allocated or anything. This way my main and end functions doesn't have to worry about remaining objects nor freed objects.

Edited by dejaime

Share this post


Link to post
Share on other sites


Second: You usually use an dynamic array (std::vector) to keep track of game objects.


Do you mean for specific types of objects. How would you store all of game objects in a dynamic array when they all are of different types.
You can of course std::vector<Sprite> spriteList. Also can you check if specific object exists in the list before deletion ?

Anyway this : 
int* a = new int;
delete a;
a = nullptr;
delete a; // <-- OK

means after delete pointer assign nullptr and then delete the null pointer which is safe and so it solves my problem! Thanks !

Share this post


Link to post
Share on other sites

I may have missed something, but is there a reason to keep the Background object as a pointer? Would it be better to keep it on the stack and rely on it being automatically cleaned up when the Game object dies?

You obviously use a load function to get the texture into memory, you could also use an unload function to remove it from memory if the need exists. And deciding if it should be drawn or not is also something that should exist if you have such requirements, it's easier to give it a bool isDrawn, and instead of deleting the object, simply set it as isDrawn = false. Seems like a nicer choice to me.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!