Merge branch 'master' of git@bitbucket.org:Major-/apollo.git.

data/login.xml

@@ -1,4 +1,3 @@
 <login>
-	<loader>org.apollo.io.player.impl.DummyPlayerLoader</loader>
-	<saver>org.apollo.io.player.impl.DiscardPlayerSaver</saver>
+	<serializer>org.apollo.io.player.DummyPlayerSerializer</serializer>
 </login>

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/fs/FileSystemConstants.java

@@ -27,11 +27,6 @@ public final class FileSystemConstants {
 	 */
 	public static final int BLOCK_SIZE = HEADER_SIZE + CHUNK_SIZE;
 
-	/**
-	 * The number of caches.
-	 */
-	public static final int CACHE_COUNT = 5;
-
 	/**
 	 * The size of an index.
 	 */

src/org/apollo/fs/decoder/NpcDefinitionDecoder.java

@@ -122,13 +122,13 @@ public final class NpcDefinitionDecoder {
 				buffer.getShort();
 			} else if (opcode == 106) {
 				@SuppressWarnings("unused")
-				int morphVariableBitsIndex = wrapMorphism(buffer.getShort());
+				int morphVariableBitsIndex = wrap(buffer.getShort());
 				@SuppressWarnings("unused")
-				int morphismCount = wrapMorphism(buffer.getShort());
+				int morphismCount = wrap(buffer.getShort());
 
 				int count = buffer.get() & 0xFF;
 				int[] morphisms = new int[count + 1];
-				Arrays.setAll(morphisms, index -> wrapMorphism(buffer.getShort()));
+				Arrays.setAll(morphisms, index -> wrap(buffer.getShort()));
 			} else if (opcode == 107) {
 				@SuppressWarnings("unused")
 				boolean clickable = false;
@@ -137,12 +137,12 @@ public final class NpcDefinitionDecoder {
 	}
 
 	/**
-	 * Wraps a morphism value around, returning -1 if the specified value is 65,535. TODO name
+	 * Wraps a morphism value around, returning -1 if the specified value is 65,535.
 	 * 
 	 * @param value The value.
 	 * @return -1 if {@code value} is 65,535, otherwise {@code value}.
 	 */
-	private static int wrapMorphism(int value) {
+	private static int wrap(int value) {
 		return value == 65_535 ? -1 : value;
 	}
 

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

@@ -221,7 +221,7 @@ public final class Position {
 	public boolean isWithinDistance(Position other, int distance) {
 		int deltaX = Math.abs(getX() - other.getX());
 		int deltaY = Math.abs(getY() - other.getY());
-		return deltaX <= distance && deltaY <= distance;
+		return deltaX <= distance && deltaY <= distance && getHeight() == other.getHeight();
 	}
 
 	@Override

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.
 	 */
@@ -113,7 +119,7 @@ public final class World {
 	private final Map<Long, Player> players = new HashMap<>();
 
 	/**
-	 * The {@link PluginManager}. TODO: better place than here!!
+	 * The {@link PluginManager}.
 	 */
 	private PluginManager pluginManager;
 
@@ -178,7 +184,7 @@ public final class World {
 	}
 
 	/**
-	 * Gets the plugin manager. TODO should this be here?
+	 * Gets the plugin manager.
 	 * 
 	 * @return The plugin manager.
 	 */
@@ -242,8 +248,11 @@ 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!!
+		pluginManager = manager;
 	}
 
 	/**
@@ -285,6 +294,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/CollisionFlag.java

@@ -9,16 +9,16 @@ import org.apollo.game.model.entity.Entity.EntityType;
  */
 public enum CollisionFlag {
 
-	/**
-	 * The walk east flag.
-	 */
-	MOB_EAST(1),
-
 	/**
 	 * The walk north flag.
 	 */
 	MOB_NORTH(0),
 
+	/**
+	 * The walk east flag.
+	 */
+	MOB_EAST(1),
+
 	/**
 	 * The walk south flag.
 	 */
@@ -29,16 +29,16 @@ public enum CollisionFlag {
 	 */
 	MOB_WEST(3),
 
-	/**
-	 * The projectile east flag.
-	 */
-	PROJECTILE_EAST(5),
-
 	/**
 	 * The projectile north flag.
 	 */
 	PROJECTILE_NORTH(4),
 
+	/**
+	 * The projectile east flag.
+	 */
+	PROJECTILE_EAST(5),
+
 	/**
 	 * The projectile south flag.
 	 */

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

@@ -165,16 +165,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/def/ItemDefinition.java

@@ -388,7 +388,7 @@ public final class ItemDefinition {
 	public void toNote() {
 		if (isNote()) {
 			if (description != null && description.startsWith("Swap this note at any bank for ")) {
-				return; // already converted TODO better way of checking?
+				return; // already converted.
 			}
 
 			ItemDefinition infoDef = lookup(noteInfoId);

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/Player.java

@@ -686,7 +686,7 @@ public final class Player extends Mob {
 	 */
 	public void sendInitialMessages() {
 		blockSet.add(SynchronizationBlock.createAppearanceBlock(this));
-		send(new IdAssignmentMessage(index, members)); // TODO should this be sent when we reconnect?
+		send(new IdAssignmentMessage(index, members));
 		sendMessage("Welcome to RuneScape.");
 
 		int[] tabs = InterfaceConstants.DEFAULT_INVENTORY_TABS;

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/model/inv/Inventory.java

@@ -51,7 +51,7 @@ public final class Inventory {
 	/**
 	 * A flag indicating if events are being fired.
 	 */
-	private boolean firingEvents = true; // TODO: make this reentrant
+	private boolean firingEvents = true;
 
 	/**
 	 * The items in this inventory.

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

@@ -0,0 +1,100 @@
+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);
+	}
+
+	@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);
+		}
+	}
+
+}

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

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

src/org/apollo/io/player/BinaryFileUtils.java

@@ -0,0 +1,56 @@
+package org.apollo.io.player;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+import org.apollo.util.NameUtil;
+
+/**
+ * A utility class with common functionality used by the binary player loader/ savers.
+ * 
+ * @author Graham
+ * @author Major
+ */
+public final class BinaryFileUtils {
+
+	/**
+	 * The Path to the saved games directory.
+	 */
+	private static final Path SAVED_GAMES_DIRECTORY = Paths.get("data/savedGames");
+
+	/**
+	 * Creates the saved games directory if it does not exist.
+	 */
+	static {
+		try {
+			if (!Files.exists(SAVED_GAMES_DIRECTORY)) {
+				Files.createDirectory(SAVED_GAMES_DIRECTORY);
+			}
+		} catch (IOException e) {
+			throw new UncheckedIOException("Error creating saved games directory.", e);
+		}
+	}
+
+	/**
+	 * Gets the save {@link File} for the specified player.
+	 * 
+	 * @param username The username of the player.
+	 * @return The file.
+	 */
+	public static Path getFile(String username) {
+		String filtered = NameUtil.decodeBase37(NameUtil.encodeBase37(username));
+		return SAVED_GAMES_DIRECTORY.resolve(filtered + ".dat");
+	}
+
+	/**
+	 * Sole private constructor to prevent instantiation.
+	 */
+	private BinaryFileUtils() {
+
+	}
+
+}

src/org/apollo/io/player/BinaryPlayerSerializer.java

@@ -1,13 +1,20 @@
-package org.apollo.io.player.impl;
+package org.apollo.io.player;
 
+import java.io.BufferedInputStream;
 import java.io.DataInputStream;
+import java.io.DataOutputStream;
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Set;
 
 import org.apollo.game.model.Appearance;
 import org.apollo.game.model.Item;
@@ -16,6 +23,8 @@ import org.apollo.game.model.entity.Player;
 import org.apollo.game.model.entity.Skill;
 import org.apollo.game.model.entity.SkillSet;
 import org.apollo.game.model.entity.attr.Attribute;
+import org.apollo.game.model.entity.attr.AttributeMap;
+import org.apollo.game.model.entity.attr.AttributePersistence;
 import org.apollo.game.model.entity.attr.AttributeType;
 import org.apollo.game.model.entity.attr.BooleanAttribute;
 import org.apollo.game.model.entity.attr.NumericalAttribute;
@@ -26,8 +35,6 @@ import org.apollo.game.model.entity.setting.PrivacyState;
 import org.apollo.game.model.entity.setting.PrivilegeLevel;
 import org.apollo.game.model.entity.setting.ScreenBrightness;
 import org.apollo.game.model.inv.Inventory;
-import org.apollo.io.player.PlayerLoader;
-import org.apollo.io.player.PlayerLoaderResponse;
 import org.apollo.net.codec.login.LoginConstants;
 import org.apollo.security.PlayerCredentials;
 import org.apollo.util.NameUtil;
@@ -36,28 +43,39 @@ import org.apollo.util.StreamUtil;
 import com.lambdaworks.crypto.SCryptUtil;
 
 /**
- * A {@link PlayerLoader} implementation that loads data from a binary file.
+ * A {@link PlayerSerializer} implementation that uses a binary file to store player data.
  * 
  * @author Graham
+ * @author Major
  */
-public final class BinaryPlayerLoader implements PlayerLoader {
+public final class BinaryPlayerSerializer implements PlayerSerializer {
 
 	/**
-	 * The default spawn position.
+	 * The Path to the saved games directory.
 	 */
-	private static final Position SPAWN_POSITION = new Position(3093, 3104);
+	private static final Path SAVED_GAMES_DIRECTORY = Paths.get("data/savedGames");
+
+	static {
+		try {
+			if (!Files.exists(SAVED_GAMES_DIRECTORY)) {
+				Files.createDirectory(SAVED_GAMES_DIRECTORY);
+			}
+		} catch (IOException e) {
+			throw new UncheckedIOException("Error creating saved games directory.", e);
+		}
+	}
 
 	@Override
 	public PlayerLoaderResponse loadPlayer(PlayerCredentials credentials) throws IOException {
-		File file = BinaryPlayerUtil.getFile(credentials.getUsername());
-		if (!file.exists()) {
-			Player player = new Player(credentials, SPAWN_POSITION);
-			player.getBank().add(995, 25); // 25 coins
+		Path path = getFile(credentials.getUsername());
+		if (!Files.exists(path)) {
+			Player player = new Player(credentials, TUTORIAL_ISLAND_SPAWN);
+
 			credentials.setPassword(SCryptUtil.scrypt(credentials.getPassword(), 16384, 8, 1));
 			return new PlayerLoaderResponse(LoginConstants.STATUS_OK, player);
 		}
 
-		try (DataInputStream in = new DataInputStream(new FileInputStream(file))) {
+		try (DataInputStream in = new DataInputStream(new BufferedInputStream(Files.newInputStream(path)))) {
 			String name = StreamUtil.readString(in);
 			String password = StreamUtil.readString(in);
 
@@ -67,37 +85,35 @@ public final class BinaryPlayerLoader implements PlayerLoader {
 
 			credentials.setPassword(password); // Update password to the hashed one.
 
-			PrivilegeLevel privilegeLevel = PrivilegeLevel.valueOf(in.readByte());
+			PrivilegeLevel privilege = PrivilegeLevel.valueOf(in.readByte());
 			MembershipStatus members = MembershipStatus.valueOf(in.readByte());
 
 			PrivacyState chatPrivacy = PrivacyState.valueOf(in.readByte(), true);
 			PrivacyState friendPrivacy = PrivacyState.valueOf(in.readByte(), false);
 			PrivacyState tradePrivacy = PrivacyState.valueOf(in.readByte(), false);
-			int runEnergy = in.readByte();
 			ScreenBrightness brightness = ScreenBrightness.valueOf(in.readByte());
 
 			int x = in.readUnsignedShort();
 			int y = in.readUnsignedShort();
 			int height = in.readUnsignedByte();
 
-			int genderIntValue = in.readUnsignedByte();
-			Gender gender = genderIntValue == Gender.MALE.toInteger() ? Gender.MALE : Gender.FEMALE;
+			Gender gender = (in.readUnsignedByte() == Gender.MALE.toInteger()) ? Gender.MALE : Gender.FEMALE;
 			int[] style = new int[7];
-			for (int i = 0; i < style.length; i++) {
-				style[i] = in.readUnsignedByte();
+			for (int slot = 0; slot < style.length; slot++) {
+				style[slot] = in.readUnsignedByte();
 			}
+			
 			int[] colors = new int[5];
-			for (int i = 0; i < colors.length; i++) {
-				colors[i] = in.readUnsignedByte();
+			for (int slot = 0; slot < colors.length; slot++) {
+				colors[slot] = in.readUnsignedByte();
 			}
 
 			Player player = new Player(credentials, new Position(x, y, height));
-			player.setPrivilegeLevel(privilegeLevel);
+			player.setPrivilegeLevel(privilege);
 			player.setMembers(members);
 			player.setChatPrivacy(chatPrivacy);
 			player.setFriendPrivacy(friendPrivacy);
 			player.setTradePrivacy(tradePrivacy);
-			player.setRunEnergy(runEnergy);
 			player.setScreenBrightness(brightness);
 
 			player.setAppearance(new Appearance(gender, style, colors));
@@ -141,6 +157,87 @@ public final class BinaryPlayerLoader implements PlayerLoader {
 		}
 	}
 
+	@Override
+	public void savePlayer(Player player) throws IOException {
+		Path file = getFile(player.getUsername());
+
+		try (DataOutputStream out = new DataOutputStream(Files.newOutputStream(file))) {
+			StreamUtil.writeString(out, player.getUsername());
+			StreamUtil.writeString(out, player.getCredentials().getPassword());
+			out.writeByte(player.getPrivilegeLevel().toInteger());
+			out.writeByte(player.getMembershipStatus().getValue());
+
+			out.writeByte(player.getChatPrivacy().toInteger(true));
+			out.writeByte(player.getFriendPrivacy().toInteger(false));
+			out.writeByte(player.getTradePrivacy().toInteger(false));
+			out.writeByte(player.getScreenBrightness().toInteger());
+
+			Position position = player.getPosition();
+			out.writeShort(position.getX());
+			out.writeShort(position.getY());
+			out.writeByte(position.getHeight());
+
+			Appearance appearance = player.getAppearance();
+			out.writeByte(appearance.getGender().toInteger());
+			int[] style = appearance.getStyle();
+			for (int element : style) {
+				out.writeByte(element);
+			}
+			int[] colors = appearance.getColors();
+			for (int color : colors) {
+				out.writeByte(color);
+			}
+
+			writeInventory(out, player.getInventory());
+			writeInventory(out, player.getEquipment());
+			writeInventory(out, player.getBank());
+
+			SkillSet skills = player.getSkillSet();
+			out.writeByte(skills.size());
+			for (int id = 0; id < skills.size(); id++) {
+				Skill skill = skills.getSkill(id);
+				out.writeByte(skill.getCurrentLevel());
+				out.writeDouble(skill.getExperience());
+			}
+
+			List<String> usernames = player.getFriendUsernames();
+			out.writeByte(usernames.size());
+			for (String username : usernames) {
+				out.writeLong(NameUtil.encodeBase37(username));
+			}
+
+			usernames = player.getIgnoredUsernames();
+			out.writeByte(usernames.size());
+			for (String username : usernames) {
+				out.writeLong(NameUtil.encodeBase37(username));
+			}
+
+			Set<Entry<String, Attribute<?>>> attributes = player.getAttributes().entrySet();
+			attributes.removeIf(e -> AttributeMap.getDefinition(e.getKey()).getPersistence() != AttributePersistence.PERSISTENT);
+			out.writeInt(attributes.size());
+
+			for (Entry<String, Attribute<?>> entry : attributes) {
+				String name = entry.getKey();
+				StreamUtil.writeString(out, name);
+
+				Attribute<?> attribute = entry.getValue();
+				out.writeByte(attribute.getType().getValue());
+				out.write(attribute.encode());
+			}
+		}
+	}
+
+	/**
+	 * Gets the save {@link File} for the specified player.
+	 * 
+	 * @param username The username of the player.
+	 * @return The file.
+	 */
+	private Path getFile(String username) {
+		String filtered = NameUtil.decodeBase37(NameUtil.encodeBase37(username));
+		return SAVED_GAMES_DIRECTORY.resolve(filtered + ".dat");
+	}
+
 	/**
 	 * Reads the player's {@link Attribute}s.
 	 * 
@@ -148,7 +245,7 @@ public final class BinaryPlayerLoader implements PlayerLoader {
 	 * @return The {@link Map} of attribute names to attributes.
 	 * @throws IOException If there is an error reading from the stream.
 	 */
-	private static Map<String, Attribute<?>> readAttributes(DataInputStream in) throws IOException {
+	private Map<String, Attribute<?>> readAttributes(DataInputStream in) throws IOException {
 		int count = in.readInt();
 		Map<String, Attribute<?>> attributes = new HashMap<>(count);
 
@@ -187,7 +284,7 @@ public final class BinaryPlayerLoader implements PlayerLoader {
 	 * @param inventory The inventory.
 	 * @throws IOException If an I/O error occurs.
 	 */
-	private static void readInventory(DataInputStream in, Inventory inventory) throws IOException {
+	private void readInventory(DataInputStream in, Inventory inventory) throws IOException {
 		int capacity = in.readUnsignedShort();
 
 		inventory.stopFiringEvents();
@@ -206,4 +303,27 @@ public final class BinaryPlayerLoader implements PlayerLoader {
 		}
 	}
 
+	/**
+	 * Writes an inventory to the specified output stream.
+	 * 
+	 * @param out The output stream.
+	 * @param inventory The inventory.
+	 * @throws IOException If an I/O error occurs.
+	 */
+	private void writeInventory(DataOutputStream out, Inventory inventory) throws IOException {
+		int capacity = inventory.capacity();
+		out.writeShort(capacity);
+
+		for (int slot = 0; slot < capacity; slot++) {
+			Item item = inventory.get(slot);
+			if (item != null) {
+				out.writeShort(item.getId() + 1);
+				out.writeInt(item.getAmount());
+			} else {
+				out.writeShort(0);
+				out.writeInt(0);
+			}
+		}
+	}
+
 }

src/org/apollo/io/player/DummyPlayerSerializer.java

@@ -1,35 +1,33 @@
-package org.apollo.io.player.impl;
+package org.apollo.io.player;
 
-import org.apollo.game.model.Position;
 import org.apollo.game.model.entity.Player;
 import org.apollo.game.model.entity.setting.MembershipStatus;
 import org.apollo.game.model.entity.setting.PrivilegeLevel;
-import org.apollo.io.player.PlayerLoader;
-import org.apollo.io.player.PlayerLoaderResponse;
 import org.apollo.net.codec.login.LoginConstants;
 import org.apollo.security.PlayerCredentials;
 
 /**
- * A dummy {@link PlayerLoader} implementation used for testing purposes.
+ * A {@link PlayerSerializer} that saves no data and returns an administrator member account, ideal for debugging.
  * 
  * @author Graham
+ * @author Major
  */
-public final class DummyPlayerLoader implements PlayerLoader {
-
-	/**
-	 * The default spawn position for players loaded by this loader.
-	 */
-	private static final Position DEFAULT_POSITION = new Position(3093, 3104);
+public final class DummyPlayerSerializer implements PlayerSerializer {
 
 	@Override
 	public PlayerLoaderResponse loadPlayer(PlayerCredentials credentials) {
 		int status = LoginConstants.STATUS_OK;
 
-		Player player = new Player(credentials, DEFAULT_POSITION);
+		Player player = new Player(credentials, TUTORIAL_ISLAND_SPAWN);
 		player.setPrivilegeLevel(PrivilegeLevel.ADMINISTRATOR);
 		player.setMembers(MembershipStatus.PAID);
 
 		return new PlayerLoaderResponse(status, player);
 	}
 
+	@Override
+	public void savePlayer(Player player) {
+		/* discard player */
+	}
+
 }

src/org/apollo/io/player/JdbcPlayerSerializer.java

@@ -0,0 +1,23 @@
+package org.apollo.io.player;
+
+import org.apollo.game.model.entity.Player;
+import org.apollo.security.PlayerCredentials;
+
+/**
+ * A {@link PlayerSerializer} that utilises {@code JDBC} to communicate with an SQL database containing player data.
+ * 
+ * @author Major
+ */
+public final class JdbcPlayerSerializer implements PlayerSerializer {
+
+	@Override
+	public void savePlayer(Player player) throws Exception {
+		throw new UnsupportedOperationException("JDBC saving is not supported at this time.");
+	}
+
+	@Override
+	public PlayerLoaderResponse loadPlayer(PlayerCredentials credentials) throws Exception {
+		throw new UnsupportedOperationException("JDBC loading is not supported at this time.");
+	}
+
+}

src/org/apollo/io/player/PlayerLoader.java

@@ -1,22 +0,0 @@
-package org.apollo.io.player;
-
-import org.apollo.security.PlayerCredentials;
-
-/**
- * An interface which may be extended by others which are capable of loading players. For example, implementations might
- * include text-based, binary and SQL loaders.
- * 
- * @author Graham
- */
-public interface PlayerLoader {
-
-	/**
-	 * Loads a player.
-	 * 
-	 * @param credentials The player's credentials.
-	 * @return The {@link PlayerLoaderResponse}.
-	 * @throws Exception If an error occurs.
-	 */
-	public PlayerLoaderResponse loadPlayer(PlayerCredentials credentials) throws Exception;
-
-}

src/org/apollo/io/player/PlayerLoaderResponse.java

@@ -8,9 +8,10 @@ import org.apollo.net.codec.login.LoginConstants;
 import com.google.common.base.Preconditions;
 
 /**
- * A response for the {@link PlayerLoader#loadPlayer(org.apollo.security.PlayerCredentials)} call.
+ * A response for the {@link PlayerSerializer#loadPlayer} call.
  * 
  * @author Graham
+ * @author Major
  */
 public final class PlayerLoaderResponse {
 
@@ -32,7 +33,8 @@ public final class PlayerLoaderResponse {
 	 *             {@link LoginConstants#STATUS_RECONNECTION_OK}.
 	 */
 	public PlayerLoaderResponse(int status) {
-		Preconditions.checkArgument(status != LoginConstants.STATUS_OK && status != LoginConstants.STATUS_RECONNECTION_OK, "Player required for this status code.");
+		Preconditions.checkArgument(status != LoginConstants.STATUS_OK && status != LoginConstants.STATUS_RECONNECTION_OK,
+				"Player required for this status code.");
 		this.status = status;
 		player = Optional.empty();
 	}
@@ -46,7 +48,8 @@ public final class PlayerLoaderResponse {
 	 * @throws NullPointerException If the specified player is null.
 	 */
 	public PlayerLoaderResponse(int status, Player player) {
-		Preconditions.checkArgument(status == LoginConstants.STATUS_OK || status == LoginConstants.STATUS_RECONNECTION_OK, "Player not required for this status code.");
+		Preconditions.checkArgument(status == LoginConstants.STATUS_OK || status == LoginConstants.STATUS_RECONNECTION_OK,
+				"Player not required for this status code.");
 		this.status = status;
 		this.player = Optional.of(player);
 	}

src/org/apollo/io/player/PlayerSaver.java

@@ -1,21 +0,0 @@
-package org.apollo.io.player;
-
-import org.apollo.game.model.entity.Player;
-
-/**
- * An interface which may be implemented by others which are capable of saving players. For example, implementations
- * might include text-based, binary and SQL savers.
- * 
- * @author Graham
- */
-public interface PlayerSaver {
-
-	/**
-	 * Saves a player.
-	 * 
-	 * @param player The player to save.
-	 * @throws Exception If an error occurs.
-	 */
-	public void savePlayer(Player player) throws Exception;
-
-}

src/org/apollo/io/player/PlayerSerializer.java

@@ -0,0 +1,38 @@
+package org.apollo.io.player;
+
+import org.apollo.game.model.Position;
+import org.apollo.game.model.entity.Player;
+import org.apollo.security.PlayerCredentials;
+
+/**
+ * An interface which may be implemented by others which are capable of serializing and deserializing players. For
+ * example, implementations might include text-based, binary and SQL serializers.
+ * 
+ * @author Graham
+ * @author Major
+ */
+public interface PlayerSerializer {
+
+	/**
+	 * The spawn point for Players, on Tutorial Island.
+	 */
+	Position TUTORIAL_ISLAND_SPAWN = new Position(3093, 3104);
+
+	/**
+	 * Loads a {@link Player}.
+	 * 
+	 * @param credentials The {@link PlayerCredentials}.
+	 * @return The {@link PlayerLoaderResponse}.
+	 * @throws Exception If an error occurs.
+	 */
+	public PlayerLoaderResponse loadPlayer(PlayerCredentials credentials) throws Exception;
+
+	/**
+	 * Saves a {@link Player}.
+	 * 
+	 * @param player The Player to save.
+	 * @throws Exception If an error occurs.
+	 */
+	public void savePlayer(Player player) throws Exception;
+
+}

src/org/apollo/io/player/impl/BinaryPlayerSaver.java

@@ -1,126 +0,0 @@
-package org.apollo.io.player.impl;
-
-import java.io.DataOutputStream;
-import java.io.File;
-import java.io.FileOutputStream;
-import java.io.IOException;
-import java.util.List;
-import java.util.Map.Entry;
-import java.util.Set;
-
-import org.apollo.game.model.Appearance;
-import org.apollo.game.model.Item;
-import org.apollo.game.model.Position;
-import org.apollo.game.model.entity.Player;
-import org.apollo.game.model.entity.Skill;
-import org.apollo.game.model.entity.SkillSet;
-import org.apollo.game.model.entity.attr.Attribute;
-import org.apollo.game.model.entity.attr.AttributeMap;
-import org.apollo.game.model.entity.attr.AttributePersistence;
-import org.apollo.game.model.inv.Inventory;
-import org.apollo.io.player.PlayerSaver;
-import org.apollo.util.NameUtil;
-import org.apollo.util.StreamUtil;
-
-/**
- * A {@link PlayerSaver} implementation that saves player data to a binary file.
- * 
- * @author Graham
- */
-public final class BinaryPlayerSaver implements PlayerSaver {
-
-	@Override
-	public void savePlayer(Player player) throws IOException {
-		File file = BinaryPlayerUtil.getFile(player.getUsername());
-
-		try (DataOutputStream out = new DataOutputStream(new FileOutputStream(file))) {
-			StreamUtil.writeString(out, player.getUsername());
-			StreamUtil.writeString(out, player.getCredentials().getPassword());
-			out.writeByte(player.getPrivilegeLevel().toInteger());
-			out.writeByte(player.getMembershipStatus().getValue());
-
-			out.writeByte(player.getChatPrivacy().toInteger(true));
-			out.writeByte(player.getFriendPrivacy().toInteger(false));
-			out.writeByte(player.getTradePrivacy().toInteger(false));
-			out.writeByte(player.getRunEnergy());
-			out.writeByte(player.getScreenBrightness().toInteger());
-
-			Position position = player.getPosition();
-			out.writeShort(position.getX());
-			out.writeShort(position.getY());
-			out.writeByte(position.getHeight());
-
-			Appearance appearance = player.getAppearance();
-			out.writeByte(appearance.getGender().toInteger());
-			int[] style = appearance.getStyle();
-			for (int element : style) {
-				out.writeByte(element);
-			}
-			int[] colors = appearance.getColors();
-			for (int color : colors) {
-				out.writeByte(color);
-			}
-
-			writeInventory(out, player.getInventory());
-			writeInventory(out, player.getEquipment());
-			writeInventory(out, player.getBank());
-
-			SkillSet skills = player.getSkillSet();
-			out.writeByte(skills.size());
-			for (int id = 0; id < skills.size(); id++) {
-				Skill skill = skills.getSkill(id);
-				out.writeByte(skill.getCurrentLevel());
-				out.writeDouble(skill.getExperience());
-			}
-
-			List<String> usernames = player.getFriendUsernames();
-			out.writeByte(usernames.size());
-			for (String username : usernames) {
-				out.writeLong(NameUtil.encodeBase37(username));
-			}
-
-			usernames = player.getIgnoredUsernames();
-			out.writeByte(usernames.size());
-			for (String username : usernames) {
-				out.writeLong(NameUtil.encodeBase37(username));
-			}
-
-			Set<Entry<String, Attribute<?>>> attributes = player.getAttributes().entrySet();
-			attributes.removeIf(e -> AttributeMap.getDefinition(e.getKey()).getPersistence() != AttributePersistence.PERSISTENT);
-			out.writeInt(attributes.size());
-
-			for (Entry<String, Attribute<?>> entry : attributes) {
-				String name = entry.getKey();
-				StreamUtil.writeString(out, name);
-
-				Attribute<?> attribute = entry.getValue();
-				out.writeByte(attribute.getType().getValue());
-				out.write(attribute.encode());
-			}
-		}
-	}
-
-	/**
-	 * Writes an inventory to the specified output stream.
-	 * 
-	 * @param out The output stream.
-	 * @param inventory The inventory.
-	 * @throws IOException If an I/O error occurs.
-	 */
-	private static void writeInventory(DataOutputStream out, Inventory inventory) throws IOException {
-		int capacity = inventory.capacity();
-		out.writeShort(capacity);
-
-		for (int slot = 0; slot < capacity; slot++) {
-			Item item = inventory.get(slot);
-			if (item != null) {
-				out.writeShort(item.getId() + 1);
-				out.writeInt(item.getAmount());
-			} else {
-				out.writeShort(0);
-				out.writeInt(0);
-			}
-		}
-	}
-
-}

src/org/apollo/io/player/impl/BinaryPlayerUtil.java

@@ -1,46 +0,0 @@
-package org.apollo.io.player.impl;
-
-import java.io.File;
-
-import org.apollo.util.NameUtil;
-
-/**
- * A utility class with common functionality used by the binary player loader/ savers.
- * 
- * @author Graham
- */
-public final class BinaryPlayerUtil {
-
-	/**
-	 * The saved games directory.
-	 */
-	private static final File SAVED_GAMES_DIRECTORY = new File("data/savedGames");
-
-	/**
-	 * Creates the saved games directory if it does not exist.
-	 */
-	static {
-		if (!SAVED_GAMES_DIRECTORY.exists()) {
-			SAVED_GAMES_DIRECTORY.mkdir();
-		}
-	}
-
-	/**
-	 * Gets the save {@link File} for the specified player.
-	 * 
-	 * @param username The username of the player.
-	 * @return The file.
-	 */
-	public static File getFile(String username) {
-		String filtered = NameUtil.decodeBase37(NameUtil.encodeBase37(username));
-		return new File(SAVED_GAMES_DIRECTORY, filtered + ".dat");
-	}
-
-	/**
-	 * Default private constructor to prevent instantiation.
-	 */
-	private BinaryPlayerUtil() {
-
-	}
-
-}

src/org/apollo/io/player/impl/DiscardPlayerSaver.java

@@ -1,18 +0,0 @@
-package org.apollo.io.player.impl;
-
-import org.apollo.game.model.entity.Player;
-import org.apollo.io.player.PlayerSaver;
-
-/**
- * A {@link PlayerSaver} implementation that discards player data.
- * 
- * @author Graham
- */
-public final class DiscardPlayerSaver implements PlayerSaver {
-
-	@Override
-	public void savePlayer(Player player) {
-		/* discard player */
-	}
-
-}

src/org/apollo/io/player/impl/JdbcPlayerLoader.java

@@ -1,19 +0,0 @@
-package org.apollo.io.player.impl;
-
-import org.apollo.io.player.PlayerLoader;
-import org.apollo.io.player.PlayerLoaderResponse;
-import org.apollo.security.PlayerCredentials;
-
-/**
- * A {@link PlayerLoader} that utilises {@code JDBC} to load player files.
- * 
- * @author Major
- */
-public final class JdbcPlayerLoader implements PlayerLoader {
-
-	@Override
-	public PlayerLoaderResponse loadPlayer(PlayerCredentials credentials) throws Exception {
-		throw new UnsupportedOperationException("JDBC loading is not supported at this time.");
-	}
-
-}

src/org/apollo/io/player/impl/JdbcPlayerSaver.java

@@ -1,18 +0,0 @@
-package org.apollo.io.player.impl;
-
-import org.apollo.game.model.entity.Player;
-import org.apollo.io.player.PlayerSaver;
-
-/**
- * A {@link PlayerSaver} that utilises {@code JDBC} to save the player.
- * 
- * @author Major
- */
-public final class JdbcPlayerSaver implements PlayerSaver {
-
-	@Override
-	public void savePlayer(Player player) throws Exception {
-		throw new UnsupportedOperationException("JDBC saving is not supported at this time.");
-	}
-
-}

src/org/apollo/io/player/impl/package-info.java

@@ -1,4 +0,0 @@
-/**
- * Contains various player loader/saver implementations.
- */
-package org.apollo.io.player.impl;

src/org/apollo/login/LoginService.java

@@ -8,9 +8,8 @@ import java.util.concurrent.Executors;
 
 import org.apollo.Service;
 import org.apollo.game.model.entity.Player;
-import org.apollo.io.player.PlayerLoader;
 import org.apollo.io.player.PlayerLoaderResponse;
-import org.apollo.io.player.PlayerSaver;
+import org.apollo.io.player.PlayerSerializer;
 import org.apollo.net.codec.login.LoginConstants;
 import org.apollo.net.codec.login.LoginRequest;
 import org.apollo.net.release.Release;
@@ -25,6 +24,7 @@ import org.xml.sax.SAXException;
  * The {@link LoginService} manages {@link LoginRequest}s.
  * 
  * @author Graham
+ * @author Major
  */
 public final class LoginService extends Service {
 
@@ -34,14 +34,9 @@ public final class LoginService extends Service {
 	private final ExecutorService executor = Executors.newCachedThreadPool(new NamedThreadFactory("LoginService"));
 
 	/**
-	 * The current {@link PlayerLoader}.
+	 * The current {@link PlayerSerializer}.
 	 */
-	private PlayerLoader loader;
-
-	/**
-	 * The current {@link PlayerSaver}.
-	 */
-	private PlayerSaver saver;
+	private PlayerSerializer serializer;
 
 	/**
 	 * Creates the login service.
@@ -70,24 +65,16 @@ public final class LoginService extends Service {
 		}
 
 		if (!rootNode.getName().equals("login")) {
-			throw new IOException("Unexpected root node name.");
+			throw new IOException("Unexpected root node name, expected 'login'.");
 		}
 
-		XmlNode loaderNode = rootNode.getChild("loader");
-		if (loaderNode == null || !loaderNode.hasValue()) {
-			throw new IOException("No loader child node or value.");
+		XmlNode serializer = rootNode.getChild("serializer");
+		if (serializer == null || !serializer.hasValue()) {
+			throw new IOException("No serializer child node or value.");
 		}
 
-		XmlNode saverNode = rootNode.getChild("saver");
-		if (saverNode == null || !saverNode.hasValue()) {
-			throw new IOException("No saver child node or value.");
-		}
-
-		Class<?> loaderClazz = Class.forName(loaderNode.getValue());
-		Class<?> saverClazz = Class.forName(saverNode.getValue());
-
-		loader = (PlayerLoader) loaderClazz.newInstance();
-		saver = (PlayerSaver) saverClazz.newInstance();
+		Class<?> clazz = Class.forName(serializer.getValue());
+		this.serializer = (PlayerSerializer) clazz.newInstance();
 	}
 
 	/**
@@ -95,7 +82,7 @@ public final class LoginService extends Service {
 	 */
 	@Override
 	public void start() {
-		/* empty - here for consistency with other services */
+
 	}
 
 	/**
@@ -110,7 +97,7 @@ public final class LoginService extends Service {
 			// TODO check archive 0 CRCs
 			session.handlePlayerLoaderResponse(request, new PlayerLoaderResponse(LoginConstants.STATUS_GAME_UPDATED));
 		} else {
-			executor.submit(new PlayerLoaderWorker(loader, session, request));
+			executor.submit(new PlayerLoaderWorker(serializer, session, request));
 		}
 	}
 
@@ -121,7 +108,7 @@ public final class LoginService extends Service {
 	 * @param player The player to save.
 	 */
 	public void submitSaveRequest(GameSession session, Player player) {
-		executor.submit(new PlayerSaverWorker(saver, session, player));
+		executor.submit(new PlayerSaverWorker(serializer, session, player));
 	}
 
 }

src/org/apollo/login/PlayerLoaderWorker.java

@@ -3,8 +3,8 @@ package org.apollo.login;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-import org.apollo.io.player.PlayerLoader;
 import org.apollo.io.player.PlayerLoaderResponse;
+import org.apollo.io.player.PlayerSerializer;
 import org.apollo.net.codec.login.LoginConstants;
 import org.apollo.net.codec.login.LoginRequest;
 import org.apollo.net.session.LoginSession;
@@ -22,9 +22,9 @@ public final class PlayerLoaderWorker implements Runnable {
 	private static final Logger logger = Logger.getLogger(PlayerLoaderWorker.class.getName());
 
 	/**
-	 * The player loader.
+	 * The PlayerSerializer.
 	 */
-	private final PlayerLoader loader;
+	private final PlayerSerializer loader;
 
 	/**
 	 * The request.
@@ -39,11 +39,11 @@ public final class PlayerLoaderWorker implements Runnable {
 	/**
 	 * Creates a {@link PlayerLoaderWorker} which will do the work for a single player load request.
 	 * 
-	 * @param loader The current player loader.
+	 * @param loader The {@link PlayerSerializer}.
 	 * @param session The {@link LoginSession} which initiated the request.
 	 * @param request The {@link LoginRequest} object.
 	 */
-	public PlayerLoaderWorker(PlayerLoader loader, LoginSession session, LoginRequest request) {
+	public PlayerLoaderWorker(PlayerSerializer loader, LoginSession session, LoginRequest request) {
 		this.loader = loader;
 		this.session = session;
 		this.request = request;

src/org/apollo/login/PlayerSaverWorker.java

@@ -4,7 +4,7 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import org.apollo.game.model.entity.Player;
-import org.apollo.io.player.PlayerSaver;
+import org.apollo.io.player.PlayerSerializer;
 import org.apollo.net.session.GameSession;
 
 /**
@@ -27,7 +27,7 @@ public final class PlayerSaverWorker implements Runnable {
 	/**
 	 * The player saver.
 	 */
-	private final PlayerSaver saver;
+	private final PlayerSerializer saver;
 
 	/**
 	 * The game session.
@@ -41,7 +41,7 @@ public final class PlayerSaverWorker implements Runnable {
 	 * @param session The game session.
 	 * @param player The player to save.
 	 */
-	public PlayerSaverWorker(PlayerSaver saver, GameSession session, Player player) {
+	public PlayerSaverWorker(PlayerSerializer saver, GameSession session, Player player) {
 		this.saver = saver;
 		this.session = session;
 		this.player = player;

src/org/apollo/net/codec/login/LoginDecoder.java

@@ -105,14 +105,14 @@ public final class LoginDecoder extends StatefulFrameDecoder<LoginDecoderState>
 	 */
 	private void decodeHeader(ChannelHandlerContext ctx, ByteBuf buffer, List<Object> out) {
 		if (buffer.readableBytes() >= 2) {
-			int loginType = buffer.readUnsignedByte();
+			int type = buffer.readUnsignedByte();
 
-			if (loginType != LoginConstants.TYPE_STANDARD && loginType != LoginConstants.TYPE_RECONNECTION) {
+			if (type != LoginConstants.TYPE_STANDARD && type != LoginConstants.TYPE_RECONNECTION) {
 				writeResponseCode(ctx, LoginConstants.STATUS_LOGIN_SERVER_REJECTED_SESSION);
 				return;
 			}
 
-			reconnecting = loginType == LoginConstants.TYPE_RECONNECTION;
+			reconnecting = type == LoginConstants.TYPE_RECONNECTION;
 			loginLength = buffer.readUnsignedByte();
 
 			setState(LoginDecoderState.LOGIN_PAYLOAD);
@@ -129,53 +129,51 @@ public final class LoginDecoder extends StatefulFrameDecoder<LoginDecoderState>
 	private void decodePayload(ChannelHandlerContext ctx, ByteBuf buffer, List<Object> out) {
 		if (buffer.readableBytes() >= loginLength) {
 			ByteBuf payload = buffer.readBytes(loginLength);
-			int clientVersion = 255 - payload.readUnsignedByte();
+			int version = 255 - payload.readUnsignedByte();
 
-			int releaseNumber = payload.readUnsignedShort();
+			int release = payload.readUnsignedShort();
 
-			int lowMemoryFlag = payload.readUnsignedByte();
-			if (lowMemoryFlag != 0 && lowMemoryFlag != 1) {
+			int memoryStatus = payload.readUnsignedByte();
+			if (memoryStatus != 0 && memoryStatus != 1) {
 				writeResponseCode(ctx, LoginConstants.STATUS_LOGIN_SERVER_REJECTED_SESSION);
 				return;
 			}
 
-			boolean lowMemory = lowMemoryFlag == 1;
+			boolean lowMemory = memoryStatus == 1;
 
-			int[] archiveCrcs = new int[FileSystemConstants.ARCHIVE_COUNT];
-			for (int i = 0; i < 9; i++) {
-				archiveCrcs[i] = payload.readInt();
+			int[] crcs = new int[FileSystemConstants.ARCHIVE_COUNT];
+			for (int index = 0; index < 9; index++) {
+				crcs[index] = payload.readInt();
 			}
 
-			int securePayloadLength = payload.readUnsignedByte();
-			if (securePayloadLength != loginLength - 41) {
+			int length = payload.readUnsignedByte();
+			if (length != loginLength - 41) {
 				writeResponseCode(ctx, LoginConstants.STATUS_LOGIN_SERVER_REJECTED_SESSION);
 				return;
 			}
 
-			ByteBuf securePayload = payload.readBytes(securePayloadLength);
+			ByteBuf secure = payload.readBytes(length);
 
-			BigInteger bigInteger = new BigInteger(securePayload.array());
-			bigInteger = bigInteger.modPow(NetworkConstants.RSA_EXPONENT, NetworkConstants.RSA_MODULUS);
+			BigInteger value = new BigInteger(secure.array());
+			value = value.modPow(NetworkConstants.RSA_EXPONENT, NetworkConstants.RSA_MODULUS);
+			secure = Unpooled.wrappedBuffer(value.toByteArray());
 
-			securePayload = Unpooled.wrappedBuffer(bigInteger.toByteArray());
-
-			int secureId = securePayload.readUnsignedByte();
-			if (secureId != 10) {
+			int id = secure.readUnsignedByte();
+			if (id != 10) {
 				writeResponseCode(ctx, LoginConstants.STATUS_LOGIN_SERVER_REJECTED_SESSION);
 				return;
 			}
 
-			long clientSeed = securePayload.readLong();
-			long reportedServerSeed = securePayload.readLong();
-			if (reportedServerSeed != serverSeed) {
+			long clientSeed = secure.readLong();
+			long reportedSeed = secure.readLong();
+			if (reportedSeed != serverSeed) {
 				writeResponseCode(ctx, LoginConstants.STATUS_LOGIN_SERVER_REJECTED_SESSION);
 				return;
 			}
 
-			int uid = securePayload.readInt();
-
-			String username = BufferUtil.readString(securePayload);
-			String password = BufferUtil.readString(securePayload);
+			int uid = secure.readInt();
+			String username = BufferUtil.readString(secure);
+			String password = BufferUtil.readString(secure);
 
 			if (password.length() < 6 || password.length() > 20 || username.isEmpty() || username.length() > 12) {
 				writeResponseCode(ctx, LoginConstants.STATUS_INVALID_CREDENTIALS);
@@ -189,8 +187,8 @@ public final class LoginDecoder extends StatefulFrameDecoder<LoginDecoderState>
 			seed[3] = (int) serverSeed;
 
 			IsaacRandom decodingRandom = new IsaacRandom(seed);
-			for (int i = 0; i < seed.length; i++) {
-				seed[i] += 50;
+			for (int index = 0; index < seed.length; index++) {
+				seed[index] += 50;
 			}
 
 			IsaacRandom encodingRandom = new IsaacRandom(seed);
@@ -198,9 +196,7 @@ public final class LoginDecoder extends StatefulFrameDecoder<LoginDecoderState>
 			PlayerCredentials credentials = new PlayerCredentials(username, password, usernameHash, uid);
 			IsaacRandomPair randomPair = new IsaacRandomPair(encodingRandom, decodingRandom);
 
-			LoginRequest request = new LoginRequest(credentials, randomPair, reconnecting, lowMemory, releaseNumber, archiveCrcs, clientVersion);
-
-			out.add(request);
+			out.add(new LoginRequest(credentials, randomPair, reconnecting, lowMemory, release, crcs, version));
 		}
 	}
 
@@ -208,11 +204,11 @@ public final class LoginDecoder extends StatefulFrameDecoder<LoginDecoderState>
 	 * Writes a response code to the client and closes the current channel.
 	 *
 	 * @param ctx The context of the channel handler.
-	 * @param responseCode The response code to write.
+	 * @param response The response code to write.
 	 */
-	private void writeResponseCode(ChannelHandlerContext ctx, int responseCode) {
+	private void writeResponseCode(ChannelHandlerContext ctx, int response) {
 		ByteBuf buffer = ctx.alloc().buffer(1);
-		buffer.writeByte(responseCode);
+		buffer.writeByte(response);
 
 		ctx.writeAndFlush(buffer).addListener(ChannelFutureListener.CLOSE);
 	}

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

@@ -18,7 +18,6 @@ public final class ThirdObjectActionMessageDecoder extends MessageDecoder<ThirdO
 
 	@Override
 	public ThirdObjectActionMessage decode(GamePacket packet) {
-		// TODO ripped out of some Winterlove-based server, so probably wrong
 		GamePacketReader reader = new GamePacketReader(packet);
 		int x = (int) reader.getUnsigned(DataType.SHORT, DataOrder.LITTLE);
 		int y = (int) reader.getUnsigned(DataType.SHORT);

src/org/apollo/net/session/Session.java

@@ -13,7 +13,7 @@ public abstract class Session {
 	/**
 	 * The channel.
 	 */
-	private final Channel channel;
+	protected final Channel channel;
 
 	/**
 	 * Creates a session for the specified channel.

src/org/apollo/net/session/UpdateSession.java

@@ -34,7 +34,7 @@ public final class UpdateSession extends Session {
 
 	@Override
 	public void destroy() {
-		// TODO implement
+		channel.close();
 	}
 
 	@Override

src/org/apollo/util/StatefulFrameDecoder.java

@@ -38,6 +38,16 @@ public abstract class StatefulFrameDecoder<T extends Enum<T>> extends ByteToMess
 		setState(state);
 	}
 
+	/**
+	 * Sets a new state.
+	 * 
+	 * @param state The new state.
+	 * @throws NullPointerException If the state is {@code null}.
+	 */
+	public final void setState(T state) {
+		this.state = Objects.requireNonNull(state, "State cannot be null.");
+	}
+
 	@Override
 	protected final void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception {
 		decode(ctx, in, out, state);
@@ -54,14 +64,4 @@ public abstract class StatefulFrameDecoder<T extends Enum<T>> extends ByteToMess
 	 */
 	protected abstract void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out, T state) throws Exception;
 
-	/**
-	 * Sets a new state.
-	 * 
-	 * @param state The new state.
-	 * @throws NullPointerException If the state is {@code null}.
-	 */
-	public final void setState(T state) {
-		this.state = Objects.requireNonNull(state, "State cannot be null.");
-	}
-
 }