Add NpcMovementTask which randomly moves bounded NPCs around the map, fix Npc#equals, bug fixes for Pathfinding and CollisionMatrix.

data/plugins/entity/spawning/npc-spawn.rb

@@ -33,11 +33,12 @@ end
 
 # Spawns the specified npc and applies the properties in the hash.
 def spawn(npc, hash)
-  $world.register(npc)
   unless hash.empty?
-    hash = decode_hash(npc.position, hash)   # Use npc.position here because sector registry events (called by World.register) can be hooked
-    apply_decoded_hash(npc, hash)            # into and someone might do something daft like move the npc immediately after it gets spawned.
+    hash = decode_hash(npc.position, hash)
+    apply_decoded_hash(npc, hash)
   end
+
+  $world.register(npc)
 end
 
 # Returns an npc with the id and position specified by the hash.
@@ -54,7 +55,7 @@ def apply_decoded_hash(npc, hash)
   hash.each do |key, value|
     case key
       when :face            then npc.turn_to(value)
-      when :boundary        then npc.boundary = value
+      when :boundary        then npc.boundaries = value
       when :spawn_animation then npc.play_animation(Animation.new(value))
       when :spawn_graphic   then npc.play_graphic(Graphic.new(value))
       else raise "Unrecognised key #{key} - value #{value}."

src/org/apollo/game/model/World.java

@@ -30,6 +30,7 @@ import org.apollo.game.model.event.EventListener;
 import org.apollo.game.model.event.EventListenerChainSet;
 import org.apollo.game.scheduling.ScheduledTask;
 import org.apollo.game.scheduling.Scheduler;
+import org.apollo.game.scheduling.impl.NpcMovementTask;
 import org.apollo.io.EquipmentDefinitionParser;
 import org.apollo.util.MobRepository;
 import org.apollo.util.NameUtil;
@@ -97,6 +98,11 @@ public final class World {
 	 */
 	private final EventListenerChainSet events = new EventListenerChainSet();
 
+	/**
+	 * The ScheduledTask that moves Npcs.
+	 */
+	private NpcMovementTask npcMovement;
+
 	/**
 	 * The {@link MobRepository} of {@link Npc}s.
 	 */
@@ -242,8 +248,12 @@ public final class World {
 		placeEntities(objects);
 		logger.fine("Loaded " + objects.length + " static objects.");
 
+		npcMovement = new NpcMovementTask(); // Must be exactly here because of ordering issues.
+		scheduler.schedule(npcMovement);
+
 		manager.start();
 		pluginManager = manager; // TODO move!!
+
 	}
 
 	/**
@@ -285,6 +295,10 @@ public final class World {
 		if (success) {
 			Sector sector = sectors.fromPosition(npc.getPosition());
 			sector.addEntity(npc);
+
+			if (npc.hasBoundaries()) {
+				npcMovement.addNpc(npc);
+			}
 		} else {
 			logger.warning("Failed to register npc, repository capacity reached: [count=" + npcRepository.size() + "]");
 		}

src/org/apollo/game/model/area/Sector.java

@@ -14,6 +14,7 @@ import org.apollo.game.model.area.collision.CollisionMatrix;
 import org.apollo.game.model.entity.Entity;
 import org.apollo.game.model.entity.Entity.EntityType;
 
+import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
 
@@ -175,14 +176,13 @@ public final class Sector {
 		Set<Entity> local = entities.get(old);
 
 		if (local == null || !local.remove(entity)) {
-			throw new IllegalArgumentException("Entity belongs in this sector but does not exist.");
+			throw new IllegalArgumentException("Entity belongs in this sector (" + this + ") but does not exist.");
 		}
 
 		local = entities.computeIfAbsent(position, key -> new HashSet<>(DEFAULT_SET_SIZE));
 
 		local.add(entity);
 		notifyListeners(entity, SectorOperation.MOVE);
-
 	}
 
 	/**
@@ -225,9 +225,14 @@ public final class Sector {
 	 */
 	public boolean traversable(Position position, EntityType entity, Direction direction) {
 		CollisionMatrix matrix = matrices[position.getHeight()];
-		int x = position.getLocalX(), y = position.getLocalY();
+		int x = position.getX(), y = position.getY();
 
-		return matrix.traversable(x, y, entity, direction);
+		return !matrix.untraversable(x % SECTOR_SIZE, y % SECTOR_SIZE, entity, direction);
+	}
+
+	@Override
+	public String toString() {
+		return MoreObjects.toStringHelper(this).add("coordinates", coordinates).toString();
 	}
 
 	/**

src/org/apollo/game/model/area/collision/CollisionMatrix.java

@@ -166,16 +166,16 @@ public final class CollisionMatrix {
 	}
 
 	/**
-	 * Returns whether or not an Entity of the specified {@link EntityType type} can traverse the tile at the specified
-	 * coordinate pair.
+	 * Returns whether or not an Entity of the specified {@link EntityType type} cannot traverse the tile at the
+	 * specified coordinate pair.
 	 * 
 	 * @param x The x coordinate.
 	 * @param y The y coordinate.
 	 * @param entity The {@link EntityType}.
 	 * @param direction The {@link Direction} the Entity is approaching from.
-	 * @return {@code true} if the tile at the specified coordinate pair is traversable, {@code false} if not.
+	 * @return {@code true} if the tile at the specified coordinate pair is not traversable, {@code false} if not.
 	 */
-	public boolean traversable(int x, int y, EntityType entity, Direction direction) {
+	public boolean untraversable(int x, int y, EntityType entity, Direction direction) {
 		CollisionFlag[] flags = CollisionFlag.forType(entity);
 		int north = 0, east = 1, south = 2, west = 3;
 

src/org/apollo/game/model/entity/Npc.java

@@ -1,6 +1,5 @@
 package org.apollo.game.model.entity;
 
-import java.util.Arrays;
 import java.util.Optional;
 
 import org.apollo.game.model.Position;
@@ -20,9 +19,9 @@ import com.google.common.base.Preconditions;
 public final class Npc extends Mob {
 
 	/**
-	 * The positions representing the bounds (i.e. walking limits) of this Npc.
+	 * The Positions representing the boundaries (i.e. walking limits) of this Npc.
 	 */
-	private Position[] boundary;
+	private Optional<Position[]> boundaries;
 
 	/**
 	 * Creates a new Npc with the specified id and {@link Position}.
@@ -31,18 +30,20 @@ public final class Npc extends Mob {
 	 * @param position The position.
 	 */
 	public Npc(int id, Position position) {
-		this(position, NpcDefinition.lookup(id));
+		this(position, NpcDefinition.lookup(id), null);
 	}
 
 	/**
 	 * Creates a new Npc with the specified {@link NpcDefinition} and {@link Position}.
 	 * 
-	 * @param position The position.
-	 * @param definition The definition.
+	 * @param position The Position.
+	 * @param definition The NpcDefinition.
+	 * @param boundaries The boundary Positions.
 	 */
-	public Npc(Position position, NpcDefinition definition) {
+	public Npc(Position position, NpcDefinition definition, Position[] boundaries) {
 		super(position, definition);
 
+		this.boundaries = Optional.ofNullable(boundaries);
 		init();
 	}
 
@@ -50,19 +51,19 @@ public final class Npc extends Mob {
 	public boolean equals(Object obj) {
 		if (obj instanceof Npc) {
 			Npc other = (Npc) obj;
-			return position.equals(other.position) && Arrays.equals(boundary, other.boundary) && getId() == other.getId();
+			return index == other.index && getId() == other.getId();
 		}
 
 		return false;
 	}
 
 	/**
-	 * Gets the boundary of this Npc.
+	 * Gets the boundaries of this Npc.
 	 * 
-	 * @return The boundary.
+	 * @return The boundaries.
 	 */
-	public Position[] getBoundary() {
-		return boundary.clone();
+	public Optional<Position[]> getBoundaries() {
+		return boundaries.isPresent() ? Optional.of(boundaries.get().clone()) : Optional.empty();
 	}
 
 	@Override
@@ -79,30 +80,29 @@ public final class Npc extends Mob {
 		return definition.get().getId();
 	}
 
+	/**
+	 * Returns whether or not this Npc has boundaries.
+	 * 
+	 * @return {@code true} if this Npc has boundaries, {@code false} if not.
+	 */
+	public boolean hasBoundaries() {
+		return boundaries.isPresent();
+	}
+
 	@Override
 	public int hashCode() {
 		final int prime = 31;
-		int result = prime * position.hashCode() + Arrays.hashCode(boundary);
-		return prime * result + getId();
+		return prime * index + getId();
 	}
 
 	/**
-	 * Indicates whether or not this Npc is bound to a specific set of coordinates.
+	 * Sets the boundaries of this Npc.
 	 * 
-	 * @return {@code true} if the Npc is bound, otherwise {@code false}.
+	 * @param boundaries The boundaries.
 	 */
-	public boolean isBound() {
-		return boundary == null;
-	}
-
-	/**
-	 * Sets the boundary of this Npc.
-	 * 
-	 * @param boundary The boundary.
-	 */
-	public void setBoundary(Position[] boundary) {
-		Preconditions.checkArgument(boundary.length == 2, "Boundary count must be 2.");
-		this.boundary = boundary.clone();
+	public void setBoundaries(Position[] boundaries) {
+		Preconditions.checkArgument(boundaries.length == 2, "Boundary count must be 2.");
+		this.boundaries = Optional.of(boundaries.clone());
 	}
 
 	@Override

src/org/apollo/game/model/entity/path/AStarPathfindingAlgorithm.java

@@ -23,7 +23,7 @@ import org.apollo.game.model.Position;
  * 
  * @author Major
  */
-final class AStarPathfindingAlgorithm extends PathfindingAlgorithm {
+public final class AStarPathfindingAlgorithm extends PathfindingAlgorithm {
 
 	/**
 	 * The heuristic.

src/org/apollo/game/model/entity/path/PathfindingAlgorithm.java

@@ -1,7 +1,7 @@
 package org.apollo.game.model.entity.path;
 
 import java.util.Deque;
-import java.util.Set;
+import java.util.Optional;
 
 import org.apollo.game.model.Direction;
 import org.apollo.game.model.Position;
@@ -9,7 +9,8 @@ import org.apollo.game.model.World;
 import org.apollo.game.model.area.Sector;
 import org.apollo.game.model.area.SectorRepository;
 import org.apollo.game.model.entity.Entity.EntityType;
-import org.apollo.game.model.entity.GameObject;
+
+import com.google.common.base.Preconditions;
 
 /**
  * An algorithm used to find a path between two {@link Position}s.
@@ -19,46 +20,48 @@ import org.apollo.game.model.entity.GameObject;
 abstract class PathfindingAlgorithm {
 
 	/**
-	 * The repository of sectors.
+	 * The repository of Sectors.
 	 */
-	private static final SectorRepository repository = World.getWorld().getSectorRepository();
+	private static final SectorRepository REPOSITORY = World.getWorld().getSectorRepository();
 
 	/**
 	 * Finds a valid path from the origin {@link Position} to the target one.
 	 * 
-	 * @param origin The origin position.
-	 * @param target The target position.
-	 * @return The {@link Deque} containing the positions to go through.
+	 * @param origin The origin Position.
+	 * @param target The target Position.
+	 * @return The {@link Deque} containing the Positions to go through.
 	 */
 	public abstract Deque<Position> find(Position origin, Position target);
 
 	/**
-	 * Returns whether or not the tile at the specified position is walkable. FIXME do this properly w/tile collision
-	 * data!
-	 *
-	 * @param position The {@link Position}.
-	 * @return {@code true} if the tile is walkable, otherwise {@code false}.
+	 * Returns whether or not a {@link Position} walking one step in any of the specified {@link Direction}s would lead
+	 * to is traversable.
+	 * 
+	 * @param current The current Position.
+	 * @param directions The Directions that should be checked.
+	 * @return {@code true} if any of the Directions lead to a traversable tile, otherwise {@code false}.
 	 */
-	protected boolean traversable(Position position) {
-		Sector sector = repository.get(position.getSectorCoordinates());
-		Set<GameObject> objects = sector.getEntities(position, EntityType.GAME_OBJECT);
-
-		return objects.stream().anyMatch(object -> object.getDefinition().isSolid());
+	protected boolean traversable(Position current, Direction... directions) {
+		return traversable(current, Optional.empty(), directions);
 	}
 
 	/**
-	 * Returns whether or not the {@link Position}s walking one step in a specified {@link Direction} would lead to is
-	 * traversable.
+	 * Returns whether or not a {@link Position} walking one step in any of the specified {@link Direction}s would lead
+	 * to is traversable.
 	 * 
-	 * @param position The starting position.
-	 * @param directions The directions that should be checked.
-	 * @return {@code true} if any of the directions lead to a traversable tile, otherwise {@code false}.
+	 * @param current The current Position.
+	 * @param boundaries The {@link Optional} containing the Position boundaries.
+	 * @param directions The Directions that should be checked.
+	 * @return {@code true} if any of the Directions lead to a traversable tile, otherwise {@code false}.
 	 */
-	protected boolean traversable(Position position, Direction... directions) {
-		int height = position.getHeight();
+	protected boolean traversable(Position current, Optional<Position[]> boundaries, Direction... directions) {
+		Preconditions.checkArgument(directions != null && directions.length > 0, "Directions array cannot be null.");
+		int height = current.getHeight();
+
+		Position[] positions = boundaries.isPresent() ? boundaries.get() : new Position[0];
 
 		for (Direction direction : directions) {
-			int x = position.getX(), y = position.getY();
+			int x = current.getX(), y = current.getY();
 			int value = direction.toInteger();
 
 			if (value >= Direction.NORTH_WEST.toInteger() && value <= Direction.NORTH_EAST.toInteger()) {
@@ -73,7 +76,9 @@ abstract class PathfindingAlgorithm {
 				x--;
 			}
 
-			if (traversable(new Position(x, y, height))) {
+			Position next = new Position(x, y, height);
+			Sector sector = REPOSITORY.get(next.getSectorCoordinates());
+			if (sector.traversable(next, EntityType.NPC, direction) && (positions.length == 0 || inside(next, positions))) {
 				return true;
 			}
 		}
@@ -81,4 +86,18 @@ abstract class PathfindingAlgorithm {
 		return false;
 	}
 
+	/**
+	 * Returns whether or not the specified {@link Position} is inside the specified {@code boundary}.
+	 * 
+	 * @param position The Position.
+	 * @param boundary The boundary Positions.
+	 * @return {@code true} if the specified Position is inside the boundary, {@code false} if not.
+	 */
+	private boolean inside(Position position, Position[] boundary) {
+		int x = position.getX(), y = position.getY();
+		Position min = boundary[0], max = boundary[1];
+
+		return x >= min.getX() && y >= min.getY() && x <= max.getX() && y <= max.getY();
+	}
+
 }

src/org/apollo/game/model/entity/path/SimplePathfindingAlgorithm.java

@@ -2,6 +2,7 @@ package org.apollo.game.model.entity.path;
 
 import java.util.ArrayDeque;
 import java.util.Deque;
+import java.util.Optional;
 
 import org.apollo.game.model.Direction;
 import org.apollo.game.model.Position;
@@ -12,7 +13,12 @@ import org.apollo.game.model.Position;
  *
  * @author Major
  */
-final class SimplePathfindingAlgorithm extends PathfindingAlgorithm {
+public final class SimplePathfindingAlgorithm extends PathfindingAlgorithm {
+
+	/**
+	 * The Optional containing the boundary Positions.
+	 */
+	private Optional<Position[]> boundaries = Optional.empty();
 
 	@Override
 	public Deque<Position> find(Position origin, Position target) {
@@ -22,6 +28,19 @@ final class SimplePathfindingAlgorithm extends PathfindingAlgorithm {
 		return addHorizontal(origin, target, positions);
 	}
 
+	/**
+	 * Finds a valid path from the origin {@link Position} to the target one.
+	 * 
+	 * @param origin The origin Position.
+	 * @param target The target Position.
+	 * @param boundaries The boundary Positions, which are marking as untraversable.
+	 * @return The {@link Deque} containing the Positions to go through.
+	 */
+	public Deque<Position> find(Position origin, Position target, Position[] boundaries) {
+		this.boundaries = Optional.of(boundaries);
+		return find(origin, target);
+	}
+
 	/**
 	 * Adds the necessary and possible horizontal {@link Position}s to the existing {@link Deque}.
 	 * <p>
@@ -33,33 +52,33 @@ final class SimplePathfindingAlgorithm extends PathfindingAlgorithm {
 	 * if so, we traverse horizontally (see {@link #addHorizontal}); if not, return the current path.
 	 * </ul>
 	 * 
-	 * @param current The current position.
+	 * @param start The current position.
 	 * @param target The target position.
 	 * @param positions The deque of positions.
 	 * @return The deque of positions containing the path.
 	 */
-	private Deque<Position> addHorizontal(Position current, Position target, Deque<Position> positions) {
-		int x = current.getX(), y = current.getY(), height = current.getHeight();
-		int dx = x - target.getX();
+	private Deque<Position> addHorizontal(Position start, Position target, Deque<Position> positions) {
+		int x = start.getX(), y = start.getY(), height = start.getHeight();
+		int dx = x - target.getX(), dy = y - target.getY();
 
 		if (dx > 0) {
-			Position west = new Position(x - 1, y, height);
+			Position current = start;
 
-			while (traversable(west) && dx-- > 0) {
-				west = new Position(--x, y, height);
-				positions.addLast(west);
+			while (traversable(current, boundaries, Direction.WEST) && dx-- > 0) {
+				current = new Position(--x, y, height);
+				positions.addLast(current);
 			}
 		} else if (dx < 0) {
-			Position east = new Position(x + 1, y, height);
+			Position current = start;
 
-			while (traversable(east) && dx++ < 0) {
-				east = new Position(++x, y, height);
-				positions.addLast(east);
+			while (traversable(current, boundaries, Direction.EAST) && dx++ < 0) {
+				current = new Position(++x, y, height);
+				positions.addLast(current);
 			}
 		}
 
 		Position last = new Position(x, y, height);
-		if (!current.equals(last) && traversable(last, Direction.NORTH, Direction.SOUTH)) {
+		if (!start.equals(last) && dy != 0 && traversable(last, boundaries, (dy > 0) ? Direction.SOUTH : Direction.NORTH)) {
 			return addVertical(last, target, positions);
 		}
 
@@ -77,33 +96,33 @@ final class SimplePathfindingAlgorithm extends PathfindingAlgorithm {
 	 * if so, we traverse horizontally (see {@link #addHorizontal}); if not, return the current path.
 	 * </ul>
 	 * 
-	 * @param current The current position.
+	 * @param start The current position.
 	 * @param target The target position.
 	 * @param positions The deque of positions.
 	 * @return The deque of positions containing the path.
 	 */
-	private Deque<Position> addVertical(Position current, Position target, Deque<Position> positions) {
-		int x = current.getX(), y = current.getY(), height = current.getHeight();
-		int dy = y - target.getY();
+	private Deque<Position> addVertical(Position start, Position target, Deque<Position> positions) {
+		int x = start.getX(), y = start.getY(), height = start.getHeight();
+		int dy = y - target.getY(), dx = x - target.getX();
 
 		if (dy > 0) {
-			Position south = new Position(x, y - 1, height);
+			Position current = start;
 
-			while (traversable(south) && dy-- > 0) {
-				south = new Position(x, --y, height);
-				positions.addLast(south);
+			while (traversable(current, boundaries, Direction.SOUTH) && dy-- > 0) {
+				current = new Position(x, --y, height);
+				positions.addLast(current);
 			}
 		} else if (dy < 0) {
-			Position north = new Position(x, y + 1, height);
+			Position current = start;
 
-			while (traversable(north) && dy++ < 0) {
-				north = new Position(x, ++y, height);
-				positions.addLast(north);
+			while (traversable(current, boundaries, Direction.NORTH) && dy++ < 0) {
+				current = new Position(x, ++y, height);
+				positions.addLast(current);
 			}
 		}
 
 		Position last = new Position(x, y, height);
-		if (!last.equals(target) && traversable(last, Direction.EAST, Direction.WEST)) {
+		if (!last.equals(target) && dx != 0 && traversable(last, boundaries, (dx > 0) ? Direction.WEST : Direction.EAST)) {
 			return addHorizontal(last, target, positions);
 		}
 

src/org/apollo/game/scheduling/impl/NpcMovementTask.java

@@ -0,0 +1,101 @@
+package org.apollo.game.scheduling.impl;
+
+import java.util.Comparator;
+import java.util.Deque;
+import java.util.PriorityQueue;
+import java.util.Queue;
+import java.util.Random;
+
+import org.apollo.game.model.Position;
+import org.apollo.game.model.entity.Npc;
+import org.apollo.game.model.entity.WalkingQueue;
+import org.apollo.game.model.entity.path.SimplePathfindingAlgorithm;
+import org.apollo.game.scheduling.ScheduledTask;
+
+import com.google.common.base.Preconditions;
+
+/**
+ * A {@link ScheduledTask} that causes {@link Npc}s to randomly walk around in their boundary.
+ *
+ * @author Major
+ */
+public final class NpcMovementTask extends ScheduledTask {
+
+	/**
+	 * The delay between executions of this task, in pulses.
+	 */
+	private static final int DELAY = 5;
+
+	/**
+	 * The random number generator used to calculate how many Npcs should be moved per execution.
+	 */
+	private static final Random RANDOM = new Random();
+
+	/**
+	 * The comparator used to sort the Npcs in the PriorityQueue.
+	 */
+	private static final Comparator<Npc> RANDOM_COMPARATOR = (first, second) -> RANDOM.nextInt(2) - 1;
+
+	/**
+	 * The PathfindingAlgorithm used by this Task.
+	 */
+	private final SimplePathfindingAlgorithm algorithm = new SimplePathfindingAlgorithm();
+
+	/**
+	 * The Queue of Npcs.
+	 */
+	private final Queue<Npc> npcs = new PriorityQueue<>(RANDOM_COMPARATOR);
+
+	/**
+	 * Creates the NpcMovementTask.
+	 */
+	public NpcMovementTask() {
+		super(DELAY, false);
+	}
+
+	/**
+	 * Adds the {@link Npc} to this {@link ScheduledTask}.
+	 * 
+	 * @param npc The Npc to add.
+	 */
+	public void addNpc(Npc npc) {
+		Preconditions.checkArgument(npc.hasBoundaries(), "Cannot add an npc with no boundaries to the NpcMovementTask.");
+		npcs.offer(npc);
+		System.out.println("Adding npc to movement task: " + npc.getId());
+	}
+
+	@Override
+	public void execute() {
+		int count = RANDOM.nextInt(npcs.size() / 50 + 5);
+		for (int iterations = 0; iterations < count; iterations++) {
+			Npc npc = npcs.poll();
+			if (npc == null) {
+				break;
+			}
+
+			Position[] boundary = npc.getBoundaries().get();
+			Position current = npc.getPosition();
+			Position min = boundary[0], max = boundary[1];
+			int currentX = current.getX(), currentY = current.getY();
+
+			boolean negativeX = RANDOM.nextBoolean(), negativeY = RANDOM.nextBoolean();
+			int x = RANDOM.nextInt(negativeX ? (currentX - min.getX()) : (max.getX() - currentX));
+			int y = RANDOM.nextInt(negativeY ? (currentY - min.getY()) : (max.getY() - currentY));
+
+			int dx = negativeX ? -x : x;
+			int dy = negativeY ? -y : y;
+			Position next = new Position(currentX + dx, currentY + dy);
+
+			Deque<Position> positions = algorithm.find(current, next, boundary);
+			WalkingQueue queue = npc.getWalkingQueue();
+
+			Position first = positions.pollFirst();
+			if (first != null && queue.addFirstStep(first)) {
+				positions.forEach(npc.getWalkingQueue()::addStep);
+			}
+
+			npcs.offer(npc);
+		}
+	}
+
+}