A* Pathfinding Problem...like big one.

Hi, I tried to implement A*-Pathfinding, but it does not work.

It crashes on me, as in the Java-App freezes. It freezes before the render() method is called, so its stuck somewhere in my A*-Algorithm.

Short description of the Map, its a 2D-Array of Integers and a 1 is a wall, 0 is nothing.
Thats why I had to do everything in y,x not x,y.

This is my Pathfinding Class:


public class Pathfinder {

	Map map;
	ArrayList<Square> open = new ArrayList<Square>();
	ArrayList<Square> closed = new ArrayList<Square>();
	
	
	Pathfinder(Map map) {
		this.map = map;
	}
	
	public ArrayList<int[]> getPath(int sy, int sx, int gy, int gx) {
		
		open.add(new Square(sy,sx));
		
		boolean found = false;
		
		while (!found) {
			
			Square current;
			int minF = 0;
			int ind = 0;
			
			if (open.size() > 0) {
				for (Square s : open) {
					if (minF == 0)
						s.F = minF;
					
					if (s.F < minF) {
						minF = s.F;
						ind = open.indexOf(s);
					}
				}
			} else {
				System.out.println("Openlist empty. No Path found.");
				return null;
			}
			
			
			closed.add(open.get(ind));
			
			
			if (open.get(ind).x == gx && open.get(ind).y == gy)
				found = true;
			
			open.remove(ind);
			
			current = closed.get(closed.size()-1);
			
			//Get adjacent squares
			ArrayList<Square> adjectant = getAdjacent(current);
			
			//Parse adjacent squares
			for (Square s: adjectant) {
				
				if (map.getAt(s.y, s.x) == 1 || closed.contains(s)) {
					if (!open.contains(s)) {
					
						s.parent = current;
						s.H = (int) ( 10*( Math.sqrt(current.x-gx) + Math.sqrt(current.y-gy)));
						s.getF();
						open.add(s);
					} else {
						if (s.G < current.G) {
							s.parent = current;
							s.H = (int) ( 10*( Math.sqrt(current.x-gx) + Math.sqrt(current.y-gy)));
							s.getF();
						}
					}
						
				}
				
				
			}
			
			
		}
		
		boolean saved = false;
		ArrayList<int[]> nodes_backwards = new ArrayList<int[]>();
		ArrayList<int[]> nodes = new ArrayList<int[]>();
		
		Square s = closed.get(closed.size()-1);
		
		
		while (!saved) {
			
			int[] tmp = {s.y,s.x};
			nodes_backwards.add(tmp);
			s = s.parent;
			
			if (s.x == sx && s.y == sy) {
				saved = true;
			}
			
			
		}
		
		for (int i=nodes_backwards.size()-1; i >= 0; i--) {
			nodes.add(nodes_backwards.get(i));
		}
		
		
		return nodes;
	}
	
	private ArrayList<Square> getAdjacent(Square current) {
		
		ArrayList<Square> adjacent = new ArrayList<Square>();
		int x = current.x;
		int y = current.y;
		
		if (y-1 >= 0) {
		
			if (x-1 >= 0)
				adjacent.add(new Square(y-1,x-1,true));				
			
			adjacent.add(new Square(y-1,x));
			
			if (x+1 <= 10)
				adjacent.add(new Square(y-1,x+1,true));				
			
		}
		
		if (x-1 >= 0)
			adjacent.add(new Square(y,x-1));
		
		if (x+1 <= 10)
			adjacent.add(new Square(y,x+1));				
		
		if (y+1 <= 10) {
			
			if (x-1 >= 0)
				adjacent.add(new Square(y+1,x-1,true));				
			
			adjacent.add(new Square(y+1,x));
			
			if (x+1 <= 10)
				adjacent.add(new Square(y+1,x+1,true));				
			
		}				
		
		
		return adjacent;
	}
	
}


My adjacent finding method is pretty shitty. Because I didn’t know how else to get all the adjacent tiles, without running into problems at the borders.

Square class:


public class Square {

	int F,G;
	int H;
	int x,y;
	Square parent;
	boolean diag;

	Square(int y, int x) {
		this.x = x;
		this.y = y;
		G = 10;
	}
	
	Square(int y, int x, boolean diag) {
		this.x = x;
		this.y = y;
		
		if (diag)
			G = 14;
	}
	
	public void getF() {
		F = G + H;
	}
	
}

And my Map class:

public class Map {

	int[][] gitter = new int[10][10];
	
	Map() {
		
		for (int i=0; i < gitter.length; i++) {
        	for (int f=0; f < gitter.length; f++) {
        		gitter[i][f] = 0;
        	}
        	
        }
		
		gitter[0][0] = 1;
		
		
		gitter[5][5] = 1;
		gitter[4][5] = 1;
		gitter[3][5] = 1;
		
		gitter[3][6] = 1;
		gitter[3][7] = 1;
		gitter[3][8] = 1;
		
		gitter[7][7] = 1;
		gitter[7][8] = 1;
		
		
	}
	
	public int getAt(int y,int x) {
		return gitter[x][y];
	}
	
	public int[] getNode(int y,int x) {
		int[] pos = {64*(x-1),64*y};
		System.out.println(pos[0] +","+pos[1]);
		return pos;
	}
	
	
}

(gitter is german for grid)

replace

ArrayList<Square> closed = new ArrayList<Square>();

with

HashSet<Square> closed = new HashSet<Square>();

But how do I retrieve values from the Hashset? There is not .get() function.
Something, something iterator?

Oh and does the rest I programmed even look moderatly fine, or was the HashSet the first thing that u saw?

For closed nodes, you only need .add(…) and .contains(…)

That did not seem to fix my problem, it still freezes on loading.
Here is my new Pathfinder class, are you sure that that was the only problem, I wouldn’t be confused if there are some more mistakes I made. I followed the instructions, but maybe I missed or missunderstood something.


public class Pathfinder {

	Map map;
	ArrayList<Square> open = new ArrayList<Square>();
	HashSet<Square> closed = new HashSet<Square>();
	
	
	Pathfinder(Map map) {
		this.map = map;
	}
	
	public ArrayList<int[]> getPath(int sy, int sx, int gy, int gx) {
		Iterator itr = closed.iterator();
		open.add(new Square(sy,sx));
		
		boolean found = false;
		
		while (!found) {
			
			Square current;
			int minF = 0;
			int ind = 0;
			
			if (open.size() > 0) {
				for (Square s : open) {
					if (minF == 0)
						s.F = minF;
					
					if (s.F < minF) {
						minF = s.F;
						ind = open.indexOf(s);
					}
				}
			} else {
				System.out.println("Openlist empty. No Path found.");
				return null;
			}
			
			
			closed.add(open.get(ind));
			
			
			if (open.get(ind).x == gx && open.get(ind).y == gy)
				found = true;
			
			current = open.get(ind);
			
			open.remove(ind);
			
			//Get adjacent squares
			ArrayList<Square> adjectant = getAdjacent(current);
			
			//Parse adjacent squares
			for (Square s: adjectant) {
				
				if (map.getAt(s.y, s.x) == 1 || closed.contains(s)) {
					if (!open.contains(s)) {
					
						s.parent = current;
						s.H = (int) ( 10*( Math.sqrt(current.x-gx) + Math.sqrt(current.y-gy)));
						s.getF();
						open.add(s);
					} else {
						if (s.G < current.G) {
							s.parent = current;
							s.H = (int) ( 10*( Math.sqrt(current.x-gx) + Math.sqrt(current.y-gy)));
							s.getF();
						}
					}
						
				}
				
				
			}
			
			
		}
		
		ArrayList<int[]> nodes_backwards = new ArrayList<int[]>();
		ArrayList<int[]> nodes = new ArrayList<int[]>();
		
		while (itr.hasNext()) {
			Square s = (Square) itr.next();
			int[] tmp = {s.y,s.x};
			nodes_backwards.add(tmp);
			s = s.parent;
			
			if (s.x == sx && s.y == sy) {
				break;
			}
			
			
		}
		
		for (int i=nodes_backwards.size()-1; i >= 0; i--) {
			nodes.add(nodes_backwards.get(i));
		}
		
		
		return nodes;
	}
	
	private ArrayList<Square> getAdjacent(Square current) {
		
		ArrayList<Square> adjacent = new ArrayList<Square>();
		int x = current.x;
		int y = current.y;
		
		if (y-1 >= 0) {
		
			if (x-1 >= 0)
				adjacent.add(new Square(y-1,x-1,true));				
			
			adjacent.add(new Square(y-1,x));
			
			if (x+1 <= 10)
				adjacent.add(new Square(y-1,x+1,true));				
			
		}
		
		if (x-1 >= 0)
			adjacent.add(new Square(y,x-1));
		
		if (x+1 <= 10)
			adjacent.add(new Square(y,x+1));				
		
		if (y+1 <= 10) {
			
			if (x-1 >= 0)
				adjacent.add(new Square(y+1,x-1,true));				
			
			adjacent.add(new Square(y+1,x));
			
			if (x+1 <= 10)
				adjacent.add(new Square(y+1,x+1,true));				
			
		}				
		
		
		return adjacent;
	}
	
}

I’m gonna try to be as nice as I can, but this thread is extremely similar to another thread where the OP was really rude. You have the same problem (in your code, not your attitude xD). You’re creating two Square objects with the same x and y and expect them to be the same object. All your contains() methods will always return false, and you’ll keep visiting old squares and adding more and more neighbors until you run out of memory.

It’s also in the wrong place. Finding the grid entries adjacent to a grid entry is the job of your grid, not of the path finder. I’d move it into grid, remove the magic numbers, and reduce repetition with a loop.

Thanks guys. I also moved more stuff around and tried to reduce clutter and some problems that might occur.

This is what I have:

public class Pathfinder {

	Map map;
	ArrayList<Square> open = new ArrayList<Square>();
	HashSet<Square> closed = new HashSet<Square>();
	
	
	Pathfinder(Map map) {
		this.map = map;
	}
	
	public ArrayList<int[]> getPath(int sy, int sx, int gy, int gx) {
		Iterator itr = closed.iterator();
		open.add(new Square(sy,sx));
		
		boolean found = false;
		
		while (!found) {
			
			Square current = null;
			int minF = 0;
			int ind = 0;
			
			if (open.size() > 0) {
				for (Square s : open) {
					if (minF == 0)
						s.F = minF;
					
					if (s.F <= minF) {
						minF = s.F;
						ind = open.indexOf(s);
						current = open.get(ind);
						closed.add(current);
						
						if (open.get(ind).x == gx && open.get(ind).y == gy)
							found = true;
						
						open.remove(ind);
					}
				}
			
				//Get adjacent squares
				ArrayList<Square> adjectant = map.getAdjacent(current);
				
				//Parse adjacent squares
				for (Square s: adjectant) {
					
					if (map.getAt(s.y, s.x) == 1 || containsNode(closed,s)) {
						if (!containsNode(open,s)) {
						
							s.parent = current;
							s.H = (int) ( 10*( Math.sqrt(current.x-gx) + Math.sqrt(current.y-gy)));
							s.getF();
							open.add(s);
						} else {
							if (s.G < current.G) {
								s.parent = current;
								s.H = (int) ( 10*( Math.sqrt(current.x-gx) + Math.sqrt(current.y-gy)));
								s.getF();
							}
						}
							
					}
					
					
				}
				
			} else {
				System.out.println("Openlist empty. No Path found.");
				return null;
			}
			
		}
		
		//Save Path (first backwards then forwards)
		ArrayList<int[]> nodes_backwards = new ArrayList<int[]>();
		ArrayList<int[]> nodes = new ArrayList<int[]>();
		
		while (itr.hasNext()) {
			Square s = (Square) itr.next();
			int[] tmp = {s.y,s.x};
			nodes_backwards.add(tmp);
			s = s.parent;
			
			if (s.x == sx && s.y == sy) {
				break;
			}
			
			
		}
		
		for (int i=nodes_backwards.size()-1; i >= 0; i--) {
			nodes.add(nodes_backwards.get(i));
		}
		
		
		return nodes;
	}
	
	private boolean containsNode(ArrayList<Square> alist, Square s) {
		
		for (Square aSq: alist) {
			if (aSq.x == s.x && aSq.y == s.y)
				return true;
			
		}
		
		return false;
	}
	
	private boolean containsNode(HashSet<Square> alist, Square s) {
		Iterator<Square> itr = closed.iterator();
		
		while (itr.hasNext()) {
			Square aSq = itr.next();
			if (aSq.x == s.x && aSq.y == s.y)
				return true;
			
		}
		
		return false;
	}
	
}
public class Map {

	int[][] grid = new int[10][10];
	int HEIGHT = 10;
	int WIDTH = 10;
	
	Map() {
		
		for (int i=0; i < HEIGHT; i++) {
        	for (int f=0; f < WIDTH; f++) {
        		grid[i][f] = 0;
        	}
        	
        }
		
		grid[0][0] = 1;
		
		
		grid[5][5] = 1;
		grid[4][5] = 1;
		grid[3][5] = 1;
		
		grid[3][6] = 1;
		grid[3][7] = 1;
		grid[3][8] = 1;
		
		grid[7][7] = 1;
		grid[7][8] = 1;
		
		
	}
	
	public int getAt(int y,int x) {
		return grid[x][y];
	}
	
	public int[] getNode(int y,int x) {
		int[] pos = {64*(x-1),64*y};
		System.out.println(pos[0] +","+pos[1]);
		return pos;
	}
	
	public ArrayList<Square> getAdjacent(Square current) {
		
		ArrayList<Square> adjacent = new ArrayList<Square>();
		int x = current.x;
		int y = current.y;
		
		for (int i=-1; i<=1;i++) {
			for (int f=-1; f<=1;f++) {
				
				try {
					getAt(i,f);
					//No Errors continue:
					adjacent.add(new Square(y+i,x+f));
					
				}catch (Exception e) {
					//No adjacent found.
				}
				
			}
		}
		
		
	return adjacent;
	}
	
}

[quote]Exception in thread “LWJGL Application” java.util.ConcurrentModificationException
at java.util.AbstractList$Itr.checkForComodification(Unknown Source)
at java.util.AbstractList$Itr.next(Unknown Source)
at de.pariapluto.apps.Pathfinder.getPath(Pathfinder.java:31)
at de.pariapluto.apps.MainGame.create(MainGame.java:35)
at com.badlogic.gdx.backends.lwjgl.LwjglApplication.mainLoop(LwjglApplication.java:102)
at com.badlogic.gdx.backends.lwjgl.LwjglApplication.access$000(LwjglApplication.java:42)
at com.badlogic.gdx.backends.lwjgl.LwjglApplication$1.run(LwjglApplication.java:89)
[/quote]
(Line 25 in the HTML Version of Pathfinder in this Post)

this is the Error I’m getting. It has something to with the iteration and synchronisation. But I’m irritated since the open list, is an ArrayList and shouldn’t have anything todo with the iterator I’m using for the HashSet.

You cannot modify the list/set you’re iterating over with an Iterator. In your code, you are creating an iterator over the CLOSED list in your first line in getPath() and then using it to construct your path later. That Iterator is completely unnecessary. Why would you want to iterate over the closed list? It contains ALL squares that have been checked, which isn’t at all you want. You want to recursively add the parents of the goal Square.

You still haven’t solved this. Both your closed list and your open list will be filled infinitely until you run out of VM memory. You seem to not grasp how object orientation works here.
If you create two objects (Squares in this case), even if you give them the same values (x, y in this case), they will NOT be the same object!

boolean same = new Square(x, y) == new Square(x, y); // same = false

You are creating a new instance of Square, which obviously cannot exist anywhere else because it is new, and then checking if it already is on the closed and open lists. The new Square cannot ever already be on the closed or open list, and you will keep filling your lists with duplicates Squares that are different instances but have the same (x, y) values.

This is a very bad idea. For every node that is outside of the map, the VM will create an ArrayIndexOutOfBoundsException, which is just silently discarded. It is much faster to do a manual bounds test. The new Square() part here is also the source of the open/closed list problem I wrote about above.

Your heuristic function also looks really weird. Doing two sqrt()s (I don’t even know why you’re doing that) kind of defeats the purpose of having a heuristic in the first place as it’s supposed to be fast. If I recall correctly the most common heuristic is

s.H = Math.abs(current.x-gx) + Math.abs(current.y-gy);

but I’m no expert.

There are quite a few problems with your code. You’ll have to work a lot just to get things working, and even more to get it to work fast, but don’t give up! I’ll help you more if you want (I have only told you the problems, no real solutions xD), but I think you should try to solve it for yourself. I won’t do it for you, you know. xD

Just to expand on this, you can create an equals method in Square that would look something like this:

    public boolean equals(Square square)
    {
        if(_x == square.getX() && _y == square.getY())
            return true;
        
        return false;
    }

This way you can at least superficially determine whether or not the Squares are the same. They may be different instances (which is not best practise in itself) but they would at least be comparable. You could then replace:

boolean same = new Square(x, y) == new Square(x, y);

with

boolean same = new Square(x, y).equals(new Square(x, y)); // same = true
boolean same = new Square(x, y).equals(new Square(x, y + 1)); // same = false

EDIT: I actually prefer to make a static equals method:


public static boolean equals(Square square1, Square square2)
{
    if(square1.getX() == square2.getX() && square1.getY() == square2.getY())
        return true;
        
    return false;
}

// called with:
boolean same = Square.equals(new Square(x, y), new Square(x, y)); //same == true

Yeah, that would work if he wasn’t using hashes. A HashSomething won’t be able to locate the right bucket as the hash codes between the two “equal” square objects are different. You’d have to override hashCode() in Square too.

@Override
public int hashCode() {
    return x + y << 16;
}

This one should work, but I’m sure it isn’t optimal.

You can use Lombok (http://projectlombok.org) to autogenerate .equals() and .hashcode(). Bit of overkill for a simple class, but it comes in handy for others. The nice thing about using lombok to do it is, if you add a field somewhere, you don’t have to remind yourself to update .equals/.hashcode