Add 'MOVE' operation type to Sector, make minor code improvements to Mob and Npc.

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

@@ -13,7 +13,6 @@ import org.apollo.game.model.entity.Entity;
 import org.apollo.game.model.entity.Entity.EntityType;
 
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 
 /**
@@ -23,16 +22,16 @@ import com.google.common.collect.ImmutableSet;
  */
 public final class Sector {
 
-	/**
-	 * The default size of newly-created sets, to reduce memory usage.
-	 */
-	private static final int DEFAULT_SET_SIZE = 2;
-
 	/**
 	 * The width and length of a sector, in tiles.
 	 */
 	public static final int SECTOR_SIZE = 8;
 
+	/**
+	 * The default size of newly-created sets, to reduce memory usage.
+	 */
+	private static final int DEFAULT_SET_SIZE = 2;
+
 	/**
 	 * The sector coordinates of this sector.
 	 */
@@ -72,6 +71,7 @@ public final class Sector {
 	 * register it to this sector.
 	 * 
 	 * @param entity The entity.
+	 * @throws IllegalArgumentException If the entity does not belong in this sector.
 	 */
 	public void addEntity(Entity entity) {
 		Position position = entity.getPosition();
@@ -82,17 +82,6 @@ public final class Sector {
 		notifyListeners(entity, SectorOperation.ADD);
 	}
 
-	/**
-	 * Checks that the specified {@link Position} is included in this sector.
-	 * 
-	 * @param position The position.
-	 * @throws IllegalArgumentException If the specified position is not included in this sector.
-	 */
-	private void checkPosition(Position position) {
-		Preconditions.checkArgument(coordinates.equals(SectorCoordinates.fromPosition(position)),
-				"Position is not included in this sector.");
-	}
-
 	/**
 	 * Checks if this sector contains the specified entity.
 	 * <p>
@@ -118,24 +107,24 @@ public final class Sector {
 	}
 
 	/**
-	 * Gets a shallow copy of the {@link List} of {@link Entity} objects at the specified {@link Position}. The returned
+	 * Gets a shallow copy of the {@link Set} of {@link Entity} objects at the specified {@link Position}. The returned
-	 * type will be {@link ImmutableList}.
+	 * type will be immutable.
 	 * 
 	 * @param position The position containing the entities.
 	 * @return The list.
 	 */
-	public List<Entity> getEntities(Position position) {
+	public Set<Entity> getEntities(Position position) {
-		return ImmutableList.copyOf(entities.computeIfAbsent(position, key -> new HashSet<>(DEFAULT_SET_SIZE)));
+		return ImmutableSet.copyOf(entities.computeIfAbsent(position, key -> new HashSet<>(DEFAULT_SET_SIZE)));
 	}
 
 	/**
-	 * Gets a shallow copy of the {@link List} of {@link Entity}s with the specified {@link EntityType}. The returned
+	 * Gets a shallow copy of the {@link Set} of {@link Entity}s with the specified {@link EntityType}. The returned
-	 * list will be an {@link ImmutableList}. Type will be inferred from the call, so ensure that the entity type and
+	 * type will be immutable. Type will be inferred from the call, so ensure that the entity type and the reference
-	 * the reference correspond, or this method will fail at runtime.
+	 * correspond, or this method will fail at runtime.
 	 * 
 	 * @param position The {@link Position} containing the entities.
 	 * @param type The {@link EntityType}.
-	 * @return The list of entities.
+	 * @return The set of entities.
 	 */
 	public <T extends Entity> Set<T> getEntities(Position position, EntityType type) {
 		Set<Entity> local = entities.computeIfAbsent(position, key -> new HashSet<>(DEFAULT_SET_SIZE));
@@ -145,6 +134,34 @@ public final class Sector {
 		return ImmutableSet.copyOf(filtered);
 	}
 
+	/**
+	 * Moves the {@link Entity} that was in the specified {@code old} {@link Position}, to the current position of the
+	 * entity.
+	 * <p>
+	 * Both the {@code old} and current positions of the entity must belong to this sector.
+	 * 
+	 * @param old The old position of the entity.
+	 * @param entity The entity to move.
+	 * @throws IllegalArgumentException If either of the positions do not belong to this sector.
+	 */
+	public void moveEntity(Position old, Entity entity) {
+		Position position = entity.getPosition();
+		checkPosition(old);
+		checkPosition(position);
+
+		Set<Entity> local = entities.get(old);
+
+		if (local == null || !local.remove(entity)) {
+			throw new IllegalArgumentException("Entity belongs in this sector but does not exist.");
+		}
+
+		local = entities.computeIfAbsent(position, key -> new HashSet<>(DEFAULT_SET_SIZE));
+
+		local.add(entity);
+		notifyListeners(entity, SectorOperation.MOVE);
+
+	}
+
 	/**
 	 * Notifies the {@link SectorListener}s registered to this sector that an update has occurred.
 	 * 
@@ -174,4 +191,15 @@ public final class Sector {
 		notifyListeners(entity, SectorOperation.REMOVE);
 	}
 
+	/**
+	 * Checks that the specified {@link Position} is included in this sector.
+	 * 
+	 * @param position The position.
+	 * @throws IllegalArgumentException If the specified position is not included in this sector.
+	 */
+	private void checkPosition(Position position) {
+		Preconditions.checkArgument(coordinates.equals(SectorCoordinates.fromPosition(position)),
+				"Position is not included in this sector.");
+	}
+
 }

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

@@ -12,6 +12,11 @@ public enum SectorOperation {
 	 */
 	ADD,
 
+	/**
+	 * The move operation, when an entity has moved positions, but is still in the same sector.
+	 */
+	MOVE,
+
 	/**
 	 * The remove operation, when an entity has been removed from a sector.
 	 */

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

@@ -419,15 +419,20 @@ public abstract class Mob extends Entity {
 	 * @param position The position.
 	 */
 	public final void setPosition(Position position) {
+		Position old = this.position;
 		SectorRepository repository = World.getWorld().getSectorRepository();
-		Sector current = repository.fromPosition(this.position);
+		Sector current = repository.fromPosition(old);
 
-		Sector next = position.inside(current) ? current : repository.fromPosition(position);
+		if (position.inside(current)) {
+			this.position = position;
+			current.moveEntity(old, this);
+		} else {
+			Sector next = repository.fromPosition(position);
+			current.removeEntity(this);
+			this.position = position; // addEntity relies on the position being updated, so do that first.
 
-		current.removeEntity(this);
+			next.addEntity(this);
-		this.position = position; // addEntity relies on the position being updated, so do that first.
+		}
-
-		next.addEntity(this);
 	}
 
 	/**

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

@@ -13,19 +13,19 @@ import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
 
 /**
- * An {@link Npc} is a {@link Mob} that is not being controlled by a player.
+ * A {@link Mob} that is not controlled by a player.
  * 
  * @author Major
  */
 public final class Npc extends Mob {
 
 	/**
-	 * The positions representing the bounds (i.e. walking limits) of this npc.
+	 * The positions representing the bounds (i.e. walking limits) of this Npc.
 	 */
 	private Position[] boundary;
 
 	/**
-	 * Creates a new npc with the specified id and {@link Position}.
+	 * Creates a new Npc with the specified id and {@link Position}.
 	 * 
 	 * @param id The id.
 	 * @param position The position.
@@ -35,7 +35,7 @@ public final class Npc extends Mob {
 	}
 
 	/**
-	 * Creates a new npc with the specified {@link NpcDefinition} and {@link Position}.
+	 * Creates a new Npc with the specified {@link NpcDefinition} and {@link Position}.
 	 * 
 	 * @param position The position.
 	 * @param definition The definition.
@@ -57,7 +57,7 @@ public final class Npc extends Mob {
 	}
 
 	/**
-	 * Gets the boundary of this npc.
+	 * Gets the boundary of this Npc.
 	 * 
 	 * @return The boundary.
 	 */
@@ -71,7 +71,7 @@ public final class Npc extends Mob {
 	}
 
 	/**
-	 * Gets the id of this npc.
+	 * Gets the id of this Npc.
 	 * 
 	 * @return The id.
 	 */
@@ -87,16 +87,16 @@ public final class Npc extends Mob {
 	}
 
 	/**
-	 * Indicates whether or not this npc is bound to a specific set of coordinates.
+	 * Indicates whether or not this Npc is bound to a specific set of coordinates.
 	 * 
-	 * @return {@code true} if the npc is bound, otherwise {@code false}.
+	 * @return {@code true} if the Npc is bound, otherwise {@code false}.
 	 */
 	public boolean isBound() {
 		return boundary == null;
 	}
 
 	/**
-	 * Sets the boundary of this npc.
+	 * Sets the boundary of this Npc.
 	 * 
 	 * @param boundary The boundary.
 	 */
@@ -111,7 +111,7 @@ public final class Npc extends Mob {
 	}
 
 	/**
-	 * Transforms this npc into the npc with the specified id.
+	 * Transforms this Npc into the Npc with the specified id.
 	 * 
 	 * @param id The id.
 	 */
@@ -123,10 +123,10 @@ public final class Npc extends Mob {
 	}
 
 	/**
-	 * Initialises this npc.
+	 * Initialises this Npc.
 	 */
 	private void init() {
-		// This has to be here instead of in Mob#init because of ordering issues - the player cannot be added to the
+		// This has to be here instead of in Mob#init because of ordering issues - the Npc cannot be added to the
 		// sector until their credentials have been set, which is only done after the super constructors are called.
 		Sector sector = World.getWorld().getSectorRepository().get(position.getSectorCoordinates());
 		sector.addEntity(this);