So, I'm programming this 2D sidescroller, and was working on adding a map format for it, along with collision today. Well I had a background image that needed to be the exact size of the screen, but when I reference the h and w ints in the SDL_Surface class, it crashes the game. :P Code: http://www.teamgaben.com/sdl_game.rar [Edited by - Essel on January 18, 2009 11:53:33 AM]

Quote:Original post by Essel
Well I had a background image that needed to be the exact size of the screen, but when I reference the h and w ints in the SDL_Surface class, it crashes the game.

Have you checked if you're actually pointing at a surface? (assuming its a pointer). If you try to get the w/h from a surface thats not there you'll get a segmentation fault and your program will crash.

Normally, most people don't want to go crawling through files and files of sourcecode looking for someone else's bugs. Hey, we barely want to go crawling through sourcecode looking for our own bugs. [wink]

You are much more likely to get help in the future if you copy and paste the relevant* parts of your code in between two [ source ] and [ /source ] tags, to place them in a nice scrollbox for us to easily read.

*It's just as bad if you copy and paste your entire code. Only post the parts that are messing up.

I downloaded the code (which I rarely do), and there are 16 source files for me to shift through, when you sound like already know where it's crashing at.

Anyway, here is what I've found:



When you SDL_FreeSurface() a surface, it does not point it to NULL after it frees it.

As a result, when you do things like this: (CMap::Load() function in cmap.cpp)


m_pSky ends up pointing to garbage. You can't check it for NULL, because it's not pointing to NULL, it's pointing to some seemingly random portion of memory.

You should set every SDL_Surface to NULL after freeing them, unless you are in a destructor, so you are 100% sure the pointer isn't being used anymore.

SDL_FreeSurface( m_pSky );m_pSky = NULL;

I see that you did this several times: (CMap::Load() function in cmap.cpp)

if ( m_pSky = NULL )		return false;

This sets m_pSky to NULL, it doesn't check if it is NULL, so this always returns false.
You want two equal signs, like this:

if ( m_pSky == NULL )		return false;

If you make this mistake alot, you can type your if-statements backwards, like this:

if ( NULL == m_pSky )		return false;

And the compiler will catch alot of those mistakes when you are compiling. Example:

if ( NULL = m_pSky ) //Compiler: "I can't set NULL to anything! Error!"		return false;

Ofcourse, this doesn't work when comparing two variables, and personally, I don't like this way, but it's a option I thought I'd mention.


You are using C++, right? I mean, you have .cpp files, you have C++ style classes, you are using the singleton pattern... So why aren't you using C++ style strings or C++'s file streams? It makes things alot easier on yourself. [smile]



So, in short, your problem is most likely this: (I wasn't able to compile and confirm, I don't have Visual C++ properly configured yet)



I'm tired, it's late, if I didn't explain anything well, please go ahead and ask and I'll answer tomorrow.

I just want to tell you, though, that your code looks very neat and clean. Excellent job! It's not commented too well in comments, but clean code is its own comment, and I had no trouble reading your code.

My only recommendation (since I'm also still a young programmer) is to embrace C++'s standard library. If you use C++, you might as well use the default tools it gives you, or get tools to replace the defaults (Like boost, which I personally haven't used).

It doesn't make sense to use the poor tools for the job when better ones are sitting two feet away. But hey, if it works for you, then go with what's comfortable and what helps you complete your game. Just try not to let comfort hinder progress. [smile]

Thanks for reading the code and helping me out, I woke up this morning realizing that I hadn't told you where exactly it was crashing at. Anyway I can't believe I didn't catch those if statements not having ==.