Wrong search in A* pathing code

Question

I have some C++ code I wrote to find an A* path, but it's behaving strangely. There's quite a bit of code here, so I'll split it into chunks and try to explain what I'm doing. I'm not gonna explain how A* pathing works. I assume if you're trying to help you already know the algorithm.

First off, here's my function for calculating the h value of a node:

int
calculateH(int cX, int cY, int eX, int eY, int horiCost = 10, int diagCost = 14) {
int h;
int xDist = abs(eX - cX);
int yDist = abs(eY - cY);

if (xDist > yDist)
    h = (diagCost * yDist) + (horiCost * (xDist - yDist));
else
    h = (diagCost * xDist) + (horiCost * (yDist - xDist));

return h;
}

I'm pretty sure there's no problem here; pretty simple stuff.

Next my Node class. And I know, I know, make those variables private and use getters; I just did it this way for testing purposes.

class Node {
public:
Node(int x_, int y_, int g_, int h_, int pX_, int pY_, bool list_) {
    x = x_;
    y = y_;
    g = g_;
    h = h_;
    pX = pX_;
    pY = pY_;
    list = list_;
};
int x, y, g, h, pX, pY;
bool list;
};

Each Node has an X and Y variable. I only store G and H, not F, and calculate F when I need it (which is only once in my code). Then there's the Parent X and Y values. List is a boolean: fale = open list, true = closed list.

I also have a Object class. The only variables that matter here are X, Y, and Passable, all accessed through getters.
Now here's the start of my actual pathfinding code. It returns a string of numbers corresponding to directions as shown below:
432
501
678
So 1 means move right, 8 means go down and right, 0 means don't go anywhere.

string
findPath(int startX, int startY, int finishX, int finishY) {
// Check if we're already there.
if (startX == finishX && startY == finishY)
    return "0";
// Check if the space is occupied.
for (int k = 0; k < objects.size(); k ++)
    if ((objects[k] -> getX() == finishX) &&
        (objects[k] -> getY() == finishY) &&
        (!objects[k] -> canPass()))
        return "0";
// The string that contains our path.
string path = "";
// The identifier of the current node.
int currentNode = 0;
// The list of nodes.
vector<Node> nodes;
// Add the starting node to the closed list.
nodes.push_back(Node(startX, startY, 0,
    calculateH(startX, startY, finishX, finishY),
    startX, startY, true));

Now we loop through until we find the destination. Note that sizeLimit is just to make sure we don't loop forever (it WONT if I can fix this code. As of right now it's very necessary). Everything from this point on, until I mark otherwise, is inside the i j loops.

int sizeLimit = 0;
while ((nodes[currentNode].x != finishX) | (nodes[currentNode].y != finishY)) {

    // Check the surrounding spaces.
    for (int i = -1;  i <= 1; i ++) {
        for (int j = -1; j <= 1; j ++) {
            bool isEmpty = true;
            // Check if there's a wall there.
            for (int k = 0; k < objects.size(); k ++) {
                if ((objects[k] -> getX() == (nodes[currentNode].x + i)) &&
                    (objects[k] -> getY() == (nodes[currentNode].y + j)) &&
                    (!objects[k] -> canPass())) {
                    isEmpty = false;
                }
            }

Next part:

 // Check if it's on the closed list.
            for (int k = 0; k < nodes.size(); k ++) {
                if ((nodes[k].x == (nodes[currentNode].x + i)) &&
                    (nodes[k].y == (nodes[currentNode].y + j)) &&
                    (nodes[k].list)) {
                    isEmpty = false;
                }
            }

Continuing on:

// Check if it's on the open list.
            for (int k = 0; k < nodes.size(); k ++) {
                if ((nodes[k].x == (nodes[currentNode].x + i)) &&
                    (nodes[k].y == (nodes[currentNode].y + j)) &&
                    (!nodes[k].list)) {
                    // Check if the G score is lower from here.
                    if (nodes[currentNode].g + 10 + (abs(i * j) * 4) <= nodes[k].g) {
                        nodes[k].g = nodes[currentNode].g + 10 + (abs(i * j) * 4);
                        nodes[k].pX = nodes[currentNode].x;
                        nodes[k].pY = nodes[currentNode].y;
                    }
                    isEmpty = false;
                }
            }

This is the last part of the i j loop:

if (isEmpty) {
                nodes.push_back(Node(nodes[currentNode].x + i,
                    nodes[currentNode].y + j,
                    nodes[currentNode].g + 10 + (abs(i * j) * 4),
                    calculateH(nodes[currentNode].x + i, nodes[currentNode].y + j, finishX, finishY),
                    nodes[currentNode].x,
                    nodes[currentNode].y,
                    false));
            }
    }
}

Now we find the Node with the lowest F score, change it to the current node, and add it to the closed list. The protection against infinite lopping is also finished up here:

// Set the current node to the one with the lowest F score.
    int lowestF = (nodes[currentNode].g + nodes[currentNode].h);
    int lowestFIndex = currentNode;
    for (int k = 0; k < nodes.size(); k ++) {
        if (((nodes[k].g + nodes[k].h) <= lowestF) &&
            (!nodes[k].list)) {
            lowestF = (nodes[k].g + nodes[k].h);
            lowestFIndex = k;
        }
    }
    currentNode = lowestFIndex;
    // Change it to the closed list.
    nodes[currentNode].list = true;

    sizeLimit ++;
    if (sizeLimit > 1000)
        return "";
}

The problem I'm having is that it wont find certain paths. It seems to never work if the path goes up or left at any point. Down, left, and right all work fine. Most of the time anyway. I have absolutely no idea what's causing this problem; at one point I even tried manually following my code to see where the problem was. No surprise that didn't work.

And one more thing: if you're counting my curly braces (first of all wow, you have more dedication than I thought), you'll notice I'm missing a close brace at the end. Not to mention my return statement. There's a little bit of code at the end to actually make the path that I've left out. I know that that part's not the problem however; I currently have it commented out and it still doesn't work in the same way. I added some code to tell me where it's not working, and it's at the pathfinding part, not the interpretation.

Sorry my code's so messy and inefficient. I'm new to c++, so any critical advice on my technique is welcome as well.

Answer 1

I think that when you are looking for the next "currentNode", you should not start with lowestF = (nodes[currentNode].g + nodes[currentNode].h); because that value, in principle, will (always) be lower-or-equal-to any other nodes in the open-set. Just start with the value of std::numeric_limits<int>::max() or some very large number, or use a priority queue instead of an std::vector to store the nodes (like std::priority_queue, or boost::d_ary_heap_indirect).

I'm pretty sure that is the problem. And since your heuristic can very often be equal to the actual path-distance obtained, there is a good chance that following nodes in the open-set turn out to have the same cost (g+h) as the currentNode. This would explain why certain paths get explored and others don't and why it gets stuck.