Search code examples
javaalgorithma-star

trying to implement an A* algorithm after hours of trying I thought I'd ask for someone to review my code


I've been trying to implement an A* algorithm in a tower defense style game and after hours of trying I thought I'd ask for some insight in my logic flaws. I've been trying to create it via http://web.mit.edu/eranki/www/tutorials/search/ & https://stackoverflow.com/a/5602061/1550619 But and infinite problem occurs when trying to generate the successors/neighbors to an AStarNode(I think) - First time implementing A* and still learning.

private ArrayList<AStarNode> aStaring(Object o, AStarNode start, AStarNode goal) {
    ArrayList<AStarNode> closed = new ArrayList();
    ArrayList<AStarNode> open = new ArrayList();

    start.g = 0;
    start.h = estimateDistance(start, goal);
    start.f = 0;

    open.add(start);

    while(!open.isEmpty()){
        AStarNode q = null;

        for(AStarNode asn : open){
            if(q == null || asn.f < q.f){
                q = asn;
            }
        }

        open.remove(q);
        closed.add(q);
        for(AStarNode succesor : q.generateSuccesors()){
            if(closed.contains(succesor)){
                System.out.println("Closed contained succesor");
                //TODO Check if walkable 
            }else{
                if(!open.contains(succesor)){
                    succesor.g = q.g+1;
                    succesor.h = estimateDistance(succesor, goal);
                    succesor.f = succesor.g + succesor.h;
                    succesor.parent = q;
                    open.add(succesor);
                }else{
                    float nextG = q.g + succesor.cost;
                    if(nextG < succesor.g){
                        open.remove(succesor);
                        closed.add(succesor);
                    }
                }
                if(succesor.x == goal.x && succesor.y == goal.y){ //path found
                    System.out.println("hurray");
                    return reconstructPath(succesor);
                }
            }
        }
    }
    return null;
}
public class AStarNode {

private MapDimension md = new MapDimension();
public AStarNode parent;
public int x,y;
public int f,g,h;
public int cost = 1;

public AStarNode(int x, int y){
    this.x = x;
    this.y = y;
}

//Looking up 4 neighbors and adding to node;
public ArrayList<AStarNode> generateSuccesors(){
    ArrayList<AStarNode> neighbors = new ArrayList<>();
    if(x+1 < md.getWidth()){
        AStarNode temp = new AStarNode(x+1,y);
        temp.parent = this;
        neighbors.add(temp);
    }
    if(x-1 > 0){
        AStarNode temp = new AStarNode(x-1,y);
        temp.parent = this;
        neighbors.add(temp);
    }
    if(y+1 < md.getHeight()){
        AStarNode temp = new AStarNode(x,y+1);
        temp.parent = this;
        neighbors.add(temp);
    }
    if(y-1 > 0){
        AStarNode temp = new AStarNode(x,y-1);
        temp.parent = this;
        neighbors.add(temp);
    }
    return neighbors;
}
}

Map:

public static final int[][] MAP = {
    {1, 1, 1, 1, 2, 2},
    {1, 1, 1, 0, 0, 2},
    {2, 0, 1, 0, 0, 0},
    {2, 0, 1, 0, 1, 1},
    {2, 2, 1, 1, 1, 1},
    {2, 2, 0, 0, 0, 1},
    {2, 1, 1, 1, 1, 1},
    {0, 1, 0, 0, 2, 2},
    {2, 1, 0, 2, 2, 2},
    {0, 1, 0, 0, 2, 2},
};

Any pointers towards the right direction would be fantastic :]


Solution

  • Everytime you run generateSuccessors, you create 4 (or less) NEW instances of AStarNode objects. This is not a problem per se, but AStarNode does not define hashCode and equals. So two nodes with the same coordinates are not considered equals, so closed.contains(successor) will NEVER return true.

    Implement hashCode and equals on AStarNode (or get your IDE to do it for you). Something as simple as:

    public int hashCode(){
       return x*y;
    }
    
    public boolean equals(Object o){
      if (o instanceof AStarNode){
        return x==((AStarNode)o).x && y==((AStarNode)o).y;
      }
      return false;
    }