Jump to content
  • Advertisement
Sign in to follow this  
Diton9000

level loading code causes error

This topic is 4873 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

My level loading code causes a windows error. Can you help?
//LevelLoad.h
#ifndef LEVEL_LOAD_H
#define LEVEL_LOAD_H

#include <fstream>
#include <string>
using namespace::std;

#define NORTH 1;
#define EAST 2;
#define SOUTH 3;
#define WEST 4;

#define ERROR 1;
#define OK 0;

class Node
{
public:
	string NorthFacingBMP, EastFacingBMP, SouthFacingBMP, WestFacingBMP;
	int NorthNode, EastNode, SouthNode, WestNode;
	void Draw(int direction);
};

class Level
{
public:
	Node *Map;
	ifstream levelFile;
	int numberOfNodes;
	int LoadLevel(const char *filename);
};

#endif


//levelload.cpp
#include "LevelLoad.h"
#include <fstream>
#include <string>
using namespace::std;

int Level::LoadLevel(const char *filename)
{
	levelFile.open(filename);
	levelFile >> numberOfNodes;
	if(levelFile.fail())
	{
		return ERROR;
	}
	Map = new Node[numberOfNodes]; 
	int counter = 0;
	while (counter <= numberOfNodes)
	{
		levelFile >> Map[counter].NorthFacingBMP;
		levelFile >> Map[counter].NorthNode;
		levelFile >> Map[counter].EastFacingBMP;
		levelFile >> Map[counter].EastNode;
		levelFile >> Map[counter].SouthFacingBMP;
		levelFile >> Map[counter].SouthNode;
		levelFile >> Map[counter].WestFacingBMP;
		levelFile >> Map[counter].WestNode;

		counter++;
	}
	return OK;
}

//main.cpp
#include "LevelLoad.h"




int main (/*int argc, char *argv[]*/)
{
	Level test;
	test.LoadLevel("test.txt");
	delete [] test.Map;
	return 0;
}

test.txt is this:
1
asdf.asdf
2
asfd
2
asdf
4
asdf
3

Share this post


Link to post
Share on other sites
Advertisement
Guest Anonymous Poster
I would recommend using a for loop instead of that while
it looks like the problem is caused by the line

while (counter <

Share this post


Link to post
Share on other sites
I would recommend using a for loop instead of that while
it looks like the problem is caused by the line

while (counter <= numberOfNodes)

try changing it to

while (counter < numberOfNodes)

because the counter increments to 1 and it tries to run through another iteration of the loop but it only creates an array with one element so Map[1] doesn't exist.

Share this post


Link to post
Share on other sites
- Classes aren't supposed to behave the way that "Level" does. In particular, the "LoadLevel" looks like it really wants to be a constructor, and the class should handle deallocation of the Map itself - i.e. in a destructor. Anything less is asking for hard-to-find bugs later on - put code in the right places (encapsulation).

- The Level also doesn't need to hold on to that ifstream object, so that should just be a local in the constructor.

- For constant values, use actual constants. Better yet, use an enum for a set of possible values that logically create their own "type" (like "direction"). #define hackery is no longer required in C++ and takes on unnecessary risk.

- Checking the ifstream fail() *after* trying to read the number of nodes is a bit of a problem.

- As mentioned, use a for loop rather than a while loop where it is called for.

- "using namespace std;" shouldn't have the :: in it. I'm surprised that compiles. Anyway, using it in *header* files is generally considered a bad idea.


//LevelLoad.h
#ifndef LEVEL_LOAD_H
#define LEVEL_LOAD_H

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

enum direction { NORTH, EAST, SOUTH, WEST, NUM_DIRECTIONS };

class Node {
// With proper design, we won't need these to be public.
std::string BMPNames[4];
int adjacentNodes[4];

public:
void Draw(direction d);
Node() {} // To be stored in a std::vector (keep reading), Nodes need to be
// default-constructible - thus we must explicitly define this ctor, since
// there is another one provided. The default-constructed objects won't
// really be useful, so we need to be careful to initialize every object
// that's needed (see Level constructor, and comments about making this an
// inner class).
Node(std::istream& in);
};

class Level {
// I will use the standard library class std::vector to hold the map.
// It automatically tracks the length of data storage, and behaves basically
// like a "resizable array" with minimal overhead - and also does memory
// management for us.
std::vector<Node> map;
// A helper function for loading.
void init(std::istream& in);
public:
// Note that we can't just do this with a temporary ifstream (like
// "init(ifstream(filename))", because temporaries need to be kept constant,
// and reading from a file stream changes that stream object.
Level(const char *filename) { std::ifstream in(filename); init(in); }
Level(std::istream& in) { init(in); }
};

#endif




//levelload.cpp
#include "LevelLoad.h"

using namespace std;

void Node::Draw(direction d) {
// do something with BMPNames[d] and adjacentNodes[d]
}

Node::Node(istream& in) {
// In constructors, normally the best way to indicate a fatal error ("oops,
// not possible to construct an object after all") is to throw an exception.
// Here we want to do that if there's a problem with the input stream; we do
// that by telling the input stream object to treat any problems as
// exceptional, thus:
in.exceptions(ifstream::eofbit | ifstream::failbit | ifstream::badbit);
for (int i = 0; i < NUM_DIRECTIONS; i++) {
in >> BMPNames >> adjacentNodes;
// We can chain these actions the same way we would with printing to cout.
// Note that this is treating the items as separated by *any* whitespace;
// if you really want *line* at a time behaviour, investigate std::getline.
}
}

// A helper function for loading. The user will not call this directly, but
// instead just use a constructor. The user has the option to open the file
// himself, or pass a string literal and let that constructor open the file.

void Level::init(istream& in) {
// As before, enforce stream validity.
// For our purposes we only really need to do this here, but I've done it
// in the Node class as well so that it can "stand on its own".
// Another option would be to make the Node an inner class of Level, since
// outside code probably shouldn't be mucking around with Nodes directly at
// all.
in.exceptions(ifstream::eofbit | ifstream::failbit | ifstream::badbit);

// Count the nodes (in a local variable!) and resize the vector accordingly
int nodeCount; in >> nodeCount;
map.resize(nodeCount);
// Read everything in
for (int i = 0; i < nodeCount; i++) {
map = Node(in);
}
}




//main.cpp
#include "LevelLoad.h"

int main() {
Level test("test.txt"); // and it will clean itself up.
}

Share this post


Link to post
Share on other sites
Another windows error when I added sdl surfaces to the node class for each direction and try to load BMP's into them. The problem is, SDL_LoadBMP takes a const char *, and I read in strings. How do I fix this?

BTW, thanks for helping with the code. I will fix it soon, but I never learned how to use exceptions or vectors, so I have to look into that.

Share this post


Link to post
Share on other sites
Use std::string's c_str method. It returns a const char pointer to a null terminated string.

cheers
sam

Share this post


Link to post
Share on other sites
Are the images actually there with the indicated path names?

Assuming it's nothing that simple, show the relevant code (new declaration of the Node, and the code that's attempting to load the surfaces).

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!