New remove method in Inventory

- Document transaction classes

data/plugins/skill/fishing/fishing.rb

@@ -68,7 +68,7 @@ class FishingAction < DistancedAction
       fish = options.sample # TODO: it's a ~70/30 chance, not 50/50
 
       if successful_catch(fishing_level, fish.level)
-        inventory.remove(bait) unless bait.nil?
+        inventory.remove(bait, 1) unless bait.nil?
         inventory.add(fish.id)
 
         name = fish.name

game/src/main/org/apollo/game/model/inv/Inventory.java

@@ -413,13 +413,18 @@ public final class Inventory {
 	}
 
 	/**
-	 * Removes one item with the specified id.
+	 * Removes the Item at the specified {@code slot}.
 	 *
-	 * @param id The id.
-	 * @return {@code true} if the item was removed, {@code false} otherwise.
+	 * @param slot The slot of the Item to remove.
+	 * @return The Item removed, wrapped in an Optional.
 	 */
-	public boolean remove(int id) {
-		return remove(id, 1) == 1;
+	public Optional<Item> remove(int slot) {
+		Item item = get(slot);
+		if (item != null) {
+			remove(item);
+			return Optional.of(item);
+		}
+		return Optional.empty();
 	}
 
 	/**
@@ -434,7 +439,7 @@ public final class Inventory {
 	public boolean remove(int... ids) {
 		boolean successful = true;
 		for (int id : ids) {
-			successful &= remove(id);
+			successful &= remove(id, 1) == 1;
 		}
 
 		return successful;

game/src/main/org/apollo/game/model/inv/transaction/ItemTransaction.java

@@ -3,48 +3,96 @@ package org.apollo.game.model.inv.transaction;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.List;
+import java.util.Optional;
 import java.util.Queue;
 import java.util.Set;
 
+import org.apollo.game.model.Item;
 import org.apollo.game.model.inv.Inventory;
 
+/**
+ * Represents a transaction of {@link Item}s.
+ */
 public final class ItemTransaction {
 
+	/**
+	 * A {@link Queue} of TransactionBlocks not yet committed.
+	 */
 	private final Queue<TransactionBlock> queuedBlocks = new ArrayDeque<>();
-	private final List<TransactionBlock> committedBlocks = new ArrayList<>();
-	private final Set<Inventory> inventories = new HashSet<>();
-	private TransactionListener listener;
 
+	/**
+	 * A {@link List} of TransactionBlocks that were successfully committed.
+	 */
+	private final List<TransactionBlock> committedBlocks = new ArrayList<>();
+
+	/**
+	 * A {@link Set} of {@link Inventory}s that were synchronized during this ItemTransaction.
+	 */
+	private final Set<Inventory> inventories = new HashSet<>();
+
+	/**
+	 * This transactions listener, wrapped in an {@link Optional}.
+	 */
+	private Optional<TransactionListener> listener = Optional.empty();
+
+	/**
+	 * Sets this ItemTransactions {@link TransactionListener}.
+	 *
+	 * @param listener The TransactionListener to set.
+	 */
 	public void setListener(TransactionListener listener) {
-		this.listener = listener;
+		this.listener = Optional.of(listener);
 	}
 
+	/**
+	 * Synchronizes the specified Inventory with this ItemTransaction.
+	 * 
+	 * @param inventory The Inventory to synchronize.
+	 */
 	public void sync(Inventory inventory) {
 		inventories.add(inventory);
 	}
 
+	/**
+	 * Queues the specified TransactionBlock to be committed.
+	 *
+	 * @param block The TransactionBlock to commit.
+	 */
 	public void append(TransactionBlock block) {
 		queuedBlocks.add(block);
 	}
 
+	/**
+	 * Refreshes all of the synchronized inventories.
+	 */
 	public void refresh() {
 		inventories.forEach(Inventory::forceRefresh);
 	}
 
+	/**
+	 * Reverts all of the committed TransactionBlocks and notifies failure.
+	 */
 	public void rollback() {
 		committedBlocks.forEach(TransactionBlock::revert);
-		listener.alertFailure();
+		listener.ifPresent(TransactionListener::alertFailure);
 	}
 
+	/**
+	 * Attempts to commit all of the {@link #queuedBlocks queued TransactionBlocks}.
+	 * 
+	 * @return {@code true} if we were able to successfully commit all of the
+	 *         TransactionBlocks, otherwise {@code false} and all previously
+	 *         committed TransactionBlocks are reverted back to their original
+	 *         state.
+	 */
 	public boolean commit() {
-		for (TransactionBlock change : queuedBlocks) {
-			if (!change.commit()) {
+		for (TransactionBlock block : queuedBlocks) {
+			if (!block.commit()) {
 				rollback();
 				return false;
 			}
-			committedBlocks.add(change);
+			committedBlocks.add(block);
 		}
 		refresh();
 		return true;

game/src/main/org/apollo/game/model/inv/transaction/SlottedTransactionItem.java

@@ -0,0 +1,68 @@
+package org.apollo.game.model.inv.transaction;
+
+import java.util.Optional;
+
+import org.apollo.game.model.Item;
+import org.apollo.game.model.inv.Inventory;
+
+/**
+ * A TransactionItem that lies within a slot in some Inventory.
+ */
+public final class SlottedTransactionItem extends TransactionItem {
+
+	/**
+	 * The slot of the Item.
+	 */
+	private final int slot;
+
+	/**
+	 * The Item.
+	 */
+	private Optional<Item> removedItem = Optional.empty();
+
+	/**
+	 * Creates a new {@link SlottedTransactionItem}.
+	 * 
+	 * @param inventory The Inventory this TransactionItem is within.
+	 * @param slot The slot of the Item.
+	 */
+	public SlottedTransactionItem(Inventory inventory, int slot) {
+		super(inventory);
+		this.slot = slot;
+	}
+
+	@Override
+	public boolean deleteSuccessful() {
+		return removedItem.isPresent();
+	}
+
+	/**
+	 * Updates the Item in the specified {@code slot} with the specified Item.
+	 * 
+	 * @param item The new Item.
+	 * @return The old Item.
+	 */
+	public Item update(Item item) {
+		Item removed = inventory.set(slot, item);
+		return removed;
+	}
+
+	@Override
+	public Item delete() {
+		if (removedItem.isPresent()) {
+			throw new IllegalStateException("Item already deleted.");
+		}
+		removedItem = inventory.remove(slot);
+		return removedItem.get();
+	}
+
+	@Override
+	public void revert() {
+		if (!removedItem.isPresent()) {
+			throw new IllegalStateException("No changes to revert.");
+		}
+		inventory.set(slot, removedItem.get());
+		removedItem = Optional.empty();
+	}
+
+}

game/src/main/org/apollo/game/model/inv/transaction/StackedTransactionItem.java

@@ -0,0 +1,61 @@
+package org.apollo.game.model.inv.transaction;
+
+import java.util.Optional;
+
+import org.apollo.game.model.Item;
+import org.apollo.game.model.inv.Inventory;
+
+/**
+ * A TransactionItem that is stacked within an Inventory.
+ */
+public final class StackedTransactionItem extends TransactionItem {
+
+	/**
+	 * The stacked Item.
+	 */
+	private final Item item;
+
+	/**
+	 * The removed Item.
+	 */
+	private Optional<Item> removedItem = Optional.empty();
+
+	/**
+	 * Creates a new {@link StackedTransactionItem}.
+	 *
+	 * @param inventory The Inventory this TransactionItem is within.
+	 * @param item The stacked Item.
+	 */
+	public StackedTransactionItem(Inventory inventory, Item item) {
+		super(inventory);
+		this.item = item;
+	}
+
+	@Override
+	public boolean deleteSuccessful() {
+		if (!removedItem.isPresent()) {
+			return false;
+		}
+		return removedItem.get().equals(item);
+	}
+
+	@Override
+	public Item delete() {
+		if (removedItem.isPresent()) {
+			throw new IllegalStateException("Item already deleted.");
+		}
+		int slot = inventory.slotOf(item.getId());
+		removedItem = inventory.remove(slot);
+		return removedItem.get();
+	}
+
+	@Override
+	public void revert() {
+		if (!removedItem.isPresent()) {
+			throw new IllegalStateException("No changes to revert.");
+		}
+		inventory.add(removedItem.get());
+		removedItem = Optional.empty();
+	}
+
+}

game/src/main/org/apollo/game/model/inv/transaction/TransactionBlock.java

@@ -1,9 +1,20 @@
 package org.apollo.game.model.inv.transaction;
 
+/**
+ * Represents a single transaction block in an {@link ItemTransaction}.
+ */
 public interface TransactionBlock {
 
+	/**
+	 * Attempts to commit changes made in this transaction.
+	 * 
+	 * @return {@code true} iff this transaction succeeds, otherwise {@code false}.
+	 */
 	boolean commit();
 
+	/**
+	 * Reverts this transaction back to its original state.
+	 */
 	void revert();
 
 }

game/src/main/org/apollo/game/model/inv/transaction/TransactionBuilder.java

@@ -10,8 +10,8 @@ public final class TransactionBuilder {
 	private final Inventory inventory;
 
 	private TransactionBuilder(Inventory inventory) {
-		transaction.sync(inventory);
 		this.inventory = inventory;
+		transaction.sync(inventory);
 	}
 
 	public TransactionBuilder failureMessage(Player player, String message) {

game/src/main/org/apollo/game/model/inv/transaction/TransactionFailureListener.java

@@ -2,11 +2,27 @@ package org.apollo.game.model.inv.transaction;
 
 import org.apollo.game.model.entity.Player;
 
+/**
+ * A TransactionListener which listens for failure events.
+ */
 public final class TransactionFailureListener implements TransactionListener {
 
+	/**
+	 * The Player.
+	 */
 	private final Player player;
+
+	/**
+	 * The message sent upon failure.
+	 */
 	private final String message;
 
+	/**
+	 * Creates a new {@link TransactionFailureListener}.
+	 * 
+	 * @param player The Player.
+	 * @param message The message sent upon failure.
+	 */
 	public TransactionFailureListener(Player player, String message) {
 		this.player = player;
 		this.message = message;

game/src/main/org/apollo/game/model/inv/transaction/TransactionItem.java

@@ -3,18 +3,44 @@ package org.apollo.game.model.inv.transaction;
 import org.apollo.game.model.Item;
 import org.apollo.game.model.inv.Inventory;
 
+/**
+ * Represents an Item during an ItemTransaction.
+ */
 public abstract class TransactionItem {
 
+	/**
+	 * The Inventory this TransactionItem is within.
+	 */
 	protected final Inventory inventory;
 
+	/**
+	 * Creates a new {@link TransactionItem}.
+	 *
+	 * @param inventory The Inventory this TransactionItem is within.
+	 */
 	protected TransactionItem(Inventory inventory) {
 		this.inventory = inventory;
 	}
 
-	public abstract Item delete();
+	/**
+	 * Performs a transaction which moves, adds, swaps or deletes an Item from
+	 * the Inventory.
+	 * 
+	 * @return The Item after the ItemTransaction has occurred.
+	 */
+	public abstract Item delete(); // TODO: Better name
 
-	public abstract boolean deleteSuccessful();
+	/**
+	 * Tests whether or not the transaction was successful.
+	 * 
+	 * @return {@code true} if this ItemTransaction was successful, otherwise
+	 *         {@code false}.
+	 */
+	public abstract boolean deleteSuccessful(); // TODO: Better name
 
+	/**
+	 * Reverts this ItemTransaction back to its original state.
+	 */
 	public abstract void revert();
 
 }

game/src/main/org/apollo/game/model/inv/transaction/TransactionListener.java

@@ -1,7 +1,13 @@
 package org.apollo.game.model.inv.transaction;
 
+/**
+ * Listens for events from an ItemTransaction.
+ */
 public interface TransactionListener {
 
+	/**
+	 * Called when an ItemTransaction has failed.
+	 */
 	void alertFailure();
 
 }