help with binary file i/o

Started by
15 comments, last by graveyard filla 19 years, 7 months ago
hi, im working on a 2d RPG and recently have tried changing the way i read / write my map files. since i changed it so that the map file is now a whopping 1 meg each, i decided to switch to binary files (from 1 meg to 96k!). anyway, for some reason im having a problem reading in the file. the program crashes because one of the values i read in is something like 2 million, and then i go on to resize a vector to fit 2 million Tile's, which crash's the game. anyway, is there something special i have to do to get this to work that im missing? id appreciate it greatly if anyone could spot out what exactly im doing wrong... heres the Save() code which writes out the file:

void Map::Save()
{
	
	//start the file out
	ofstream fout(map_path.c_str(), ios::binary);
	
	//the first thing we do, is write to this file, its own file path and handle
	fout.write((char *)(&map_path), sizeof(map_path)); //fout << map_path <<' '; 
	fout.write((char *)(&map_handle), sizeof(map_handle));//fout << map_handle <<' ';

	
	//first at the top of this map, we will write out how many MST's are on this map
	int t = map_swap_tiles.size();
	fout.write((char *)(&t), sizeof(t));

	//now we will write out the handle and path to each MST, and its coordinates, and of course its source (repeating code here =()
	for(int i = 0; i < t; i++)
	{	
		vector<Map_Swap_Tile> &tmp = map_swap_tiles;

		fout.write((char *)(&tmp.dst_path), sizeof(tmp.dst_path));
	    fout.write((char *)(&tmp.dst_handle), sizeof(tmp.dst_handle));

		fout.write((char *)(&tmp.src_path), sizeof(tmp.src_path));
		fout.write((char *)(&tmp.src_handle), sizeof(tmp.src_handle));

		fout.write((char *)(&tmp.xpos), sizeof(tmp.xpos));
		fout.write((char *)(&tmp.ypos), sizeof(tmp.ypos));

		fout.write((char *)(&tmp.land_x), sizeof(tmp.land_x));
		fout.write((char *)(&tmp.land_y), sizeof(tmp.land_y));
	}


	
	//first print out which tilemap this map uses...
	fout.write((char *)(&current_tmap), sizeof(current_tmap));//fout << current_tmap << ' ';
  
	//now print out the height/width of this map
	fout.write((char *)(&height), sizeof(height));//fout << height << ' ';
	fout.write((char *)(&width), sizeof(width));//fout << width  << ' ';

	// now output how many NPC's are in the map, the NPC's script, and the NPC's location	
    t = npcs.size();
	fout.write((char *)(t), sizeof(t));
	
	//for each NPC output that npc's data
	for(int i = 0; i < npcs.size(); i++)
	{
		//first the script
		fout.write((char *)(&npcs.script_path), sizeof(npcs.script_path));

		//now its position
		fout.write((char *)(&npcs.xpos), sizeof(npcs.xpos));
		fout.write((char *)(&npcs.ypos), sizeof(npcs.ypos));
	}


	//now print the ID #'s for the tiles along with any data with them
	for(int y = 0; y < height; y++)
	{
		for(int x = 0; x < width; x++)
		{
			//write out how many layers this tile has
			int p = current_map[y][x].layers.size();
          	fout.write((char *)(&p), sizeof(p));
		
			//write out all data for each layer
			for(int i = 0; i < p; i++)
			{
				fout.write((char *)(&current_map[y][x].layers.x_map_loc), sizeof(current_map[y][x].layers.x_map_loc));
				fout.write((char *)(&current_map[y][x].layers.y_map_loc), sizeof(current_map[y][x].layers.y_map_loc));	
				fout.write((char *)(&current_map[y][x].layers.z), sizeof(current_map[y][x].layers.z));
			}

			fout.write((char *)(&current_map[y][x].flags), sizeof(current_map[y][x].flags));
		}
	}


	//lastly close the file!
	fout.close();

}

now, heres the code i use to load in a map.. it *should* be doing exactly what im doing in save....

void Map::Open()
{
	
	//open the file
	ifstream file_in(map_path.c_str(), ios::binary);

	// Error Check
	if(!file_in)
	{
		cout << "COULDNT OPEN MAP FROM INSIDE OF MAP::OPEN, FILENAME: " + map_path<<endl;
		systm.Log("COULDNT OPEN MAP FROM INSIDE OF MAP::OPEN, FILENAME: " + map_path);
		file_in.close();
		return;
	}


	Clear_Before_Load();
	 
	//the first thing we do, is read to this file, its own file path and handle
    file_in.read((char *)(&map_path), sizeof(map_path)); //file_in << map_path <<' '; 
	file_in.read((char *)(&map_handle), sizeof(map_handle));//file_in << map_handle <<' ';

	int t;
	//read how many MST's are on the map
	file_in.read((char *)(&t), sizeof(t));//file_in << map_swap_tiles.size() << ' ';

	vector<Map_Swap_Tile> &tmp = map_swap_tiles;
	tmp.resize(t);

	//now we will read out the handle and path to each MST, and its coordinates, and of course its source (repeating code here =()
	for(int i = 0; i < t; i++)
	{
	

		file_in.read((char *)(&tmp.dst_path), sizeof(tmp.dst_path));
	    file_in.read((char *)(&tmp.dst_handle), sizeof(tmp.dst_handle));

		file_in.read((char *)(&tmp.src_path), sizeof(tmp.src_path));
		file_in.read((char *)(&tmp.src_handle), sizeof(tmp.src_handle));

		file_in.read((char *)(&tmp.xpos), sizeof(tmp.xpos));
		file_in.read((char *)(&tmp.ypos), sizeof(tmp.ypos));

		file_in.read((char *)(&tmp.land_x), sizeof(tmp.land_x));
		file_in.read((char *)(&tmp.land_y), sizeof(tmp.land_y));

	}
	
	//read in the tilemap path
	file_in.read((char *)(&current_tmap), sizeof(current_tmap));//file_in << current_tmap << ' ';
  
	//now read in h / w
	file_in.read((char *)(&height), sizeof(height));//file_in << height << ' ';
	file_in.read((char *)(&width), sizeof(width));//file_in << width  << ' ';

    //grab how many NPC's are on the map
	file_in.read((char *)(t), sizeof(t));
	npcs.resize(t);

	//for each NPC, read in that NPC's info
	for(int i = 0; i < t; i++)
	{
		//first the script
		file_in.read((char *)(&npcs.script_path), sizeof(npcs.script_path));

		//now its position
		file_in.read((char *)(&npcs.xpos), sizeof(npcs.xpos));
		file_in.read((char *)(&npcs.ypos), sizeof(npcs.ypos));
	}


	//now print the ID #'s for the tiles along with any data with them
	for(int y = 0; y < height; y++)
	{
		for(int x = 0; x < width; x++)
		{

			//grab how many layers this tile has
			int p;
          	file_in.read((char *)(&p), sizeof(p));
			
			//resize our tile to fit all the layers
			current_map[y][x].layers.resize(p);

			//read in all the data for each layer
			for(int i = 0; i < p; p++)
			{
				current_map[y][x].layers.texture = this->tilemap_texture[0];

				file_in.read((char *)(&current_map[y][x].layers.x_map_loc), sizeof(current_map[y][x].layers.x_map_loc));
				file_in.read((char *)(&current_map[y][x].layers.y_map_loc), sizeof(current_map[y][x].layers.y_map_loc));
				file_in.read((char *)(&current_map[y][x].layers.z), sizeof(current_map[y][x].layers.z));
			}

			file_in.read((char *)(&current_map[y][x].flags), sizeof(current_map[y][x].flags));

		}
	}


    file_in.close();	

}//end of open()

the part where things get screwed up, are in the Open function, towards the end, this line:

//grab how many layers this tile has
			int p;
          	file_in.read((char *)(&p), sizeof(p));
			
			//resize our tile to fit all the layers
			current_map[y][x].layers.resize(p);
for some reason, it crashes here because im trying to resize the vector to something like 2,000,232,23.... this number should NOT be here! each tile can only have a max of 5 layers. ive traced it and it seems everything before this is read in properly, its just this one line thats screwed up... thanks for any help!
FTA, my 2D futuristic action MMORPG
Advertisement
I suspect there are a lot of problems here, but I can point out some problems with the way you are writing the path string.
I'm guessing map_path is a std::string, am I right? If that is the case, you should not be using the sizeof operator to determine its length. Use map_path.length() instead. Secondly, because the length of the string is variable, you need to write the length out to the file first, so that the loader function knows how much data to read in to the string.
//savingfout.write((char*)map_path.length(), sizeof(int)); // first write the lengthfout.write((char*)map_path.c_str(), map_path.length() * sizeof(char)); // then write the string itself//loadingint path_length;char *buf[MAX_PATH_LENGTH];file_in.read((char*)&path_length, sizeof(int)); // first read in the lengthfile_in.read(buf, path_length); // then read that many bytes into a buffermap_path = buf;


There may be a more elegant way to read the string that doesn't require a buffer, but this should demonstrate the point.
You are not the one beautiful and unique snowflake who, unlike the rest of us, doesn't have to go through the tedious and difficult process of science in order to establish the truth. You're as foolable as anyone else. And since you have taken no precautions to avoid fooling yourself, the self-evident fact that countless millions of humans before you have also fooled themselves leads me to the parsimonious belief that you have too.--Daniel Rutter
ahh... didnt know that... thanks, ill try that out.. i was thinking there would be something weird with containers.. thanks again
FTA, my 2D futuristic action MMORPG
It's not just "containers"; writing any non-POD type out byte-for-byte (i.e. via fwrite()) is Bad News(TM). The struct that you write out has no concept of the data it points to, it just has that data's *current* location in memory. Which will *not* be the same next time you load the program. There won't even necessarily be anything meaningful at the address listed in the file -> BOOM.

[google] "serialization".
ive been trying to write 2 functions for reading / writing a string to a file, but im getting errors. heres the function to read:

void Map::Read_String(const string &str,const ifstream &file_in){	//loading	int str_len;	char *buff[100];	file_in.read((char*)&str_len, sizeof(int)); // first read in the length	file_in.read(buff,str_len); // then read that many bytes into a buffer	str = buff;}


heres the errors:
ERRORS:c:\Documents and Settings\doo\Desktop\projects\fta\ftanewRPG\leveleditor\FTAedit\Map.cpp(1529): error C2662: 'std::basic_istream<_Elem,_Traits>::read' : cannot convert 'this' pointer from 'const std::ifstream' to 'std::basic_istream<_Elem,_Traits> &'        with        [            _Elem=char,            _Traits=std::char_traits<char>        ]        and        [            _Elem=char,            _Traits=std::char_traits<char>        ]        Conversion loses qualifiersc:\Documents and Settings\doo\Desktop\projects\fta\ftanewRPG\leveleditor\FTAedit\Map.cpp(1530): error C2662: 'std::basic_istream<_Elem,_Traits>::read' : cannot convert 'this' pointer from 'const std::ifstream' to 'std::basic_istream<_Elem,_Traits> &'        with        [            _Elem=char,            _Traits=std::char_traits<char>        ]        and        [            _Elem=char,            _Traits=std::char_traits<char>        ]        Conversion loses qualifiers


the write function gives similar errors, so ill just post this. thanks for anymore help!
FTA, my 2D futuristic action MMORPG
Original post by graveyard filla
First,
int str_len;char *buff[100];file_in.read((char*)&str_len, sizeof(int)); // first read in the lengthfile_in.read(buff,str_len); // then read that many bytes into a bufferstr = buff;


First off, I can identify two possible error. Unless you write a null terminated character to the string (which you would not if you simply wrote just as much bytes of the string as indicated by the string::length return value), you should always zero-fill your character array that you read in.

Now that I read again, make that three error.
You have an array of 100 character pointers there. I think you meant char buff[100].

Which brings us to the second problem. What if you had a string that is more than 100 characters? That can lead to a simple buffer overrun (since you never check the size and limit it to max - 1 for null terminated character)

Next, to your actual error message. From a quick flip to the MSDN help, it seems to indicate that basic_istream::read is not a const qualified member (i.e. internal state of basic_istream is allowed to change). However, you are passing in a const qualified basic_istream object. Calling a non-const member on a const object would generate a compilation error. There are two solutions (three if you intend to go into the stream header and change it, but if you do that I gotta kill you until you die from it).

I just noticed your str is a constant as well. Amazed you can change its state via assignment ;)

[edit]And another suggestion from Jesse, check the return values from read. [edit]

First, the way which I am still tempted to kill you, const_cast the reference from a const object to a non-const one. Avoid doing this. I cannot think of any instances yet that one should do this, but still, avoid doing this. You are violating the agreement you had with the declaraton on the parameters itself.

Second, remove that const qualifier off file_in parameter.

Third (the way which I begin to kill you), is to go into the source, check out what internal member variables that are modified by the basic_istream::read method, and mark those as mutable, and mark basic_istream::read as const. DO NOT DO THIS (but I listed it for completeness purpose.)

I present two methods below, one is a safer method, below is a bit risky since it makes assumptions on the internal implementation.

void Map::Read_String(string &str,ifstream &file_in){	int str_len;	int bytes_read = file_in.read((char*)&str_len, sizeof(int));	if (bytes_read == sizeof(int) && str_len > 0)	{		std::auto_ptr<char> buff(new char[str_len + 1]);		std::memset(buff.get(), 0, str_len + 1);		bytes_read = file_in.read(buff.get(),str_len);		if (bytes_read == str_len)			str = buff.get();	}}


void Map::Read_String(string &str,ifstream &file_in){	int str_len;	int bytes_read = file_in.read((char*)&str_len, sizeof(int));	if (bytes_read == sizeof(int) && str_len > 0)	{		str.resize(str_len);		bytes_read = file_in.read(&(*str.begin()),str_len);		if (bytes_read != str_len)		{			// handle error!		}	}}


[Edited by - dot on September 1, 2004 9:14:04 PM]
thanks for your help, i never used an auto ptr before but im assuming it cleans up the memory when it falls out of scope? thats pretty sweet.. anyway, are you sure read() can return an int? for some rason its not working, and the site i looked at said it didnt either. thanks alot for anymore help.

EDIT:

also, i cant get my Write_String() function to work either for some reason. i dont know what im doing wrong...

void Map::Write_String(string &str,ofstream &fout){	//how to write a std::string 	fout.write((char*)str.length(), sizeof(int)); // first write the length    fout.write(str.c_str(),str.length() * sizeof(char)); // then write the string itself (char*)}


the way i use it is something like..
ofstream fout(map_path,ios::binary);fout << stuff;Write_String(map_path,fout);


what i mean by it not working, is the file i get in the end is a blank file with nothing in it...

thanks for any help
FTA, my 2D futuristic action MMORPG
Yes, I'm sorry, I'm a bit rusty of the C++ (Too much C#/Java :/ )

Ok, now that i've recheck the documentation, it returns a basic_istream& object. Now, there is two way.

if (file_in.read(...))

Which actually call file_in.operator void*(), which actually is used to check if the stream is in good state since the last operation, in this case, the read.

Or you can check the individual state bits via rdstate. I think it is as follows
if (file_in.rdstate( ) & std::ios::eofbit)

I'm not sure the exact bit flag to check, since I dont have the specification here that might say which bit flag it set if read fails.

I know I've been bombarding some rather obscure advanced stuff and has been rather lengthy in my post, but I rather you know as much side of the situation as possible then just concentrate on just one solution and ignore the rest :)

And once again, sorry for that wrong code source above.

PS. Yeah you're right on auto_ptr, basically it manages a block of memory and frees it upon destruction. Now, however, it uses delete <type> instead of delete[] <type>. To think of it, maybe I should not have used it since there is no guarantee that new and new[] allocate from the same pool. And it uses ownership transfer, not referenced counting. Should read up on it and decide where to use it since it is rather useful where you need to ensure that a heap allocated object is properly released in case of exception, before the ownership transfer is done.
basic_istream::operator<< are all used for formatted output. They are delegated to the current active locale, and converted to text. Thus, they are 'text format'. Try not to mix text output with binary output.

Try calling flush at the end of all output. Perhaps the destructor does not imply an implicit flush of output to disk.
hmm... flush didnt seem to do anything, the file output was just working fine untill i used this write_string() function instead of just doing it the wrong way and doing sizeof(string) and stuff. the file is still just blank... when i say "working fine" i dont mean actually working fine, but it was at least writing something to the file.

heres the function that uses write_string() (which i described above)

void Map::Save(){	//start the file out	ofstream fout(map_path.c_str(), ios::binary);		//the first thing we do, is write to this file, its own file path and handle		Write_String(map_path,fout);        Write_String(map_handle,fout);		int t = map_swap_tiles.size();	//first at the top of this map, we will write out how many MST's are on this map		fout.write((char *)(&t), sizeof(t));//fout << map_swap_tiles.size() << ' ';			    vector<Map_Swap_Tile> &tmp = map_swap_tiles;	//now we will write out the handle and path to each MST, and its coordinates, and of course its source (repeating code here =()	for(int i = 0; i < t; i++)	{				   Write_String(tmp.dst_path,fout);//fout.write((char *)(&tmp.dst_path), sizeof(tmp.dst_path));	       Write_String(tmp.dst_handle,fout);//fout.write((char *)(&tmp.dst_handle), sizeof(tmp.dst_handle));		   Write_String(tmp.src_path,fout);//fout.write((char *)(&tmp.src_path), sizeof(tmp.src_path));		   Write_String(tmp.src_handle,fout);//fout.write((char *)(&tmp.src_handle), sizeof(tmp.src_handle));		   fout.write((char *)(&tmp.xpos), sizeof(tmp.xpos));		   fout.write((char *)(&tmp.ypos), sizeof(tmp.ypos));		   fout.write((char *)(&tmp.land_x), sizeof(tmp.land_x));		   fout.write((char *)(&tmp.land_y), sizeof(tmp.land_y));	}		//first print out which tilemap this map uses...    Write_String(current_tmap,fout);	//now print out the height/width of this map	fout.write((char *)(&height), sizeof(height));//fout << height << ' ';	fout.write((char *)(&width), sizeof(width));//fout << width  << ' ';    t = npcs.size();	fout.write((char *)(t), sizeof(t));	for(int i = 0; i < npcs.size(); i++)	{				//first the script		Write_String(npcs.script_path,fout);		//now its position		fout.write((char *)(&npcs.xpos), sizeof(npcs.xpos));		fout.write((char *)(&npcs.ypos), sizeof(npcs.ypos));		}	//now print the ID #'s for the tiles along with any data with them	for(int y = 0; y < height; y++)	{		for(int x = 0; x < width; x++)		{			//write out how many layers this tile has			int p = current_map[y][x].layers.size();          	fout.write((char *)(&p), sizeof(p));				//write out all data for each layer			for(int i = 0; i < p; i++)			{					fout.write((char *)(&current_map[y][x].layers.x_map_loc), sizeof(current_map[y][x].layers.x_map_loc));				fout.write((char *)(&current_map[y][x].layers.y_map_loc), sizeof(current_map[y][x].layers.y_map_loc));				fout.write((char *)(&current_map[y][x].layers.z), sizeof(current_map[y][x].layers.z));				}			fout.write((char *)(&current_map[y][x].flags), sizeof(current_map[y][x].flags));				}	}		fout.flush();	fout.close();}


thanks again!
FTA, my 2D futuristic action MMORPG

This topic is closed to new replies.

Advertisement