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

Started by
15 comments, last by osirius 17 years, 9 months ago
I got my code running! But I have a heck of a lot of logical errors now. The screen pops up and a nice grid of empty squares is shown, but the grid isn't centered as I wanted it, it's pushed into the bottom right corner. And when I click on any square the window closes (without errors). But I'm still excited to have the code running :-) .

What I did is I changed the code so that you have two surfaces m_full and m_empty in the MapEditor class, then in the MapEditor constructor they are loaded with square_full.png and square_empty.png. These are passed to each tile so that all tiles also have m_full and m_empty, but it means I'm not loading the images each time a new tile is created, the tiles just point to the images I loaded in the MapEditor object.

One question I have is that I removed the destructor for the Tile class and then did SDL_FreeSurface( m_full ) and SDL_FreeSurface( m_empty ) in the MapEditor class. The way I see it is that SDL_FreeSurface frees the actual images themselves, so I only need to free them when the MapEditor object is destroyed - if I destroyed them in the Tile class then the first Tile object destroyed would free those surfaces, but all the other Tile objects destroyed after that would have no surfaces to free. Is that correct?

Here's the new code:

// 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();	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 );}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 , SDL_Surface* full, SDL_Surface* empty );	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, SDL_Surface* full, SDL_Surface* empty ){	m_x = x;	m_y = y;	m_full = full;	m_empty = empty;	m_image = m_empty;	m_isFull = false;}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;	SDL_Surface*      m_full;	SDL_Surface*      m_empty;	Screen            m_screen;	SDL_Event         m_event;};MapEditor::MapEditor(){	SDL_Init( SDL_INIT_EVERYTHING );	m_full = load_image( "square_full.png" );	m_empty = load_image( "square_empty.png" );	for( int x = 40; x <= 600; x += 40 )	{		for( int y = 40; y <= 440; y += 40 )		{			m_map.push_back( Tile( x, y, m_full, m_empty ) );		}	}}MapEditor::~MapEditor(){	SDL_FreeSurface( m_full );	SDL_FreeSurface( m_empty );	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.
Advertisement
Quote:Original post by Anonymous Poster
You can't do (a < b < c) in C++, you'll have to replace it with
((a < b) && (b < c))


Thanks. I've now replaced this part:

if( (x <= m_event.button.x < x + 40) &&    (y <= m_event.button.y < y + 40) )


... with this:

if( ( (x <= m_event.button.x) && (m_event.button.x < x+40) ) &&    ( (y <= m_event.button.y) && (m_event.button.y < y+40) ) )


The code still has the same logical errors though.

EDIT: Changed spacing in code.
-----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.
The problem with the grid being in the bottom right corner is simple: I had created tiles up to 600 as the x value and 440 as the y value, and the screen is 640x480. I've now changed it so tiles are made up to 560 in the x value and 400 in the y value, and I have a nice, centered grid. I'm still debugging the rest.
-----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.
Not to knock what you are doing, but conventionally, rather than store each tile's x and y, the approach would be to compute these as you draw them:

std::vector<Tile> map;int TileWidth=32,TileHeight=32;void init(){    map.resize(10*10); // or whatever}void draw(){    int offsetx=Whatever,offsety=Whatever; // to centre or whatever    int index=0;    for(int y=0;y<10;++y)        {        for(int x=0;x<10;++x)            {            int dx=offsetx+(x*TileWidth);            int dy=offsety+(y*TileHeight);            draw(dx,dy,map[index++].GetImage());            }        }}


sort of thing. This means you can move the map around the screen independantly of the tiles and saves a pair of co-ordinates in each tile structure.

All looks top notch though apart from that minor detail so just ignore me if you want [smile].
It works! I found out what I did wrong: I'd forgotten to add break; statements to the cases of switch( m_event.type ) in MapEditor::start(). The program works flawlessly now: the window opens, I can click on the tiles to fill them in or unfill them, then Xing out the window closes the program cleanly. Woo!

Thanks for the help everyone.
-----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 EasilyConfused
Not to knock what you are doing, but conventionally, rather than store each tile's x and y, the approach would be to compute these as you draw them:

*** Source Snippet Removed ***

sort of thing. This means you can move the map around the screen independantly of the tiles and saves a pair of co-ordinates in each tile structure.


Thanks, that sounds like a good idea. I'll modify my code using that idea later.

EDIT: Actually, when I went through my code and thought about how to implement that idea I decided against it, my way turns out to be simpler.

[Edited by - osirius on July 20, 2006 12:06:45 PM]
-----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 osirius
One question I have is that I removed the destructor for the Tile class and then did SDL_FreeSurface( m_full ) and SDL_FreeSurface( m_empty ) in the MapEditor class. The way I see it is that SDL_FreeSurface frees the actual images themselves, so I only need to free them when the MapEditor object is destroyed - if I destroyed them in the Tile class then the first Tile object destroyed would free those surfaces, but all the other Tile objects destroyed after that would have no surfaces to free. Is that correct?


Is it? :-)

I'd like to know.
-----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 topic is closed to new replies.

Advertisement