SDL Problem

Started by
11 comments, last by BitMaster 11 years ago
When you load an image or resource, you check if it is NULL and then exit if it is. Very good!

However, you don't go far enough. Your code needs to tell you where/what fail, when it fails, and hopefully even why.

For example, you have this code:
bool loadFiles()
{
	background = loadBMP("background.bmp");
	if(background == NULL)
		return false;
	 
	if(!soundMgr->loadFile())
		return false;
	
	font = TTF_OpenFont("robotic.ttf", 24);
	if(font == NULL)
		return false;
	 
	return true;
}
It's (really truly) excellent that you check if those things return NULL, but if that function fails and returns false, you have zero clue what part of it failed. Was the font failing to load or was it the image? Or was it the sound? You have no idea - but your code does. So you need to have your code report the details of failures to you.

I said, "Your first line of defense in programming is listening to the compiler, and your second line of defense is your own code that error-checks your game code."
Exiting on failure isn't enough - your second line of defense needs to be bolstered to explain and report the errors. When the function fails, you need to know where it failed, and how.

One way is to use asserts() for things that are never supposed to fail, but you don't always want your program crashing to the desktop whenever image file #2057 failed to load.

Instead, you need some kind of logging system. Whether you report your errors to a log system that creates text files with the error logs in it, or whether you pump the output to the command prompt, you need some kind of textual description when things fail.

SDL redirects the std::cout and std::cerr output streams to text files, so you can take advantage of that. Report your errors to std::cout or std::cerr, and SDL will create files next to your executables called 'stdout.txt' and 'stderr.txt' so you can read the output even if your program crashed.

So, let's say your code is running along, and it comes across this line:
background = loadBMP("background.bmp");

First, you need to check for failures. This is good, you already got that covered:
if(background == NULL)
Second, you need to report the failure, to communicate to the developer or user where your code failed.
std::cout << "Failed in the 'loadFiles()' function." << std::endl;

Then you need to report what failed:
std::cout << "Failed to load 'background.bmp'" << std::endl;
Further, if you are able to, you need to communicate when it failed. By 'when', I mean the circumstances surrounding it. This could be the timing of when it failed, or even better, a callstack if you can get one. A callstack is the list of functions that led up to the function you are currently in. In your case, it'd output something like:
  • loadBMP() failed, called from:
  • loadFiles(), which was called from
  • main()
This is especially important as projects get larger and larger, and loadBMP() might get called from dozens of different locations. However, C++ doesn't have built-in standard support for callstacks (sad.png), so only compiler-specific methods exist. This is something you can ignore for now, but being aware of their existence as a very useful bit of information to report.

Also included in the 'when' is any variables that are relevant to the failure. In this situation, the only variable involved ("background.bmp") was already reported as part of the 'what'.
'When' is the circumstances surrounding the failure that led, or contributed, to the failure.

If possible, you'll want to report why the function failed. The only one who knows why the function failed is the function itself, so different APIs have different ways of reporting what errors occurred. Some APIs return special values that mean certain types of failures, other APIs have other methods.

SDL uses a function called SDL_GetError() to return a text description of what went wrong. Oftentimes, it's not too detailed and contains redundant information with what you are already reporting, but it's good to log that information also. When hunting down bugs, every little bit helps.
std::cout << "SDL_GetError() says: " << SDL_GetError() << std::endl;
Finally, you need to handle the failures. To handle a failure means to have your code decide what to do when something goes wrong.
In your existing code, your method of handling the failure is to exit. This is not always the ideal solution, because sometimes you can recover from errors, and sometimes failures are minor. But exiting is at least some kind of solution, and definitely needs to be done if your program can't continue running without a certain resource.
return false;

So putting it together, your code might look like this:
background = loadBMP("background.bmp");

if(background == NULL) //Did a failure happen?
{
	std::cerr << "In loadFiles():\n"                               //Report [Where] it happened.
			  << "Failed to load 'background.bmp'\n"               //Report [What] happened.
			  << "SDL_GetError(): \"" << SDL_GetError() << "\"\n"  //Report [Why/How] it happened.
			  << "<The callstack, if you got it>"                  //Report [When] it happened.
			  << "<the values of any variables that contributed to the failure>" //Also part of [When].
			  << "-----------------------------------------" << std::endl;
				 
	return false;
}
But let's think about this for a second further. This is alot of information to report. If your loadFile() loads multiple images, it'd look like this:
bool loadFiles()
{
	myImage1 = loadBMP("myImage1.bmp");
	
	if(myImage1 == NULL) //Did a failure happen?
	{
		std::cerr << "In loadFiles():\n"                               //Report [Where] it happened.
				  << "Failed to load 'myImage1.bmp'\n"               //Report [What] happened.
				  << "SDL_GetError(): \"" << SDL_GetError() << "\"\n"  //Report [Why/How] it happened.
				  << "<The callstack, if you got it>"                  //Report [When] it happened.
				  << "<the values of any variables that contributed to the failure>" //Also part of [When].
				  << "-----------------------------------------" << std::endl;
					 
		return false;
	}
	
	myImage2 = loadBMP("myImage2.bmp");
	
	if(myImage2 == NULL) //Did a failure happen?
	{
		std::cerr << "In loadFiles():\n"                               //Report [Where] it happened.
				  << "Failed to load 'myImage2.bmp'\n"               //Report [What] happened.
				  << "SDL_GetError(): \"" << SDL_GetError() << "\"\n"  //Report [Why/How] it happened.
				  << "<The callstack, if you got it>"                  //Report [When] it happened.
				  << "<the values of any variables that contributed to the failure>" //Also part of [When].
				  << "-----------------------------------------" << std::endl;
					 
		return false;
	}
	
	myImage3 = loadBMP("myImage3.bmp");
	
	if(myImage3 == NULL) //Did a failure happen?
	{
		std::cerr << "In loadFiles():\n"                               //Report [Where] it happened.
				  << "Failed to load 'myImage3.bmp'\n"               //Report [What] happened.
				  << "SDL_GetError(): \"" << SDL_GetError() << "\"\n"  //Report [Why/How] it happened.
				  << "<The callstack, if you got it>"                  //Report [When] it happened.
				  << "<the values of any variables that contributed to the failure>" //Also part of [When].
				  << "-----------------------------------------" << std::endl;
					 
		return false;
	}
	 
	return true;
}
That's alot of super-redundant information that is needlessly copied and pasted, and makes the function loadFile() harder to read.
So the error reporting should actually go into the function that actually failed, and not the function that decides how to handle the failure. In this case, loadBMP() should handle all of loadBMP()'s own error reporting. loadFile() only needs to respond to the failure and decide what to do next.
SDL_Surface *loadBMP(std::string filename)
{
	SDL_Surface *tempSurface = SDL_LoadBMP(filename.c_str());
	if(!tempSurface)
	{
		std::cerr << "In loadImage():\n"
				  << "\tFailed to load \"" << filename << "\"\n"
				  << "\tReason: SDL_LoadBMP() returned NULL\n"
				  << "\tSDL_GetError(): \"" << SDL_GetError() << "\"\n"
				  << "-----------------------------------------" << std::endl;
		
		return NULL; //Use nullptr if you're using the "C++11" standard of C++.
	}
	
	SDL_Surface *optimizedImage = SDL_DisplayFormat(tempSurface);
	SDL_FreeSurface(tempSurface);
	
	if(!optimizedImage)
	{
		std::cerr << "In loadImage():\n"
				  << "\tFailed to load \"" << filename << "\"\n"
				  << "\tReason: SDL_DisplayFormat() returned NULL\n"
				  << "\tSDL_GetError(): \"" << SDL_GetError() << "\"\n"
				  << "-----------------------------------------" << std::endl;
		
		return NULL; //Use nullptr if you're using the "C++11" standard of C++.
	}
	
	Uint32 color = SDL_MapRGB(optimizedImage->format, 0, 0xFF, 0xFF);
	SDL_SetColorKey(optimizedImage, SDL_SRCCOLORKEY, color);
	
	return optimizedImage;
}
Then any function that uses loadBMP() looks nice and neat:
bool loadFiles()
{
	myImage1 = loadBMP("myImage1.bmp");
	if(!myImage1)
		return false;
	
	myImage2 = loadBMP("myImage2.bmp");
	if(!myImage2)
		return false;
	
	myImage3 = loadBMP("myImage3.bmp");
	if(!myImage3)
		return false;
	 
	return true;
}
Or even:
bool loadFiles()
{
	myImage1 = loadBMP("myImage1.bmp");
	myImage2 = loadBMP("myImage2.bmp");
	myImage3 = loadBMP("myImage3.bmp");
	 
	return (myImage1 && myImage2 && myImage3);
}
Though this last version has different logic: It loads all the files, even if some failed, and then checks for failure. In some situations, this may not be desired (you don't want to wait for 10 minutes of loading, only to find out it failed in the first 20 seconds but decided to load the rest before exiting anyway).

So loadBMP() reports (to the log files) the loadBMP() problems, let's the calling functions know that an error occured, and the functions calling loadBMP() then decide how to handle the problem.

Since you are exiting immediately on failure, you could assert():
void loadFiles()
{
	assert(myImage1 = loadBMP("myImage1.bmp"));
	assert(myImage2 = loadBMP("myImage2.bmp"));
	assert(myImage3 = loadBMP("myImage3.bmp"));
}
This at least gives a pop-up box on failure (instead of a silent exit), and gives the piece of code that failed the assertion.

However, just because 'myImage2' failed to load, you still might want to go through your regular freeing of resources before you fully exit.
But, as you've experienced yourself, if a program failed you at least want to pop up a message box letting the user know that something failed (and hopefully give them enough information to report back to the developers so the developers can fix the problem). A silent exit or a messy crash to the desktop leaves the users confused and unsure what steps they should take next.

By doing this:
bool loadFiles()
{
	myImage1 = loadBMP("myImage1.bmp");
	if(!myImage1)
		return false;
	
	myImage2 = loadBMP("myImage2.bmp");
	if(!myImage2)
		return false;
	
	myImage3 = loadBMP("myImage3.bmp");
	if(!myImage3)
		return false;
	 
	return true;
}
And then this:
int main(int argc, char** argv)
{
	if(!initSDL())
	{
		DisplayPopUpBox("Failed to initialize SDL.");
		return -1;
	}
	 
	 
	if(!loadFiles())
	{
		DisplayPopUpBox("Failed to load the resources.");
		return -1;
	}
	
	//etc...
		
}
You can at least let the user know that something went wrong, and then you also have detailed information in your log files (stderr.txt and stdout.txt) for you, the developer, to track down the problem.

In the same way loadBMP() reports its own errors, I'm sure you can make your other functions like soundMgr->loadFile() do the same.

Then, next time you run the code, your own code will tell you what the problem is, and defense line #2 will be a whole lot stronger.

As a random shot at what might be causing the crash, you accidentally call 'loadFiles()' before you call 'soundMgr->initSound()'.
Since 'loadFiles()' calls 'soundMgr->loadFile()', this means you are loading your sound files before you are initializing your sound system, which might fail to work. If this is the case, your error reporting will inform you of this. I also don't see you calling Mix_Init() anywhere.

Best of luck with your project!
Advertisement

Ok, I had trouble logging onto the site... So it's taken a bit of time to reply.

I've sorted out that problem I originally had. I will put more effort into catching errors and finding out what part of my code is having trouble. By doing this I managed to find out that it was where I was trying to load the background file. I had it saved as a PNG while I was trying to load a BMP. I changed the file format of my image.

Now I am just having a hell of a lot of trouble with the SDL_mixer library. When I debug, it stalls at the point of loading a sound file. I've tried searching on the internet, but I'm not getting any answers.

Then I started getting this System Error:

"The program can't start because libstdc++-6.dll is missing from your computer. Try reinstalling the program to fix this problem."

I have no idea what file this is, I wasn't getting this error before. I tried redownloading the SDL_mixer library, remaking the project and the same thing happens...

Unless I'm mistaken libstdc++-6.dll is part of the MinGW runtime. If that DLL was not included I would strongly suggest not using the whole package since whoever compiled it didn't take good care of the dependencies and probably made others mistakes too that will come back to bite you.

That said, I would suggest learning to compile your own libraries instead of trawling the web until you find a DLL with the right name. You will be learning a lot doing that and you will most importantly be able to control your dependencies much better. And while that will initially take a bit longer, in the long run you will be able to avoid/solve yourself the whole class of problems this thread is about.

This topic is closed to new replies.

Advertisement