Public Group

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

## Recommended Posts

I've searched around quite a bit to find out how I would load a map from a file. Very few articles entertain the actual idea, and fewer yet the specific language details for doing so. This is kind of a compounded post based on topic_id=431385.

I'm trying to make a very dynamic map loader, I don't want it so that you need much more than map information, which makes sense, right? (ie. no number of faces needed prior to reading from file-- which brings me to the new() operator.) Coded in dev-c++ 4.9.9.2. Here's my fully working example:

#include <iostream>
#include <fstream>
#include <string>

using namespace std;

int iTexture = 1;
int * p;             // used in runtime allocation of memory

void save_map();

int main ()
{
save_map();

bool himom;    // temporary: used only to allow view prior to program exit
cin >> himom;  // temporary: used only to allow view prior to program exit
delete[] p;    // deletes memory after loaded from **MAYBE: outside the scope?
return false;
}

void save_map()
{
float fVertex[2][2] = {{2.0f,3.0f},{5.0f,6.0f}};
ofstream myfile ("map1.txt", ios::trunc);         // opens file for write
// next line: prints vertex data to file... in lieu of actual save routine
myfile << "1;2;3;\n4;5;6;\n7;8;9;\n1;0;1;\n2;1;0;\n3;1;1;\n4;1;0;\n5;0;1;";
myfile.close();
}

{
bool bFaceEnd = false;                    // exit loop when no more unread faces
int a = 0;
ifstream file ("map1.txt");               // open file for input

if (file.is_open())                       // if file is open...
{
while (bFaceEnd == false)               // ...and not done reading faces
{
file.getline (line,5,';');            // get value from file, filter ;
iTexture = atoi(line);                // value from file and convert from string to int
if (iTexture != 0)                    // if value from file is not 0...
{
p = new int[a+3];                   // add 3 ints to array for storage
p[a] = iTexture;                    // assign first int as texture data

file.getline (line,5,';');          // get next value from file
p[a+1] = atoi(line);                // assign the second element as x value

file.getline (line,5,';');          // get next value from file
p[a+2] = atoi(line);                // assign the third element as y value
cout << "texture : " << p[a] << " x: " << p[a+1] << " y: " << p[a+2] << '\n';
a = a + 3; // increase so that the (p=new int[a+3]) creates new elements.
}
else                                  // if the texture data from file is 0
{
bFaceEnd = true;                    // end the while loop
cout << "total point(s) loaded: " << (a/3) << '\n'; // print the check
file.close();                       // close the file
}
}
}
else cout << "Read failed!";              // if the files not open... go die somewhere
}


My question is, since I'm declaring the map data (p) inside the load_map() function using new() are there any ideas to cleanly have the information made global from inside the function, say to an OpenGL draw loop? I'm new to new()... if you know what I mean. Secondly, I couldn't find if what I've done anywhere: Is it ok to be whoring out the new() operator with very dynamic variables like this? Any suggestions in general? I'm kind of in unfamiliar territory here and making it up as I go. Like I mentioned earlier, there isn't alot of valuable information on a way to do this, so at the very least: I hope this helps someone else! Thanks, -TickNSwisted-

##### Share on other sites
Ok, to answer your question, simply have the function return a pointer to the map data. Do that like:

MapData *LoadMap(){    MapData *loadedMap = new MapData;    //load the map into loadedMap;    return loadedMap;}

This will return a pointer to the loaded map data. To get that data from whatever function you call the mapload in, to the draw loop, it really depends on how your arcitecture is set up. Just load the pointer into a variable that both functions can access.

As for using the new operater, I had trouble following your code, but it looks like you are creating a bunch of memory leaks. You keep defining new arrays (p) in each loop, without saving one you just made anywhere. This means that it is lost forever, and will keep taking up memory until you restart the computer.

It only seems as though the code is working, when in reality it is not, because you are displaying the values before you lose them in memory. One trick that I think may help you is saving and loading structs, here read this article, and scroll down to the "binary input and output". Pay attention when he loads and saves structs, because you could make an X,Y,Z, struct.

http://www.gamedev.net/reference/articles/article1127.asp

##### Share on other sites
I'd recommend creating a Map class, that will store all the map information, and that offers a LoadFromFile(string fileName) and a SaveToFile(string fileName) function. Plus some more functions to use this map data in your game.

Once you've written that, all you have to do is create a Map object and call the appropriate functions. You can then change the inner workings of this class when you need to, without having to touch any other code. For example, instead of using a int*, you can use a std::vector<int>. This is a dynamically resizeable list, much safer and easier to use than an int pointer.

What new does, is create some space on the heap memory. Variabeles declared globally, or inside a function, are created on the stack, and cleared up once that function ends (or for global variabeles, once the program ends). Things on the heap aren't cleared up, untill you call delete on them (note that when you use new[], you must use delete[]). This is interesting: when you create an int pointer inside a function, and call new on it, and then leave the function, the int pointer gets removed from the stack... but the memory that it pointed to is not removed from the heap. Since you just lost the pointer to it, there's no way to remove it anymore.
That should be a good reason to go for a standard data structure like std::vector or std::list. There's no need to spend so much time making sure you're not leaking memory for such a simple thing. Pointers are usefull sometimes, but in most cases you don't need to use them. And if you think you have to, consider using references or const references instead.

As for not needing the number of faces, personally I would store that in a map file, and I'd write an editor that automatically puts such information at the start of a map file. Such header information does make loading a map easier to code usually. :)

##### Share on other sites
Thanks, Captain P, Foot Soldier. I'm new to the whole vector approach. You guys are right-- it is nicer than the pointer/new/delete/leak I was using earlier.
#include <string>#include <iostream>#include <fstream>#include <vector>using namespace std;typedef vector <int> vec;bool save_map (const char *filename){ ofstream myfile (filename, ios::trunc);         // opens file for write // next line: prints vertex data to file... in lieu of actual save routine myfile << "1;2;3;\n4;5;6;\n7;8;9;\n1;0;1;\n2;1;0;\n3;1;1;\n4;1;0;\n5;0;1;\nI like cake!!!"; myfile.close(); return true;}bool load_map (const char *filename){ vec t, x, y;                       // yey, first time using these fun little vectors static char line[5];               // create storage for loading the value from file bool bFaceEnd = false;             // create bool to end the tile while loop loading int  iNumFace = 0;                 // create storage for dynamic counts of the faces ifstream file (filename);          // open file for input, passed from function call if (file.is_open())                // if file is open...  {   while (bFaceEnd == false)        // ...and not done reading faces    {     file.getline (line,5,';');     // assign value from file to char:5:line, filter;     t.push_back(atoi(line));       // convert char::line to int and assign to vec::t     if (atoi(line) != 0)           // if value pulled from file is not valid texture      {       file.getline (line,5,';');   // assign value from file to char:5:line, filter;       x.push_back(atoi(line));     // convert char::line to int and assign to vec::x       file.getline (line,5,';');   // assign value from file to char:5:line, filter;       y.push_back (atoi(line));    // convert char::line to int and assign to vec::y              iNumFace++;                  // increase value of int::iNumFace to count faces      }     else                           // if value pulled from file is not valid texture      {       bFaceEnd = true;             // ... end the while loop using the bool bFaceEnd       cout << "total point(s) loaded: " << (iNumFace) << '\n';       for (int i = 0; i < iNumFace; ++i)        { cout <<"texture : "<<t<<" x: "<<x<<" y: "<<y<<'\n'; }       file.close();                // close the file       t.clear();                   // clear the t vector data       x.clear();                   // clear the x vector data       y.clear();                   // clear the y vector data      }    }  } else                               // if the files not open  {   cout << "Read failed!";   return false;  }               return true;}int main (){ save_map("map1.txt"); load_map("map1.txt"); bool himom = false; cin >> himom; return false;}

I'm thinking of how I might impliment the class so that the vectors can be on more of a global scope whereas the function can keep applying the data stream to the class, depositing the data in memory?

class map
{
public:
vec t, x, y;
void tile (vec, vec, vec);
};

void map::tile (vec a, vec b, vec c)
{t = a; x = b; y = c;}

{
...
map[0].tile (t,x,y) ??

-TickNSwisted-

##### Share on other sites
A bit of a late reply to your last post, but here's a few comments. First, I wouldn't name variabeles 't', 'x' and such. Those names don't describe what the variabele is for. 'i' is fine for a loop counter, since we're so used to that, but I've got no idea what t and x stand for. Oh, x isn't the horizontal position? You get the idea. ;)
In this case however, you're better off writing a 3D vector class, one that stores a x, y and z value. This allows you to bundle those variabeles together, rather than splitting them across 3 lists. A more elegant solution.

Here's also a different approach for your Map class, a more object oriented approach, as it hides the actual level data, and provides functions for outside code to interact with your level data. The fun thing of that is that you can change how you store your data, make it more efficient, while still keeping the same functions, the same interface, to the outside world.
And if something goes wrong with the level data, then it's either inside this class, or with how it's being used. Some random global x, y or t variabele has nothing to do with it anymore.

// The Level class contains functions to load level data from a file and to retreive this dataclass Level{private:	// The level data list of numbers. No 't' or 'x', we want descriptive names here! ;)	// Note the Vec3D type: I'd put x, y and t together into one class, a 3D vector class.	// A well-designed 3D vector class is very usefull anyway, as you'll usually do a	// lot of math with them in 3D games. Overloaded operators surely are nice in such situations. :)	vector<Vec3D> mapData;public:	// The constructor: this function gets called when a Level object is created.	Level();		// The destructor: when a Level object is removed, this function gets called.	// If you had newed anything in the constructor, make sure you delete it here.	~Level();		// This function loads level data from a given file. Notice the std::string here:	// it's much safer and easier to use than a char*.	// char* are the C way of doing text, strings are the C++ way.	void LoadFromFile(string fileName);		// The save to file function: this takes the level data from our levelData vector	// and outputs it to the given file.	void SaveToFile(string fileName);		// I don't know how you're going to interact with your map data, but you'll	// likely need some way for other code to access the map data. This is more	// design-related however: maybe you want a DrawMap function, instead of	// letting the rendering code retreive this data each frame.	Vec3D GetTile(int x, int y);};int main(){	Level firstLevel;	firstLevel.LoadFromFile("mapname.ext");	firstLevel.SaveToFile("map2.ext");	getchar();	// Yep, used to wait for user input... :)	return 1;}

Of course, this is just an outline, to give you an idea. If some terms are unclear, give them a Google. I'm sure you'll find operator overloading interesting, for example. Good luck! :)

##### Share on other sites
I'm going to give my advice in several passes, showing the result of following the advice after each pass.

PASS 1:
- Don't artificially pause your programs at the end. Learn to run them from the command line, or how to do the necessary configuration with your IDE to pause them at the end of the program run. (This has been discussed to death around here so I won't go into detail.) Also, the return type of main is int, so you should return an int (i.e. 0), not a bool that then gets cast implicitly to int. Except that you don't need to do that at all, because as a special case, main() returns 0 if it reaches the end (this is well-defined and standard, and ONLY applies to main()).

- Some reformatting; you don't have to follow how I do it, but if I were you I'd seriously rethink single-space indent... you want to be able to see those indents and it really helps to be able to make sure things are lined up when you are matching indentation to scope. Also, I recommend *not* putting a space between function names and the opening parenthesis for arguments (and similarly for constructor calls); this distinguishes them from language constructs (for, while, etc.)

- Don't make every function return something if there isn't a need. In particular, returning whether an operation "was successful" is useless if the calling code is going to ignore that result. (You should also consider throwing an exception in the case of failure.)

- Rely on RAII. Stream objects *do not need* to be .close()d explicitly. .close() is provided for *special circumstances* when you need the stream to close before the end of the object's scope. Similarly, there is no need to .clear() a vector that is about to die; the vector's destructor will look after that. .clear() is for when you need to take everything out of a vector *and then keep using the vector*.

- Don't comment every line. You should know what they do. Comments are for explaining why you're doing things the way you are.

- Use early-out logic to avoid extra indentation of code.

#include <string>#include <iostream>#include <fstream>#include <vector>using namespace std;typedef vector<int> vec;void save_map(const char *filename) {  ofstream myfile(filename, ios::trunc);  // We don't have real map data right now; this is a proxy  myfile << "1;2;3;\n4;5;6;\n7;8;9;\n1;0;1;\n2;1;0;\n3;1;1;\n4;1;0;\n5;0;1;\nI like cake!!!";}void load_map(const char *filename) {  vec t, x, y;  static char line[5];  bool bFaceEnd = false;  int  iNumFace = 0;  ifstream file(filename);  if (!file.is_open()) {    cout << "Read failed!";    return;  } // or throw an exception.  while (bFaceEnd == false) {    file.getline(line,5,';');    t.push_back(atoi(line));    if (atoi(line) != 0) {      file.getline(line,5,';');      x.push_back(atoi(line));      file.getline(line,5,';');      y.push_back(atoi(line));             iNumFace++;    } else {      bFaceEnd = true;      cout << "total point(s) loaded: " << (iNumFace) << '\n';      for (int i = 0; i < iNumFace; ++i) {        cout << "texture : " << t << " x: " << x << " y: " << y << '\n';       }    }  }}int main() {  save_map("map1.txt");  load_map("map1.txt");}

PASS 2:

- Use std::string for filenames and just convert it when you make calls to the library; you'll appreciate the extra flexibility later when you need to "construct" a filename. (Also, the existing code *does not need or use* <string> at all. After these changes it will :) )

- We can also make good use of std::string by reading lines with the *free function* std::getline. You should probably forget about the member function version. Oh, and don't use static variables without a good reason (i.e. needing to remember the contents from one function call to the next).

- Your data format makes things needlessly difficult. C++ is designed (like C) for interpreting text files as sequences of "tokens", i.e. items separated by whitespace. Therefore our task is much easier if we just put space between the numbers instead of semicolons. Also, we should make those newlines meaningful, by checking that there are three numbers per line.

The way we'll do this is to read each line (in the normal, '\n' delimited sense :) ) with the free function std::getline, and then parse three numbers out of it.

- Don't use atoi(). It's ancient and badly designed. What if there really is a number there and the number is 0? You have no way to tell. The modern C++ library offers a much better solution: just attempt to read the numbers with operator>>.

"But wait," you say, "I thought we were going to read a line at a time into std::strings. How are we going to read the numbers out of the strings?" Simple: with a std::stringstream. :) These objects simply behave like streams that are "sourced" from (or "sinked" to, or both) a std::string instead of a file on disk (or the console, in the case of std::cin and std::cout). This is polymorphism at work. (Much nicer than in C where we have to distinguish sprintf() from fprintf().)

- With proper design, we don't need a "sentinel" value. Instead, we just keep trying to read lines until we hit the end. (I will handle errors by simply ignoring any data read in from erroneous lines.)

We can also check for "extra" data on a line by simply trying to read into a char variable after the numbers. If that *succeeds*, we discard the line as not matching our format; if it *fails*, we are happy.

This means we'll be getting rid of 'bFaceEnd' since we are just going to read to the end of the file. (In more complex files where there is more than one kind of thing being stored, you will usually want to put a length count ahead of each "chunk" anyway, or else a sentinel, yes... but for now, we will not worry about that. KISS.)

#include <string>#include <iostream>#include <fstream>#include <vector>#include <sstream> // for std::stringstream.using namespace std;typedef vector<int> vec;void save_map(const std::string& filename) {  ofstream myfile(filename.c_str(), ios::trunc);  // We don't have real map data right now; this is a proxy  myfile << "1 2 3\n4 5 6\n7 8 9\n1 0 1\n2 1 0\n3 1 1\n4 1 0\n5 0 1";}void load_map(const std::string& filename) {  vec ts, xs, ys;  std::string line;  int  iNumFace = 0;  ifstream file(filename.c_str());  if (!file.is_open()) {    cout << "Read failed!";    return;  } // or throw an exception.  // Normally you should not try to play around with checking for EOF. It tends  // not to work like you'd expect. This idiom is standard:  while (std::getline(file, line)) {    char garbage;    int t, x, y;    std::stringstream ss(line);    // We can read this like: "If (we can) read from ss into t, into x, (and)    // into y, and cannot (subsequently) read from ss into garbage,"    if ((ss >> t >> x >> y) && !(ss >> garbage)) {       // ... then the line is valid.      ts.push_back(t);      xs.push_back(x);      ys.push_back(y);        iNumFace++;    }  }  cout << "total point(s) loaded: " << iNumFace << '\n';  for (int i = 0; i < iNumFace; ++i) {    cout << "texture : " << ts << " x: " << xs << " y: " << ys << '\n';   }}int main() {  save_map("map1.txt");  load_map("map1.txt");}

PASS 3:

- I've already gotten rid of bFaceEnd, but I'd like to object to its naming anyway. In C++, 'bool' is a REAL type; there is no need to try to carry the type information in the name. Don't use Hungarian notation like this; C++ offers you tools to do stronger type-checking if you need it, and it becomes type checking done by the compiler (which can't mess it up) instead of the programmer. Hungarian notation makes your names uglier, adds extra typing, adds extra potential maintenance, and like all things that add extra maintenance, adds an opportunity for your code to start *lying to you* (i.e. you change the type of a variable and forget to change its name). Just don't do it. Similarly, iNumFace is a poor name.

- Remove the debugging information from the function. If it were information we really wanted to present to the user, we should do it outside the load function, because it's not logically a part of the task of loading.

- In order to be able to "get at" the data from the rest of the program, the logical option is to *return* it from the load function. See that return type we freed up by not abusing it for a success code? :) This also gives us a chance to show off more std::vector functionality. Of course, it seems like we need to return multiple things (three vectors, but)...

- Prefer sequences of structures instead of making several "parallel" sequences. That is, bind together sets of data that are related to each other. In our case, a t-value, an x-value and a y-value logically represent something (a "face", I gather), so we make a struct that represents a face, and create and return a vector of faces.

- Above, I was explicitly qualifying a bunch of stuff in namespace std:: that I didn't need to ;) So that's cleaned up here.

- We can cause the stream library to take care of throwing an appropriate exception for us if the file-opening fails. It looks a little ugly, but the alternatives are at least as bad. There's a minor design flaw in streams here.

#include <string>#include <iostream>#include <fstream>#include <vector>#include <sstream> // for std::stringstream.struct face {  int t, x, y;};using namespace std;typedef vector<face> vec;void save_map(const string& filename) {  ofstream myfile(filename.c_str(), ios::trunc);  // We don't have real map data right now; this is a proxy  myfile << "1 2 3\n4 5 6\n7 8 9\n1 0 1\n2 1 0\n3 1 1\n4 1 0\n5 0 1";}vec load_map(const std::string& filename) {  vec result;  std::string line;  ifstream file;  file.exceptions(ios::failbit);  file.open(filename.c_str());  while (std::getline(file, line)) {    char garbage;    face f;    std::stringstream ss(line);    if ((ss >> f.t >> f.x >> f.y) && !(ss >> garbage)) {       result.push_back(f);    }  }  return result;}int main() {  save_map("map1.txt");  try {    vec faces = load_map("map1.txt");    cout << "total point(s) loaded: " << faces.size() << '\n';    for (vec::iterator it = faces.begin(); it != faces.end(); ++it) {      cout << "texture : " << it->t << " x: " << it->x << " y: " << it->y << '\n';     }  } catch (ifstream::failure& e) {    cout << "Read failed!\n";  }}

PASS 4 (advanced, optional, maybe not worth it for a simple example like this but good stuff to know):

- We make our struct more intelligent by giving it functionality for being read in and written out. This improves the factoring of the code by delegating responsibility.

- It also lets us avoid the explicit loop over the container and use a much more powerful tool from the standard library ;)

#include <string>#include <iostream>#include <fstream>#include <vector>#include <sstream>#include <algorithm> // for std::copy.#include <iterator> // for std::ostream_iterator.struct face {  int t, x, y;};istream& operator>>(istream& is, face& f) {  return is >> f.t >> f.x >> f.y;}ostream& operator<<(ostream& os, const face& f) {  return os << "texture : " << f.t << " x: " << f.x << " y: " << f.y; }using namespace std;typedef vector<face> vec;void save_map(const string& filename) {  ofstream myfile(filename.c_str(), ios::trunc);  // We don't have real map data right now; this is a proxy  myfile << "1 2 3\n4 5 6\n7 8 9\n1 0 1\n2 1 0\n3 1 1\n4 1 0\n5 0 1";}vec load_map(const std::string& filename) {  vec result;  std::string line;  ifstream file;  file.exceptions(ios::failbit);  file.open(filename.c_str());  while (std::getline(file, line)) {    char garbage;    face f;    std::stringstream ss(line);    if ((ss >> f) && !(ss >> garbage)) {       result.push_back(f);    }  }  return result;}int main() {  save_map("map1.txt");  try {    vec faces = load_map("map1.txt");    cout << "total point(s) loaded: " << faces.size() << '\n';    // We output all the faces by literally copying their textual    // representations to the output stream.    copy(faces.begin(), faces.end(), ostream_iterator<face>(cout, "\n");  } catch (ifstream::failure& e) {    cout << "Read failed!\n";  }}

1. 1
Rutin
38
2. 2
3. 3
4. 4
5. 5

• 11
• 9
• 12
• 14
• 9
• ### Forum Statistics

• Total Topics
633350
• Total Posts
3011473
• ### Who's Online (See full list)

There are no registered users currently online

×