Minor refactoring of class chests handling. Fixes #318

src/main/java/com/garbagemule/MobArena/ArenaListener.java

@@ -3,6 +3,7 @@ package com.garbagemule.MobArena;
 import java.util.*;
 
 import com.garbagemule.MobArena.events.ArenaKillEvent;
+import com.garbagemule.MobArena.util.ClassChests;
 import org.bukkit.ChatColor;
 import org.bukkit.Location;
 import org.bukkit.Material;
@@ -1062,52 +1063,11 @@ public class ArenaListener
             public void run() {
                 if (!className.equalsIgnoreCase("random")) {
                     if (useClassChests) {
-                        // Check for stored class chests first
                         ArenaClass ac = plugin.getArenaMaster().getClasses().get(className.toLowerCase());
-                        Location loc = ac.getClassChest();
+                        if (ClassChests.assignClassFromStoredClassChest(arena, p, ac)) {
-                        Block blockChest;
+                            return;
-                        if (loc != null) {
-                            blockChest = loc.getBlock();
-                        } else {
-                            // Otherwise, start the search
-                            BlockFace backwards = ((org.bukkit.material.Sign) sign.getData()).getFacing().getOppositeFace();
-                            Block blockSign   = sign.getBlock();
-                            Block blockBelow  = blockSign.getRelative(BlockFace.DOWN);
-                            Block blockBehind = blockBelow.getRelative(backwards);
-
-                            // If the block below this sign is a class sign, swap the order
-                            if (blockBelow.getType() == Material.WALL_SIGN || blockBelow.getType() == Material.SIGN_POST) {
-                                String className = ChatColor.stripColor(((Sign) blockBelow.getState()).getLine(0)).toLowerCase();
-                                if (arena.getClasses().containsKey(className)) {
-                                    blockSign = blockBehind;  // Use blockSign as a temp while swapping
-                                    blockBehind = blockBelow;
-                                    blockBelow = blockSign;
-                                }
-                            }
-
-                            // TODO: Make number of searches configurable
-                            // First check the pillar below the sign
-                            blockChest = findChestBelow(blockBelow, 6);
-
-                            // Then, if no chest was found, check the pillar behind the sign
-                            if (blockChest == null) blockChest = findChestBelow(blockBehind, 6);
                         }
-                        
+                        if (ClassChests.assignClassFromClassChestSearch(arena, p, ac, sign)) {
-                        // If a chest was found, get the contents
-                        if (blockChest != null) {
-                            InventoryHolder holder = (InventoryHolder) blockChest.getState();
-                            ItemStack[] contents = holder.getInventory().getContents();
-                            // Guard against double-chests for now
-                            if (contents.length > 36) {
-                                ItemStack[] newContents = new ItemStack[36];
-                                System.arraycopy(contents, 0, newContents, 0, 36);
-                                contents = newContents;
-                            }
-                            arena.assignClassGiveInv(p, className, contents);
-                            Messenger.tell(p, Msg.LOBBY_CLASS_PICKED, TextUtils.camelCase(className));
-                            if (price > 0D) {
-                                Messenger.tell(p, Msg.LOBBY_CLASS_PRICE,  plugin.economyFormat(price));
-                            }
                             return;
                         }
                         // Otherwise just fall through and use the items from the config-file
@@ -1125,15 +1085,6 @@ public class ArenaListener
             }
         });
     }
-    
-    private Block findChestBelow(Block b, int left) {
-        if (left < 0) return null;
-        
-        if (b.getType() == Material.CHEST || b.getType() == Material.TRAPPED_CHEST) {
-            return b;
-        }
-        return findChestBelow(b.getRelative(BlockFace.DOWN), left - 1);
-    }
 
     public void onPlayerQuit(PlayerQuitEvent event) {
         Player p = event.getPlayer();

src/main/java/com/garbagemule/MobArena/commands/user/PickClassCommand.java

@@ -9,6 +9,7 @@ import com.garbagemule.MobArena.commands.CommandInfo;
 import com.garbagemule.MobArena.commands.Commands;
 import com.garbagemule.MobArena.framework.Arena;
 import com.garbagemule.MobArena.framework.ArenaMaster;
+import com.garbagemule.MobArena.util.ClassChests;
 import com.garbagemule.MobArena.util.TextUtils;
 import org.bukkit.Location;
 import org.bukkit.block.Block;
@@ -89,22 +90,7 @@ public class PickClassCommand implements Command
 
         if (!lowercase.equalsIgnoreCase("random")) {
             if (arena.getSettings().getBoolean("use-class-chests", false)) {
-                Location loc = ac.getClassChest();
+                if (ClassChests.assignClassFromStoredClassChest(arena, p, ac)) {
-                if (loc != null) {
-                    Block blockChest = loc.getBlock();
-                    InventoryHolder holder = (InventoryHolder) blockChest.getState();
-                    ItemStack[] contents = holder.getInventory().getContents();
-                    // Guard against double-chests for now
-                    if (contents.length > 36) {
-                        ItemStack[] newContents = new ItemStack[36];
-                        System.arraycopy(contents, 0, newContents, 0, 36);
-                        contents = newContents;
-                    }
-                    arena.assignClassGiveInv(p, lowercase, contents);
-                    Messenger.tell(p, Msg.LOBBY_CLASS_PICKED, TextUtils.camelCase(lowercase));
-                    if (price > 0D) {
-                        Messenger.tell(p, Msg.LOBBY_CLASS_PRICE, am.getPlugin().economyFormat(price));
-                    }
                     return true;
                 }
                 // No linked chest? Fall through to config-file

src/main/java/com/garbagemule/MobArena/util/ClassChests.java

@@ -0,0 +1,108 @@
+package com.garbagemule.MobArena.util;
+
+import com.garbagemule.MobArena.ArenaClass;
+import com.garbagemule.MobArena.Messenger;
+import com.garbagemule.MobArena.Msg;
+import com.garbagemule.MobArena.framework.Arena;
+import org.bukkit.ChatColor;
+import org.bukkit.Location;
+import org.bukkit.Material;
+import org.bukkit.block.Block;
+import org.bukkit.block.BlockFace;
+import org.bukkit.block.Sign;
+import org.bukkit.entity.Player;
+import org.bukkit.inventory.InventoryHolder;
+import org.bukkit.inventory.ItemStack;
+
+public class ClassChests {
+
+    public static boolean assignClassFromStoredClassChest(Arena arena, Player player, ArenaClass ac) {
+        if (!arena.getSettings().getBoolean("use-class-chests", false)) {
+            return false;
+        }
+
+        Location loc = ac.getClassChest();
+        if (loc == null) {
+            return false;
+        }
+
+        Block block = loc.getBlock();
+        if (!(block.getState() instanceof InventoryHolder)) {
+            Messenger.warning("Class chest location for class '" + ac.getConfigName() + "' is not a chest!");
+            return false;
+        }
+
+        assignClassAndGrantChestItems(arena, player, ac, block);
+        return true;
+    }
+
+    public static boolean assignClassFromClassChestSearch(Arena arena, Player player, ArenaClass ac, Sign sign) {
+        if (!arena.getSettings().getBoolean("use-class-chests", false)) {
+            return false;
+        }
+
+        // Start the search
+        BlockFace backwards = ((org.bukkit.material.Sign) sign.getData()).getFacing().getOppositeFace();
+        Block blockSign   = sign.getBlock();
+        Block blockBelow  = blockSign.getRelative(BlockFace.DOWN);
+        Block blockBehind = blockBelow.getRelative(backwards);
+
+        // If the block below this sign is a class sign, swap the order
+        if (blockBelow.getType() == Material.WALL_SIGN || blockBelow.getType() == Material.SIGN_POST) {
+            String className = ChatColor.stripColor(((Sign) blockBelow.getState()).getLine(0)).toLowerCase();
+            if (arena.getClasses().containsKey(className)) {
+                blockSign = blockBehind;  // Use blockSign as a temp while swapping
+                blockBehind = blockBelow;
+                blockBelow = blockSign;
+            }
+        }
+
+        // TODO: Make number of searches configurable
+        // First check the pillar below the sign
+        Block block = findChestBelow(blockBelow, 6);
+
+        // Then, if no chest was found, check the pillar behind the sign
+        if (block == null) {
+            block = findChestBelow(blockBehind, 6);
+        }
+
+        // If it's still null, we have no class chest
+        if (block == null) {
+            return false;
+        }
+
+        assignClassAndGrantChestItems(arena, player, ac, block);
+        return true;
+    }
+
+    private static Block findChestBelow(Block b, int left) {
+        if (left < 0) {
+            return null;
+        }
+        if (b.getType() == Material.CHEST || b.getType() == Material.TRAPPED_CHEST) {
+            return b;
+        }
+        return findChestBelow(b.getRelative(BlockFace.DOWN), left - 1);
+    }
+
+    private static void assignClassAndGrantChestItems(Arena arena, Player player, ArenaClass ac, Block block) {
+        String classname = ac.getLowercaseName();
+        InventoryHolder holder = (InventoryHolder) block.getState();
+        ItemStack[] contents = holder.getInventory().getContents();
+
+        // Guard against double-chests for now
+        if (contents.length > 36) {
+            ItemStack[] newContents = new ItemStack[36];
+            System.arraycopy(contents, 0, newContents, 0, 36);
+            contents = newContents;
+        }
+        arena.assignClassGiveInv(player, classname, contents);
+        Messenger.tell(player, Msg.LOBBY_CLASS_PICKED, TextUtils.camelCase(classname));
+
+        double price = ac.getPrice();
+        if (price > 0D) {
+            Messenger.tell(player, Msg.LOBBY_CLASS_PRICE, arena.getPlugin().economyFormat(price));
+        }
+    }
+
+}