Fix sector code (hopefully for the last time) and resolve order conflicts from the use of hashCode etc; add hashCode and equals to any class extending Entity; remove magic constant when resetting the interacting mob.

src/org/apollo/game/message/handler/impl/ObjectActionVerificationHandler.java

@@ -1,6 +1,7 @@
 package org.apollo.game.message.handler.impl;
 
 import java.util.List;
+import java.util.Set;
 
 import org.apollo.game.message.handler.MessageHandler;
 import org.apollo.game.message.handler.MessageHandlerContext;
@@ -36,14 +37,9 @@ public final class ObjectActionVerificationHandler extends MessageHandler<Object
 
 		Position position = message.getPosition();
 		Sector sector = repository.fromPosition(position);
-		List<GameObject> objects = sector.getEntities(position, EntityType.GAME_OBJECT);
+		Set<GameObject> objects = sector.getEntities(position, EntityType.GAME_OBJECT);
 
-		if (!containsObject(id, objects)) {
+		if (!player.getPosition().isWithinDistance(position, 15) || !containsObject(id, objects)) {
-			ctx.breakHandlerChain();
-			return;
-		}
-
-		if (!player.getPosition().isWithinDistance(position, 15)) {
 			ctx.breakHandlerChain();
 			return;
 		}
@@ -62,8 +58,8 @@ public final class ObjectActionVerificationHandler extends MessageHandler<Object
 	 * @param objects The list of objects.
 	 * @return {@code true} if the list does contain the object with the specified id, otherwise {@code false}.
 	 */
-	private static boolean containsObject(int id, List<GameObject> objects) {
+	private static boolean containsObject(int id, Set<GameObject> objects) {
-		return objects.stream().filter(object -> object.getId() == id).findAny().isPresent();
+		return objects.stream().anyMatch(object -> object.getId() == id);
 	}
 
 }

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

@@ -1,5 +1,8 @@
 package org.apollo.game.model;
 
+import org.apollo.game.model.area.Sector;
+import org.apollo.game.model.area.SectorCoordinates;
+
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Preconditions;
 
@@ -23,7 +26,7 @@ public final class Position {
 	/**
 	 * The packed integer containing the {@code height, x}, and {@code y} variables.
 	 */
-	private final int position;
+	private final int packed;
 
 	/**
 	 * Creates a position at the default height.
@@ -45,14 +48,14 @@ public final class Position {
 	public Position(int x, int y, int height) {
 		Preconditions.checkArgument(height >= 0 && height < HEIGHT_LEVELS, "Height level out of bounds.");
 
-		position = height << 30 | (y & 0x7FFF) << 15 | x & 0x7FFF;
+		packed = height << 30 | (y & 0x7FFF) << 15 | x & 0x7FFF;
 	}
 
 	@Override
 	public boolean equals(Object obj) {
 		if (obj instanceof Position) {
 			Position other = (Position) obj;
-			return position == other.position;
+			return packed == other.packed;
 		}
 
 		return false;
@@ -94,7 +97,7 @@ public final class Position {
 	 * @return The height level.
 	 */
 	public int getHeight() {
-		return position >> 30;
+		return packed >> 30;
 	}
 
 	/**
@@ -147,6 +150,15 @@ public final class Position {
 		return Math.max(deltaX, deltaY);
 	}
 
+	/**
+	 * Returns the {@link SectorCoordinates} of the {@link Sector} this position is inside.
+	 * 
+	 * @return The sector coordinates.
+	 */
+	public SectorCoordinates getSectorCoordinates() {
+		return SectorCoordinates.fromPosition(this);
+	}
+
 	/**
 	 * Gets the x coordinate of the sector this position is in.
 	 * 
@@ -171,7 +183,7 @@ public final class Position {
 	 * @return The x coordinate.
 	 */
 	public int getX() {
-		return position & 0x7FFF;
+		return packed & 0x7FFF;
 	}
 
 	/**
@@ -180,12 +192,23 @@ public final class Position {
 	 * @return The y coordinate.
 	 */
 	public int getY() {
-		return (position >> 15) & 0x7FFF;
+		return (packed >> 15) & 0x7FFF;
 	}
 
 	@Override
 	public int hashCode() {
-		return position;
+		return packed;
+	}
+
+	/**
+	 * Returns whether or not this position is inside the specified {@link Sector}.
+	 * 
+	 * @param sector The sector.
+	 * @return {@code true} if this position is inside the specified sector, otherwise {@code false}.
+	 */
+	public boolean inside(Sector sector) {
+		SectorCoordinates coordinates = sector.getCoordinates();
+		return coordinates.equals(getSectorCoordinates());
 	}
 
 	/**
@@ -204,7 +227,7 @@ public final class Position {
 	@Override
 	public String toString() {
 		return MoreObjects.toStringHelper(this).add("x", getX()).add("y", getY()).add("height", getHeight())
-				.add("sector x", getTopLeftSectorX()).add("sector y", getTopLeftSectorY()).toString();
+				.add("sector", getSectorCoordinates()).toString();
 	}
 
 }

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

@@ -3,6 +3,7 @@ package org.apollo.game.model;
 import java.io.BufferedInputStream;
 import java.io.FileInputStream;
 import java.io.InputStream;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.logging.Logger;
@@ -133,7 +134,7 @@ public final class World {
 	/**
 	 * This world's {@link SectorRepository}.
 	 */
-	private final SectorRepository sectorRepository = new SectorRepository(false);
+	private final SectorRepository sectors = SectorRepository.immutable();
 
 	/**
 	 * Creates the world.
@@ -222,7 +223,7 @@ public final class World {
 	 * @return The sector repository.
 	 */
 	public SectorRepository getSectorRepository() {
-		return sectorRepository;
+		return sectors;
 	}
 
 	/**
@@ -236,10 +237,10 @@ public final class World {
 	public void init(int release, IndexedFileSystem fs, PluginManager manager) throws Exception {
 		this.releaseNumber = release;
 
-		ItemDefinitionDecoder itemDefDecoder = new ItemDefinitionDecoder(fs);
+		ItemDefinitionDecoder itemDecoder = new ItemDefinitionDecoder(fs);
-		ItemDefinition[] itemDefs = itemDefDecoder.decode();
+		ItemDefinition[] items = itemDecoder.decode();
-		ItemDefinition.init(itemDefs);
+		ItemDefinition.init(items);
-		logger.fine("Loaded " + itemDefs.length + " item definitions.");
+		logger.fine("Loaded " + items.length + " item definitions.");
 
 		try (InputStream is = new BufferedInputStream(new FileInputStream("data/equipment-" + release + ".dat"))) {
 			EquipmentDefinitionParser parser = new EquipmentDefinitionParser(is);
@@ -249,14 +250,14 @@ public final class World {
 		}
 
 		NpcDefinitionDecoder npcDecoder = new NpcDefinitionDecoder(fs);
-		NpcDefinition[] npcDefs = npcDecoder.decode();
+		NpcDefinition[] npcs = npcDecoder.decode();
-		NpcDefinition.init(npcDefs);
+		NpcDefinition.init(npcs);
-		logger.fine("Loaded " + npcDefs.length + " npc definitions.");
+		logger.fine("Loaded " + npcs.length + " npc definitions.");
 
 		ObjectDefinitionDecoder objectDecoder = new ObjectDefinitionDecoder(fs);
-		ObjectDefinition[] objDefs = objectDecoder.decode();
+		ObjectDefinition[] objectDefs = objectDecoder.decode();
-		ObjectDefinition.init(objDefs);
+		ObjectDefinition.init(objectDefs);
-		logger.fine("Loaded " + objDefs.length + " object definitions.");
+		logger.fine("Loaded " + objectDefs.length + " object definitions.");
 
 		GameObjectDecoder staticDecoder = new GameObjectDecoder(fs);
 		GameObject[] objects = staticDecoder.decode();
@@ -283,10 +284,7 @@ public final class World {
 	 * @param entities The entities.
 	 */
 	private void placeEntities(Entity... entities) {
-		for (Entity entity : entities) {
+		Arrays.stream(entities).forEach(entity -> sectors.fromPosition(entity.getPosition()).addEntity(entity));
-			Sector sector = sectorRepository.fromPosition(entity.getPosition());
-			sector.addEntity(entity);
-		}
 	}
 
 	/**
@@ -306,7 +304,7 @@ public final class World {
 		boolean success = npcRepository.add(npc);
 
 		if (success) {
-			Sector sector = sectorRepository.fromPosition(npc.getPosition());
+			Sector sector = sectors.fromPosition(npc.getPosition());
 			sector.addEntity(npc);
 		} else {
 			logger.warning("Failed to register npc, repository capacity reached: [count=" + npcRepository.size() + "]");
@@ -329,7 +327,7 @@ public final class World {
 		boolean success = playerRepository.add(player);
 		if (success) {
 			players.put(NameUtil.encodeBase37(username), player);
-			Sector sector = sectorRepository.fromPosition(player.getPosition());
+			Sector sector = sectors.fromPosition(player.getPosition());
 			sector.addEntity(player);
 
 			logger.info("Registered player: " + player + " [count=" + playerRepository.size() + "]");
@@ -357,11 +355,9 @@ public final class World {
 	 */
 	public void unregister(final Npc npc) {
 		if (npcRepository.remove(npc)) {
-			Sector sector = sectorRepository.fromPosition(npc.getPosition());
+			Sector sector = sectors.fromPosition(npc.getPosition());
 
-			if (!sector.removeEntity(npc)) {
+			sector.removeEntity(npc);
-				logger.warning("Could not remove npc from their sector.");
-			}
 		} else {
 			logger.warning("Could not find npc " + npc + " to unregister!");
 		}
@@ -377,10 +373,8 @@ public final class World {
 			players.remove(NameUtil.encodeBase37(player.getUsername()));
 			logger.info("Unregistered player: " + player + " [count=" + playerRepository.size() + "]");
 
-			Sector sector = sectorRepository.fromPosition(player.getPosition());
+			Sector sector = sectors.fromPosition(player.getPosition());
-			if (!sector.removeEntity(player)) {
+			sector.removeEntity(player);
-				logger.warning("Could not remove player from their sector.");
-			}
 
 			logoutDispatcher.dispatch(player);
 		} else {

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

@@ -2,15 +2,19 @@ package org.apollo.game.model.area;
 
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 import org.apollo.game.model.Position;
 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;
 
 /**
  * An 8x8 area of the map.
@@ -19,6 +23,11 @@ import com.google.common.collect.ImmutableList;
  */
 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.
 	 */
@@ -32,7 +41,7 @@ public final class Sector {
 	/**
 	 * A map of positions to entities in that position.
 	 */
-	private final Map<Position, List<Entity>> entities = new HashMap<>();
+	private final Map<Position, Set<Entity>> entities = new HashMap<>();
 
 	/**
 	 * A list of listeners registered to this sector.
@@ -66,27 +75,37 @@ public final class Sector {
 	 */
 	public void addEntity(Entity entity) {
 		Position position = entity.getPosition();
-		List<Entity> entities = this.entities.get(position);
+		checkPosition(position);
-		if (entities == null) {
+		Set<Entity> local = entities.computeIfAbsent(position, key -> new HashSet<>(DEFAULT_SET_SIZE));
-			entities = new ArrayList<>();
-		}
 
-		entities.add(entity);
+		local.add(entity);
-		this.entities.put(position, entities);
 		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>
+	 * This method operates in constant time.
 	 * 
 	 * @param entity The entity.
 	 * @return {@code true} if this sector contains the entity, otherwise {@code false}.
 	 */
 	public boolean contains(Entity entity) {
 		Position position = entity.getPosition();
-		List<Entity> entities = this.entities.get(position);
+		Set<Entity> local = entities.get(position);
 
-		return entities != null && entities.contains(entity);
+		return local != null && local.contains(entity);
 	}
 
 	/**
@@ -98,17 +117,6 @@ public final class Sector {
 		return coordinates;
 	}
 
-	/**
-	 * Gets an {@link ImmutableList} containing every {@link Entity} in this sector.
-	 * 
-	 * @return The list.
-	 */
-	public List<Entity> getEntities() {
-		List<Entity> combined = new ArrayList<>();
-		this.entities.values().forEach(entities -> combined.addAll(entities));
-		return ImmutableList.copyOf(combined);
-	}
-
 	/**
 	 * Gets a shallow copy of the {@link List} of {@link Entity} objects at the specified {@link Position}. The returned
 	 * type will be {@link ImmutableList}.
@@ -117,13 +125,7 @@ public final class Sector {
 	 * @return The list.
 	 */
 	public List<Entity> getEntities(Position position) {
-		List<Entity> entities = this.entities.get(position);
+		return ImmutableList.copyOf(entities.computeIfAbsent(position, key -> new HashSet<>(DEFAULT_SET_SIZE)));
-		if (entities == null) {
-			this.entities.put(position, new ArrayList<>());
-			return ImmutableList.of();
-		}
-
-		return ImmutableList.copyOf(entities);
 	}
 
 	/**
@@ -135,16 +137,12 @@ public final class Sector {
 	 * @param type The {@link EntityType}.
 	 * @return The list of entities.
 	 */
-	public <T extends Entity> List<T> getEntities(Position position, EntityType type) {
+	public <T extends Entity> Set<T> getEntities(Position position, EntityType type) {
-		List<Entity> entities = this.entities.get(position);
+		Set<Entity> local = entities.computeIfAbsent(position, key -> new HashSet<>(DEFAULT_SET_SIZE));
-		if (entities == null) {
-			this.entities.put(position, new ArrayList<>());
-			return ImmutableList.of();
-		}
 
 		@SuppressWarnings("unchecked")
-		List<T> filtered = (List<T>) entities.stream().filter(e -> e.getEntityType() == type).collect(Collectors.toList());
+		Set<T> filtered = (Set<T>) local.stream().filter(entity -> entity.getEntityType() == type).collect(Collectors.toSet());
-		return ImmutableList.copyOf(filtered);
+		return ImmutableSet.copyOf(filtered);
 	}
 
 	/**
@@ -161,17 +159,24 @@ public final class Sector {
 	 * Removes a {@link Entity} from this sector.
 	 * 
 	 * @param entity The entity.
-	 * @return {@code true} if the entity was removed, otherwise {@code false}.
+	 * @throws IllegalArgumentException If the entity does not belong in this sector, or if it was never added.
 	 */
-	public boolean removeEntity(Entity entity) {
+	public void removeEntity(Entity entity) {
-		Position position = entity.getPosition();
+		try {
-		List<Entity> entities = this.entities.get(position);
+			Position position = entity.getPosition();
+			checkPosition(position);
+
+			Set<Entity> local = entities.get(position);
+
+			if (local == null || !local.remove(entity)) {
+				throw new IllegalArgumentException("Entity belongs in this sector but does not exist.");
+			}
 
-		if (entities != null && entities.remove(entity)) {
 			notifyListeners(entity, SectorOperation.REMOVE);
-			return true;
+		} catch (Exception e) {
+			e.printStackTrace();
+			throw e;
 		}
-		return false;
 	}
 
 }

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

@@ -2,11 +2,14 @@ package org.apollo.game.model.area;
 
 import org.apollo.game.model.Position;
 
+import com.google.common.base.MoreObjects;
+
 /**
  * An immutable class representing the coordinates of a sector, where the coordinates ({@code x, y}) are the top-left of
  * the sector.
  * 
  * @author Graham
+ * @author Major
  */
 public final class SectorCoordinates {
 
@@ -43,12 +46,12 @@ public final class SectorCoordinates {
 
 	@Override
 	public boolean equals(Object obj) {
-		if (obj == null || getClass() != obj.getClass()) {
+		if (obj instanceof SectorCoordinates) {
-			return false;
+			SectorCoordinates other = (SectorCoordinates) obj;
+			return x == other.x && y == other.y;
 		}
 
-		final SectorCoordinates other = (SectorCoordinates) obj;
+		return false;
-		return x == other.x && y == other.y;
 	}
 
 	/**
@@ -74,4 +77,9 @@ public final class SectorCoordinates {
 		return x << 16 | y;
 	}
 
+	@Override
+	public String toString() {
+		return MoreObjects.toStringHelper(this).add("x", x).add("y", y).toString();
+	}
+
 }

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

@@ -17,6 +17,27 @@ import com.google.common.collect.ImmutableList;
  */
 public final class SectorRepository {
 
+	/**
+	 * Returns an immutable sector repository, where {@link Sector}s cannot be added or removed.
+	 * <p>
+	 * Note that, internally, sectors are added lazily (i.e. only when necessary). As such, repositories are (again,
+	 * internally) not actually immutable, so do not rely on such behaviour.
+	 * 
+	 * @return The sector repository.
+	 */
+	public static SectorRepository immutable() {
+		return new SectorRepository(false);
+	}
+
+	/**
+	 * Returns a mutable sector repository, where {@link Sector}s may be removed.
+	 * 
+	 * @return The sector repository.
+	 */
+	public static SectorRepository mutable() {
+		return new SectorRepository(true);
+	}
+
 	/**
 	 * Whether or not sectors can be removed from this repository.
 	 */
@@ -32,7 +53,7 @@ public final class SectorRepository {
 	 * 
 	 * @param permitRemoval If removal (of {@link Sector}s) from this repository should be permitted.
 	 */
-	public SectorRepository(boolean permitRemoval) {
+	private SectorRepository(boolean permitRemoval) {
 		this.permitRemoval = permitRemoval;
 	}
 
@@ -44,11 +65,12 @@ public final class SectorRepository {
 	 * @throws UnsupportedOperationException If the coordinates of the provided sector are already mapped (and hence the
 	 *             existing sector would be replaced), and removal of sectors is not permitted.
 	 */
-	public void add(Sector sector) {
+	private void add(Sector sector) {
 		Preconditions.checkNotNull(sector, "Sector cannot be null.");
 		if (sectors.containsKey(sector.getCoordinates()) && !permitRemoval) {
 			throw new UnsupportedOperationException("Cannot add a sector with the same coordinates as an existing sector.");
 		}
+
 		sectors.put(sector.getCoordinates(), sector);
 	}
 
@@ -97,6 +119,7 @@ public final class SectorRepository {
 			sector = new Sector(coordinates);
 			add(sector);
 		}
+
 		return sector;
 	}
 
@@ -121,6 +144,7 @@ public final class SectorRepository {
 		if (!permitRemoval) {
 			throw new UnsupportedOperationException("Cannot remove sectors from this repository.");
 		}
+
 		return sectors.remove(sector.getCoordinates()) != null;
 	}
 

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

@@ -73,4 +73,10 @@ public abstract class Entity {
 		return position;
 	}
 
+	@Override
+	public abstract boolean equals(Object obj);
+
+	@Override
+	public abstract int hashCode();
+
 }

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

@@ -31,6 +31,16 @@ public final class GameObject extends Entity {
 		this.packed = id << 8 | type << 2 | orientation;
 	}
 
+	@Override
+	public boolean equals(Object obj) {
+		if (obj instanceof GameObject) {
+			GameObject other = (GameObject) obj;
+			return position.equals(other.position) && packed == other.packed;
+		}
+
+		return false;
+	}
+
 	/**
 	 * Gets the definition of this object.
 	 * 
@@ -72,6 +82,11 @@ public final class GameObject extends Entity {
 		return (packed >> 2) & 0x3F;
 	}
 
+	@Override
+	public int hashCode() {
+		return packed;
+	}
+
 	@Override
 	public String toString() {
 		return MoreObjects.toStringHelper(this).add("id", getId()).add("type", getType()).add("rotation", getRotation())

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

@@ -3,6 +3,7 @@ package org.apollo.game.model.entity;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 
 import org.apollo.game.action.Action;
 import org.apollo.game.model.Animation;
@@ -11,7 +12,6 @@ import org.apollo.game.model.Graphic;
 import org.apollo.game.model.Position;
 import org.apollo.game.model.World;
 import org.apollo.game.model.area.Sector;
-import org.apollo.game.model.area.SectorCoordinates;
 import org.apollo.game.model.area.SectorRepository;
 import org.apollo.game.model.def.NpcDefinition;
 import org.apollo.game.model.entity.attr.Attribute;
@@ -20,6 +20,7 @@ import org.apollo.game.model.inv.Inventory;
 import org.apollo.game.model.inv.Inventory.StackMode;
 import org.apollo.game.model.inv.InventoryConstants;
 import org.apollo.game.scheduling.impl.SkillNormalizationTask;
+import org.apollo.game.sync.block.InteractingMobBlock;
 import org.apollo.game.sync.block.SynchronizationBlock;
 import org.apollo.game.sync.block.SynchronizationBlockSet;
 
@@ -31,11 +32,6 @@ import org.apollo.game.sync.block.SynchronizationBlockSet;
  */
 public abstract class Mob extends Entity {
 
-	/**
-	 * This mob's current action.
-	 */
-	private transient Action<?> action;
-
 	/**
 	 * The attribute map of this mob.
 	 */
@@ -49,23 +45,13 @@ public abstract class Mob extends Entity {
 	/**
 	 * This mob's npc definition. A player only uses this if they are appearing as an npc.
 	 */
-	protected NpcDefinition definition;
+	protected Optional<NpcDefinition> definition;
 
 	/**
 	 * This mob's equipment.
 	 */
 	protected final Inventory equipment = new Inventory(InventoryConstants.EQUIPMENT_CAPACITY, StackMode.STACK_ALWAYS);
 
-	/**
-	 * The position this mob is facing towards.
-	 */
-	private transient Position facingPosition = position;
-
-	/**
-	 * This mob's first movement direction.
-	 */
-	private transient Direction firstDirection = Direction.NONE;
-
 	/**
 	 * The index of this mob.
 	 */
@@ -81,6 +67,31 @@ public abstract class Mob extends Entity {
 	 */
 	protected final Inventory inventory = new Inventory(InventoryConstants.INVENTORY_CAPACITY);
 
+	/**
+	 * This mob's skill set.
+	 */
+	protected final SkillSet skillSet = new SkillSet();
+
+	/**
+	 * This mob's walking queue.
+	 */
+	protected final transient WalkingQueue walkingQueue = new WalkingQueue(this);
+
+	/**
+	 * This mob's current action.
+	 */
+	private transient Action<?> action;
+
+	/**
+	 * The position this mob is facing towards.
+	 */
+	private transient Position facingPosition = position;
+
+	/**
+	 * This mob's first movement direction.
+	 */
+	private transient Direction firstDirection = Direction.NONE;
+
 	/**
 	 * This mob's list of local npcs.
 	 */
@@ -96,28 +107,30 @@ public abstract class Mob extends Entity {
 	 */
 	private transient Direction secondDirection = Direction.NONE;
 
-	/**
-	 * This mob's skill set.
-	 */
-	protected final SkillSet skillSet = new SkillSet();
-
 	/**
 	 * Indicates whether this mob is currently teleporting or not.
 	 */
 	private transient boolean teleporting = false;
 
 	/**
-	 * This mob's walking queue.
+	 * Creates the mob with the specified initial {@link Position}.
-	 */
-	protected final transient WalkingQueue walkingQueue = new WalkingQueue(this);
-
-	/**
-	 * Creates a new mob with the specified initial {@link Position}.
 	 *
-	 * @param position The initial position.
+	 * @param position The position.
 	 */
 	public Mob(Position position) {
+		this(position, null);
+	}
+
+	/**
+	 * Creates the mob.
+	 * 
+	 * @param position The initial position.
+	 * @param definition The {@link NpcDefinition}.
+	 */
+	public Mob(Position position, NpcDefinition definition) {
 		super(position);
+		this.definition = Optional.ofNullable(definition);
+
 		init();
 	}
 
@@ -170,7 +183,7 @@ public abstract class Mob extends Entity {
 	 * @return The npc definition.
 	 */
 	public final NpcDefinition getDefinition() {
-		return definition;
+		return definition.get();
 	}
 
 	/**
@@ -183,6 +196,7 @@ public abstract class Mob extends Entity {
 			return secondDirection == Direction.NONE ? new Direction[] { firstDirection } : new Direction[] { firstDirection,
 					secondDirection };
 		}
+
 		return Direction.EMPTY_DIRECTION_ARRAY;
 	}
 
@@ -288,10 +302,12 @@ public abstract class Mob extends Entity {
 	}
 
 	/**
-	 * Initialises this mob.
+	 * Returns whether or not this mob has an {@link NpcDefinition}.
+	 * 
+	 * @return {@code true} if this mob has an npc definition, {@code false} if not.
 	 */
-	private void init() {
+	public final boolean hasNpcDefinition() {
-		World.getWorld().schedule(new SkillNormalizationTask(this));
+		return definition.isPresent();
 	}
 
 	/**
@@ -342,7 +358,7 @@ public abstract class Mob extends Entity {
 	 */
 	public final void resetInteractingMob() {
 		interactingMob = null;
-		blockSet.add(SynchronizationBlock.createInteractingMobBlock(65535));
+		blockSet.add(SynchronizationBlock.createInteractingMobBlock(InteractingMobBlock.RESET_INDEX));
 	}
 
 	/**
@@ -358,10 +374,11 @@ public abstract class Mob extends Entity {
 	/**
 	 * Sets this mob's {@link NpcDefinition}.
 	 * 
-	 * @param definition The definition.
+	 * @param definition The definition. Must not be {@code null}.
+	 * @throws NullPointerException If the specified definition is {@code null}.
 	 */
 	public final void setDefinition(NpcDefinition definition) {
-		this.definition = definition;
+		this.definition = Optional.of(definition);
 	}
 
 	/**
@@ -403,17 +420,14 @@ public abstract class Mob extends Entity {
 	 */
 	public final void setPosition(Position position) {
 		SectorRepository repository = World.getWorld().getSectorRepository();
-		Sector newSector = repository.fromPosition(position);
+		Sector current = repository.fromPosition(this.position);
 
-		if (SectorCoordinates.fromPosition(this.position) != SectorCoordinates.fromPosition(position)) {
+		Sector next = position.inside(current) ? current : repository.fromPosition(position);
-			Sector oldSector = repository.fromPosition(this.position);
-			oldSector.removeEntity(this);
-		} else {
-			newSector.removeEntity(this);
-		}
 
-		this.position = position;
+		current.removeEntity(this);
-		newSector.addEntity(this);
+		this.position = position; // addEntity relies on the position being updated, so do that first.
+
+		next.addEntity(this);
 	}
 
 	/**
@@ -446,6 +460,7 @@ public abstract class Mob extends Entity {
 			if (this.action.equals(action)) {
 				return false;
 			}
+
 			stopAction();
 		}
 
@@ -500,4 +515,11 @@ public abstract class Mob extends Entity {
 		blockSet.add(SynchronizationBlock.createTurnToPositionBlock(position));
 	}
 
+	/**
+	 * Initialises this mob.
+	 */
+	private void init() {
+		World.getWorld().schedule(new SkillNormalizationTask(this));
+	}
+
 }

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

@@ -1,6 +1,11 @@
 package org.apollo.game.model.entity;
 
+import java.util.Arrays;
+import java.util.Optional;
+
 import org.apollo.game.model.Position;
+import org.apollo.game.model.World;
+import org.apollo.game.model.area.Sector;
 import org.apollo.game.model.def.NpcDefinition;
 import org.apollo.game.sync.block.SynchronizationBlock;
 
@@ -14,11 +19,6 @@ import com.google.common.base.Preconditions;
  */
 public final class Npc extends Mob {
 
-	/**
-	 * This npc's id.
-	 */
-	private int id;
-
 	/**
 	 * The positions representing the bounds (i.e. walking limits) of this npc.
 	 */
@@ -31,19 +31,29 @@ public final class Npc extends Mob {
 	 * @param position The position.
 	 */
 	public Npc(int id, Position position) {
-		this(NpcDefinition.lookup(id), position);
+		this(position, NpcDefinition.lookup(id));
 	}
 
 	/**
 	 * Creates a new npc with the specified {@link NpcDefinition} and {@link Position}.
 	 * 
-	 * @param definition The definition.
 	 * @param position The position.
+	 * @param definition The definition.
 	 */
-	public Npc(NpcDefinition definition, Position position) {
+	public Npc(Position position, NpcDefinition definition) {
-		super(position);
+		super(position, definition);
-		this.definition = definition;
+
-		this.id = definition.getId();
+		init();
+	}
+
+	@Override
+	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 false;
 	}
 
 	/**
@@ -52,7 +62,7 @@ public final class Npc extends Mob {
 	 * @return The boundary.
 	 */
 	public Position[] getBoundary() {
-		return boundary;
+		return boundary.clone();
 	}
 
 	@Override
@@ -66,7 +76,14 @@ public final class Npc extends Mob {
 	 * @return The id.
 	 */
 	public int getId() {
-		return id;
+		return definition.get().getId();
+	}
+
+	@Override
+	public int hashCode() {
+		final int prime = 31;
+		int result = prime * position.hashCode() + Arrays.hashCode(boundary);
+		return prime * result + getId();
 	}
 
 	/**
@@ -85,12 +102,12 @@ public final class Npc extends Mob {
 	 */
 	public void setBoundary(Position[] boundary) {
 		Preconditions.checkArgument(boundary.length == 4, "Boundary count must be 4.");
-		this.boundary = boundary;
+		this.boundary = boundary.clone();
 	}
 
 	@Override
 	public String toString() {
-		return MoreObjects.toStringHelper(this).add("id", definition.getId()).add("name", definition.getName()).toString();
+		return MoreObjects.toStringHelper(this).add("id", getId()).add("name", definition.get().getName()).toString();
 	}
 
 	/**
@@ -99,9 +116,20 @@ public final class Npc extends Mob {
 	 * @param id The id.
 	 */
 	public void transform(int id) {
-		Preconditions.checkArgument(id >= 0 && id < NpcDefinition.count(), "Id to transform to is out of bounds.");
+		Preconditions.checkElementIndex(id, NpcDefinition.count(), "Id to transform to is out of bounds.");
-		definition = NpcDefinition.lookup(this.id = id);
+
+		definition = Optional.of(NpcDefinition.lookup(id));
 		blockSet.add(SynchronizationBlock.createTransformBlock(id));
 	}
 
+	/**
+	 * 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
+		// 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);
+	}
+
 }

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

@@ -18,6 +18,7 @@ import org.apollo.game.message.impl.UpdateRunEnergyMessage;
 import org.apollo.game.model.Appearance;
 import org.apollo.game.model.Position;
 import org.apollo.game.model.World;
+import org.apollo.game.model.area.Sector;
 import org.apollo.game.model.inter.InterfaceConstants;
 import org.apollo.game.model.inter.InterfaceListener;
 import org.apollo.game.model.inter.InterfaceSet;
@@ -82,6 +83,21 @@ public final class Player extends Mob {
 	 */
 	private PlayerCredentials credentials;
 
+	@Override
+	public int hashCode() {
+		return credentials.hashCode();
+	}
+
+	@Override
+	public boolean equals(Object obj) {
+		if (obj instanceof Player) {
+			Player other = (Player) obj;
+			return credentials.equals(other.credentials);
+		}
+
+		return false;
+	}
+
 	/**
 	 * A flag which indicates there are npcs that couldn't be added.
 	 */
@@ -206,6 +222,7 @@ public final class Player extends Mob {
 	public Player(PlayerCredentials credentials, Position position) {
 		super(position);
 		this.credentials = credentials;
+
 		init();
 	}
 
@@ -923,6 +940,11 @@ public final class Player extends Mob {
 	private void init() {
 		initInventories();
 		initSkills();
+
+		// This has to be here instead of in Mob#init because of ordering issues - the player 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);
 	}
 
 	/**

src/org/apollo/game/sync/block/InteractingMobBlock.java

@@ -7,18 +7,23 @@ package org.apollo.game.sync.block;
  */
 public final class InteractingMobBlock extends SynchronizationBlock {
 
+	/**
+	 * The index used to reset the interacting mob.
+	 */
+	public static final int RESET_INDEX = 65_535;
+
 	/**
 	 * The index of the mob.
 	 */
-	private final int mobIndex;
+	private final int index;
 
 	/**
 	 * Creates the interacting mob block.
 	 * 
-	 * @param mobIndex The index of the current interacting mob.
+	 * @param index The index of the current interacting mob.
 	 */
-	InteractingMobBlock(int mobIndex) {
+	InteractingMobBlock(int index) {
-		this.mobIndex = mobIndex;
+		this.index = index;
 	}
 
 	/**
@@ -26,8 +31,8 @@ public final class InteractingMobBlock extends SynchronizationBlock {
 	 * 
 	 * @return The index.
 	 */
-	public int getInteractingMobIndex() {
+	public int getIndex() {
-		return mobIndex;
+		return index;
 	}
 
 }

src/org/apollo/game/sync/block/SynchronizationBlock.java

@@ -34,9 +34,11 @@ public abstract class SynchronizationBlock {
 	 * @return The appearance block.
 	 */
 	public static SynchronizationBlock createAppearanceBlock(Player player) {
-		return new AppearanceBlock(player.getEncodedName(), player.getAppearance(), player.getSkillSet().getCombatLevel(), 0,
+		int combat = player.getSkillSet().getCombatLevel();
-				player.getEquipment(), player.getPrayerIcon(), player.isSkulled(), player.getDefinition() == null ? -1 : player
+		int id = player.hasNpcDefinition() ? player.getDefinition().getId() : -1;
-						.getDefinition().getId());
+
+		return new AppearanceBlock(player.getEncodedName(), player.getAppearance(), combat, 0, player.getEquipment(),
+				player.getPrayerIcon(), player.isSkulled(), id);
 	}
 
 	/**

src/org/apollo/game/sync/task/PhasedSynchronizationTask.java

@@ -38,9 +38,12 @@ public final class PhasedSynchronizationTask extends SynchronizationTask {
 	public void run() {
 		try {
 			task.run();
+		} catch (Exception e) { // TODO better solution...
+			e.printStackTrace();
+			// The executor suppresses any exceptions thrown as part of the task, so we catch and print here as
+			// rethrowing them does nothing.
 		} finally {
 			phaser.arriveAndDeregister();
 		}
 	}
-
 }

src/org/apollo/game/sync/task/PrePlayerSynchronizationTask.java

@@ -53,7 +53,6 @@ public final class PrePlayerSynchronizationTask extends SynchronizationTask {
 
 			Position position = player.getPosition();
 			player.setLastKnownSector(position);
-
 			player.send(new SectorChangeMessage(position));
 		}
 	}

src/org/apollo/net/release/r317/MouseClickMessageDecoder.java

@@ -19,19 +19,21 @@ public final class MouseClickMessageDecoder extends MessageDecoder<MouseClickMes
 		int read;
 		if (reader.getLength() == 2) {
 			read = (int) reader.getUnsigned(DataType.SHORT);
-			int clickCount = (read >> 12);
+			int clicks = (read >> 12);
 			int dX = (read >> 6) & 0x3f;
 			int dY = read & 0x3f;
-			return new MouseClickMessage(clickCount, dX, dY, true);
+			return new MouseClickMessage(clicks, dX, dY, true);
 		} else if (reader.getLength() == 3) {
 			read = (int) reader.getUnsigned(DataType.TRI_BYTE) & ~0x800000;
 		} else {
 			read = (int) reader.getUnsigned(DataType.INT) & ~0xc0000000;
 		}
-		int clickCount = (read >> 19);
+
+		int clicks = (read >> 19);
 		int x = (read & 0x7f) % 765;
 		int y = (read & 0x7f) / 765;
-		return new MouseClickMessage(clickCount, x, y, false);
+
+		return new MouseClickMessage(clicks, x, y, false);
 	}
 
 }

src/org/apollo/net/release/r317/NpcSynchronizationMessageEncoder.java

@@ -217,7 +217,7 @@ public final class NpcSynchronizationMessageEncoder extends MessageEncoder<NpcSy
 	 * @param builder The builder.
 	 */
 	private static void putInteractingMobBlock(InteractingMobBlock block, GamePacketBuilder builder) {
-		builder.put(DataType.SHORT, block.getInteractingMobIndex());
+		builder.put(DataType.SHORT, block.getIndex());
 	}
 
 	/**

src/org/apollo/net/release/r317/PlayerSynchronizationMessageEncoder.java

@@ -378,7 +378,7 @@ public final class PlayerSynchronizationMessageEncoder extends MessageEncoder<Pl
 	 * @param builder The builder.
 	 */
 	private static void putInteractingMobBlock(InteractingMobBlock block, GamePacketBuilder builder) {
-		builder.put(DataType.SHORT, DataOrder.LITTLE, block.getInteractingMobIndex());
+		builder.put(DataType.SHORT, DataOrder.LITTLE, block.getIndex());
 	}
 
 	/**

src/org/apollo/net/release/r317/Release317.java

@@ -19,8 +19,8 @@ import org.apollo.game.message.impl.OpenInterfaceSidebarMessage;
 import org.apollo.game.message.impl.PlayerSynchronizationMessage;
 import org.apollo.game.message.impl.PositionMessage;
 import org.apollo.game.message.impl.PrivacyOptionMessage;
-import org.apollo.game.message.impl.SectorChangeMessage;
 import org.apollo.game.message.impl.RemoveTileItemMessage;
+import org.apollo.game.message.impl.SectorChangeMessage;
 import org.apollo.game.message.impl.SendFriendMessage;
 import org.apollo.game.message.impl.SendObjectMessage;
 import org.apollo.game.message.impl.ServerChatMessage;
@@ -128,7 +128,7 @@ public final class Release317 extends Release {
 		register(237, new MagicOnItemMessageDecoder());
 
 		register(3, new FocusUpdateMessageDecoder());
-		register(241, new MouseClickMessageDecoder());
+		register(45, new MouseClickMessageDecoder());
 		register(86, new ArrowKeyMessageDecoder());
 		register(95, new PrivacyOptionMessageDecoder());
 

src/org/apollo/net/release/r377/MouseClickMessageDecoder.java

@@ -16,23 +16,24 @@ public final class MouseClickMessageDecoder extends MessageDecoder<MouseClickMes
 	@Override
 	public MouseClickMessage decode(GamePacket packet) {
 		GamePacketReader reader = new GamePacketReader(packet);
-		int read, clickCount, x, y;
+		int read, clicks, x, y;
 
 		if (reader.getLength() == 2) {
 			read = (int) reader.getUnsigned(DataType.SHORT);
-			clickCount = (read >> 12);
+			clicks = (read >> 12);
 			x = (read >> 6) & 0x3f;
 			y = read & 0x3f;
-			return new MouseClickMessage(clickCount, x, y, true);
+			return new MouseClickMessage(clicks, x, y, true);
 		} else if (reader.getLength() == 3) {
 			read = (int) reader.getUnsigned(DataType.TRI_BYTE) & ~0x800000;
 		} else {
 			read = (int) reader.getUnsigned(DataType.INT) & ~0xc0000000;
 		}
-		clickCount = (read >> 19);
+		
+		clicks = (read >> 19);
 		x = (read & 0x7f) % 765;
 		y = (read & 0x7f) / 765;
-		return new MouseClickMessage(clickCount, x, y, false);
+		return new MouseClickMessage(clicks, x, y, false);
 	}
 
 }

src/org/apollo/net/release/r377/NpcSynchronizationMessageEncoder.java

@@ -217,7 +217,7 @@ public final class NpcSynchronizationMessageEncoder extends MessageEncoder<NpcSy
 	 * @param builder The builder.
 	 */
 	private static void putInteractingMobBlock(InteractingMobBlock block, GamePacketBuilder builder) {
-		builder.put(DataType.SHORT, DataOrder.LITTLE, block.getInteractingMobIndex());
+		builder.put(DataType.SHORT, DataOrder.LITTLE, block.getIndex());
 	}
 
 	/**

src/org/apollo/net/release/r377/PlayerSynchronizationMessageEncoder.java

@@ -378,7 +378,7 @@ public final class PlayerSynchronizationMessageEncoder extends MessageEncoder<Pl
 	 * @param builder The builder.
 	 */
 	private static void putInteractingMobBlock(InteractingMobBlock block, GamePacketBuilder builder) {
-		builder.put(DataType.SHORT, DataTransformation.ADD, block.getInteractingMobIndex());
+		builder.put(DataType.SHORT, DataTransformation.ADD, block.getIndex());
 	}
 
 	/**

src/org/apollo/security/PlayerCredentials.java

@@ -106,19 +106,17 @@ public final class PlayerCredentials {
 
 	@Override
 	public int hashCode() {
-		return (int) encodedUsername;
+		return Long.hashCode(encodedUsername);
 	}
 
 	@Override
 	public boolean equals(Object obj) {
-		if (this == obj) {
+		if (obj instanceof PlayerCredentials) {
-			return true;
+			PlayerCredentials other = (PlayerCredentials) obj;
-		} else if (obj == null || getClass() != obj.getClass()) {
+			return encodedUsername == other.encodedUsername;
-			return false;
 		}
 
-		PlayerCredentials other = (PlayerCredentials) obj;
+		return false;
-		return encodedUsername == other.encodedUsername;
 	}
 
 }