Quote:Original post by Jouei
The following code here!
http://www.hexdollhosting.com/MapLoader.txt
0) HTML code for links works on this forum.
1) Don't link to code off-site unless it's *really* long (I'm talking thousands of lines) - and even then, you should try to create a smaller example for demonstration instead. Longish code snippets go in [source]
[/source] tags within your post, as shown below. Shorter snippets can go between [code][/code] tags.
(EDIT: Editing the post unescaped the tags, so I had to re-edit to re-escape them. Grr.)
2) You can hit 'edit post' on my post to see how this stuff works (it won't let you save any edits, of course). But you're supposed to have looked this stuff up in the FAQ anyway.
Quote:Was designed to load in a map for 3d purposes it seems to work fine execept for one large problem.
Strange turn of phrase there :)
Anyway, I can hardly make any sense of the rest of what you wrote, but I'd have to agree with what jpetrie said. The best "linked list expert" to help you is the one who wrote a linked list for you and shipped it with your compiler.
But while we're on the topic of things being outdated: There is no such thing as 'stdio.h' in C++ any more (it's called 'cstdio' now), and if you're going to include 'fstream', you might consider using it - FILE* is horrible, arcane C hackery that is actually coming from stdio.h (because your compiler is letting you use C headers); fstream provides things like the std::ifstream class instead.
Also, C++ doesn't make you assign structs member-by-member. You can just assign the struct instances directly, for simple structures. For more complicated ones, you can explain to the compiler how to do it (i.e., overload an assignment operator), and then continue as before. Although, with proper use of the standard library, it is unusual to make "more complicated ones". :)
(Oh, and the singular of 'vertices' is 'vertex'.)
#include <windows.h>// We won't need stdio.h - or cstdio - any more.// But we will need these:#include <list>#include <string>#include <sstream>#include <iostream>#include <fstream>using namespace std;bool DisplaySector(int ID);struct Vertex { float x; float y; float z; float Tx; float Ty; // In C++, structs and classes are interchangeable: it's just syntactic // sugar ('struct' simply means a 'class' where 'public:' is assumed for // base classes and members unless otherwise specified). Thus, they can, // and often should, have constructors: Vertex(float x, float y, float z, float Tx, float Ty) : x(x), y(y), z(z), Tx(Tx), Ty(Ty) {}};struct Polygon { int TexId; Vertex v1; Vertex v2; Vertex v3; // Note how complex objects are passed by const reference. Polygon(int TexId, const Vertex& v1, const Vertex& v2, const Vertex& v3) : TexId(TexId), v1(v1), v2(v2), v3(v3) {} // Also note that this will automatically copy the passed-in Vertices into // the Polygon being constructed: the originals still exist separately.};// Using the standard library container, the "structure of the list" is// separated out from the actual objects. This makes it easier to maintain the// code. Of course, not having to write the actual linked-list code helps a// lot, too. :)typedef std::list<Polygon> Polylist;class Sector { public: int ID; // Don't repeat the class name in member names; it doesn't add information. Polylist polygons; // The constructor and destructor will no longer need to do anything for // this class, so we eliminate them all together. The std::list will get its // own default constructor and destructor called automatically where // appropriate. // Don't return 'bool' from a function if it could only ever return the // one value ('true', in the case of the original code). The return value // is meaningless. void AddPoly(int TexId, const Vertex& v1, const Vertex& v2, const Vertex& v3); // Also, only make comments that are meaningful - in particular, don't write // things that are immediately obvious from the corresponding code. Sector(int ID) : ID(ID) {}};typedef std::list<Sector> Sectorlist;void Sector::AddPoly(int TexId, const Vertex& v1, const Vertex& v2, const Vertex& v3) { // This becomes much easier. All we do is construct a Polygon, and tell the // 'polygons' list to accept it at the end. We don't even need to make a // temporary variable to hold that Polygon, because we won't refer to it // again (just like how you don't need to make a temporary variable to hold // a number when you pass it to a function). // Also notice how, for consistency, I kept the parameters in the same order // for all the relevant functions. polygons.push_back(Polygon(TexId, v1, v2, v3));}class Map { public: Sectorlist sectors; bool AddSector(const string& SectorFile, int ID);};// Here's how we do file I/O and string manipulation in C++.// In the original code, BTW, the ID-reading process basically left behind the// "rest of a string" to be handled by the rest of the polygon-reading process.// This is sort of messy; I'm instead going to make a function that reads in// a whole polygon at once, assuming that all the vertex data is on the// same line as the texture ID. I'll have that function just handle that line,// and do the line-skipping in the main loop.Polygon readPolygon(istream& is) { // This is actually not how we'd normally write these things in C++. Instead // we would define operator overloads that would let us do things like // 'Map mymap; thefile >> mymap;'. // Initialize variables with their initial values, where possible. Also, // declare things near first use, not all at the top of the function. int id; is >> id; // One problem with the original code is that it tries to read a texture ID // before every vertex, rather than just before each polygon. Notice how // better-organized code neatly sidesteps those kinds of problems. float x, y, z, Tx, Ty; Vertex v[3]; for (int i = 0; i < 3; ++i) { is >> x >> y >> z >> Tx >> Ty; v = Vertex(x, y, z, Tx, Ty); } return Polygon(id, v[0], v[1], v[2]);}bool Map::AddSector(const string& SectorFile, int ID) { // Here's how we get that file object in C++: ifstream file(SectorFile.c_str(), ios::trunc); // The 'trunc' flag translates to what the 't' did in the old "magic string". // The 'r' is implied by the fact that we have an ifstream rather than an // ofstream. if (!file) { // we can still check it the same way. MessageBox(NULL, "Sector could not load", "May be corrrupt", MB_OK); return false; } // We don't need to pass around a buffer, because we can make a std::string // locally (although even in the old code, it would have been better to make // the buffer locally) and it will automatically be the right size. The C++ // standard library provides a real string object that is not simply some // array of chars - it handles the memory management for you. // Also, the same function will automatically work with the console input // the same way as it does with files, in case you ever need that // functionality. C++ stream objects (such as std::istream) are 'polymorphic'; // the decision about how to get bytes is made by the object itself, and not // by the name of the function called. Sector s(ID); string line; // I'm also going to restructure your loop in a more sensible way: while (getline(is, line)) { // There's no need to check for special stuff at the beginning of the line, // because all lines *not* starting with '#' are ignored *anyway*. if (!line.empty() && line[0] == '#') { // We can treat strings like streams, too: s.polygons.push_back(readPolygon(stringstream(line))); } } // Now that we finished making the sector, we can add it. sectors.push_back(s); // There's no need to "close" explicitly. return true;}Map map;// This would normally be a member function of the Map instead...bool LoadMap(const string& fileName) { // Oh, I see you're somewhat familiar with iostreams after all. Why not // use them consistently? :) // Anyway, we only read from the file, so don't also open it for writing. int ID = 1; ifstream ifs(fileName.c_str()); if (!ifs) { MessageBox(NULL, "Map could not load", "May be corrrupt", MB_OK); return false; } string sectorName; while (ifs >> sectorName) { map.AddSector(sectorName, ID); ID++; } return true;}bool DisplaySector(int ID) { Sectorlist& sectors = map.sectors; // make an alias to save typing. // The original code does something similar with pointers. // We don't need to handle an empty list specially, because it simply // contains zero elements (so the loop executes zero times). // Here's how we can look at each element of the list in turn: for (Sectorlist::iterator sit = sectors.begin(); sit != sectors.end(); ++sit) { // Notice that the "iterator" can be used like a pointer: if (sit->ID == ID) { // The original looping logic made no sense: it said "do this at least // once, until the pointer is not null", but then also included a check // for a null pointer (because it could be null the first time). Clearly // we didn't want to always do the loop at least once - a plain 'while' // loop was more appropriate. // But with the iterators, a for-loop is appropriate: Polylist& polygons = sit->polygons; for (Polylist::iterator pit = polygons.begin(); pit != polygons.end(); ++pit) { // There was another bug here: v1.x was printed instead of v3.x. // The code really should use an array instead. But I'll let it pass // for now... the *real* fix involves more complex stuff and I can only // explain so much at a time. cout << temp->v3.x; cout << " " << temp->v3.y; cout << " " << temp->v3.z; cout << " " << temp->v3.Tx; cout << " " << temp->v3.Ty; cout << "\n"; cout << " " << temp->v2.x; cout << " " << temp->v2.y; cout << " " << temp->v2.z; cout << " " << temp->v2.Tx; cout << " " << temp->v2.Ty; cout << "\n"; cout << " " << temp->v1.x; cout << " " << temp->v1.y; cout << " " << temp->v1.z; cout << " " << temp->v1.Tx; cout << " " << temp->v1.Ty; cout << "\n"; } return true; } } return false;}int main() { cout << "Trying to load map"; LoadMap("Map1.mp"); cout << "\nMap loaded!"; DisplaySector(1); DisplaySector(2); // Please don't pause your programs artificially at the end.}Please note that the above code does NOT fix all the design problems with the code (and in fact I didn't even make sure it would compile), but should at least give you some good ideas.
