Resolve merge conflict.

data/plugins/chat/private-messaging/messaging.rb

@@ -6,7 +6,7 @@ java_import 'org.apollo.game.model.entity.setting.PrivacyState'
 
 on :message, :private_chat do |ctx, player, message|
   friend = $world.get_player(message.username)
-  friend.send(ForwardPrivateChatMessage.new(player.username, player.privilege_level, message.compressed_message)) if interaction_permitted(player, friend)
+  friend.send(ForwardPrivateChatMessage.new(player.username, player.privilege_level, message.compressed_chat)) if interaction_permitted(player, friend)
 end
 
 # Checks if the sender is permitted to interact with the friend they have added:

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

@@ -118,11 +118,9 @@ public final class GameObjectDecoder {
 			block = true;
 		}
 
-		Predicate<Integer> walls = (value) -> value >= ObjectType.LENGTHWISE_WALL.getValue()
+		Predicate<Integer> walls = (value) -> value >= ObjectType.LENGTHWISE_WALL.getValue() && value <= ObjectType.RECTANGULAR_CORNER.getValue() || value == ObjectType.DIAGONAL_WALL.getValue();
-				&& value <= ObjectType.RECTANGULAR_CORNER.getValue() || value == ObjectType.DIAGONAL_WALL.getValue();
 
-		Predicate<Integer> roofs = (value) -> value > ObjectType.DIAGONAL_INTERACTABLE.getValue()
+		Predicate<Integer> roofs = (value) -> value > ObjectType.DIAGONAL_INTERACTABLE.getValue() && value < ObjectType.FLOOR_DECORATION.getValue();
-				&& value < ObjectType.FLOOR_DECORATION.getValue();
 
 		if (walls.test(type) || roofs.test(type)) {
 			block = true;

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

@@ -128,7 +128,7 @@ public final class ObjectDefinitionDecoder {
 				data.getShort();
 				data.getShort();
 				int count = data.get();
-				for (int i = 0; i <= count; i++){
+				for (int i = 0; i <= count; i++) {
 					data.getShort();
 				}
 			} else {

src/org/apollo/game/GamePulseHandler.java

@@ -25,7 +25,7 @@ public final class GamePulseHandler implements Runnable {
 	 * 
 	 * @param service The {@link GameService}.
 	 */
-	GamePulseHandler(GameService service) {
+	protected GamePulseHandler(GameService service) {
 		this.service = service;
 	}
 
@@ -33,10 +33,11 @@ public final class GamePulseHandler implements Runnable {
 	public void run() {
 		try {
 			service.pulse();
-		} catch (Throwable t) {
+		} catch (Throwable reason) {
-			logger.log(Level.SEVERE, "Exception during pulse.", t);
+			logger.log(Level.SEVERE, "Exception occured during pulse!", reason);
+		} finally {
+			service.shutdown(false);
 		}
-
 	}
 
 }

src/org/apollo/game/GameService.java

@@ -19,6 +19,7 @@ import org.apollo.game.sync.ClientSynchronizer;
 import org.apollo.io.MessageHandlerChainParser;
 import org.apollo.login.LoginService;
 import org.apollo.net.session.GameSession;
+import org.apollo.util.MobRepository;
 import org.apollo.util.NamedThreadFactory;
 import org.apollo.util.xml.XmlNode;
 import org.apollo.util.xml.XmlParser;
@@ -89,61 +90,23 @@ public final class GameService extends Service {
 		return chainGroup;
 	}
 
-	/**
-	 * Initializes the game service.
-	 * 
-	 * @throws IOException If there is an error with the file (e.g. does not exist, cannot be read, does not contain
-	 *             valid nodes).
-	 * @throws SAXException If there is an error parsing the file.
-	 * @throws ReflectiveOperationException If a MessageHandler could not be created.
-	 */
-	private void init() throws IOException, SAXException, ReflectiveOperationException {
-		try (InputStream is = new FileInputStream("data/messages.xml")) {
-			MessageHandlerChainParser chainGroupParser = new MessageHandlerChainParser(is);
-			chainGroup = chainGroupParser.parse(world);
-		}
-
-		try (InputStream is = new FileInputStream("data/synchronizer.xml")) {
-			XmlParser parser = new XmlParser();
-			XmlNode rootNode = parser.parse(is);
-
-			if (!rootNode.getName().equals("synchronizer")) {
-				throw new IOException("Invalid root node name.");
-			}
-
-			XmlNode activeNode = rootNode.getChild("active");
-			if (activeNode == null || !activeNode.hasValue()) {
-				throw new IOException("No active node/value.");
-			}
-
-			Class<?> clazz = Class.forName(activeNode.getValue());
-			synchronizer = (ClientSynchronizer) clazz.newInstance();
-		}
-	}
-
 	/**
 	 * Called every pulse.
 	 */
 	public void pulse() {
 		synchronized (this) {
-			LoginService loginService = getContext().getService(LoginService.class);
+			finalizeUnregisters();
 
-			int unregistered = 0;
+			MobRepository<Player> players = world.getPlayerRepository();
-			Player old;
+			for (Player player : players) {
-			while (unregistered < UNREGISTERS_PER_CYCLE && (old = oldPlayers.poll()) != null) {
+				GameSession session = player.getSession();
-				loginService.submitSaveRequest(old.getSession(), old);
-				unregistered++;
-			}
-
-			for (Player p : world.getPlayerRepository()) {
-				GameSession session = p.getSession();
 				if (session != null) {
 					session.handlePendingMessages(chainGroup);
 				}
 			}
 
 			world.pulse();
-			synchronizer.synchronize(world.getPlayerRepository(), world.getNpcRepository());
+			synchronizer.synchronize(players, world.getNpcRepository());
 		}
 	}
 
@@ -169,8 +132,15 @@ public final class GameService extends Service {
 	}
 
 	/**
-	 * Starts the game service.
+	 * Shuts down this game service.
+	 * 
+	 * @param natural Whether or not the shutdown was expected.
 	 */
+	public void shutdown(boolean natural) {
+		scheduledExecutor.shutdownNow();
+		// TODO: Other events that should happen upon natural or unexpected shutdown.
+	}
+
 	@Override
 	public void start() {
 		scheduledExecutor.scheduleAtFixedRate(new GamePulseHandler(this), GameConstants.PULSE_DELAY, GameConstants.PULSE_DELAY,
@@ -186,4 +156,51 @@ public final class GameService extends Service {
 		oldPlayers.add(player);
 	}
 
+	/**
+	 * Finalizes the unregistration of Player's queued to be unregistered.
+	 */
+	private void finalizeUnregisters() {
+		LoginService loginService = getContext().getService(LoginService.class);
+
+		for (int count = 0; count < UNREGISTERS_PER_CYCLE; count++) {
+			Player player = oldPlayers.poll();
+			if (player == null) {
+				break;
+			}
+
+			loginService.submitSaveRequest(player.getSession(), player);
+		}
+	}
+
+	/**
+	 * Initializes the game service.
+	 * 
+	 * @throws IOException If there is an error accessing the file.
+	 * @throws SAXException If there is an error parsing the file.
+	 * @throws ReflectiveOperationException If a MessageHandler could not be created.
+	 */
+	private void init() throws IOException, SAXException, ReflectiveOperationException {
+		try (InputStream input = new FileInputStream("data/messages.xml")) {
+			MessageHandlerChainParser chainGroupParser = new MessageHandlerChainParser(input);
+			chainGroup = chainGroupParser.parse(world);
+		}
+
+		try (InputStream input = new FileInputStream("data/synchronizer.xml")) {
+			XmlParser parser = new XmlParser();
+			XmlNode root = parser.parse(input);
+
+			if (!root.getName().equals("synchronizer")) {
+				throw new IOException("Invalid root node name.");
+			}
+
+			XmlNode active = root.getChild("active");
+			if (active == null || !active.hasValue()) {
+				throw new IOException("No active node/value.");
+			}
+
+			Class<?> clazz = Class.forName(active.getValue());
+			synchronizer = (ClientSynchronizer) clazz.newInstance();
+		}
+	}
+
 }

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

@@ -9,6 +9,7 @@ import org.apollo.game.model.inter.EnterAmountListener;
 import org.apollo.game.model.inter.bank.BankConstants;
 import org.apollo.game.model.inter.bank.BankDepositEnterAmountListener;
 import org.apollo.game.model.inter.bank.BankUtils;
+import org.apollo.game.model.inter.bank.BankWithdrawEnterAmountListener;
 
 /**
  * A {@link MessageHandler} that handles withdrawing and depositing items from/to a player's bank.
@@ -90,7 +91,7 @@ public final class BankMessageHandler extends MessageHandler<ItemActionMessage>
 		int amount = optionToAmount(message.getOption());
 
 		if (amount == -1) {
-			EnterAmountListener listener = new BankDepositEnterAmountListener(player, message.getSlot(), message.getId());
+			EnterAmountListener listener = new BankWithdrawEnterAmountListener(player, message.getSlot(), message.getId());
 			player.getInterfaceSet().openEnterAmountDialogue(listener);
 		} else if (!BankUtils.withdraw(player, message.getSlot(), message.getId(), amount)) {
 			ctx.breakHandlerChain();

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

@@ -73,6 +73,8 @@ public final class EquipItemHandler extends MessageHandler<ItemOptionMessage> {
 		Item weapon = equipment.get(EquipmentConstants.WEAPON);
 		Item shield = equipment.get(EquipmentConstants.SHIELD);
 
+		// XXX: This is still pretty ugly in some parts, improve.
+
 		if (definition.isTwoHanded()) {
 			int slotsRequired = weapon != null && shield != null ? 1 : 0;
 			if (inventory.freeSlots() < slotsRequired) {

src/org/apollo/game/message/impl/NpcActionMessage.java

@@ -29,7 +29,7 @@ public abstract class NpcActionMessage extends Message {
 	 */
 	public NpcActionMessage(int option, int index) {
 		this.option = option;
-		this.index = index - 1;
+		this.index = index;
 	}
 
 	/**

src/org/apollo/game/message/impl/PlayerActionMessage.java

@@ -25,11 +25,11 @@ public abstract class PlayerActionMessage extends Message {
 	 * Creates a player action message.
 	 * 
 	 * @param option The option number.
-	 * @param playerIndex The index of the player.
+	 * @param index The index of the player.
 	 */
-	public PlayerActionMessage(int option, int playerIndex) {
+	public PlayerActionMessage(int option, int index) {
 		this.option = option;
-		this.index = playerIndex - 1;
+		this.index = index;
 	}
 
 	/**

src/org/apollo/game/message/impl/PlayerSynchronizationMessage.java

@@ -53,8 +53,7 @@ public final class PlayerSynchronizationMessage extends Message {
 	 * @param localPlayers The number of local players.
 	 * @param segments A list of segments.
 	 */
-	public PlayerSynchronizationMessage(Position lastKnownRegion, Position position, boolean regionChanged,
+	public PlayerSynchronizationMessage(Position lastKnownRegion, Position position, boolean regionChanged, SynchronizationSegment segment, int localPlayers, List<SynchronizationSegment> segments) {
-			SynchronizationSegment segment, int localPlayers, List<SynchronizationSegment> segments) {
 		this.lastKnownRegion = lastKnownRegion;
 		this.position = position;
 		this.regionChanged = regionChanged;

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

@@ -226,8 +226,7 @@ public final class Position {
 
 	@Override
 	public String toString() {
-		return MoreObjects.toStringHelper(this).add("x", getX()).add("y", getY()).add("height", getHeight())
+		return MoreObjects.toStringHelper(this).add("x", getX()).add("y", getY()).add("height", getHeight()).add("region", getRegionCoordinates()).toString();
-				.add("region", getRegionCoordinates()).toString();
 	}
 
 }

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

@@ -41,8 +41,7 @@ public final class Region {
 		@Override
 		public void execute(Region region, Entity entity, EntityUpdateType update) {
 			EntityType type = entity.getEntityType();
-			if (type != EntityType.PLAYER && type != EntityType.NPC
+			if (type != EntityType.PLAYER && type != EntityType.NPC && (type != EntityType.STATIC_OBJECT || update == EntityUpdateType.REMOVE)) {
-					&& (type != EntityType.STATIC_OBJECT || update == EntityUpdateType.REMOVE)) {
 				region.record(entity, update);
 			}
 		}
@@ -190,8 +189,7 @@ public final class Region {
 
 		Set<EntityType> set = new HashSet<>(Arrays.asList(types));
 		@SuppressWarnings("unchecked")
-		Set<T> filtered = (Set<T>) local.stream().filter(entity -> set.contains(entity.getEntityType()))
+		Set<T> filtered = (Set<T>) local.stream().filter(entity -> set.contains(entity.getEntityType())).collect(Collectors.toSet());
-				.collect(Collectors.toSet());
 		return ImmutableSet.copyOf(filtered);
 	}
 
@@ -291,8 +289,7 @@ public final class Region {
 	 * @throws IllegalArgumentException If the specified position is not included in this Region.
 	 */
 	private void checkPosition(Position position) {
-		Preconditions.checkArgument(coordinates.equals(RegionCoordinates.fromPosition(position)),
+		Preconditions.checkArgument(coordinates.equals(RegionCoordinates.fromPosition(position)), "Position is not included in this Region.");
-				"Position is not included in this Region.");
 	}
 
 	/**

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

@@ -18,6 +18,6 @@ public interface RegionListener {
 	 * @param entity The affected {@link Entity}.
 	 * @param type The type of {@link EntityUpdateType}.
 	 */
-	public abstract void execute(Region region, Entity entity, EntityUpdateType type);
+	public void execute(Region region, Entity entity, EntityUpdateType type);
 
 }

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

@@ -184,8 +184,7 @@ public final class CollisionMatrix {
 
 	@Override
 	public String toString() {
-		return MoreObjects.toStringHelper(this).add("width", width).add("length", length).add("matrix", Arrays.toString(matrix))
+		return MoreObjects.toStringHelper(this).add("width", width).add("length", length).add("matrix", Arrays.toString(matrix)).toString();
-				.toString();
 	}
 
 	/**

src/org/apollo/game/model/area/update/package-info.java

@@ -1,4 +1,4 @@
 /**
- * Contains snapshot-related classes. 
+ * Contains snapshot-related classes.
  */
 package org.apollo.game.model.area.update;

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

@@ -198,8 +198,8 @@ public abstract class Mob extends Entity {
 	 */
 	public final Direction[] getDirections() {
 		if (firstDirection != Direction.NONE) {
-			return secondDirection == Direction.NONE ? new Direction[] { firstDirection } : new Direction[] { firstDirection,
+			return secondDirection == Direction.NONE ? new Direction[] { firstDirection } : new Direction[] {
-					secondDirection };
+					firstDirection, secondDirection };
 		}
 
 		return Direction.EMPTY_DIRECTION_ARRAY;

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

@@ -942,8 +942,7 @@ public final class Player extends Mob {
 
 	@Override
 	public String toString() {
-		return MoreObjects.toStringHelper(this).add("username", getUsername()).add("privilege", privilegeLevel)
+		return MoreObjects.toStringHelper(this).add("username", getUsername()).add("privilege", privilegeLevel).add("client version", getClientVersion()).toString();
-				.add("client version", getClientVersion()).toString();
 	}
 
 	/**
@@ -962,11 +961,9 @@ public final class Player extends Mob {
 		InventoryListener fullBankListener = new FullInventoryListener(this, FullInventoryListener.FULL_BANK_MESSAGE);
 		InventoryListener appearanceListener = new AppearanceInventoryListener(this);
 
-		InventoryListener syncInventoryListener = new SynchronizationInventoryListener(this,
+		InventoryListener syncInventoryListener = new SynchronizationInventoryListener(this, SynchronizationInventoryListener.INVENTORY_ID);
-				SynchronizationInventoryListener.INVENTORY_ID);
 		InventoryListener syncBankListener = new SynchronizationInventoryListener(this, BankConstants.BANK_INVENTORY_ID);
-		InventoryListener syncEquipmentListener = new SynchronizationInventoryListener(this,
+		InventoryListener syncEquipmentListener = new SynchronizationInventoryListener(this, SynchronizationInventoryListener.EQUIPMENT_ID);
-				SynchronizationInventoryListener.EQUIPMENT_ID);
 
 		inventory.addListener(syncInventoryListener);
 		inventory.addListener(fullInventoryListener);

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

@@ -7,6 +7,7 @@ import java.util.List;
 import org.apollo.game.model.skill.SkillListener;
 
 import com.google.common.base.Preconditions;
+import com.google.common.primitives.Ints;
 
 /**
  * Represents the set of the player's skills.
@@ -58,7 +59,6 @@ public final class SkillSet {
 	 */
 	public static int getLevelForExperience(double experience) {
 		Preconditions.checkArgument(experience >= 0 && experience <= MAXIMUM_EXP, "Experience must be between 0 and " + MAXIMUM_EXP + ", inclusive.");
-
 		for (int level = 1; level <= 98; level++) {
 			if (experience < EXPERIENCE_FOR_LEVEL[level + 1]) {
 				return level; // TODO binary search?
@@ -140,10 +140,10 @@ public final class SkillSet {
 		int ranged = skills[Skill.RANGED].getMaximumLevel();
 		int magic = skills[Skill.MAGIC].getMaximumLevel();
 
-		double base = (defence + hitpoints + Math.floor(prayer / 2)) * 0.25;
+		double base = Ints.max(strength + attack, magic * 2, ranged * 2);
-		double melee = (attack + strength) * 0.325;
+		double combat = (base * 1.3 + defence + hitpoints + prayer / 2) / 4;
 
-		this.combat = (int) (base + Math.max(melee, Math.max(ranged, magic) * 0.4875));
+		this.combat = (int) combat;
 	}
 
 	/**
@@ -209,7 +209,7 @@ public final class SkillSet {
 	 * @return The total level.
 	 */
 	public int getTotalLevel() {
-		return Arrays.stream(skills).map(Skill::getMaximumLevel).reduce(0, (total, level) -> total + level);
+		return Arrays.stream(skills).mapToInt(Skill::getMaximumLevel).sum();
 	}
 
 	/**

src/org/apollo/game/model/entity/attr/StringAttribute.java

@@ -33,7 +33,7 @@ public final class StringAttribute extends Attribute<String> {
 	public byte[] encode() {
 		byte[] bytes = value.getBytes(Charset.forName("UTF-8"));
 		int length = bytes.length;
-				
+
 		bytes = Arrays.copyOf(bytes, length + 1);
 		bytes[length - 1] = 0;
 		return bytes;

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

@@ -91,8 +91,7 @@ public abstract class GameObject extends Entity {
 
 	@Override
 	public String toString() {
-		return MoreObjects.toStringHelper(this).add("id", getId()).add("type", getType()).add("orientation", getOrientation())
+		return MoreObjects.toStringHelper(this).add("id", getId()).add("type", getType()).add("orientation", getOrientation()).toString();
-				.toString();
 	}
 
 	@Override

src/org/apollo/game/model/entity/obj/ObjectGroup.java

@@ -38,8 +38,7 @@ public enum ObjectGroup {
 	 * @throws IllegalArgumentException If there is no ObjectGroup with the specified value.
 	 */
 	public static ObjectGroup valueOf(int value) {
-		return Arrays.stream(values()).filter(group -> group.value == value).findAny()
+		return Arrays.stream(values()).filter(group -> group.value == value).findAny().orElseThrow(() -> new IllegalArgumentException("No ObjectGroup with a value of " + value + " exists."));
-				.orElseThrow(() -> new IllegalArgumentException("No ObjectGroup with a value of " + value + " exists."));
 	}
 
 	/**

src/org/apollo/game/model/entity/obj/package-info.java

@@ -1,4 +1,4 @@
 /**
- * Contains object-related classes. 
+ * Contains object-related classes.
  */
 package org.apollo.game.model.entity.obj;

src/org/apollo/game/model/skill/SynchronizationSkillListener.java

@@ -1,5 +1,7 @@
 package org.apollo.game.model.skill;
 
+import java.util.stream.IntStream;
+
 import org.apollo.game.message.impl.UpdateSkillMessage;
 import org.apollo.game.model.entity.Player;
 import org.apollo.game.model.entity.Skill;
@@ -29,14 +31,14 @@ public final class SynchronizationSkillListener extends SkillAdapter {
 
 	@Override
 	public void levelledUp(SkillSet set, int id, Skill skill) {
-		player.getBlockSet().add(SynchronizationBlock.createAppearanceBlock(player));
+		if (Skill.isCombatSkill(id)) {
+			player.getBlockSet().add(SynchronizationBlock.createAppearanceBlock(player));
+		}
 	}
 
 	@Override
 	public void skillsUpdated(SkillSet set) {
-		for (int id = 0; id < set.size(); id++) {
+		IntStream.range(0, set.size()).forEach(id -> skillUpdated(set, id, set.getSkill(id)));
-			player.send(new UpdateSkillMessage(id, set.getSkill(id)));
-		}
 	}
 
 	@Override

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

@@ -78,8 +78,7 @@ public final class PrePlayerSynchronizationTask extends SynchronizationTask {
 	 * @param updates The {@link Map} containing {@link Region} updates.
 	 * @param snapshots The Map containing Region snapshots.
 	 */
-	public PrePlayerSynchronizationTask(Player player, Map<RegionCoordinates, List<RegionUpdateMessage>> updates,
+	public PrePlayerSynchronizationTask(Player player, Map<RegionCoordinates, List<RegionUpdateMessage>> updates, Map<RegionCoordinates, List<RegionUpdateMessage>> snapshots) {
-			Map<RegionCoordinates, List<RegionUpdateMessage>> snapshots) {
 		this.player = player;
 		this.updates = updates;
 		this.snapshots = snapshots;
@@ -209,8 +208,7 @@ public final class PrePlayerSynchronizationTask extends SynchronizationTask {
 		int deltaX = current.getLocalX(last);
 		int deltaY = current.getLocalY(last);
 
-		return deltaX <= Position.MAX_DISTANCE || deltaX >= (VIEWPORT_WIDTH - Position.MAX_DISTANCE - 1)
+		return deltaX <= Position.MAX_DISTANCE || deltaX >= (VIEWPORT_WIDTH - Position.MAX_DISTANCE - 1) || deltaY <= Position.MAX_DISTANCE || deltaY >= (VIEWPORT_WIDTH - Position.MAX_DISTANCE - 1);
-				|| deltaY <= Position.MAX_DISTANCE || deltaY >= (VIEWPORT_WIDTH - Position.MAX_DISTANCE - 1);
 	}
 
 	/**
@@ -223,8 +221,7 @@ public final class PrePlayerSynchronizationTask extends SynchronizationTask {
 	 * @param repository The {@link RegionRepository} containing the Regions.
 	 * @return The Optional containing the GroupedRegionUpdateMessage.
 	 */
-	private Optional<GroupedRegionUpdateMessage> toUpdateMessage(RegionUpdateMode mode, Position position,
+	private Optional<GroupedRegionUpdateMessage> toUpdateMessage(RegionUpdateMode mode, Position position, RegionCoordinates coordinates, RegionRepository repository) {
-			RegionCoordinates coordinates, RegionRepository repository) {
 		List<RegionUpdateMessage> messages;
 
 		/*
@@ -240,8 +237,7 @@ public final class PrePlayerSynchronizationTask extends SynchronizationTask {
 				messages = updates.get(coordinates);
 				if (messages == null) {
 					synchronized (updates) {
-						messages = updates.computeIfAbsent(coordinates,
+						messages = updates.computeIfAbsent(coordinates, coords -> repository.get(coords).getUpdates(position.getHeight()));
-								coords -> repository.get(coords).getUpdates(position.getHeight()));
 					}
 				}
 
@@ -250,8 +246,7 @@ public final class PrePlayerSynchronizationTask extends SynchronizationTask {
 				messages = snapshots.get(coordinates);
 				if (messages == null) {
 					synchronized (snapshots) {
-						messages = snapshots.computeIfAbsent(coordinates,
+						messages = snapshots.computeIfAbsent(coordinates, coords -> repository.get(coords).getSnapshot(position.getHeight()));
-								coords -> repository.get(coords).getSnapshot(position.getHeight()));
 					}
 				}
 
@@ -260,8 +255,7 @@ public final class PrePlayerSynchronizationTask extends SynchronizationTask {
 				throw new IllegalArgumentException("Unrecognised RegionUpdateMode " + mode + ".");
 		}
 
-		return messages.isEmpty() ? Optional.empty() : Optional
+		return messages.isEmpty() ? Optional.empty() : Optional.of(new GroupedRegionUpdateMessage(position, coordinates, messages));
-				.of(new GroupedRegionUpdateMessage(position, coordinates, messages));
 	}
 
 }

src/org/apollo/net/NetworkConstants.java

@@ -72,8 +72,7 @@ public final class NetworkConstants {
 			Preconditions.checkState(rsa != null, "Root node must have a child named 'rsa'.");
 
 			XmlNode modulus = rsa.getChild("modulus"), exponent = rsa.getChild("private-exponent");
-			Preconditions.checkState(modulus != null && exponent != null,
+			Preconditions.checkState(modulus != null && exponent != null, "Rsa node must have two children: 'modulus' and 'private-exponent'.");
-					"Rsa node must have two children: 'modulus' and 'private-exponent'.");
 
 			RSA_MODULUS = new BigInteger(modulus.getValue());
 			RSA_EXPONENT = new BigInteger(exponent.getValue());
@@ -82,8 +81,7 @@ public final class NetworkConstants {
 			Preconditions.checkState(ports != null, "Root node must have a child named 'ports'.");
 
 			XmlNode http = ports.getChild("http"), service = ports.getChild("service"), jaggrab = ports.getChild("jaggrab");
-			Preconditions.checkState(http != null && service != null && jaggrab != null,
+			Preconditions.checkState(http != null && service != null && jaggrab != null, "Ports node must have three children: 'http', 'service', and 'jaggrab'.");
-					"Ports node must have three children: 'http', 'service', and 'jaggrab'.");
 
 			HTTP_PORT = Integer.parseInt(http.getValue());
 			SERVICE_PORT = Integer.parseInt(service.getValue());

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

@@ -44,8 +44,7 @@ public final class GroupedRegionUpdateMessageEncoder extends MessageEncoder<Grou
 		for (RegionUpdateMessage update : message.getMessages()) {
 			System.out.println("==== Sending " + update + " as part of grum");
 			@SuppressWarnings("unchecked")
-			MessageEncoder<RegionUpdateMessage> encoder = (MessageEncoder<RegionUpdateMessage>) release.getMessageEncoder(update
+			MessageEncoder<RegionUpdateMessage> encoder = (MessageEncoder<RegionUpdateMessage>) release.getMessageEncoder(update.getClass());
-					.getClass());
 
 			GamePacket packet = encoder.encode(update);
 			builder.put(DataType.BYTE, packet.getOpcode());

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

@@ -194,8 +194,8 @@ public final class NpcSynchronizationMessageEncoder extends MessageEncoder<NpcSy
 	private static void putGraphicBlock(GraphicBlock block, GamePacketBuilder builder) {
 		Graphic graphic = block.getGraphic();
 		builder.put(DataType.SHORT, graphic.getId());
-        builder.put(DataType.INT, graphic.getHeight() << 16 | graphic.getDelay() & 0xFFFF);
+		builder.put(DataType.INT, graphic.getHeight() << 16 | graphic.getDelay() & 0xFFFF);
-    }
+	}
 
 	/**
 	 * Puts a hit update block into the specified builder.

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

@@ -20,8 +20,7 @@ public final class RemoveObjectMessageEncoder extends MessageEncoder<RemoveObjec
 		builder.put(DataType.BYTE, DataTransformation.NEGATE, message.getType() << 2 | message.getOrientation());
 		builder.put(DataType.BYTE, message.getPositionOffset());
 
-		System.out.println("Sending rm obj: type=" + message.getType() + ", orient=" + message.getOrientation() + ",posoff="
+		System.out.println("Sending rm obj: type=" + message.getType() + ", orient=" + message.getOrientation() + ",posoff=" + Integer.toBinaryString(message.getPositionOffset()));
-				+ Integer.toBinaryString(message.getPositionOffset()));
 		return builder.toGamePacket();
 	}
 

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

@@ -32,8 +32,7 @@ public final class GroupedRegionUpdateMessageEncoder extends MessageEncoder<Grou
 	 *
 	 * @param encoders The Map of RegionUpdateMessages to MessageEncoders.
 	 */
-	public GroupedRegionUpdateMessageEncoder(
+	public GroupedRegionUpdateMessageEncoder(Map<Class<? extends RegionUpdateMessage>, MessageEncoder<? extends RegionUpdateMessage>> encoders) {
-			Map<Class<? extends RegionUpdateMessage>, MessageEncoder<? extends RegionUpdateMessage>> encoders) {
 		this.encoders = ImmutableMap.copyOf(encoders);
 	}
 
@@ -49,8 +48,7 @@ public final class GroupedRegionUpdateMessageEncoder extends MessageEncoder<Grou
 			@SuppressWarnings("unchecked")
 			MessageEncoder<RegionUpdateMessage> encoder = (MessageEncoder<RegionUpdateMessage>) encoders.get(update);
 
-			Preconditions.checkState(encoder != null, update.getClass()
+			Preconditions.checkState(encoder != null, update.getClass() + " does not have a registered encoder in GroupedRegionUpdateMessageEncoder.");
-					+ " does not have a registered encoder in GroupedRegionUpdateMessageEncoder.");
 
 			GamePacket packet = encoder.encode(update);
 			builder.put(DataType.BYTE, packet.getOpcode());

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

@@ -194,7 +194,7 @@ public final class NpcSynchronizationMessageEncoder extends MessageEncoder<NpcSy
 	private static void putGraphicBlock(GraphicBlock block, GamePacketBuilder builder) {
 		Graphic graphic = block.getGraphic();
 		builder.put(DataType.SHORT, graphic.getId());
-        builder.put(DataType.INT, graphic.getHeight() << 16 | graphic.getDelay() & 0xFFFF);
+		builder.put(DataType.INT, graphic.getHeight() << 16 | graphic.getDelay() & 0xFFFF);
 	}
 
 	/**

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

@@ -221,8 +221,7 @@ public final class Release377 extends Release {
 		regionUpdates.put(SendObjectMessage.class, new SendObjectMessageEncoder());
 		regionUpdates.put(RemoveObjectMessage.class, new RemoveObjectMessageEncoder());
 
-		for (Map.Entry<Class<? extends RegionUpdateMessage>, MessageEncoder<? extends RegionUpdateMessage>> entry : regionUpdates
+		for (Map.Entry<Class<? extends RegionUpdateMessage>, MessageEncoder<? extends RegionUpdateMessage>> entry : regionUpdates.entrySet()) {
-				.entrySet()) {
 			@SuppressWarnings("unchecked")
 			Class<RegionUpdateMessage> clazz = (Class<RegionUpdateMessage>) entry.getKey();
 			@SuppressWarnings("unchecked")

src/org/apollo/update/ComparableChannelRequest.java

@@ -9,8 +9,7 @@ import io.netty.channel.Channel;
  *
  * @param <T> The type of request.
  */
-public final class ComparableChannelRequest<T extends Comparable<T>> extends ChannelRequest<T> implements
+public final class ComparableChannelRequest<T extends Comparable<T>> extends ChannelRequest<T> implements Comparable<ComparableChannelRequest<T>> {
-		Comparable<ComparableChannelRequest<T>> {
 
 	/**
 	 * Creates the ComparableChannelRequest.

src/org/apollo/update/OnDemandRequestWorker.java

@@ -5,7 +5,6 @@ import io.netty.buffer.Unpooled;
 import io.netty.channel.Channel;
 
 import java.io.IOException;
-import java.nio.ByteBuffer;
 
 import org.apollo.fs.FileDescriptor;
 import org.apollo.fs.IndexedFileSystem;
@@ -20,7 +19,7 @@ import org.apollo.net.codec.update.OnDemandResponse;
 public final class OnDemandRequestWorker extends RequestWorker<OnDemandRequest, IndexedFileSystem> {
 
 	/**
-	 * The maximum length of a chunk, in bytes.
+	 * The maximum length of a chunk, in {@code byte}s.
 	 */
 	private static final int CHUNK_LENGTH = 500;
 
@@ -41,19 +40,13 @@ public final class OnDemandRequestWorker extends RequestWorker<OnDemandRequest,
 
 	@Override
 	protected void service(IndexedFileSystem fs, Channel channel, OnDemandRequest request) throws IOException {
-		FileDescriptor desc = request.getFileDescriptor();
+		FileDescriptor descriptor = request.getFileDescriptor();
+		ByteBuf buffer = Unpooled.wrappedBuffer(fs.getFile(descriptor));
+		int length = buffer.readableBytes();
 
-		ByteBuffer buffer = fs.getFile(desc);
+		for (int chunk = 0; buffer.readableBytes() > 0; chunk++) {
-		int length = buffer.remaining();
+			int chunkSize = Math.min(buffer.readableBytes(), CHUNK_LENGTH);
-
+			OnDemandResponse response = new OnDemandResponse(descriptor, length, chunk, buffer.readBytes(chunkSize));
-		for (int chunk = 0; buffer.remaining() > 0; chunk++) {
-			int chunkSize = Math.min(buffer.remaining(), CHUNK_LENGTH);
-
-			byte[] data = new byte[chunkSize];
-			buffer.get(data, 0, data.length);
-			ByteBuf chunkData = Unpooled.wrappedBuffer(data, 0, chunkSize);
-
-			OnDemandResponse response = new OnDemandResponse(desc, length, chunk, chunkData);
 			channel.writeAndFlush(response);
 		}
 	}

src/org/apollo/util/MobRepository.java

@@ -1,7 +1,6 @@
 package org.apollo.util;
 
 import java.util.Iterator;
-import java.util.NoSuchElementException;
 
 import org.apollo.game.model.entity.Mob;
 
@@ -11,6 +10,7 @@ import com.google.common.base.Preconditions;
  * A {@link MobRepository} is a repository of {@link Mob}s that are currently active in the game world.
  * 
  * @author Graham
+ * @author Ryley
  * @param <T> The type of Mob.
  */
 public final class MobRepository<T extends Mob> implements Iterable<T> {
@@ -19,57 +19,57 @@ public final class MobRepository<T extends Mob> implements Iterable<T> {
 	 * The {@link Iterator} implementation for the MobRepository.
 	 * 
 	 * @author Graham
+	 * @author Ryley
 	 */
 	private final class MobRepositoryIterator implements Iterator<T> {
 
 		/**
-		 * The current index of this iterator.
+		 * The repository of {@link Mob}s this {@link Iterator} iterates over.
 		 */
-		private int index = 0;
+		private final MobRepository<T> repository;
 
 		/**
-		 * The previous index of this iterator.
+		 * The current index of this iterator.
 		 */
-		private int previousIndex = -1;
+		private int currentIndex;
+
+		/**
+		 * The amount of indexes found.
+		 */
+		private int foundIndex;
+
+		/**
+		 * Constructs a new {@link MobRepositoryIterator} with the specified MobRepository.
+		 * 
+		 * @param repository The repository of Mob's this Iterator iterates over.
+		 */
+		private MobRepositoryIterator(MobRepository<T> repository) {
+			this.repository = repository;
+		}
 
 		@Override
 		public boolean hasNext() {
-			for (int i = index; i < mobs.length; i++) {
+			if (foundIndex == repository.size()) {
-				if (mobs[i] != null) {
+				return false;
-					index = i;
+			}
+
+			while (currentIndex < repository.capacity()) {
+				if (mobs[currentIndex++] != null) {
+					foundIndex++;
 					return true;
 				}
 			}
 			return false;
 		}
 
-		@SuppressWarnings("unchecked")
 		@Override
 		public T next() {
-			T mob = null;
+			return repository.get(currentIndex);
-			for (int i = index; i < mobs.length; i++) {
-				if (mobs[i] != null) {
-					mob = (T) mobs[i];
-					index = i;
-					break;
-				}
-			}
-
-			if (mob == null) {
-				throw new NoSuchElementException("Mob does not exist.");
-			}
-
-			previousIndex = index;
-			index++;
-			return mob;
 		}
 
-		@SuppressWarnings("unchecked")
 		@Override
 		public void remove() {
-			Preconditions.checkState(previousIndex != -1, "Cannot remove as the repository is empty.");
+			repository.remove(currentIndex + 1);
-			MobRepository.this.remove((T) mobs[previousIndex]);
-			previousIndex = -1;
 		}
 
 	}
@@ -79,11 +79,6 @@ public final class MobRepository<T extends Mob> implements Iterable<T> {
 	 */
 	private final Mob[] mobs;
 
-	/**
-	 * The position of the next free index.
-	 */
-	private int pointer = 0;
-
 	/**
 	 * The current size of this repository.
 	 */
@@ -95,7 +90,7 @@ public final class MobRepository<T extends Mob> implements Iterable<T> {
 	 * @param capacity The maximum number of Mobs that can be present in the repository.
 	 */
 	public MobRepository(int capacity) {
-		this.mobs = new Mob[capacity];
+		mobs = new Mob[capacity];
 	}
 
 	/**
@@ -105,41 +100,23 @@ public final class MobRepository<T extends Mob> implements Iterable<T> {
 	 * @return {@code true} if the Mob was added, {@code false} if the size has reached the capacity of this repository.
 	 */
 	public boolean add(T mob) {
-		if (size == mobs.length) {
+		if (size == capacity()) {
 			return false;
 		}
 
-		int index = -1;
+		for (int index = 0; index < capacity(); index++) {
-		for (int i = pointer; i < mobs.length; i++) {
+			if (mobs[index] != null) {
-			if (mobs[i] == null) {
+				continue;
-				index = i;
-				break;
 			}
+
+			mobs[index] = mob;
+			mob.setIndex(index + 1);
+			size++;
+
+			return true;
 		}
 
-		if (index == -1) {
+		return false;
-			for (int i = 0; i < pointer; i++) {
-				if (mobs[i] == null) {
-					index = i;
-					break;
-				}
-			}
-		}
-
-		if (index == -1) {
-			return false; // shouldn't happen, but just in case
-		}
-
-		mobs[index] = mob;
-		mob.setIndex(index + 1);
-
-		if (index == mobs.length - 1) {
-			pointer = 0;
-		} else {
-			pointer = index;
-		}
-		size++;
-		return true;
 	}
 
 	/**
@@ -160,13 +137,15 @@ public final class MobRepository<T extends Mob> implements Iterable<T> {
 	 */
 	@SuppressWarnings("unchecked")
 	public T get(int index) {
-		Preconditions.checkElementIndex(index, mobs.length, "Mob index is out of bounds.");
+		if (index < 1 || index >= capacity() + 1) {
-		return (T) mobs[index];
+			throw new IndexOutOfBoundsException("Mob index is out of bounds.");
+		}
+		return (T) mobs[index - 1];
 	}
 
 	@Override
 	public Iterator<T> iterator() {
-		return new MobRepositoryIterator();
+		return new MobRepositoryIterator(this);
 	}
 
 	/**
@@ -176,18 +155,27 @@ public final class MobRepository<T extends Mob> implements Iterable<T> {
 	 * @return {@code true} if the Mob was removed, {@code false} if not.
 	 */
 	public boolean remove(T mob) {
-		int index = mob.getIndex() - 1;
+		Preconditions.checkNotNull(mob);
-		if (index < 0 || index >= mobs.length) {
+		return remove(mob.getIndex());
+	}
+
+	/**
+	 * Removes a Mob from the repository by the specified index.
+	 * 
+	 * @param index The index of the Mob to remove.
+	 * @return {@code true} if the Mob at the specified index was removed otherwise {@code false}.
+	 */
+	public boolean remove(int index) {
+		Mob mob = get(index);
+
+		if (mob.getIndex() != index) {
 			return false;
 		}
 
-		if (mobs[index] == mob) {
+		mobs[index - 1] = null;
-			mobs[index] = null;
+		mob.setIndex(-1);
-			mob.setIndex(-1);
+		size--;
-			size--;
+		return true;
-			return true;
-		}
-		return false;
 	}
 
 	/**

src/org/apollo/util/xml/XmlNode.java

@@ -146,14 +146,14 @@ public final class XmlNode implements Iterable<XmlNode> {
 		return value;
 	}
 
-    /**
+	/**
-     * Gets the value of this node, wrapped in an {@link Optional}
+	 * Gets the value of this node, wrapped in an {@link Optional}
-     *
+	 *
-     * @return The value of this node if it exists otherwise {@link Optional#empty()} is returned
+	 * @return The value of this node if it exists otherwise {@link Optional#empty()} is returned
-     */
+	 */
-    public Optional<String> getOptionalValue() {
+	public Optional<String> getOptionalValue() {
-        return Optional.ofNullable(value);
+		return Optional.ofNullable(value);
-    }
+	}
 
 	/**
 	 * Checks if this node has a value.