Hello.

In my Player class, when the destructor is called to destroy its bitmap, it causes the game to become unresponsive and ultimately crash.

player.hpp

player.cpp (the parts that matter)

For the time being, I have the destructor code commented out, to avoid crashing, but this potentially leads to memory leaks. My Player object is defined in main() and is initialized with Player.init(480, 360) (duh). I'm not sure why this causes a crash. Any ideas?

I appreciate your feedback.

Your player destructor will be called just before main returns.
You might possibly already have freed everything allegro related by then. Since you haven't posted your main function, I can merely make assumptions here.

Possible solutions are, create a destroy method, which might fit in your design, as you already use an init method; use new and delete; run the game (and create the player) in a different method; use a code block to limit the scope of your player.

Also

void Player::init(int screen_width, int screen_height) {
   ...
   
   Map.init("txt/map.txt");
   for (int i = 0; i < number_of_potatoes; i++) Potato[i].init(screen_width, screen_height);
   ...
}

Very bad design, the player should not initialize the map or other game objects.

You should post your main().

Yes, show us your whole program. No doubt, it will be a beautiful demonstration of the fact that C++'s RAII idiom doesn't work in the general case. :p

Hey guys, thanks for the feedback--it is much appreciated!

Taron, I took your advice and incorporated a custom destructor function for my Player object, and called upon it once main was ready to die; it worked like a charm! Also, you mentioned my poor design choice; this I know, but was done on the fly so it's not too bad for just testing things. What would you suggest in the future? Should I define the objects in main next time and do their functions there?

Torhu and Beoran, thanks for your interest in my endeavors as well; I don't think I need to post my main anymore, as the above worked.

You don't need to destroy or free anything before exiting, it's taken care of by Allegro and the operating system. al_uninstall_system gets called automatically after main returns, see atexit. When the process is done, any allocated memory, open files, etc. still left will be cleaned up by the OS.

Interesting! I had always utilized al_destroy_x to deallocate things, because I thought that it would otherwise lead to memory leaks. This is great to know then! Now, what would happen in the event that the game crashes prior to a clean shut-down; would it result in memory issues then? Thanks for the input!

No, if a process crashes, modern operating systems will completely free all the memory the process requested.

I'd like to note though that if you're using a memory checking tool, such as valgrind, that will complain if you don't deallocate your memory on exit manually. But yes, it's not strictly required since the OS takes care of that.

You still need to free the memory of anything that doesn't stay allocated for the whole time the program runs.

Just to make sure, the problem that was happening with the OP is the lack of rule of three, correct? C++ requires classes to have a... -- let's see if I still remember -- a default constructor, a copy constructor, and a destructor. ...Otherwise, destructors are just called when an instance is created. I'm still not sure what that thing is all about, but oh well.

As for the design choices taron mention, pretty sure it was just about the fact that the map creation should be independent from the player creation. For your game, it probably doesn't matter, as you probably can't have the map without a player, but it's certainly not the most logical design choice.

No the problem is that because of RAII, his destructor was most likely called AFTER allegro had been shut down, hence the unloading failed. That's what I ment with RAII being useless in the general case.

In this case you need to implement a custom destroy() method anyway, since RAII doesn't give you much control over when the destructor will be called exactly, since C++ arranges for it to be called behind your back.

Or you need to do as taron said and add a block around the usage of the object that uses the RAII destructor to force the compiler from calling the destructor sooner rather than at the end of the program / function...

Things like these are why I don't think C++ is to an improvement over C. I'd rather call `{Player player = player_new() ; /* ... */ ; player_free(player); }` at the right time and place and be done with it, rather than having to remember the 13 pitfalls of RAII.

atexit runs after main exits (and therefore, after objects local to main get destroyed) but before global destructors run, hence the problem. Allegro was shut down by the time al_destroy_bitmap was called.

Solution : Make player a global pointer and new it when you enter main and delete it before you leave main. This way your object is still global (if you really need it to be, which you don't). Or make player local to main.

And your player class looks more like a game class, since it contains game or world objects.

And there are only 3 times when a destructor is called. When a block goes out of scope. When you call delete. Or when global destructors run after atexit.

Edgar Reynaldo,

Sure, but if you use a pointer together with new/delete, you are not using the famous C++ RAII idiom anymore. Which is exactly the point I was making: RAII isn't useful in he general case. What you suggest is equivalent to malloc()/free(), or wrapped versions of them, in C.

Why not use C then and save yourself the complexity? C isn't going to coddle you, but it just means that we have to remember one rule: "close every resource you open and free all memory you allocate" in stead of 37 rules in C++ like "destructors are called when..." , "rule of three...", "don't raise exceptions in...", etc... . As long as you're clear about the ownership policies of the resources, it's as easy as opening a door, going through it, and then remembering to close the door behind you.

If both Allegro and the operating system purges resources upon shutdown, what purpose then would Allegro's al_destroy_bitmap function offer? Considering how the system handles it for you, it seems to reason that the function itself is moot in the eyes of the programmer. No? Better yet, in what instance would it be a good idea to use the function?

This new information has perplexed me as to why the function exists to you and I if the system manages deletion for you.

If you had a utility to view bitmaps, then each additional viewed bitmap would take up more memory and eventually you'd run out.

Could you elaborate on that a bit more? What exactly do you mean by "utility" (that's fairly ambiguous in my books, sorry.)

I just meant if your program loads lots of bitmaps, and doesn't need them for the entire run of the program, it should release the ones that aren't needed any more to free up memory for the next ones. Memory leak.

Shegoth,

The operating system does not "manage" the memory for you. The operating system just notices that your program hasn't deallocated memory or released certain resources when it exits and goes "GRRRR! another badly written program that didn't clean up after itself! I'll better take care of that or other programs will be affected by it!" And then the OS deallocates all that left-over memory and releases the resources that program uses.

While the program is running, you are responsible for releasing resources and memory. In C, this is done manually. In C++, there are destructors that may or may not help you to do this behind your back.

For example, suppose you have a level background that gets loaded every time the level changes.

ALLEGRO_BITMAP * background = NULL;

bool background_load(int level) {
   char background_name[100];
   sprintf(background_name, "background_%d.png", level);
   background = al_load_bitmap(background_name);
   return (background != NULL);
}

The function background_load above has a memory leak. When you load the background, the reference to allocated memory you had in background is lost and overwritten, without freeing it.

bool background_load(int level) {
   char background_name[100];
   sprintf(background_name, "background_%d.png", level);
   if (background) { 
     al_destroy_bitmap(background);
     background = NULL;  
   }
   background = al_load_bitmap(background_name);
   return (background != NULL);
}

This is already a bit better. Now, the old background will be freed, and the memory it used released, before the new background is loaded.

Open the door, go though it, and close it again. Load the resource, use it and then destroy it. It's as simple as that.

Edit: I forgot two ; in the return statements.

Aah okay, that makes sense; thank you.

Edit
Beautiful example, beoran! I think that pretty much clears that up now. I was confused with memory leaks between run-time and exiting... But have a good idea now. Thanks!

I recommended a global pointer because it looked like his program depended on global access. Not because it was a better practice.

And Resource Acquisition Is Initialization is useful to a degree. It means when you create a resource, initialize something that will take care of freeing it for you.

And if you're gonna argue theres more reason to use C I will counter with the fact that you have to free the resources you create whether it is in C or C++, but C++ can be nicer because you don't have to remember to do it for every object, just to take care of it properly in the destructor. No C/C++ flame war please.

Hey I'm cool!

No the global pointer is fine. The rage against globals is overrated.

And of course, I don't want to start any flame war. I'm just griping about C++. If you like it, then be my guest in using it.

However, I remain convinced, that for the beginning programmers, it would be best if they started learning plain C first, and only then C++. You'll learn both languages much better that way. Learn to walk before you learn to run (into trouble?).

Meh. I learned both at the same time as I went along. But there were some nice resources for C++ online that I took advantage of like cplusplus.com and the sgi stl reference.

Classes aren't all that complex, but there are some caveats to creating one, and operator overloading can be confusing at first if you don't have a good reference. I don't know, I just learned C as I was learning C++. C is the base of C++ anyway. So by learning C++ you are also learning C. Meh IDK w/e.

Oh, and you can still use a global pointer with a local object. Just don't use it outside of main!

Then let us test your C skills.

int function1( int (*ptr)[10] )

Valid C / Invalid C ?

Looks invalid - the function pointer parameter doesn't have any parameters of its own. It should either be int (*ptr)(void)[10] or have some other kind of input parameter. Otherwise it is not a valid function signature. I don't remember the exact syntax for an array of function pointers though.

It's actually valid.

Taken from a program I wrote.

Now you might think I set a trap for you there to create the impression that I'm an C master. I'll understand if you feel pissed, but I must say I'm not a C master.
My C++ skills are <10% of yours but I do think it's a mistake to underestimate the difficulty of C. Timorg knows why it's valid, he taught me it.

Hey, William, even I I had to break out cdecl for that one, but now I understand too. Arguably it's not that useful except in cases where you happen to have a constant size array you want to pass to a function.

And that's what I mean with "learn C first". If you learn C from learning C++, you won't learn C well. And if you don't know C well, then there's many fine points of C++ you won't be able to learn well either. Even Stoustrup learned C first.

Here's one that cost me probably days of tedious debugging.

variable = a + b + c + get_some_variable() == 5 ? 10 : 20;

Call me a dunce but I made that mistake many times.

variable = a + b + c + (get_some_variable() == 5 ? 10 : 20);

Is what I mean.

I'm guessing that's only valid in C becase C is retarded sometimes, allowing you to declare functions without any input parameters. C++ would never let you get away with that.

Edit
Ah, I see. That is an integer pointer, not a function pointer. Why does it use a function pointer syntax int (*ptr); though? That's one of the odder things that I didn't know about C.

Parenthesis are probably ignored if there's not a second set like int (*ptr)().

I was thinking it was the same as how

int array[10];
printf("%d\n",array[5]);

is equal to

int array[10];
printf("%d\n",5[array]);

For what it's worth, all three of these functions print the value in the offset passed.

Well, yes, Arthur, but the program should give you warnings when compiled because you're converting an integer to a pointer and then back to an integer when you use printf().

Maybe the following modification would be instructive: