Sign in to follow this  

Serialization problem

This topic is 3722 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I am writing a tile-based map editor for a 2d game and I am having trouble serializing and unserializing data to be written to and read from a file. mapFileDesc
struct mapFileDesc
{
	unsigned int width; // width of the level
	unsigned int height; // height of the level
	int tilesetID; // which tileset this level is using
	std::list<unit> unitList; // list of player and all enemies
	std::vector<int> layout; // vector of tiles
	std::string objectives;
	bool slaughter;
	bool boss;
	bool escape;
	bool sandbox;
	ammunitionStates ammo;

	mapFileDesc operator= (mapFileDesc src)
	{
		width = src.width;
		height = src.height;
		tilesetID = src.tilesetID;
		unitList = src.unitList;
		layout = src.layout;
	}
};

ammunitionStates in a struct of 8 unsigned ints, and unit looks like this
struct unit
{
	POINT location;
	UINT type;
	bool selected;
	bool boss;
	bool slaughter;
};

mapFileDesc has complex data types, so it isnt a POD type, so cant be serialized directly. I am using std::fstream to read from and write to files, and have gotten everything working except the std::list of units. Whenever I try to pull anything out of the file, the type variable, which should be somewhere in the 0-50 range, usually ends up in the low to mid 3 billions. It's a similar story for the location, and the three bools usually end up being true when I expect them to be false; Here are the serialization functions I'm writing.
int MFIOSave(mapFileDesc src,const std::string& filename)
{

	std::ofstream out;
	char* data;
	size_t len = 0;

	out.open(filename.c_str(),std::ios::binary);

	//signature
	char sig[] = "TMF";
	out.write(sig,sizeof(char)*3);

	// header
	int temp = (int)src.layout.size();
	out.write((char*)&temp,sizeof(int));
		temp = 1;//(int)src.unitList.size(); // fix this
	out.write((char*)&temp,sizeof(int));
		temp = (int)src.objectives.size();
	out.write((char*)&temp,sizeof(int));

	//data
	out.write((char*)&src.width,sizeof(int));
	out.write((char*)&src.height,sizeof(int));
	out.write((char*)&src.tilesetID,sizeof(int));
	data = new char[src.objectives.size()];
	for(unsigned int i=0; i<src.objectives.size(); i++)
	{
		data[i] = src.objectives.at(i);
	}
	out.write(data,(std::streamsize)((sizeof(char))*src.objectives.size()));
	delete[] data;
	out.write((char*)&src.slaughter,sizeof(bool));
	out.write((char*)&src.boss,sizeof(bool));
	out.write((char*)&src.escape,sizeof(bool));
	out.write((char*)&src.sandbox,sizeof(bool));
	out.write((char*)&src.ammo,sizeof(ammunitionStates));
	
	//layout
	size_t vLength = src.layout.size();
	data = (char*)new int[vLength];
	for(size_t i=0; i<vLength; i++)
	{
		data[i] = src.layout.at(i);
	}

	out.write(data,sizeof(int)*(int)vLength);
	delete[] data;

	/*//create an array to hold units
	data = (char*)new unit*[src.unitList.size()];
	std::list<unit*>::iterator it = src.unitList.begin();
	std::list<unit*>::iterator itEnd = src.unitList.end();
	int index = 0;
	while (it != itEnd)
	{
		(unit*)data = (*it);
		it++;
		index++;
	}
	//write the whole array
	out.write((char*)data,(std::streamsize)((sizeof(unit)) * src.unitList.size()));
	delete[]data;*/

	/*//setup iterators
	std::list<unit*>::iterator it = src.unitList.begin();
	std::list<unit*>::iterator itEnd = src.unitList.end();
	while (it != itEnd)
	{
		(*it)->type;
		out.write((char*)(*it),sizeof(unit));

		it++;
	}*/
	
	/*std::list<unit>::iterator it = src.unitList.begin();
	std::list<unit>::iterator itEnd = src.unitList.end();
	while (it != itEnd)
	{
		out.write((char*)&(*it),sizeof(unit));

		it++;
	}*/

	/*size_t numUnits = src.unitList.size();
	data = (char*)new unit[numUnits];
	std::list<unit>::iterator it = src.unitList.begin();
	std::list<unit>::iterator itEnd = src.unitList.end();
	int index = 0;
	while (it != itEnd)
	{
		unit* uPtr = (unit*)data;
		uPtr = uPtr + index;
		uPtr->type = (*it).type;
		uPtr->boss = (*it).boss;
		uPtr->location.x = (*it).location.x;
		uPtr->location.y = (*it).location.y;
		uPtr->selected = (*it).selected;
		uPtr->slaughter = (*it).slaughter;

		index++;
		it++;
	}

	int writeLen = sizeof(unit)*(std::streamsize)numUnits;
	out.write(data,writeLen);

	delete[] data;*/

	unit grarg;
	grarg.location.x = 100;
	grarg.location.y = 100;
	grarg.type = 10;
	grarg.selected = false;
	grarg.boss = false;
	grarg.slaughter = false;
	out.write((char*)&grarg,sizeof(unit));

	out.close();

	return MFIO_OK;
}

int MFIOLoad(mapFileDesc* dest,const std::string& filename)
{
	std::ifstream in;
	in.open(filename.c_str(),std::ios::binary);

	//signature
	char str[3];
	in.read(str,(sizeof(char)*3));

	//header
	int vectorLength = 0;
	int numUnits = 0;
	int objectivesStrLen = 0;
	in.read((char*)&vectorLength,sizeof(int));
	in.read((char*)&numUnits,sizeof(int));
	in.read((char*)&objectivesStrLen,sizeof(int));

		//data
		in.read((char*)&dest->width,sizeof(int));
		in.read((char*)&dest->height,sizeof(int));
		in.read((char*)&dest->tilesetID,sizeof(int));

		//objectives string
		char* data = new char[objectivesStrLen];
		in.read(data,objectivesStrLen);
		dest->objectives = data;
		delete[] data;
		dest->objectives.resize(objectivesStrLen);

		//bools
		in.read((char*)&dest->slaughter,sizeof(bool));
		in.read((char*)&dest->boss,sizeof(bool));
		in.read((char*)&dest->escape,sizeof(bool));
		in.read((char*)&dest->sandbox,sizeof(bool));

		//ammo amounts
		in.read((char*)&dest->ammo,sizeof(ammunitionStates));

		//map layout
		data = (char*)new int[vectorLength];
		in.read(data,vectorLength);
		for(int i=0; i<vectorLength; i++)
		{
			dest->layout.push_back(data[i]);
		}
		delete[] data;

	/* data = (char*)new unit[numUnits];
	for(int i=0; i<numUnits; i++)
	{
	}
	delete[] data;*/

	/* unit* uData = new unit;
	for(int i=0; i<numUnits; i++)
	{
		in.read((char*)uData,sizeof(unit));
		uData->type;

		dest->unitList.push_front( uData );
	}
	delete[] uData;*/

	/* unit* uPtr = new unit;
	for(int i=0; i<numUnits; i++)
	{
		in.read((char*)uPtr,sizeof(unit));

		dest->unitList.push_front( (*uPtr) );
	}
	delete uPtr;*/

		/* //unit list
		data = (char*)new unit[numUnits];
		in.read(data,sizeof(unit)*numUnits);

		for(int i=0; i<numUnits; i++)
		{
			unit temp;
			unit* uPtr = (unit*)data;
			uPtr = uPtr+i;
			temp.type = uPtr->type;
			//temp.location.x = ((unit*)data)[i].location.x;
			//temp.location.x = ((unit*)data)[i].location.y;
			//temp.slaughter = ((unit*)data)[i].slaughter;
			//temp.boss = ((unit*)data)[i].boss;
			//temp.selected = ((unit*)data)[i].selected;

			dest->unitList.push_back(temp);
		}

		delete[] data;*/
		
		unit grarg;
		in.read((char*)&grarg,sizeof(unit));
		grarg.type;

	in.close();
	return MFIO_OK;
}
}

I'll leave the commented lines of code in there as a sad testimonial to the hours I've spent trying different versions to get this to work. The most recent version just writes hand coded data in a single unit, but I still cant read it out. Can you show me why this is going wrong?

Share this post


Link to post
Share on other sites
First off, you're writing the code in a very C like way, not really C++.

I'd suggest trying to read and write simple data using the iostreams. Then, design a file IO system to read and write more complex data.

So, to write the file as binary you need to create a new stream class as the STL doesn't do binary output easily:

class BinaryOutStream : public ofstream
{
public:
BinaryOutStream (char *filename) :
ofstream (filename, ios_base::out | ios_base::binary)
{

}

~BinaryOutStream ()
{
close ();
}
};

The STL for C++ uses the << operator for writing data to a stream, so it would be ideal to replicate this. Adding this:

BinaryOutStream &operator << (const bool value)
{
write (reinterpret_cast <const char *> (&value), sizeof value);
return *this;
}

will allow bool type values to be written to the output stream as a binary value. Note I've used the better C++ cast. You can implement other types in a similar way. However, you'll get problems if you manually implement the PODs when you get to things like the size_t type. Instead, let the compiler do the hard work:

template <class T>
BinaryOutStream &operator << (T value)
{
write (reinterpret_cast <const char *> (&value), sizeof value);
return *this;
}

Now this takes care of all the POD types.

Suppose a more complex type:

class ComplexType
{
public:
ComplexType () :
value0 (counter++),
value1 (counter++),
value2 (counter++)
{
}

private:
static int counter;
int value0;
int value1;
int value2;
};

int ComplexType::counter = 1;

You can add a friend function that allows the structure to be written:

friend BinaryOutStream &operator << (BinaryOutStream &stream, ComplexType &value)
{
stream << value.value0 << value.value1 << value.value2;
return stream;
}

To use the above:

void main (void)
{
BinaryOutStream stream ("c:\\test.bin");
bool value = true;
ComplexType complex_value;
stream << value << complex_value;
}

But what about things like std::vector?
Well, this isn't that hard to do, add the following to the stream class:

template <class T>
BinaryOutStream &operator << (const vector <T> value)
{
*this << value.size ();

for (vector <T>::const_iterator it = value.begin (), end = value.end () ; it != end ; ++it)
{
*this << *it;
}

return *this;
}

and, in case you want to write a pointer:

template <class T>
BinaryOutStream &operator << (const T *value)
{
*this << *value;
return *this;
}

To use the above updates:

void main (void)
{
BinaryOutStream stream ("c:\\test.bin");
ComplexType complex_value;
vector <ComplexType> vector_complex_value;
vector_complex_value.push_back (complex_value);
vector_complex_value.push_back (complex_value);
vector_complex_value.push_back (complex_value);
vector_complex_value.push_back (complex_value);
stream << vector_complex_value;
}

Do a similar thing for the ifstream.

Hope that helps.

Skizz

Share this post


Link to post
Share on other sites
I didn't study the code too deeply, but:

0) Are you *sure* you read *everything* in the *exact* order it's written?

1) Implement it as a member function, to avoid constantly repeating 'src.'. You know that whole 'OOP' thing? Yeah.

2) There's no reason for dumping containers into an array just for serialization. Just write each element of the container to the file. To do this, you'll need to provide serialization for the element type as well, but you should anyway.

3) In C++, we have namespaces. They are there to help you. The C++ standards committee isn't trying to waste your time typing std:: in front of everything; they're trying to help you organize your code without inventing your own ugly prefixing conventions. (With a namespace, you gain the flexibility to omit the prefixes with using-declarations.)

4) Instead of defining a bunch of separate const values for the return value, use an enumeration. They're a little more useful in C++ than in C.

5) Avoid C-style casts. Also, I strongly recommend the little template function shown below for writing primitives to file (Skizz, the operator<< and operator>> are for formatted, "human-readable" text serialization, which isn't necessarily desirable). It makes sure the types match up.

6) Take advantage of RAII: open your streams in the constructor where possible, and don't waste the effort to .close() them when you don't need to.

7) Lots of other minor style things that add up, which are a lot more easily illustrated than described...


template <typename T>
void writePrimitive(T& what, std::ostream& to) {
to.write(reinterpret_cast<char*>(&what), sizeof(T));
}

template <typename T>
void writeArray(T* begin, int count, std::ostream& to) {
to.write(reinterpret_cast<char*>(begin), sizeof(T) * count);
}

namespace MapFile {
struct Desc {
// as before, but also declare the member functions.
};

enum Result { OK, ERROR };

Result Desc::Save(const std::string& filename) {
std::ofstream out(filename.c_str(), std::ios::binary);

// Don't you suppose you should check the stream state here? Otherwise, why
// are you returning a success/error code that will always be success?
if (!out) { return ERROR; }

// signature
char sig[] = "TMF";
writeArray(sig, 3, out);

// header
writePrimitive(layout.size(), out);
writePrimitive(unitList.size(), out);
writePrimitive(objectives.size(), out);

// data
writePrimitive(width, out);
writePrimitive(height, out);
writePrimitive(tilesetID, out);

// For the string, you can write its .data():
writeArray(objectives.data(), objectives.size(), out);

writePrimitive(slaughter, out);
writePrimitive(boss, out);
writePrimitive(escape, out);
writePrimitive(sandbox, out);
writePrimitive(ammo, out); // are you sure it's POD?

// For the layout, we'll have to iterate through.
// I'll avoid the fancy <algorithm> stuff for now ;)
for (std::vector<int>::iterator it = layout.begin(); it != layout.end(); ++it) {
writePrimitive(*it, out);
}

// Similarly for the unit list.
for (std::list<unit>::iterator it = unitList.begin(); it != unitList.end(); ++it) {
writePrimitive(*it, out);
}

return OK;
}
}

// Loading left as an exercise.

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
5) Avoid C-style casts. Also, I strongly recommend the little template function shown below for writing primitives to file (Skizz, the operator<< and operator>> are for formatted, "human-readable" text serialization, which isn't necessarily desirable). It makes sure the types match up.

I know, which is why I created my own stream class ("class BinaryOutStream : public ofstream") and overrode the << operator to pass through to the write function of the base class which writes binary data (much like your templates, only I templatised the vector type as well). This allows for the formatted style (use of <<) which is cleaner in my opinion than lots of write calls. You could also, in theory, use the same write function (the "friend BinaryOutStream &operator << (BinaryOutStream &stream, ComplexType &value)") to write either text or binary just by providing either an instance of the ofstream class or the derived BinaryOutStream. The code above won't do this as it stands.

Skizz

Share this post


Link to post
Share on other sites
I'm sorry about the ugly C-style code. I had some ideas for cleaning everything up, but I should have started with a better design. I have gotten a lot more ideas from your posts.

OOP conventions aside, you both seem to agree on more and simpler writes. Everything works the way I expect up to the point where I try to write/read a (char*)&unit. I still dont know why, but my unit struct does not appear to be valid for direct serialization.

I checked the write/read order a couple of times and everything looks right.

I am pretty sure ammo is a POD, it just has 8 unsigned ints in it, no member functions, no overloaded operators. It also reads and writes fine.

Could the POINT be where this is going wrong? I'm going to break the whole struct down into primitive types and write them in an order I can be in control of and see if that fixes the problem. I'm also going to do some refactoring. Thank you.

Share this post


Link to post
Share on other sites

This topic is 3722 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this