problem code isolated but makes no sense

Started by
13 comments, last by Dave Hunt 18 years, 10 months ago
I have a complex problem in my tile engine that only occurs when i strictly use a loadMap function. For some reason when i read the tiles name from the text file into a string buffer on the third Node in the linked list it overwrites the second nodes stringHandle (ie bitmap3.bmp) with whatever is being read into it. So when it parses the list of surfaces it returns that second node which is not the correct surface, so every single tile painted is the second node (which is actually the first because the very first node is reserved as NULL anyway.) So it paints the first tile listed in the file for all the tiles. I've isolated the exact line this occurs so my question is why? here is the code:

//load the map of file specified
void map::loadMap(char* tileFile, videoMemory* memoryAlloc)
{  
	//debug buffer for breaking down file into components
	char tileBitmap[256];
	char buffer[128] = {""};
	long tileID;
	bool bWalkable;
	int xOfs, yOfs, zOfs;

	//debug
	printf ("next node string: %s", memoryAlloc->pFirst);

	//dont forget to create an origin (!!!INVALID COORDS BUT VALID RECT!!!)
	SDL_Surface* tileImage = memoryAlloc->querySurface("tile0.bmp");
	this->pOrigin = new CgameTile(-1, -1, tileImage, -1, "tile0.bmp", false);

	ifstream mapStream (tileFile);

	if (! mapStream.is_open())
  { cout << "Error opening file"; return; }


	//get header first!! first two lines are read as HEADER!
	mapStream.getline(buffer, 128);
	mapStream.getline(buffer, 128);

	while(!mapStream.eof())
	{
			//break line down into components
			
			mapStream >> tileID;
			mapStream >> xOfs;
			mapStream >> yOfs;
			//node 2 of the link list handleString changes to 
			//tileBitmap for unknown reason
			mapStream >> &tileBitmap[0];  //on this line
			mapStream >> zOfs;
			mapStream >> bWalkable;

			if(this->maxHeight<zOfs) { this->maxHeight=zOfs; }
			if(this->sizeOfMapX<xOfs) {this->sizeOfMapX=xOfs; }
			if(this->sizeOfMapY<yOfs) {this->sizeOfMapY=yOfs; }

			/*debug*/
			//cout << tileID << "," << xOfs << "," << yOfs << "," << tileBitmap << "," << zOfs << endl;

			tileImage = memoryAlloc->querySurface(&tileBitmap[0]);

			/*read data                        feed it into the constructor */
			CgameTile* newNode = new CgameTile(xOfs, yOfs, tileImage, zOfs, &tileBitmap[0], tileID, bWalkable);
			this->addTail(newNode);
	}
	return;
}

i've experienced this before somewhat, i would guess its some kind of memory going to a part of an array its not supposed to and overiting the handlestring (how i have no idea) but i already investigated tileBitmap by "watching" it and it goes to only the amount allocated and then terminates '\0' so it looks sound. so whats going on? thanks for any help
Advertisement
One thing I notice is that you are passing the address of tileBitmap to your CGameTile constructor. Unless the constructor makes a copy of the string, then all of your CGameTile instances will be using the same value for tileBitmap.
isnt that what i want? the value of tileBitmap changes through each iteration until the end of the file.

The problem is when it reads from the file into the buffer (tileBitmap), it overwrites the videoMemory->pHead->pNext->handleString to whatever is being
(CgameSurface)
written into the buffer. So every CgameTile points to the same CgameSurface because the name is changed when it checks to see if it exists.

So why would reading from a file to a buffer (tileBitmap) overwrite somthing as unrelated as that to cause me these problems.
Quote:Original post by JTR1080
isnt that what i want?


I don't know, since there is not enough code/information for me to know what you are doing with it.

You're talking about linked lists and 'stringHandle' and there is nothing in the code you posted that is related to that execept a call to addTail(). Perhaps if you posted more code (and some headers) I would be able to tell what was going on.
loadMap() is part of the 'map' class which interfaces with the 'tile' class and 'VideoMemory' (handles surfaces) class to manage a 'map'

here is the main.cpp code of the tile engine
/*	TileWhirl'd  ISOMETRIC TILING ENGINE - Loads tiles from a file or writes to a file	You can interface with the engine through the map source file functions, NOT WITH	TILE.CPP.*//*	VideoMemory System - creates a list of active surfaces and has functions for periodic	garbage collection; if a node contains an empty surface.  It is usefull so surfaces	can be queried and reused with little effort.*/#include <stdio.h>#include "SDL.h"#include "videoMemory.h"#include "map.h"SDL_Surface *screen;SDL_Rect *screenRect;SDL_Cursor *SDLcursor;	void cleanup(void);	void panUp(void);	void panDown(void);	void panLeft(void);	void panRight(void);	void panUpRight(void);	void panUpLeft(void);	void panDownRight(void);	void panDownLeft(void);	//global var	int startCenX;	int startCenY;static SDL_Cursor *init_system_cursor(const char *image[]);#define XOFS 11 videoMemory* vMemory = new videoMemory;map* ourMap = new map;bool bRunning = true;int main(int nArg, char* pszArgs[]){	//initialize SDL	if(SDL_Init(SDL_INIT_VIDEO|SDL_INIT_AUDIO)<0) 	{		printf ("Error initializing %s \n", SDL_GetError());		return -1;	}	//cleanup at exit	atexit(cleanup);	//initialize the screen	SDL_Surface* image = new SDL_Surface;	image = SDL_LoadBMP("tile.bmp");          	SDL_WM_SetIcon(image,NULL);	SDL_WM_SetCaption("TileWhirl'd  ISOMETRIC TILING ENGINE", "tile.bmp");	screen=SDL_SetVideoMode(1024, 768, 0, SDL_HWSURFACE|SDL_DOUBLEBUF);	if(screen==NULL) { printf ("Could not initialize: %s\n", SDL_GetError()); }	screenRect = new SDL_Rect;	screenRect->x = 0;	screenRect->y = 0;	screenRect->w = screen->w;	screenRect->h = screen->h;	SDL_SetClipRect(screen, screenRect);	//ourMap->randomMap(vMemory, 30, 30);	//ourMap->createMap(vMemory, 30, 30); //MINIMUM SCROLL SIZE 23 23 this is for 1024x768	ourMap->loadMap("created.txt", vMemory);      //23 23 is a 0-22x0-22 map	if(!ourMap->pOrigin) { printf ("Error could not load map!"); }	SDL_Event event;	 startCenX = 15;   //min startX 11	 startCenY = 15;   //min startY 11	SDLcursor = SDL_GetCursor();	while(bRunning)	{				SDL_FillRect(screen, screenRect, NULL);                          //loading dif maps		vMemory->killDeadNodes();   //i don't see how i will need this until i start		ourMap->renderMap(startCenX, startCenY); //11 11		while(SDL_PollEvent(&event))		{			switch(event.type)			{				case SDL_KEYDOWN:					switch(event.key.keysym.sym)					{					case SDLK_q:						bRunning=false;						break;					//move down and to the right					case SDLK_KP3:						panDownRight();						break;					//move up and to the left					case SDLK_KP7:						panUpLeft();						break;					//move up and to the right					case SDLK_KP9:						panUpRight();						break;					//move down and to the left					case SDLK_KP1:						panDownLeft();						break;					/////STRAIGHT MOVEMENT					//move up					case SDLK_KP8:						panUp();						break;					//move down					case SDLK_KP2:						panDown();						break;					//move left					case SDLK_KP4:						panLeft();						break;					//move right					case SDLK_KP6:						panRight();						break;					default:						break;					}					break;				case SDL_KEYUP:					switch(event.key.keysym.sym)					{					case SDLK_ESCAPE:						bRunning=false;						break;					default:						break;					}					break;				case SDL_QUIT:					bRunning=false;					break; 			}/*switch(eventtype*/		}/*while(pollevent)*/	} /*while(running)*/	ourMap->saveMap("created.txt");	return 0;}void cleanup(){	delete ourMap; //unloads the map in active memory	delete vMemory;	delete screenRect;	SDL_FreeCursor(SDLcursor);	SDL_Quit();	return;}///////////////////////////////////////////////////////////////////   MOVEMENT FUNCTIONS:  ORIENTED TO THE VIEW PORT ///////////////////////////////////////////////////////////////////////////void panUp(){	//add X && Y	if((startCenX+XOFS)+1<ourMap->sizeOfMapX && (startCenY+XOFS)+1<ourMap->sizeOfMapY)	{ 		startCenX++;		startCenY++;	}	return;}  void panDown(){	//subtract X && Y	if((startCenY-XOFS)-1>=0 && (startCenX-XOFS)-1>=0)	{		startCenY--;		startCenX--;	}	return;}void panLeft(){	//X- y+	if((startCenX-XOFS)-1>=0 && (startCenY+XOFS)+1<ourMap->sizeOfMapY)	{		startCenX--;		startCenY++;	}	return;}void panRight(){	//X+ y-	if((startCenY-XOFS)-1>=0 && (startCenX+XOFS)+1<ourMap->sizeOfMapX)	{		startCenX++;		startCenY--;	}	return;}//now the angled movementsvoid panUpRight(){	if((startCenY-XOFS)-1>=0) { startCenY--; }	return;}void panUpLeft(){	if((startCenX-XOFS)-1>=0) { startCenX--; }	return;}void panDownRight(){	if((startCenX+XOFS)+1<ourMap->sizeOfMapX) { startCenX++; }	return;}void panDownLeft(){	if((startCenY+XOFS)+1<ourMap->sizeOfMapY) { startCenY++; }	return;}

I will include the header files for each other class in the project:
CgameObj.h
// gameObj.h: interface for the CgameObj class.// Abstract base class, Doesn't require an implementation file////////////////////////////////////////////////////////////////////////technically it isnt an abstract class because its not virtual though right?#if !defined(AFX_GAMEOBJ_H__2545A58E_50BC_4C99_9E1A_1CEDD79D2AF0__INCLUDED_)#define AFX_GAMEOBJ_H__2545A58E_50BC_4C99_9E1A_1CEDD79D2AF0__INCLUDED_#if _MSC_VER >= 1000#pragma once#endif // _MSC_VER >= 1000#include "SDL.h"class CgameObj  {protected:	unsigned long objID;	unsigned int xOfs;  //offset from 0,0 on the X axis (if it was 5,4 then 5)	unsigned int yOfs;  //offset on the Y axis	SDL_Surface *objSurface;  //pointer to the surface of the currentTile	SDL_Rect *objRect;	int height;  //for depth and overlays//void addSurface(char* surfaceName);};#endif // !defined(AFX_GAMEOBJ_H__2545A58E_50BC_4C99_9E1A_1CEDD79D2AF0__INCLUDED_)

CgameTile.h
// gameTile.h: interface for the CgameTile class.// //////////////////////////////////////////////////////////////////////#if !defined(AFX_GAMETILE_H__729E4C47_9AF1_48F2_ACCF_655C56BFDA2C__INCLUDED_)#define AFX_GAMETILE_H__729E4C47_9AF1_48F2_ACCF_655C56BFDA2C__INCLUDED_#if _MSC_VER >= 1000#pragma once#endif // _MSC_VER >= 1000#include "gameObj.h"class CgameTile : protected CgameObj  {public:	CgameTile();	//"Creating a tile"	CgameTile(int xPos, int yPos, SDL_Surface *tileSurface, int objHeight, char* tileBitmap, bool bWalk);	//"loading a tile"	CgameTile(int xPos, int yPos, SDL_Surface *tileSurface, int objHeight, char* tileBitmap, long tileID, bool bWalk);	virtual ~CgameTile();	CgameTile *NextNode;	CgameTile *PrevNode;	bool walkable;private:	char tileBitmap[256];friend class map;};#endif // !defined(AFX_GAMETILE_H__729E4C47_9AF1_48F2_ACCF_655C56BFDA2C__INCLUDED_)

CSurfaces.h
// surfaces.h: interface for the Csurfaces class.////////////////////////////////////////////////////////////////////////#if !defined(AFX_SURFACES_H__BFA82B16_E07C_4190_B5A9_F725997A3083__INCLUDED_)#define AFX_SURFACES_H__BFA82B16_E07C_4190_B5A9_F725997A3083__INCLUDED_#if _MSC_VER >= 1000#pragma once#endif // _MSC_VER >= 1000#include "SDL.h"#include <string>using namespace std;class Csurfaces  {private:	SDL_Surface* pSurface;  //reference surface	char* handleSurface;   //handle to that surface	Csurfaces* pNext;		//next node	Csurfaces* pPrev;		//prev nodepublic:	Csurfaces();	Csurfaces(char* handleName);	virtual ~Csurfaces();friend class videoMemory;};#endif // !defined(AFX_SURFACES_H__BFA82B16_E07C_4190_B5A9_F725997A3083__INCLUDED_)

map.h
// map.h: interface for the map class.////////////////////////////////////////////////////////////////////////#if !defined(AFX_MAP_H__80BB8323_9F61_407F_B21E_62E9B74069F0__INCLUDED_)#define AFX_MAP_H__80BB8323_9F61_407F_B21E_62E9B74069F0__INCLUDED_#if _MSC_VER >= 1000#pragma once#endif // _MSC_VER >= 1000#include "gameTile.h"#include "videoMemory.h"#include <fstream>#include <iostream>#include <string>//these are only used for random map gen #include <cstdlib>#include <ctime>using namespace std;class map  {public:	map();	virtual ~map();	CgameTile* pOrigin;	int sizeOfMapX;	int sizeOfMapY;	int maxHeight;	//member functions	void loadMap(char* fileName, videoMemory* memoryAlloc);	void saveMap(char* fileName);	void createMap(videoMemory* memoryAlloc, int xBounds, int yBounds);	void randomMap(videoMemory* memoryAlloc, int xBounds, int yBounds);	void renderMap(int centerCoordX, int centerCoordY);	void unloadMap();private:	void addTail(CgameTile* tail);	CgameTile* getTile(int xCoord, int yCoord, int zCoord);};#endif // !defined(AFX_MAP_H__80BB8323_9F61_407F_B21E_62E9B74069F0__INCLUDED_)

videoMemory.h
// videoMemory.h: interface for the videoMemory class.////////////////////////////////////////////////////////////////////////#if !defined(AFX_VIDEOMEMORY_H__3F889092_C3D6_45BA_8FDE_1EAA97A5581E__INCLUDED_)#define AFX_VIDEOMEMORY_H__3F889092_C3D6_45BA_8FDE_1EAA97A5581E__INCLUDED_#if _MSC_VER >= 1000#pragma once#endif // _MSC_VER >= 1000#include "surfaces.h"class videoMemory  {public:	videoMemory();	virtual ~videoMemory();	Csurfaces* pFirst;	//member functions	void killDeadNodes(void);	SDL_Surface* querySurface(char* handleString);private:	void addSurface(Csurfaces* newSurface);	void unloadVideoMemory(void);};#endif // !defined(AFX_VIDEOMEMORY_H__3F889092_C3D6_45BA_8FDE_1EAA97A5581E__INCLUDED_)
I'm sure the problem isn't in main.cpp. It must be somewhere in you class implementations. Also, you referred to 'handleString', but there is no member variable named 'handleString'. There is a 'handleSurface'. Perhaps that is what you meant. At any rate, there is nothing in the code you posted that would cause this.

P.S. Instead of using 'code' tags, you should use 'source' tags for large pieces of source code.
You forgot the mapStream.Close().
Quote:Original post by Anonymous Poster
You forgot the mapStream.Close().


The mapStream will be closed by the destructor when the function returns.
As mentioned, there is only a single tileBitmap array, and it's local to the function. You may iteratively put different stuff in it, but if you are doing something silly like making an assignment "myTitle = inputTitle" in the CGameTile constructor, then all of your tiles point at the same character buffer - which isn't even valid after the function ends.

C++ provides tools for dealing with this sort of thing in a way that's easy and *works*. In particular, it provides std::string (which also avoids problems with needing a particular buffer length).

void map::loadMap(const char* tileFile, videoMemory* memoryAlloc) {      std::string tileBitmap; // I assume this is the name of a bitmap file  // anyway, whether you read into a char[] or a std::string with >>, it will  // only read one "word"  std::string ignored; // "buffer" line	  long tileID;  bool bWalkable;  int xOfs, yOfs, zOfs;  //debug  cerr << "next node string: " << memoryAlloc->pFirst;  //dont forget to create an origin (!!!INVALID COORDS BUT VALID RECT!!!)  SDL_Surface* tileImage = memoryAlloc->querySurface("tile0.bmp");  pOrigin = new CgameTile(-1, -1, tileImage, -1, "tile0.bmp", false);  ifstream mapStream (tileFile);  if (!mapStream.is_open()) { cerr << "Error opening file"; return; }  //get header first!! first two lines are read as HEADER!  getline(mapStream, ignored);  getline(mapStream, ignored);  // While able to read in a whole line,  while(mapStream >> tileID >> xOfs >> yOfs >> tileBitmap >> zOfs >> bWalkable){    // update maxima    if(maxHeight<zOfs) { maxHeight=zOfs; }    if(sizeOfMapX<xOfs) { sizeOfMapX=xOfs; }    if(sizeOfMapY<yOfs) { sizeOfMapY=yOfs; }    // debug    // cerr << tileID << "," << xOfs << "," << yOfs << "," << tileBitmap     //      << "," << zOfs << endl;    // You can get a const char* back from a std::string like this:    tileImage = memoryAlloc->querySurface(tileBitmap.c_str());    // Data is read; feed to ctor    // I will leave it to you to fix the ctor to accept the std::string.    // That way, it will actually copy as needed.    CgameTile* newNode = new CgameTile(xOfs, yOfs, tileImage, zOfs, tileBitmap, tileID, bWalkable);    addTail(newNode);  }}


By the way:

- The initialization of a character buffer to {""} doesn't do what you think it does - you wanted either just "" or perhaps {0}. With the code as given, the first character is initialized to """ interpreted as a character" - where "" is a string literal, i.e. a pointer to somewhere in memory that contains an empty string. Then you get that value cast into a one-byte value and stored at the beginning of your buffer. As it happens, it doesn't matter because initialization wasn't required, but - ugh.

- "return" at the end of a void function is entirely unnecessary. Omit it.

- Why doesn't zOfs come next to xOfs and YOfs in the data (or constructor parameters)?

- You seem to be writing your own linked list; consider std::list instead. (Rr for that matter, any other standard library container - linked lists are usually taught as the conceptually-easiest resizing container to implement yourself, but are often not the best choice.)
Quote:Original post by Zahlman
As mentioned, there is only a single tileBitmap array, and it's local to the function. You may iteratively put different stuff in it, but if you are doing something silly like making an assignment "myTitle = inputTitle" in the CGameTile constructor, then all of your tiles point at the same character buffer - which isn't even valid after the function ends.

C++ provides tools for dealing with this sort of thing in a way that's easy and *works*. In particular, it provides std::string (which also avoids problems with needing a particular buffer length).


I think that is my problem, the constructor for the surface with the overloaded
value of char* handleName, sets that char* to whatever the address in memory
handleName is, which is probably tileBitmap array so when it gets changed to
the new variable it changes the handleSurface. I'm changing the members of Csurfaces to have char intstead of char* to hold the data, the only problem is converting between the two so i have to write some annoying code like
for(int i = 0; i<len(string; i++){    copy char[x] to memberChar[x];}

hopefully it works

edit: problem solved! :)

This topic is closed to new replies.

Advertisement