I won't hunt down the segfault, since this is an assignment, but I will explain a couple of standard C++ tools and patterns that are quite likely to solve this problem if the error is indeed referring to a segfault being emitted by your own program and not the build tool.
I see a lot of pointers around the place, and some of them are even being re-used (which is not kosher unless you really like your debugger). Most of these pointers are being used to store collections of data:
int *closedMap = new int[nMapWidth*nMapHeight]; // closed nodes have been checked
int *openMap = new int[nMapWidth*nMapHeight]; // open nodes have not been checked
int *dirMap = new int[nMapWidth*nMapHeight];
Let's make life far more simple:
//#include <vector>
size_t mapSize = nMapWidth * nMapHeight;
std::vector<int> closedMap(mapSize, 0); // closed nodes have been checked
std::vector<int> openMap(mapSize, 0); // open nodes have not been checked
std::vector<int> dirMap(mapSize);
This allocates three vectors of int with size 'mapSize'. The open and closed lists have their values initialized to zero.
Vectors elements can be accessed like arrays:
dirMap[10];
Vectors are aware of their own length:
dirMap.size();
The will throw a specific exception if you attempt out-of-bounds access, which will make the bug easier to find if that's what's happening. Vectors also do not require deletion. When the object goes out of scope it will automatically be destroyed and destruct all of its elements.
Vector can also resize itself dynamically:
std::vector<int> foo; //starts at zero length
int bar = 42;
foo.push_back(bar); //automagically increases its size to store new elements
foo.push_back(bar);
foo.push_back(bar);
In addition to preferring vectors over manually allocated arrays, I'm looking at the function prototype for FindPath(), and you could do something like this:
struct Position { int x, y; }; //suggestion
struct Dimensions { int width, height; }; //suggestion
// v Return the path as a vector v use a vector for the map - pass it in by const reference
std::vector<int> FindPath(const Position start, const Position target, const std::vector<unsigned char>& map, const Dimensions mapSize);
You can store the map in a vector and pass it in to the function as a const reference (if you don't know what that means please ask), which would give you bounds checking when accessing map cells.
More importantly, you can return your path as a vector. The vector will know its own size and doesn't require any voodoo to be returned. If no path is found you can simply return an empty vector. One of the added benefits of this is that you don't have to specify a limit on the path length, since the vector holding the result can be any size.
Also, if you have a 'Position' struct, you could return a std::vector<Position> instead of returning one-dimensional indices.
So that's vector. (more info http://en.cppreference.com/w/cpp/container/vector)
Another thing I saw was this:
n=new simplePatherNode(nStartX, nStartY, 0, 0); //with sizeof(simplePatherNode) == 4 * sizeof(int)
Point-blank do this instead:
simplePatherNode n(nStartX, nStartY, 0, 0);
Manual memory management is highly error prone. In C++ we try to avoid it wherever possible. 4 integers is absolutely trivial and can fit on the stack with no issues. Don't dynamically allocate something just because it's a custom type.
Those two changes can take all the raw pointers out of your code, which will make segfaulting significantly more difficult.
Some other advice just from browsing the code:
- You can avoid the braces around one-line bodies, like you do with the for-loop in main(). It's better not to do this though, as it lacks visual distinction that makes the code easier to read and it's been found to be error prone.
- You don't have to return anything from main() in C++. The compiler will assume zero.
- You specify that the map is 4x3, but the array storing the map is linear. How about this?
unsigned char pMap[] = { //const std::vector<unsigned char> map = {
1, 1, 1, 1,
0, 1, 0, 1,
0, 1, 1, 1
};
-
static simplePatherNode* n; // the current node
static simplePatherNode* n2; // the "child" node, bordering n
vs
//do not use 'static' here - the static queue may well be causing your bug
simplePatherNode* curNode;
simplePatherNode* childNode;
//but do use clear names
https://www.google.com/search?q=premature+optimization+is+the+root+of+all+evil
- Speaking of that queue, I see you manually emptying it in a couple places by popping until it's empty. This is understandable since pqueue lacks a clear() function, but there's a more concise way to do this in case you ever need it:
std::priority_queue<int> foo;
foo = std::priority_queue<int>(); //just replace it with a new queue
- That's probably enough for now. Please keep us updated on your progress.
