SOLVED: Need help debugging map editor written in C++/SDL

Started by
15 comments, last by osirius 17 years, 9 months ago
I've started writing a map editor using C++/SDL but although it compiles, when you run the executable it screws up. The idea for the map editor is that I have a grid of tiles (10 rows, 14 columns) and each tile in the grid is empty, and when the mouse clicks inside a tile it is filled in, or if the tile is already filled in then it's cleared. That's all I've done so far, and I just want to get that much working first. I'm using FreeBSD, so I compiled the program using this command:
$ g++ map_editor.cpp `sdl-config --cflags --libs` -lSDL_image
I always use that command (maybe adding or removing -lSDL_foo) and all previous programs ran fine, so there's definitely something wrong with this particular program. Then I ran the executable and got the following output in the terminal (a window popped up that was black inside, and I had to force quit the window since trying to X it out didn't work):
$ ./a.out
a.out in free(): warning: page is already free
a.out in free(): warning: chunk is already free
a.out in free(): warning: page is already free
a.out in free(): warning: chunk is already free
a.out in free(): warning: page is already free
a.out in free(): warning: chunk is already free
a.out in free(): warning: page is already free
a.out in free(): warning: chunk is already free
<snip>
a.out in free(): warning: page is already free
a.out in free(): warning: chunk is already free
a.out in free(): warning: page is already free
a.out in free(): warning: chunk is already free
a.out in free(): warning: page is already free
a.out in free(): warning: chunk is already free
a.out in free(): warning: chunk is already free
Fatal signal: Segmentation Fault (SDL Parachute Deployed)
X connection to :0.0 broken (explicit kill or server shutdown).
$
I'm not sure why it isn't working properly. Can anyone help? I've included the code below. Just so you know, I decided to take a different approach when coding this and make it as simple as possible by making it very inflexible, so I only include flexibility where absolutely necessary. That might not be the best idea for larger projects but I'm still learning and thought I'd try this approach to see what the result is like. I'm also starting to regret having the map stored as a vector of Tile objects, meaning I had to instantiate 140 tile objects. Not fun. Maybe I'll change that later. Anyway, here's the code (you can see square_full.png and square_empty.png here and here):

// map_editor.cpp

#include "SDL/SDL.h"
#include "SDL/SDL_image.h"
#include <string>
#include <vector>

// *******************************
// * Functions
// *******************************

SDL_Surface* load_image( std::string filename )
{
	SDL_Surface* A = IMG_Load( filename.c_str() );
	SDL_Surface* B = SDL_DisplayFormat( A );
	SDL_FreeSurface( A );

	return B;
}

// *******************************
// * Screen
// *******************************

class Screen
{
public:
	Screen();
	~Screen();

	void apply( int x, int y, SDL_Surface* surface );
	void show();
private:
	SDL_Surface* m_screen;
};

Screen::Screen()
{
	m_screen = SDL_SetVideoMode( 640, 480, 32, SDL_SWSURFACE );
}

Screen::~Screen()
{
	SDL_FreeSurface( m_screen );
}

void Screen::apply( int x, int y, SDL_Surface* surface )
{
	SDL_Rect offset;
	offset.x = x;
	offset.y = y;
	SDL_BlitSurface( surface, NULL, m_screen, &offset );
}

void Screen::show()
{
	SDL_Flip( m_screen );
}

// *******************************
// * Tile
// *******************************

class Tile
{
public:
	Tile();
	~Tile();

	int get_x();
	int get_y();
	void set_x( int x );
	void set_y( int y );

	void set_full();
	void set_empty();
	bool is_full();

	SDL_Surface* get_image();
private:
	int m_x;
	int m_y;
	bool m_isFull;
	SDL_Surface* m_image;
	SDL_Surface* m_full;
	SDL_Surface* m_empty;
};

Tile::Tile()
{
	m_x = 0;
	m_y = 0;
	m_isFull = false;

	m_full = load_image( "square_full.png" );
	m_empty = load_image( "square_empty.png" );
	m_image = m_empty;
}

Tile::~Tile()
{
	SDL_FreeSurface( m_image );
	SDL_FreeSurface( m_full );
	SDL_FreeSurface( m_empty );
}

int Tile::get_x()
{
	return m_x;
}

int Tile::get_y()
{
	return m_y;
}

void Tile::set_x( int x )
{
	m_x = x;
}

void Tile::set_y( int y )
{
	m_y = y;
}

void Tile::set_full()
{
	m_isFull = true;
	m_image = m_full;
}

void Tile::set_empty()
{
	m_isFull = false;
	m_image = m_empty;
}

bool Tile::is_full()
{
	return m_isFull;
}

SDL_Surface* Tile::get_image()
{
	return m_image;
}

// *******************************
// * MapEditor
// *******************************

class MapEditor
{
public:
	MapEditor();
	~MapEditor();

	void start();
private:
	std::vector<Tile> m_map;
	Screen            m_screen;
	SDL_Event         m_event;
};

MapEditor::MapEditor()
{
	SDL_Init( SDL_INIT_EVERYTHING );

	Tile tile001;
	Tile tile002;
	Tile tile003;
	Tile tile004;
	Tile tile005;
	Tile tile006;
	Tile tile007;
	Tile tile008;
	Tile tile009;
	Tile tile010;
	Tile tile011;
	Tile tile012;
	Tile tile013;
	Tile tile014;
	Tile tile015;
	Tile tile016;
	Tile tile017;
	Tile tile018;
	Tile tile019;
	Tile tile020;
	Tile tile021;
	Tile tile022;
	Tile tile023;
	Tile tile024;
	Tile tile025;
	Tile tile026;
	Tile tile027;
	Tile tile028;
	Tile tile029;
	Tile tile030;
	Tile tile031;
	Tile tile032;
	Tile tile033;
	Tile tile034;
	Tile tile035;
	Tile tile036;
	Tile tile037;
	Tile tile038;
	Tile tile039;
	Tile tile040;
	Tile tile041;
	Tile tile042;
	Tile tile043;
	Tile tile044;
	Tile tile045;
	Tile tile046;
	Tile tile047;
	Tile tile048;
	Tile tile049;
	Tile tile050;
	Tile tile051;
	Tile tile052;
	Tile tile053;
	Tile tile054;
	Tile tile055;
	Tile tile056;
	Tile tile057;
	Tile tile058;
	Tile tile059;
	Tile tile060;
	Tile tile061;
	Tile tile062;
	Tile tile063;
	Tile tile064;
	Tile tile065;
	Tile tile066;
	Tile tile067;
	Tile tile068;
	Tile tile069;
	Tile tile070;
	Tile tile071;
	Tile tile072;
	Tile tile073;
	Tile tile074;
	Tile tile075;
	Tile tile076;
	Tile tile077;
	Tile tile078;
	Tile tile079;
	Tile tile080;
	Tile tile081;
	Tile tile082;
	Tile tile083;
	Tile tile084;
	Tile tile085;
	Tile tile086;
	Tile tile087;
	Tile tile088;
	Tile tile089;
	Tile tile090;
	Tile tile091;
	Tile tile092;
	Tile tile093;
	Tile tile094;
	Tile tile095;
	Tile tile096;
	Tile tile097;
	Tile tile098;
	Tile tile099;
	Tile tile100;
	Tile tile101;
	Tile tile102;
	Tile tile103;
	Tile tile104;
	Tile tile105;
	Tile tile106;
	Tile tile107;
	Tile tile108;
	Tile tile109;
	Tile tile110;
	Tile tile111;
	Tile tile112;
	Tile tile113;
	Tile tile114;
	Tile tile115;
	Tile tile116;
	Tile tile117;
	Tile tile118;
	Tile tile119;
	Tile tile120;
	Tile tile121;
	Tile tile122;
	Tile tile123;
	Tile tile124;
	Tile tile125;
	Tile tile126;
	Tile tile127;
	Tile tile128;
	Tile tile129;
	Tile tile130;
	Tile tile131;
	Tile tile132;
	Tile tile133;
	Tile tile134;
	Tile tile135;
	Tile tile136;
	Tile tile137;
	Tile tile138;
	Tile tile139;
	Tile tile140;

	m_map.push_back( tile001 );
	m_map.push_back( tile002 );
	m_map.push_back( tile003 );
	m_map.push_back( tile004 );
	m_map.push_back( tile005 );
	m_map.push_back( tile006 );
	m_map.push_back( tile007 );
	m_map.push_back( tile008 );
	m_map.push_back( tile009 );
	m_map.push_back( tile010 );
	m_map.push_back( tile011 );
	m_map.push_back( tile012 );
	m_map.push_back( tile013 );
	m_map.push_back( tile014 );
	m_map.push_back( tile015 );
	m_map.push_back( tile016 );
	m_map.push_back( tile017 );
	m_map.push_back( tile018 );
	m_map.push_back( tile019 );
	m_map.push_back( tile020 );
	m_map.push_back( tile021 );
	m_map.push_back( tile022 );
	m_map.push_back( tile023 );
	m_map.push_back( tile024 );
	m_map.push_back( tile025 );
	m_map.push_back( tile026 );
	m_map.push_back( tile027 );
	m_map.push_back( tile028 );
	m_map.push_back( tile029 );
	m_map.push_back( tile030 );
	m_map.push_back( tile031 );
	m_map.push_back( tile032 );
	m_map.push_back( tile033 );
	m_map.push_back( tile034 );
	m_map.push_back( tile035 );
	m_map.push_back( tile036 );
	m_map.push_back( tile037 );
	m_map.push_back( tile038 );
	m_map.push_back( tile039 );
	m_map.push_back( tile040 );
	m_map.push_back( tile041 );
	m_map.push_back( tile042 );
	m_map.push_back( tile043 );
	m_map.push_back( tile044 );
	m_map.push_back( tile045 );
	m_map.push_back( tile046 );
	m_map.push_back( tile047 );
	m_map.push_back( tile048 );
	m_map.push_back( tile049 );
	m_map.push_back( tile050 );
	m_map.push_back( tile051 );
	m_map.push_back( tile052 );
	m_map.push_back( tile053 );
	m_map.push_back( tile054 );
	m_map.push_back( tile055 );
	m_map.push_back( tile056 );
	m_map.push_back( tile057 );
	m_map.push_back( tile058 );
	m_map.push_back( tile059 );
	m_map.push_back( tile060 );
	m_map.push_back( tile061 );
	m_map.push_back( tile062 );
	m_map.push_back( tile063 );
	m_map.push_back( tile064 );
	m_map.push_back( tile065 );
	m_map.push_back( tile066 );
	m_map.push_back( tile067 );
	m_map.push_back( tile068 );
	m_map.push_back( tile069 );
	m_map.push_back( tile070 );
	m_map.push_back( tile071 );
	m_map.push_back( tile072 );
	m_map.push_back( tile073 );
	m_map.push_back( tile074 );
	m_map.push_back( tile075 );
	m_map.push_back( tile076 );
	m_map.push_back( tile077 );
	m_map.push_back( tile078 );
	m_map.push_back( tile079 );
	m_map.push_back( tile080 );
	m_map.push_back( tile081 );
	m_map.push_back( tile082 );
	m_map.push_back( tile083 );
	m_map.push_back( tile084 );
	m_map.push_back( tile085 );
	m_map.push_back( tile086 );
	m_map.push_back( tile087 );
	m_map.push_back( tile088 );
	m_map.push_back( tile089 );
	m_map.push_back( tile090 );
	m_map.push_back( tile091 );
	m_map.push_back( tile092 );
	m_map.push_back( tile093 );
	m_map.push_back( tile094 );
	m_map.push_back( tile095 );
	m_map.push_back( tile096 );
	m_map.push_back( tile097 );
	m_map.push_back( tile098 );
	m_map.push_back( tile099 );
	m_map.push_back( tile100 );
	m_map.push_back( tile101 );
	m_map.push_back( tile102 );
	m_map.push_back( tile103 );
	m_map.push_back( tile104 );
	m_map.push_back( tile105 );
	m_map.push_back( tile106 );
	m_map.push_back( tile107 );
	m_map.push_back( tile108 );
	m_map.push_back( tile109 );
	m_map.push_back( tile110 );
	m_map.push_back( tile111 );
	m_map.push_back( tile112 );
	m_map.push_back( tile113 );
	m_map.push_back( tile114 );
	m_map.push_back( tile115 );
	m_map.push_back( tile116 );
	m_map.push_back( tile117 );
	m_map.push_back( tile118 );
	m_map.push_back( tile119 );
	m_map.push_back( tile120 );
	m_map.push_back( tile121 );
	m_map.push_back( tile122 );
	m_map.push_back( tile123 );
	m_map.push_back( tile124 );
	m_map.push_back( tile125 );
	m_map.push_back( tile126 );
	m_map.push_back( tile127 );
	m_map.push_back( tile128 );
	m_map.push_back( tile129 );
	m_map.push_back( tile130 );
	m_map.push_back( tile131 );
	m_map.push_back( tile132 );
	m_map.push_back( tile133 );
	m_map.push_back( tile134 );
	m_map.push_back( tile135 );
	m_map.push_back( tile136 );
	m_map.push_back( tile137 );
	m_map.push_back( tile138 );
	m_map.push_back( tile139 );
	m_map.push_back( tile140 );

	int tile = 0;
	for( int x = 40; x <= 600; x += 40 )
	{
		for( int y = 40; y <= 440; y += 40 )
		{
			m_map[ tile ].set_x( x );
			m_map[ tile ].set_y( y );
			++tile;
		}
	}
}

MapEditor::~MapEditor()
{
	SDL_Quit();
}

void MapEditor::start()
{
	bool quit = false;
	while( quit == false )
	{
		while( SDL_PollEvent(&m_event) )
		{
			switch( m_event.type )
			{
				case SDL_MOUSEBUTTONDOWN:
					for( int tile = 0; tile < m_map.size(); ++tile )
					{
						int x = m_map[tile].get_x();
						int y = m_map[tile].get_y();
						if( (x <= m_event.button.x < x + 40) &&
					 	    (y <= m_event.button.y < y + 40) )
						{
							if( m_map[tile].is_full() )
							{
								m_map[tile].set_empty();
							}
							else
							{
								m_map[tile].set_full();
							}
						}
					}
				case SDL_QUIT:
					quit = true;
			}
		}

		for( int tile = 0; tile < m_map.size(); ++tile )
		{
			int x = m_map[tile].get_x();
			int y = m_map[tile].get_y();
			SDL_Surface* image = m_map[tile].get_image();

			m_screen.apply( x, y, image );
		}

		m_screen.show();
	}
}

// *******************************
// * Main
// *******************************

int main( int argc, char* args[] )
{
	MapEditor editor;
	editor.start();

	return 0;
}


[Edited by - osirius on July 20, 2006 6:50:46 AM]
-----Build a man a fire, and he will be warm for the night. Set a man on fire and he'll be warm for the rest of his life.
Advertisement
Quote: Tile tile001-140

m_map.push_back( tile001-140 );


FOR GOD'S SAKE! USE AN ARRAY AND A FOR LOOP!
A few small coding tips:

1) You do not want that every tile loads the bitmap, instead make some kind of resource manager which keeps the textures and let the tiles refer to those textures by index (or reference). This will save you a lot of memory.

2) Indeed do not declare such an amount of variables. Beter make a look and push tiles like:

map.push_back(Tile());


3) Use the constructure initialiser list as much as possible. This will result in a slight performance increase.

About the errors: I have no idea what could cause it. Perhaps you should step through your application with the debugger and check where is goes wrong.

Crafter 2D: the open source 2D game framework

?Github: https://github.com/crafter2d/crafter2d
Twitter: [twitter]crafter_2d[/twitter]

Having taken a deep breath, and just to expand upon the points above slightly:

When you do

Tile Tile1;
Tile Tile2; // etc

then

vec.push_back(Tile1);
vec.push_back(Tile2);

what you are doing is creating a load of tiles in a default state, then when you push then onto the vector, you are creating a copy of each in the vector, also in a default state, so the above code is indeed the same as

for(int i=0;i<100;++i) vec.push_back(Tile());

HOWEVER, since you are creating tiles in default state, this is also the same as:

vec.resize(100);

or even (when creating the vector)

vector<Tile> vec(100);

or, with the vector in a class

class Whatever
{
private:
std::vector<Tile> vec;

public:
Whatever() : vec(100) { }
};

since the vector will call the default constructor for all of its objects in any of the above cases.

HTH
Thank you all for the help. In particular I didn't realise you could do vec.push_back( Tile() ), I thought you _had_ to actually declare a particular tile, such as Tile myTile(), rather than just having a generic Tile(). So that's extremely useful. I've rewritten the code using this and also made some other minor changes: the Tile class now has a function click() that takes care of clicking on that tile, and I changed the data member m_screen in the Screen class to m_screenSurface to avoid confusion with the data member m_screen in the MapEditor class. The code now still compiles but again screws up when you execute it but in a different way:

$ ./a.outa.out in free(): warning: page is already freea.out in free(): warning: chunk is already freea.out in free(): warning: page is already freea.out in free(): warning: chunk is already freea.out in free(): warning: page is already freea.out in free(): warning: chunk is already freea.out in free(): warning: page is already freea.out in free(): warning: chunk is already freea.out in free(): warning: page is already freea.out in free(): warning: chunk is already freeFatal signal: Segmentation Fault (SDL Parachute Deployed)$


I don't actually explicitly kill the app this time - a window pops up then immediately disappears, and the terminal gives Segmentation Fault and goes back to a prompt. So I'm still in the debugging phase, but here's the new code anyway:

// map_editor.cpp#include "SDL/SDL.h"#include "SDL/SDL_image.h"#include <string>#include <vector>// *******************************// * Functions// *******************************SDL_Surface* load_image( std::string filename ){	SDL_Surface* A = IMG_Load( filename.c_str() );	SDL_Surface* B = SDL_DisplayFormat( A );	SDL_FreeSurface( A );	return B;}// *******************************// * Screen// *******************************class Screen{public:	Screen();	~Screen();	void apply( int x, int y, SDL_Surface* surface );	void show();private:	SDL_Surface* m_screenSurface;};Screen::Screen(){	m_screenSurface = SDL_SetVideoMode( 640, 480, 32, SDL_SWSURFACE );}Screen::~Screen(){	SDL_FreeSurface( m_screenSurface );}void Screen::apply( int x, int y, SDL_Surface* surface ){	SDL_Rect offset;	offset.x = x;	offset.y = y;	SDL_BlitSurface( surface, NULL, m_screenSurface, &offset );}void Screen::show(){	SDL_Flip( m_screenSurface );}// *******************************// * Tile// *******************************class Tile{public:	Tile( int x, int y );	~Tile();	int get_x();	int get_y();	void click();	SDL_Surface* get_image();private:	int m_x;	int m_y;	bool m_isFull;	SDL_Surface* m_image;	SDL_Surface* m_full;	SDL_Surface* m_empty;};Tile::Tile( int x, int y ){	m_x = x;	m_y = y;	m_isFull = false;	m_full = load_image( "square_full.png" );	m_empty = load_image( "square_empty.png" );	m_image = m_empty;}Tile::~Tile(){	SDL_FreeSurface( m_image );	SDL_FreeSurface( m_full );	SDL_FreeSurface( m_empty );}int Tile::get_x(){	return m_x;}int Tile::get_y(){	return m_y;}void Tile::click(){	if( m_isFull == true )	{		m_image = m_empty;		m_isFull = false;	}	else	{		m_image = m_full;		m_isFull = true;	}}SDL_Surface* Tile::get_image(){	return m_image;}// *******************************// * MapEditor// *******************************class MapEditor{public:	MapEditor();	~MapEditor();	void start();private:	std::vector<Tile> m_map;	Screen            m_screen;	SDL_Event         m_event;};MapEditor::MapEditor(){	SDL_Init( SDL_INIT_EVERYTHING );	for( int x = 40; x <= 600; x += 40 )	{		for( int y = 40; y <= 440; y += 40 )		{			m_map.push_back( Tile(x,y) );		}	}}MapEditor::~MapEditor(){	SDL_Quit();}void MapEditor::start(){	bool quit = false;	while( quit == false )	{		while( SDL_PollEvent(&m_event) )		{			switch( m_event.type )			{				case SDL_MOUSEBUTTONDOWN:					for( int tile = 0; tile < m_map.size(); ++tile )					{						int x = m_map[tile].get_x();						int y = m_map[tile].get_y();						if( (x <= m_event.button.x < x + 40) &&					 	    (y <= m_event.button.y < y + 40) )						{							m_map[tile].click();						}					}				case SDL_QUIT:					quit = true;			}		}		for( int tile = 0; tile < m_map.size(); ++tile )		{			int x = m_map[tile].get_x();			int y = m_map[tile].get_y();			SDL_Surface* image = m_map[tile].get_image();			m_screen.apply( x, y, image );		}		m_screen.show();	}}// *******************************// * Main// *******************************int main( int argc, char* args[] ){	MapEditor editor;	editor.start();	return 0;}
-----Build a man a fire, and he will be warm for the night. Set a man on fire and he'll be warm for the rest of his life.
Quote:Original post by jeroenb
3) Use the constructure initialiser list as much as possible. This will result in a slight performance increase.


Thanks, I didn't know that, but for the moment I'll initialise everything in the constructor itself - if I used the initialiser list then some things will be initialised there, while other things, such as adding all the tiles in the MapEditor class, will have to stay in the constructor itself. So it's simpler just to have everything in the constructor.
-----Build a man a fire, and he will be warm for the night. Set a man on fire and he'll be warm for the rest of his life.
This may not be why you are getting the seg fault but will cause a seg fault.
A surface which is created via a SDL_SetVideoMode call is not owned by you and you should not free it.
Screen::Screen(){	m_screenSurface = SDL_SetVideoMode( 640, 480, 32, SDL_SWSURFACE );}Screen::~Screen(){	SDL_FreeSurface( m_screenSurface );}
Quote:Original post by Anonymous Poster
A surface which is created via a SDL_SetVideoMode call is not owned by you and you should not free it.


Thanks, I've gotten rid of the destructor in the Screen class now, however the code still crashes at runtime in the same way as before.
-----Build a man a fire, and he will be warm for the night. Set a man on fire and he'll be warm for the rest of his life.
By the way you can't do (a <
Hmm, that worked OK in the "Show Preview" option...
Anyway what I meant to say was:

You can't do (a < b < c) in C++, you'll have to replace it with
((a < b) && (b < c))

This topic is closed to new replies.

Advertisement