Search code examples
c#.netpath-findinga-star

Having an issue implementing A* pathfinding


I am trying to add pathfinding into a game that I am creating and have tried to use the A* pathfinding algorithm. It kind of works as it does eventually find a route however it can take a very long time when nodes are far apart and when I look at it when it searches it looks like it goes over the same nodes over and over again and I'm not too sure why?

This was how I did it and I'm not sure if I messed up somewhere.

internal class PathFinding
    {
        const int NO_PARENT = -1; //Constant to show that starting node and end node have no parent nodes

        private List<Object> _obstacles = new List<Object>();
        private List<Node> _open = new List<Node>();
        private List<Node> _closed = new List<Node>();

        private struct Node
        {
            //Position
            int x;
            int y;

            //Parent node position
            int parentX;
            int parentY;

            int gCost; //Distance from node to starting node
            int hCost; //Distance from node to target node
            int fCost; //Which node should be explored next gCost + hCost

            public Node(int nodeXPosition, int nodeYPosition, int parentXPosition, int parentYPosition)
            {
                x = nodeXPosition;
                y = nodeYPosition;
                parentX = parentXPosition;
                parentY = parentYPosition;
                gCost = 0;
                hCost = 0;
                fCost = 0;
            }

            public void CalculateCosts(Node startingNode, Node targetNode)
            {
                gCost = Math.Abs(x - startingNode.x) + Math.Abs(y - startingNode.y);
                hCost = Math.Abs(x - targetNode.x) + Math.Abs (y - targetNode.y);
                fCost = gCost + hCost;
            }

            public int GetFCost()
            {
                return fCost;
            }

            public int GetXPosition()
            {
                return x;
            }

            public int GetYPosition()
            {
                return y;
            }

            public Node[] ExploreNeighbours()
            {
                //0 = up
                //1 = right
                //2 = down
                //3 = left

                Node[] neighbours = new Node[4];
                neighbours[0].x = x;
                neighbours[0].y = y - 1;
                neighbours[1].x = x + 1;
                neighbours[1].y = y;
                neighbours[2].x = x;
                neighbours[2].y = y + 1;
                neighbours[3].x = x - 1;
                neighbours[3].y = y;
                //Set parent node for explored nodes
                for (int i = 0; i < neighbours.Count(); i++)
                {
                    neighbours[i].parentX = x;
                    neighbours[i].parentY = y;
                }
                return neighbours;
            }

        }

        public PathFinding(List<Object> obstacles)
        {
            _obstacles = obstacles;
        }

        public void FindRoute(int startPositionX, int startPositionY, int endPositionX, int endPositionY)
        {
            bool routeFound = false;
            Node startingNode = new Node(startPositionX,startPositionY,NO_PARENT,NO_PARENT);
            Node endNode = new Node(endPositionX,endPositionY,NO_PARENT,NO_PARENT);
            Node[] exploredNodes;

            startingNode.CalculateCosts(startingNode,endNode);
            _open.Add(startingNode);

           while(routeFound == false)
           {
                //Sort list by lowest F cost 
                _open = _open.OrderBy(x => x.GetFCost()).ToList();
                //Explore the node in open list with lowest F cost (First in list due to sorting)
                exploredNodes = _open[0].ExploreNeighbours();
                //Check to see if explored nodes already exist as well as calculate their costs
                //If explored node already then remove it from the open list. 
                //Once it has checked to makes sure that duplicates are gone add all explored nodes to the open list
                for (int i = 0; i < exploredNodes.Length; i++)
                {
                    exploredNodes[i].CalculateCosts(startingNode,endNode);
                    for (int j = 0; j < _open.Count; j++)
                    {
                        if (exploredNodes[i].GetXPosition() == _open[j].GetXPosition() && exploredNodes[i].GetYPosition() == _open[j].GetYPosition())
                        {
                            _open.RemoveAt(j);
                        }
                    }
                }

                _open.AddRange(exploredNodes);
                _closed.Add(_open[0]);
                _open.RemoveAt(0);
         
            }

        }

    }

Solution

  • You are adding nodes to the closed set, but as far as I can see you are never checking it. You do seem to be checking the open set, but that is incorrect. The open set can contain the same node multiple times, with different costs. You can remove nodes from the open set, but then you need to make sure to keep the node with the lowest cost.

    Your loop also looks wrong to me. The general algorithm should be:

    1. If the open set is empty, return with "no path found"
    2. Pick the node with the lowest cost in the open set (and remove it)
    3. Check if the node is in the closed set, if so , restart at step 1.
    4. Check if the node is the target, if so, get the calculated path and return
    5. Add all the neighbors of the node to the open set. This should include the cost and parent of the added node.
    6. Add the node to the closed set
    7. Restart at step 1

    There are also several other issues

    1. You should use HashSet<T> for the closed list and PriorityQueue<TElement,TPriority> for the open set. Both require your nodes to be equatable, so either implement Equals method, or create a IEqualityComparer<Node> to do the comparison.
    2. You do not seem to have any real description of the graph? You seem to have a list of obstacles, but are not using them in any way.
    3. It looks like you have planed for returning the path to your destination, but are missing anything to actually track this path.

    Overall I would suggest starting by simplifying the code as much as possible.

    • Start with Djikstra before attempting A*, or even just a Breadth-first search.
    • Learn how to use the debugger and step thru your program to verify that it works as you intended. Use small graphs so you can view all the data in the debugger without to much issue.
    • Decide on a graph format, images can be useful as a simple way to create mazes and view the result. Navmeshes are the more typical format used in games, but will require a bit more to create and debug.
    • Write unit tests for various cases, to ensure you do not break anything when optimizing or improving your code.