[SDL2] A (yet another) simple Pong clone to review

Started by
15 comments, last by adrian17 10 years, 10 months ago

Hi!
As the first step made a simple Pong game, both to get a bit of practice and to have a nice code foundation for future projects. I've used SDL2 and a tiny bit of sdl2_mixer, just to get the hang of it.

I'm quite happy with the code, although I guess some things could have been done better, eg. file structure. The only thing I really don't like is how I had to divide paddle's input functions to InputRight() and InputLeft(), but I didn't know any other good way of doing this (aside from making subclasses for left and right paddle, which didn't seem worth the effort to me).

https://github.com/adrian17/pong

(it's also my first time using Git, I hope I didn't mess anything up)

Regarding SDL, I have a few small "I have no idea why this happens" issues, mostly regarding toggling fullscreen with SDL_SetWindowFullscreen. For convenience, I've written these in my repo's "issues" section.

Advertisement

Your Game class contains std::vector<Particle> particles; which I feel is out-of-place. Imagine you'd want to have main menu with particles - you would have to create another vector<> in other class, yet again make same rendering loop & so on. Consider moving creating class such as ParticleSystem or something similar, so that it would simulate and render all particles.

Thanks! I've already pushed an update in which I added the ParticleManager class (in the particles.h file). I see what you mean - if I ever want to expand the particle system or make specific particle subclasses, it should make managing the code easier.

My new concern right now is, the Task Manager shows constant increase in memory usage every time I switch between Game an MainMenu. Looks like a probable memory leak, but I have yet to find it.

EDIT: I've found the leak: for some reason, when I use Mix_LoadWAV(); in the constructor and Mix_FreeChunk() in the destructor, the memory isn't freed completely. Strange...

  1. Are you sure destructor is being called? I remember having problems when compiler decided not to recompile properly.
  2. Does memory usage increase indefinitely? Possibly memory segmentation increases usage or SDL's GC/caching (if any) decides not to free memory right away.

Are you sure destructor is being called? I remember having problems when compiler decided not to recompile properly.

I think it is, and I don't see any reason why it wouldn't be called.

Memory usage didn't increase at all when I commented out all mixer-related functions. I'm also sure that the ball, board and paddle objects are destroyed.

Does memory usage increase indefinitely? Possibly memory segmentation increases usage or SDL's GC/caching (if any) decides not to free memory right away.

I checked it - after switching between MainMenu and Game for a while, reported memory usage jumped from 13MB to 30MB and was still getting higher. Every time I started the game, it increased by approx. 100KB (sound files have a total size 94KB).

Try cleaning the project and rebuilding it.

Also, put a printing statement (console print, messagebox, etc) inside the destructor to see if it is really being called correctly, although as you said, i don't really see a reason as why it wouldn't be called...

I'm sure that destructor is called - in fact, I can see Mix_FreeChunk; being called. If comment it out, the memory usage increases even faster.

That's actually quite strange - when the Mix_FreeChunk is called in the destructor, memory usage doesn't drop at all - it just influences how big the increase is when loading the WAV in the constructor. That means that the "leak" occurs when I try to load the sound into recently freed memory, am I right?

(by the way - I'm aware that i could just load the sound files when launching the game instead of loading them every time in the object constructor, but that's not solving the problem, that's getting around it. If I ever had a game with complex levels and wanted to load specific resources for each level, the problem would resurface)

Try to replace the Mix_FreeChunk() line by this:




  if(point->allocated == 1)
  delete [] point->abuf;

delete point;
point = nullptr;

if it still leaks, try the following:




delete [] point->abuf;
delete point;
point = nullptr;

Though i'm pretty sure this last example will crash the game, bt bear with me and try it anyway...

I'm no expert in SDL, i'm just reading the SDL wiki, trying to find out why it leaks memory.

It's funny, because other tutorials use the same as you to allocate & delete the audio chunk, yet no one says anything about a leak. Either the problem is something else, and we're very wrong, or no one else ever checked the process manager for leaks...

Still no improvement, with both your examples (the last one didn't cause any crashes). By the way, the sound plays properly every time.
In the meantime I've recorded a video to better visualize what's going on:

I've seen the video, went through your code a bunch of times, checked the SDL documentation, and i still can't explain why it happens...

By the way, the examples above are basically what MIX_FreeChunk() does, and the reason why the 2nd might crash the game, if because is "point->allocated" isn't 1, then it doesn't contain the audio buffer and the "point->abuf" shouldn't be deleted.

Sorry, but i can't really help anymore.

This topic is closed to new replies.

Advertisement