AStar Algorithm

[enigma7995]

I designed my own aStar algorithm but i am afraid it is running a bit slow.  Can anyone please look at it and let me know how it can be improved.

1   2   3   4   5   6   7   8   9   10   11   12   13   14   15   16   17   18   19   20   21   22   23   24   25   26   27   28   29   30   31   32   33   34   35   36   37   38   39   40   41   42   43   44   45   46   47   48   49   50   51   52   53   54   55   56   57   58   59   60   61   62   63   64   65   66   67   68   69   70   71   72   73   74   75   76   77   78   79   80   81   82   83   84   85   86   87   88   89   90   91   92   93   94   95   96   97   98   99   100   101   102   103   104   105   106   107   108   109   110   111   112   113   114   115   116   117   118   119   120   121   122   123   124   125   126   127   128   129   130   131   132   133   134   135   136   137   138   139   140   141   142   143   144   145   146   147   148   149   150   151   152   153   154   155   156   157   158   159   160   161   162   163   164   165   166   167   168   169   170   171   172   173   174   175   176   177   178   179   180   181   182   183

public class PathFinder {    private int endX, endY;    private int startX, startY;        private int[][] map;        private ArrayList<Node> open = new ArrayList<Node>();    public static ArrayList<Node> closed = new ArrayList<Node>();        public PathFinder(int[][] map){       this.map = map;    }        public ArrayList<Node> findPath(Node start, Node end){       open.clear();       closed.clear();             this.endX = end.x;       this.endY = end.y;             this.startX = start.x;       this.startY = start.y;             open.add(start);             while (open.size() > 0){          if (open.get(0).x == endX && open.get(0).y == endY){             closed.add(open.get(0));             open.remove(0);             break;          }else{             addAdj(open.get(0));          }       }             return reconstructPath();    }        long startTime;    public ArrayList<Node> findPath(int startX, int startY, int endX, int endY){             open.clear();       closed.clear();             this.endX = endX;       this.endY = endY;             this.startX = startX;       this.startY = startY;             open.add(new Node(startX, startY, startX, startY, 0));             while (open.size() > 0){          if (open.get(0).x == endX && open.get(0).y == endY){             closed.add(open.get(0));             open.remove(0);             break;          }else{             addAdj(open.get(0));                      }       }             return reconstructPath();    }        private void addAdj(Node parent){       closed.add(open.get(0));       open.remove(0);             if (map[parent.y+1][parent.x] != 1 && notClosed(parent.x,parent.y+1,Math.abs(endX-parent.x)+Math.abs(endY-(parent.y+1))+parent.fCost,parent.x,parent.y)){          if (open.size()==0){ //            long startTime = System.nanoTime();            open.add(new Node(parent.x,parent.y+1, parent.x, parent.y, Math.abs(endX-parent.x)+Math.abs(endY-(parent.y+1))+parent.fCost)); //            System.out.println("add time" + (System.nanoTime()-startTime));         }else if (notQueued(parent.x,parent.y+1)){             startTime = System.nanoTime();             for (int i=0; i<open.size(); i++){                if (Math.abs(endX-parent.x)+Math.abs(endY-(parent.y+1))+parent.fCost<open.get(i).fCost){                   open.add(i, new Node(parent.x,parent.y+1, parent.x, parent.y, Math.abs(endX-parent.x)+Math.abs(endY-(parent.y+1))+parent.fCost));                   break;                }else if (i==open.size()-1){                   open.add(new Node(parent.x,parent.y+1, parent.x, parent.y, Math.abs(endX-parent.x)+Math.abs(endY-(parent.y+1))+parent.fCost));                   break;                }               }          }       }if (map[parent.y-1][parent.x] != 1 && notClosed(parent.x,parent.y-1,Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost,parent.x,parent.y)){          if (open.size()==0){             open.add(new Node(parent.x,parent.y-1, parent.x, parent.y, Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost));          }else if (notQueued(parent.x,parent.y-1)){             for (int i=0; i<open.size(); i++){                if (Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost<open.get(i).fCost){                   open.add(i, new Node(parent.x,parent.y-1, parent.x, parent.y, Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost));                   break;                }else if (i==open.size()-1){                   open.add(new Node(parent.x,parent.y-1, parent.x, parent.y, Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost));                   break;                }             }          }       }if (map[parent.y][parent.x+1] != 1 && notClosed(parent.x+1,parent.y,Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost,parent.x,parent.y) ){          if (open.size()==0){             open.add(new Node(parent.x+1,parent.y, parent.x, parent.y, Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost));          }else if (notQueued(parent.x+1,parent.y)){             for (int i=0; i<open.size(); i++){                if (Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost<open.get(i).fCost){                   open.add(i, new Node(parent.x+1,parent.y, parent.x, parent.y, Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost));                   break;                }else if (i==open.size()-1){                   open.add(new Node(parent.x+1,parent.y, parent.x, parent.y, Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost));                   break;                }             }          }       }if (map[parent.y][parent.x-1] != 1 && notClosed(parent.x-1,parent.y,Math.abs(endX-(parent.x-1))+Math.abs(endY-parent.y)+parent.fCost,parent.x,parent.y)){          if (open.size()==0){             open.add(new Node(parent.x-1,parent.y, parent.x, parent.y, Math.abs(endX-(parent.x-1))+Math.abs(endY-parent.y)+parent.fCost));          }else if (notQueued(parent.x-1,parent.y)){             for (int i=0; i<open.size(); i++){                if (Math.abs(endX-(parent.x-1))+Math.abs(endY-parent.y)+parent.fCost<open.get(i).fCost){                   open.add(i, new Node(parent.x-1,parent.y, parent.x, parent.y, Math.abs(endX-(parent.x-1))+Math.abs(endY-parent.y)+parent.fCost));                   break;                }else if (i==open.size()-1){                   open.add(new Node(parent.x-1,parent.y, parent.x, parent.y, Math.abs(endX-(parent.x-1))+Math.abs(endY-parent.y)+parent.fCost));                   break;                }             }          }       }    }        private boolean notClosed(int x, int y, int fCost, int parentX, int parentY){       for (int i=0; i<closed.size(); i++){          if (x == closed.get(i).x && y == closed.get(i).y){             if (closed.get(i).fCost>fCost){                closed.get(i).parentX=parentX;                closed.get(i).parentY=parentY;             }             return false;          }       }       return true;    }        private boolean notQueued(int x, int y){       for (int i=0; i<open.size(); i++){          if (x == open.get(i).x && y == open.get(i).y){             return false;          }       }       return true;    }        private ArrayList<Node> reconstructPath(){       ArrayList<Node> path = new ArrayList<Node>();             int currentParentX = closed.get(closed.size()-1).parentX;       int currentParentY = closed.get(closed.size()-1).parentY;             path.add(closed.get(closed.size()-1));             boolean finished = false;             here:       while (finished == false){          for (int i=0; i<closed.size(); i++){             if (closed.get(i).x == currentParentX && closed.get(i).y == currentParentY){                path.add(0, closed.get(i));                if (currentParentX == startX && currentParentY == startY) finished = true;                                currentParentX = closed.get(i).parentX;                currentParentY = closed.get(i).parentY;                break;             }else if (i==closed.size()-1){                System.out.println("ERROR: could not reconstruct path.");                break here;             }          }       }             return path;    }

and this is the node class

1   2   3   4   5   6   7   8   9   10   11   12   13   14   15   16   17   18   19

public class Node {    public int x, y;    public int parentX, parentY;    public int fCost;        public Node(int x, int y, int parentX, int parentY, int fCost){       this.x = x;       this.y = y;             this.parentX = parentX;       this.parentY = parentY;             this.fCost = fCost;    }        public String toString(){       return "(" + x + ", " + y + ")";    } }

thank you so much in advance... any help would be great!

[pjt33]

I designed my own aStar algorithm but i am afraid it is running a bit slow.  Can anyone please look at it and let me know how it can be improved.

I would start by refactoring it to remove the massive duplication in addAdj and by documenting the meaning of map.

[enigma7995]

I designed my own aStar algorithm but i am afraid it is running a bit slow.  Can anyone please look at it and let me know how it can be improved.

I would start by refactoring it to remove the massive duplication in addAdj and by documenting the meaning of map.

by refactoring do you mean using an automated code refactorer and the map is essentially the entire world map.  Right now i have not implemented a feature to search only up to a certain amount of the map before it quits.  I am more worried that i am using bad or slow logic, or that i am just using poor programming procedures to tackle the problem.  And thank you for the quick response!

Games published by our own members! Check 'em out!

[Varkas]

You use the wrong data structures. A* is better implemented with priority queues or a data structure called "heap" (not to be mistaken with the heap which is the main memory of computers).

A* and binary heap tutorial:
http://www.policyalmanac.org/games/binaryHeaps.htm

Refactorings are often done manually or with some help from tools, seldom purely automatic.

[Riven]

For the closed nodes, you can simply use a (hash)set.

[pjt33]

I would start by refactoring it to remove the massive duplication in addAdj and by documenting the meaning of map.

by refactoring do you mean using an automated code refactorer

No, I mean asking yourself what the differences are between

1   2   3   4   5   6   7   8   9   10   11   12   13   14   15

if (map[parent.y-1][parent.x] != 1 && notClosed(parent.x,parent.y-1,Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost,parent.x,parent.y)){          if (open.size()==0){             open.add(new Node(parent.x,parent.y-1, parent.x, parent.y, Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost));          }else if (notQueued(parent.x,parent.y-1)){             for (int i=0; i<open.size(); i++){                if (Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost<open.get(i).fCost){                   open.add(i, new Node(parent.x,parent.y-1, parent.x, parent.y, Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost));                   break;                }else if (i==open.size()-1){                   open.add(new Node(parent.x,parent.y-1, parent.x, parent.y, Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost));                   break;                }             }          }       }

and

1   2   3   4   5   6   7   8   9   10   11   12   13   14   15

if (map[parent.y][parent.x+1] != 1 && notClosed(parent.x+1,parent.y,Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost,parent.x,parent.y) ){          if (open.size()==0){             open.add(new Node(parent.x+1,parent.y, parent.x, parent.y, Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost));          }else if (notQueued(parent.x+1,parent.y)){             for (int i=0; i<open.size(); i++){                if (Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost<open.get(i).fCost){                   open.add(i, new Node(parent.x+1,parent.y, parent.x, parent.y, Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost));                   break;                }else if (i==open.size()-1){                   open.add(new Node(parent.x+1,parent.y, parent.x, parent.y, Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost));                   break;                }             }          }       }

and then extracting a method which contains all of the repeated stuff. Your class is about twice as long as it needs to be, which makes it harder to everyone else (and for you in 3 months' time) to understand it quickly.

and the map is essentially the entire world map.

It's an int[][]. What does each int mean? The cost of moving into that square? The cost of moving out of that square? An index into an array of terrain properties? A height, with the cost of moving between two squares depending non-linearly on the height difference? (I was assigned a project back in university which used precisely that for the map).

[enigma7995]

Thank you everyone for your helpful responses. I am going to try and use the suggested priority queues and i will rewrite my addAdj() method.  I would also like to apologize for my lack of clarity.  All the map is right now is a big array of walkable (0) or non-walkable (1) tiles.  There are no tiles that have a greater cost than another.  Anyways what type of speed should i expect from this sort of algorithm and how much more performance should i expect out of a professionally written library.  I'm semi new to game development and I am young in general so i really appreciate your time and understanding.

[Varkas]

On a somewhat modern computer you should expect pathes of less than 200 tiles to be found in a few milliseconds. So unless you have thousands of units doing pathfinding, it should be really quick.