Fishing refactoring, fixed bad enchant distribution

Changelog.txt

@@ -37,6 +37,7 @@ Version 1.4.00-dev
  = Fixed a bug where /party kick would trigger the PartyChangeEvent for the wrong player
  = Fixed a bug where party join messages weren't displayed
  = Fixed a bug where Disarm and Deflect had wrong values
+ = Fixed Magic Hunter (Fishing ability) favoring certain enchants
  ! We're now using Bukkit sounds instead of Spout sounds.
  ! It is now possible to use a negative number for Max_Level in treasures.yml to not use a maximum level, changed default file accordingly
  ! A Fishing catch will now always contains a fish even if a treasure is found

src/main/java/com/gmail/nossr50/listeners/EntityListener.java

@@ -320,11 +320,11 @@ public class EntityListener implements Listener {
                     break;
 
                 case COOKED_FISH:   /* RESTORES 2 1/2 HUNGER - RESTORES 5 HUNGER @ 1000 */
-                    Fishing.fishermansDiet(player, Fishing.fishermansDietRankLevel1, event);
+                    Fishing.beginFishermansDiet(player, Fishing.fishermansDietRankLevel1, event);
                     break;
 
                 case RAW_FISH:      /* RESTORES 1 HUNGER - RESTORES 2 1/2 HUNGER @ 1000 */
-                    Fishing.fishermansDiet(player, Fishing.fishermansDietRankLevel2, event);
+                    Fishing.beginFishermansDiet(player, Fishing.fishermansDietRankLevel2, event);
                     break;
 
                 default:

src/main/java/com/gmail/nossr50/listeners/PlayerListener.java

@@ -2,6 +2,7 @@ package com.gmail.nossr50.listeners;
 
 import org.bukkit.Material;
 import org.bukkit.block.Block;
+import org.bukkit.entity.LivingEntity;
 import org.bukkit.entity.Player;
 import org.bukkit.event.EventHandler;
 import org.bukkit.event.EventPriority;
@@ -10,7 +11,6 @@ import org.bukkit.event.player.AsyncPlayerChatEvent;
 import org.bukkit.event.player.PlayerChangedWorldEvent;
 import org.bukkit.event.player.PlayerCommandPreprocessEvent;
 import org.bukkit.event.player.PlayerFishEvent;
-import org.bukkit.event.player.PlayerFishEvent.State;
 import org.bukkit.event.player.PlayerInteractEvent;
 import org.bukkit.event.player.PlayerJoinEvent;
 import org.bukkit.event.player.PlayerLoginEvent;
@@ -19,6 +19,7 @@ import org.bukkit.event.player.PlayerRespawnEvent;
 import org.bukkit.inventory.ItemStack;
 
 import com.gmail.nossr50.mcMMO;
+import com.gmail.nossr50.config.AdvancedConfig;
 import com.gmail.nossr50.config.Config;
 import com.gmail.nossr50.datatypes.PlayerProfile;
 import com.gmail.nossr50.locale.LocaleLoader;
@@ -82,25 +83,25 @@ public class PlayerListener implements Listener {
     public void onPlayerFish(PlayerFishEvent event) {
         Player player = event.getPlayer();
 
-        if (Misc.isNPCPlayer(player)) {
+        if (Misc.isNPCPlayer(player) || !Permissions.fishing(player)) {
             return;
         }
 
-        if (Permissions.fishing(player)) {
-            State state = event.getState();
-
-            switch (state) {
-            case CAUGHT_FISH:
-                Fishing.processResults(event);
-                break;
+        int skillLevel = Users.getProfile(player).getSkillLevel(SkillType.FISHING);
 
-            case CAUGHT_ENTITY:
-                Fishing.shakeMob(event);
-                break;
-
-            default:
-                break;
+        switch (event.getState()) {
+        case CAUGHT_FISH:
+            Fishing.beginFishing(player, skillLevel, event);
+              break;
+        case CAUGHT_ENTITY:
+            if (skillLevel >= AdvancedConfig.getInstance().getShakeUnlockLevel() && Permissions.shakeMob(player)) {
+                //TODO: Unsafe cast?
+                Fishing.beginShakeMob(player, (LivingEntity) event.getCaught(), skillLevel);
             }
+
+            break;
+        default:
+            break;
         }
     }
 

src/main/java/com/gmail/nossr50/skills/fishing/Fishing.java

@@ -1,35 +1,22 @@
 package com.gmail.nossr50.skills.fishing;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
-import org.bukkit.DyeColor;
-import org.bukkit.Location;
-import org.bukkit.Material;
 import org.bukkit.enchantments.Enchantment;
-import org.bukkit.entity.Entity;
-import org.bukkit.entity.EntityType;
 import org.bukkit.entity.Item;
 import org.bukkit.entity.LivingEntity;
 import org.bukkit.entity.Player;
-import org.bukkit.entity.Sheep;
-import org.bukkit.entity.Skeleton;
-import org.bukkit.entity.Skeleton.SkeletonType;
 import org.bukkit.event.entity.FoodLevelChangeEvent;
 import org.bukkit.event.player.PlayerFishEvent;
 import org.bukkit.inventory.ItemStack;
-import org.bukkit.material.MaterialData;
-import org.bukkit.material.Wool;
-import org.bukkit.potion.Potion;
-import org.bukkit.potion.PotionType;
 
 import com.gmail.nossr50.config.AdvancedConfig;
 import com.gmail.nossr50.config.Config;
 import com.gmail.nossr50.config.TreasuresConfig;
-import com.gmail.nossr50.datatypes.PlayerProfile;
 import com.gmail.nossr50.datatypes.treasure.FishingTreasure;
 import com.gmail.nossr50.locale.LocaleLoader;
-import com.gmail.nossr50.skills.Combat;
 import com.gmail.nossr50.skills.SkillType;
 import com.gmail.nossr50.skills.SkillTools;
 import com.gmail.nossr50.util.ItemChecks;
@@ -37,481 +24,213 @@ import com.gmail.nossr50.util.Misc;
 import com.gmail.nossr50.util.Permissions;
 import com.gmail.nossr50.util.Users;
 
-public class Fishing {
-    public static int fishingTierLevel1 = AdvancedConfig.getInstance().getFishingTierLevelsTier1();
-    public static int fishingTierLevel2 = AdvancedConfig.getInstance().getFishingTierLevelsTier2();
-    public static int fishingTierLevel3 = AdvancedConfig.getInstance().getFishingTierLevelsTier3();
-    public static int fishingTierLevel4 = AdvancedConfig.getInstance().getFishingTierLevelsTier4();
-    public static int fishingTierLevel5 = AdvancedConfig.getInstance().getFishingTierLevelsTier5();
-
-    public static int shakeChanceLevel1 = AdvancedConfig.getInstance().getShakeChanceRank1();
-    public static int shakeChanceLevel2 = AdvancedConfig.getInstance().getShakeChanceRank2();
-    public static int shakeChanceLevel3 = AdvancedConfig.getInstance().getShakeChanceRank3();
-    public static int shakeChanceLevel4 = AdvancedConfig.getInstance().getShakeChanceRank4();
-    public static int shakeChanceLevel5 = AdvancedConfig.getInstance().getShakeChanceRank5();
-    public static int shakeUnlockLevel = AdvancedConfig.getInstance().getShakeUnlockLevel();
-
-    public static int fishermansDietRankLevel1 = AdvancedConfig.getInstance().getFishermanDietRankChange();
+public final class Fishing {
+    static final AdvancedConfig ADVANCED_CONFIG = AdvancedConfig.getInstance();
+
+    // The order of the values is extremely important, Fishing.getLootTier() and ShakeMob.getShakeChance() depend on it to work properly
+    protected enum Tier {
+        FIVE(5) {@Override public int getLevel() {return ADVANCED_CONFIG.getFishingTierLevelsTier5();} @Override public int getShakeChance() {return ADVANCED_CONFIG.getShakeChanceRank5();}},
+        FOUR(4) {@Override public int getLevel() {return ADVANCED_CONFIG.getFishingTierLevelsTier4();} @Override public int getShakeChance() {return ADVANCED_CONFIG.getShakeChanceRank4();}},
+        THREE(3) {@Override public int getLevel() {return ADVANCED_CONFIG.getFishingTierLevelsTier3();} @Override public int getShakeChance() {return ADVANCED_CONFIG.getShakeChanceRank3();}},
+        TWO(2) {@Override public int getLevel() {return ADVANCED_CONFIG.getFishingTierLevelsTier2();} @Override public int getShakeChance() {return ADVANCED_CONFIG.getShakeChanceRank2();}},
+        ONE(1) {@Override public int getLevel() {return ADVANCED_CONFIG.getFishingTierLevelsTier1();} @Override public int getShakeChance() {return ADVANCED_CONFIG.getShakeChanceRank1();}};
+
+        int numerical;
+
+        private Tier(int numerical) {
+            this.numerical = numerical;
+        }
+
+        public int toNumerical() {
+            return numerical;
+        }
+
+        abstract protected int getLevel();
+        abstract protected int getShakeChance();
+    }
+
+    // TODO: Get rid of that
+    public static int fishermansDietRankLevel1 = ADVANCED_CONFIG.getFishermanDietRankChange();
     public static int fishermansDietRankLevel2 = fishermansDietRankLevel1 * 2;
     public static int fishermansDietMaxLevel = fishermansDietRankLevel1 * 5;
 
-    public static int magicHunterMultiplier = AdvancedConfig.getInstance().getFishingMagicMultiplier();
+    private Fishing() {}
 
-    public static void fishermansDiet(Player player, int rankChange, FoodLevelChangeEvent event) {
+    /**
+     * Begins Fisherman's Diet ability
+     *
+     * @param player Player using the ability
+     * @param rankChange ???
+     * @param event Event to process
+     */
+    public static void beginFishermansDiet(Player player, int rankChange, FoodLevelChangeEvent event) {
+        // TODO: The permission should probably not be checked here
+        // TODO: Also I don't like the idea of moving event around
         if (!Permissions.fishermansDiet(player)) {
             return;
         }
-    
+
         SkillTools.handleFoodSkills(player, SkillType.FISHING, event, fishermansDietRankLevel1, fishermansDietMaxLevel, rankChange);
     }
 
     /**
-     * Get the player's current fishing loot tier.
-     * 
-     * @param profile
-     *            The profile of the player
-     * @return the player's current fishing rank
+     * Begins Shake Mob ability
+     *
+     * @param player Player using the ability
+     * @param mob Targeted mob
+     * @param skillLevel Fishing level of the player
      */
-    public static int getFishingLootTier(PlayerProfile profile) {
-        int level = profile.getSkillLevel(SkillType.FISHING);
-        int fishingTier;
-
-        if (level >= fishingTierLevel5) {
-            fishingTier = 5;
-        } else if (level >= fishingTierLevel4) {
-            fishingTier = 4;
-        } else if (level >= fishingTierLevel3) {
-            fishingTier = 3;
-        } else if (level >= fishingTierLevel2) {
-            fishingTier = 2;
-        } else {
-            fishingTier = 1;
-        }
-
-        return fishingTier;
+    public static void beginShakeMob(Player player, LivingEntity mob, int skillLevel) {
+        ShakeMob.process(player, mob, skillLevel);
     }
 
     /**
-     * Get item results from Fishing.
-     * 
-     * @param player
-     *            The player that was fishing
-     * @param event
-     *            The event to modify
+     * Begins Fishing
+     *
+     * @param player Player fishing
+     * @param skillLevel Fishing level of the player
+     * @param event Event to process
      */
-    private static void getFishingResults(Player player, PlayerFishEvent event) {
-        if (player == null)
-            return;
-
-        PlayerProfile profile = Users.getProfile(player);
-        Item theCatch = (Item) event.getCaught();
-
-        if (Config.getInstance().getFishingDropsEnabled() && Permissions.fishingTreasures(player)) {
-            int skillLevel = profile.getSkillLevel(SkillType.FISHING);
-            List<FishingTreasure> rewards = new ArrayList<FishingTreasure>();
+    public static void beginFishing(Player player, int skillLevel, PlayerFishEvent event) {
+        // TODO: Find a way to not pass the event directly
+        int xp = 0;
+        FishingTreasure treasure = checkForTreasure(player, skillLevel);
 
-            for (FishingTreasure treasure : TreasuresConfig.getInstance().fishingRewards) {
-                int maxLevel = treasure.getMaxLevel();
-
-                if (treasure.getDropLevel() <= skillLevel && (maxLevel >= skillLevel || maxLevel <= 0)) {
-                    rewards.add(treasure);
-                }
-            }
-
-            if (rewards.isEmpty()) {
-                return;
-            }
+        if (treasure != null) {
+            player.sendMessage(LocaleLoader.getString("Fishing.ItemFound"));
 
-            FishingTreasure treasure = rewards.get(Misc.getRandom().nextInt(rewards.size()));
+            xp += treasure.getXp();
             ItemStack treasureDrop = treasure.getDrop();
 
-            int activationChance = Misc.calculateActivationChance(Permissions.luckyFishing(player));
-
-            if (Misc.getRandom().nextDouble() * activationChance <= treasure.getDropChance()) {
-                player.getWorld().dropItem(player.getEyeLocation(), theCatch.getItemStack());  // Drop the original item
-
-                short maxDurability = treasureDrop.getType().getMaxDurability();
-
-                if (maxDurability > 0) {
-                    treasureDrop.setDurability((short) (Misc.getRandom().nextInt(maxDurability))); // Change durability to random value
-                }
-
-                theCatch.setItemStack(treasureDrop);
-                Users.getPlayer(player).addXP(SkillType.FISHING, treasure.getXp());
+            if (Permissions.fishingMagic(player) && beginMagicHunter(player, skillLevel, treasureDrop, player.getWorld().hasStorm())) {
+                player.sendMessage(LocaleLoader.getString("Fishing.MagicFound"));
             }
+
+            // Drop the original catch at the feet of the player and set the treasure as the real catch
+            Item caught = (Item) event.getCaught();
+            Misc.dropItem(player.getEyeLocation(), caught.getItemStack());
+            caught.setItemStack(treasureDrop);
         }
 
-        SkillTools.xpProcessing(player, profile, SkillType.FISHING, Config.getInstance().getFishingBaseXP());
+        SkillTools.xpProcessing(player, Users.getProfile(player), SkillType.FISHING, Config.getInstance().getFishingBaseXP() + xp);
     }
 
     /**
-     * Process results from Fishing.
-     * 
-     * @param event
-     *            The event to modify
+     * Checks for treasure
+     *
+     * @param player Player fishing
+     * @param skillLevel Fishing level of the player
+     * @return Chosen treasure
      */
-    public static void processResults(PlayerFishEvent event) {
-        Player player = event.getPlayer();
-
-        getFishingResults(player, event);
-        Item theCatch = (Item) event.getCaught();
-
-        if (theCatch.getItemStack().getType() != Material.RAW_FISH) {
-            int lootTier = Fishing.getFishingLootTier(Users.getProfile(player));
-            int specificChance = 1;
-            boolean enchanted = false;
-            ItemStack fishingResults = theCatch.getItemStack();
-
-            player.sendMessage(LocaleLoader.getString("Fishing.ItemFound"));
-
-            if (ItemChecks.isEnchantable(fishingResults)) {
-                int activationChance = Misc.calculateActivationChance(Permissions.luckyFishing(player));
+    private static FishingTreasure checkForTreasure(Player player, int skillLevel) {
+        if (!Config.getInstance().getFishingDropsEnabled() || !Permissions.fishingTreasures(player)) {
+            return null;
+        }
 
-                if (player.getWorld().hasStorm()) {
-                    activationChance = (int) (activationChance * 0.909);
-                }
+        List<FishingTreasure> rewards = new ArrayList<FishingTreasure>();
 
-                /* CHANCE OF ITEM BEING ENCHANTED
-                 * 5% - Tier 1
-                 * 10% - Tier 2
-                 * 15% - Tier 3
-                 * 20% - Tier 4
-                 * 25% - Tier 5
-                 */
-                if (Misc.getRandom().nextInt(activationChance) <= (lootTier * magicHunterMultiplier) && Permissions.fishingMagic(player)) {
-                    for (Enchantment newEnchant : Enchantment.values()) {
-                        boolean conflicts = false;
-
-                        if (newEnchant.canEnchantItem(fishingResults)) {
-                            specificChance++;
-
-                            for (Enchantment oldEnchant : fishingResults.getEnchantments().keySet()) {
-                                conflicts = oldEnchant.conflictsWith(newEnchant);
-
-                                if (conflicts) {
-                                    specificChance--;
-                                    break;
-                                }
-                            }
-
-                            /* CHANCE OF GETTING EACH ENCHANTMENT
-                             * 50% - 1st Enchantment
-                             * 33% - 2nd Enchantment
-                             * 25% - 3rd Enchantment
-                             * 20% - 4th Enchantment
-                             * 16.66% - 5th Enchantment
-                             * 14.29% - 6th Enchantment
-                             * 12.5% - 7th Enchantment
-                             * 11.11% - 8th Enchantment
-                             */
-                            if (!conflicts && Misc.getRandom().nextInt(specificChance) < 1) {
-                                enchanted = true;
-                                int randomEnchantLevel = Misc.getRandom().nextInt(newEnchant.getMaxLevel()) + 1;
-
-                                if (randomEnchantLevel < newEnchant.getStartLevel()) {
-                                    randomEnchantLevel = newEnchant.getStartLevel();
-                                }
-
-
-                                fishingResults.addEnchantment(newEnchant, randomEnchantLevel);
-                            }
-                        }
-                    }
-                }
-            }
+        for (FishingTreasure treasure : TreasuresConfig.getInstance().fishingRewards) {
+            int maxLevel = treasure.getMaxLevel();
 
-            if (enchanted) {
-                player.sendMessage(LocaleLoader.getString("Fishing.MagicFound"));
+            if (treasure.getDropLevel() <= skillLevel && (maxLevel >= skillLevel || maxLevel <= 0)) {
+                rewards.add(treasure);
             }
         }
-    }
 
-    /**
-     * Shake a mob, have them drop an item.
-     * 
-     * @param event
-     *            The event to modify
-     */
-    public static void shakeMob(PlayerFishEvent event) {
-        Entity caughtEntity = event.getCaught();
-
-        if (!(caughtEntity instanceof LivingEntity)) {
-            return;
+        if (rewards.isEmpty()) {
+            return null;
         }
 
-        Player player = event.getPlayer();
+        FishingTreasure treasure = rewards.get(Misc.getRandom().nextInt(rewards.size()));
+        ItemStack treasureDrop = treasure.getDrop();
+        int activationChance = Misc.calculateActivationChance(Permissions.luckyFishing(player));
 
-        if (Users.getProfile(player).getSkillLevel(SkillType.FISHING) < Fishing.shakeUnlockLevel || !Permissions.shakeMob(player)) {
-            return;
+        if (Misc.getRandom().nextDouble() * activationChance > treasure.getDropChance()) {
+            return null;
         }
 
-        int randomChance = 100;
+        short maxDurability = treasureDrop.getType().getMaxDurability();
 
-        //TODO: Invert this so it matches the rest of our lucky checks...
-        if (Permissions.luckyFishing(event.getPlayer())) {
-            randomChance = 125;
+        if (maxDurability > 0) {
+            treasureDrop.setDurability((short) (Misc.getRandom().nextInt(maxDurability)));
         }
 
-        
-        final PlayerProfile profile = Users.getProfile(player);
-        int lootTier = getFishingLootTier(profile);
+        return treasure;
+    }
 
-        int dropChance = getShakeChance(lootTier);
-
-        if (Permissions.luckyFishing(player)) {
-            // With lucky perk on max level tier, its 100%
-            dropChance = (int) (dropChance * 1.25);
+    /**
+     * Processes for treasure
+     *
+     * @param player Player fishing
+     * @param skillLevel Fishing level of the player
+     * @param itemStack ItemStack to enchant
+     * @param storm World's weather
+     * @return True if the ItemStack has been enchanted
+     */
+    private static boolean beginMagicHunter(Player player, int skillLevel, ItemStack itemStack, boolean storm) {
+        if (!ItemChecks.isEnchantable(itemStack)) {
+            return false;
         }
 
-        final int DROP_CHANCE = Misc.getRandom().nextInt(100);
-        final int DROP_NUMBER = Misc.getRandom().nextInt(randomChance) + 1;
-
-        LivingEntity le = (LivingEntity) event.getCaught();
-        EntityType type = le.getType();
-        Location location = le.getLocation();
-
-        if (DROP_CHANCE < dropChance) {
-
-            switch (type) {
-            case BLAZE:
-                Misc.dropItem(location, new ItemStack(Material.BLAZE_ROD));
-                break;
+        int activationChance = Misc.calculateActivationChance(Permissions.luckyFishing(player));
 
-            case CAVE_SPIDER:
-                if (DROP_NUMBER > 50) {
-                    Misc.dropItem(location, new ItemStack(Material.SPIDER_EYE));
-                } else {
-                    Misc.dropItem(location, new ItemStack(Material.STRING));
-                }
-                break;
-
-            case CHICKEN:
-                if (DROP_NUMBER > 66) {
-                    Misc.dropItem(location, new ItemStack(Material.FEATHER));
-                } else if (DROP_NUMBER > 33) {
-                    Misc.dropItem(location, new ItemStack(Material.RAW_CHICKEN));
-                } else {
-                    Misc.dropItem(location, new ItemStack(Material.EGG));
-                }
-                break;
-
-            case COW:
-                if (DROP_NUMBER > 95) {
-                    Misc.dropItem(location, new ItemStack(Material.MILK_BUCKET));
-                } else if (DROP_NUMBER > 50) {
-                    Misc.dropItem(location, new ItemStack(Material.LEATHER));
-                } else {
-                    Misc.dropItem(location, new ItemStack(Material.RAW_BEEF));
-                }
-                break;
-
-            case CREEPER:
-                if (DROP_NUMBER > 97) {
-                    Misc.dropItem(location, new ItemStack(Material.SKULL_ITEM, 1, (short) 4));
-                } else {
-                    Misc.dropItem(location, new ItemStack(Material.SULPHUR));
-                }
-                break;
-
-            case ENDERMAN:
-                Misc.dropItem(location, new ItemStack(Material.ENDER_PEARL));
-                break;
-
-            case GHAST:
-                if (DROP_NUMBER > 50) {
-                    Misc.dropItem(location, new ItemStack(Material.SULPHUR));
-                } else {
-                    Misc.dropItem(location, new ItemStack(Material.GHAST_TEAR));
-                }
-                break;
-
-            case IRON_GOLEM:
-                if (DROP_NUMBER > 97) {
-                    Misc.dropItem(location, new ItemStack(Material.PUMPKIN));
-                } else if (DROP_NUMBER > 85) {
-                    Misc.dropItem(location, new ItemStack(Material.IRON_INGOT));
-                } else {
-                    Misc.dropItem(location, new ItemStack(Material.RED_ROSE));
-                }
-                break;
-
-            case MAGMA_CUBE:
-                Misc.dropItem(location, new ItemStack(Material.MAGMA_CREAM));
-                break;
-
-            case MUSHROOM_COW:
-                if (DROP_NUMBER > 95) {
-                    Misc.dropItem(location, new ItemStack(Material.MILK_BUCKET));
-                } else if (DROP_NUMBER > 90) {
-                    Misc.dropItem(location, new ItemStack(Material.MUSHROOM_SOUP));
-                } else if (DROP_NUMBER > 60) {
-                    Misc.dropItem(location, new ItemStack(Material.LEATHER));
-                } else if (DROP_NUMBER > 30) {
-                    Misc.dropItem(location, new ItemStack(Material.RAW_BEEF));
-                } else {
-                    Misc.dropItem(location, new ItemStack(Material.RED_MUSHROOM));
-                    Misc.randomDropItems(location, new ItemStack(Material.RED_MUSHROOM), 50, 2);
-                }
-                break;
+        if (storm) {
+            activationChance = (int) (activationChance * 0.909);
+        }
 
-            case PIG:
-                Misc.dropItem(location, new ItemStack(Material.PORK));
-                break;
+        if (Misc.getRandom().nextInt(activationChance) > getLootTier(skillLevel) * ADVANCED_CONFIG.getFishingMagicMultiplier()) {
+            return false;
+        }
 
-            case PIG_ZOMBIE:
-                if (DROP_NUMBER > 50) {
-                    Misc.dropItem(location, new ItemStack(Material.ROTTEN_FLESH));
-                } else {
-                    Misc.dropItem(location, new ItemStack(Material.GOLD_NUGGET));
-                }
-                break;
+        List<Enchantment> possibleEnchantments = new ArrayList<Enchantment>();
 
-            case SHEEP:
-                final Sheep sheep = (Sheep) le;
+        for (Enchantment enchantment : Enchantment.values()) {
+            if (enchantment.canEnchantItem(itemStack)) {
+                possibleEnchantments.add(enchantment);
+            }
+        }
 
-                if (!sheep.isSheared()) {
-                    final Wool wool = new Wool();
-                    wool.setColor(sheep.getColor());
+        // This make sure that the order isn't always the same, for example previously Unbreaking had a lot more chance to be used than any other enchant
+        Collections.shuffle(possibleEnchantments, Misc.getRandom());
 
-                    final ItemStack theWool = wool.toItemStack();
-                    theWool.setAmount(1 + Misc.getRandom().nextInt(6));
+        boolean enchanted = false;
+        int specificChance = 1;
 
-                    Misc.dropItem(location, theWool);
-                    sheep.setSheared(true);
-                }
-                break;
-
-            case SKELETON:
-                if (((Skeleton) le).getSkeletonType() == SkeletonType.WITHER) {
-                    if (DROP_NUMBER > 97) {
-                        Misc.dropItem(location, new ItemStack(Material.SKULL_ITEM, 1, (short) 1));
-                    } else if (DROP_NUMBER > 50) {
-                        Misc.dropItem(location, new ItemStack(Material.BONE));
-                    } else {
-                        Misc.dropItem(location, new ItemStack(Material.COAL));
-                        Misc.randomDropItems(location, new ItemStack(Material.COAL), 50, 2);
-                    }
-                } else {
-                    if (DROP_NUMBER > 97) {
-                        Misc.dropItem(location, new ItemStack(Material.SKULL_ITEM));
-                    } else if (DROP_NUMBER > 50) {
-                        Misc.dropItem(location, new ItemStack(Material.BONE));
-                    } else {
-                        Misc.dropItem(location, new ItemStack(Material.ARROW));
-                        Misc.randomDropItems(location, new ItemStack(Material.ARROW), 50, 2);
-                    }
-                }
-                break;
-
-            case SLIME:
-                Misc.dropItem(location, new ItemStack(Material.SLIME_BALL));
-                break;
-
-            case SNOWMAN:
-                if (DROP_NUMBER > 97) {
-                    Misc.dropItem(location, new ItemStack(Material.PUMPKIN));
-                } else {
-                    Misc.dropItem(location, new ItemStack(Material.SNOW_BALL));
-                    Misc.randomDropItems(location, new ItemStack(Material.SNOW_BALL), 50, 4);
-                }
-                break;
+        for (Enchantment possibleEnchantment : possibleEnchantments) {
+            boolean conflicts = false;
 
-            case SPIDER:
-                if (DROP_NUMBER > 50) {
-                    Misc.dropItem(location, new ItemStack(Material.SPIDER_EYE));
-                } else {
-                    Misc.dropItem(location, new ItemStack(Material.STRING));
-                }
-                break;
+            for (Enchantment currentEnchantment : itemStack.getEnchantments().keySet()) {
+                conflicts = currentEnchantment.conflictsWith(possibleEnchantment);
 
-            case SQUID:
-                ItemStack item;
-                try {
-                    item = (new MaterialData(Material.INK_SACK, DyeColor.BLACK.getDyeData())).toItemStack(1);
-                }
-                catch(Exception e) {
-                    item = (new MaterialData(Material.INK_SACK, (byte) 0)).toItemStack(1);
+                if (conflicts) {
+                    break;
                 }
-                catch(NoSuchMethodError e) {
-                    item = (new MaterialData(Material.INK_SACK, (byte) 0)).toItemStack(1);
-                }
-
-                Misc.dropItem(location, item);
-                break;
-
-            case WITCH:
-                final int DROP_NUMBER_2 = Misc.getRandom().nextInt(randomChance) + 1;
-                if (DROP_NUMBER > 95) {
-                    if (DROP_NUMBER_2 > 66) {
-                        Misc.dropItem(location, new Potion(PotionType.INSTANT_HEAL).toItemStack(1));
-                    } else if (DROP_NUMBER_2 > 33) {
-                        Misc.dropItem(location, new Potion(PotionType.FIRE_RESISTANCE).toItemStack(1));
-                    } else {
-                        Misc.dropItem(location, new Potion(PotionType.SPEED).toItemStack(1));
-                    }
-                } else {
-                    if (DROP_NUMBER_2 > 88) {
-                        Misc.dropItem(location, new ItemStack(Material.GLASS_BOTTLE));
-                    } else if (DROP_NUMBER_2 > 75) {
-                        Misc.dropItem(location, new ItemStack(Material.GLOWSTONE_DUST));
-                    } else if (DROP_NUMBER_2 > 63) {
-                        Misc.dropItem(location, new ItemStack(Material.SULPHUR));
-                    } else if (DROP_NUMBER_2 > 50) {
-                        Misc.dropItem(location, new ItemStack(Material.REDSTONE));
-                    } else if (DROP_NUMBER_2 > 38) {
-                        Misc.dropItem(location, new ItemStack(Material.SPIDER_EYE));
-                    } else if (DROP_NUMBER_2 > 25) {
-                        Misc.dropItem(location, new ItemStack(Material.STICK));
-                    } else if (DROP_NUMBER_2 > 13) {
-                        Misc.dropItem(location, new ItemStack(Material.SUGAR));
-                    } else {
-                        Misc.dropItem(location, new ItemStack(Material.POTION));
-                    }
-                }
-                break;
+            }
 
-            case ZOMBIE:
-                if (DROP_NUMBER > 97) {
-                    Misc.dropItem(location, new ItemStack(Material.SKULL_ITEM, 1, (short) 2));
-                } else {
-                    Misc.dropItem(location, new ItemStack(Material.ROTTEN_FLESH));
-                }
-                break;
+            if (!conflicts && Misc.getRandom().nextInt(specificChance) == 0) {
+                itemStack.addEnchantment(possibleEnchantment, Misc.getRandom().nextInt(possibleEnchantment.getMaxLevel()) + 1);
 
-            default:
-                break;
+                specificChance++;
+                enchanted = true;
             }
         }
 
-        Combat.dealDamage(le, 1);
+        return enchanted;
     }
 
     /**
-     * Gets chance of shake success.
-     * 
-     * @param rank
-     *            Treasure hunter rank
-     * @return The chance of a successful shake
+     * Gets the loot tier for a given skill level
+     *
+     * @param skillLevel Fishing skill level
+     * @return Loot tier
      */
-    public static int getShakeChance(int lootTier) {
-        switch (lootTier) {
-        case 1:
-            return shakeChanceLevel1;
-
-        case 2:
-            return shakeChanceLevel2;
-
-        case 3:
-            return shakeChanceLevel3;
-
-        case 4:
-            return shakeChanceLevel4;
-
-        case 5:
-            return shakeChanceLevel5;
-
-        default:
-            return 10;
+   public static int getLootTier(int skillLevel) {
+        for (Tier tier : Tier.values()) {
+            if (skillLevel >= tier.getLevel()) {
+                return tier.toNumerical();
+            }
         }
+
+        return 0;
     }
 }

src/main/java/com/gmail/nossr50/skills/fishing/FishingCommand.java

@@ -7,9 +7,6 @@ import com.gmail.nossr50.skills.SkillType;
 import com.gmail.nossr50.util.Permissions;
 
 public class FishingCommand extends SkillCommand {
-
-    AdvancedConfig advancedConfig = AdvancedConfig.getInstance();
-
     private int lootTier;
     private String magicChance;
     private String magicChanceLucky;
@@ -29,10 +26,10 @@ public class FishingCommand extends SkillCommand {
 
     @Override
     protected void dataCalculations() {
-        lootTier = Fishing.getFishingLootTier(profile);
+        lootTier = Fishing.getLootTier((int) skillValue);
 
         //TREASURE HUNTER
-        double enchantChance = lootTier * Fishing.magicHunterMultiplier;
+        double enchantChance = lootTier * AdvancedConfig.getInstance().getFishingMagicMultiplier();
 
         if (player.getWorld().hasStorm()) {
             chanceRaining = LocaleLoader.getString("Fishing.Chance.Raining");
@@ -44,7 +41,7 @@ public class FishingCommand extends SkillCommand {
         magicChanceLucky = treasureHunterStrings[1];
 
         //SHAKE
-        String[] shakeStrings = calculateAbilityDisplayValues(Fishing.getShakeChance(lootTier));
+        String[] shakeStrings = calculateAbilityDisplayValues(ShakeMob.getShakeProbability(lootTier));
         shakeChance = shakeStrings[0];
         shakeChanceLucky = shakeStrings[1];
 
@@ -107,8 +104,8 @@ public class FishingCommand extends SkillCommand {
         }
 
         if (canShake) {
-            if (skillValue < Fishing.shakeUnlockLevel) {
-                player.sendMessage(LocaleLoader.getString("Ability.Generic.Template.Lock", new Object[] { LocaleLoader.getString("Fishing.Ability.Locked.0", new Object[] { Fishing.shakeUnlockLevel }) }));
+            if (skillValue < AdvancedConfig.getInstance().getShakeUnlockLevel()) {
+                player.sendMessage(LocaleLoader.getString("Ability.Generic.Template.Lock", new Object[] { LocaleLoader.getString("Fishing.Ability.Locked.0", new Object[] { AdvancedConfig.getInstance().getShakeUnlockLevel() }) }));
             }
             else {
                 if (isLucky) {

src/main/java/com/gmail/nossr50/skills/fishing/ShakeMob.java

@@ -0,0 +1,230 @@
+package com.gmail.nossr50.skills.fishing;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Map.Entry;
+
+import org.bukkit.Material;
+import org.bukkit.entity.LivingEntity;
+import org.bukkit.entity.Player;
+import org.bukkit.entity.Sheep;
+import org.bukkit.entity.Skeleton;
+import org.bukkit.entity.Skeleton.SkeletonType;
+import org.bukkit.inventory.ItemStack;
+import org.bukkit.material.Wool;
+import org.bukkit.potion.Potion;
+import org.bukkit.potion.PotionType;
+
+import com.gmail.nossr50.skills.Combat;
+import com.gmail.nossr50.skills.fishing.Fishing.Tier;
+import com.gmail.nossr50.util.Misc;
+import com.gmail.nossr50.util.Permissions;
+
+public final class ShakeMob {
+    private ShakeMob() {}
+
+    /**
+     * Begins Tree Feller
+     *
+     * @param player Player using Shake Mob
+     * @param mob Targeted entity
+     * @param skillLevel Fishing level of the player
+     */
+    public static void process(Player player, LivingEntity mob, int skillLevel) {
+        int activationChance = Misc.calculateActivationChance(Permissions.luckyFishing(player));
+
+        if (getShakeProbability(skillLevel) <= Misc.getRandom().nextInt(activationChance)) {
+            return;
+        }
+
+        Map<ItemStack, Integer> possibleDrops = new HashMap<ItemStack, Integer>();
+
+        findPossibleDrops(mob, possibleDrops);
+
+        if (possibleDrops.isEmpty()) {
+            return;
+        }
+
+        ItemStack drop = chooseDrop(possibleDrops);
+
+        // It's possible that chooseDrop returns null if the sum of probability in possibleDrops is inferior than 100
+        if (drop == null) {
+            return;
+        }
+
+        // Extra processing depending on the mob and drop type
+        switch (mob.getType()) {
+        case SHEEP:
+            Sheep sheep = (Sheep) mob;
+
+            if (drop.getType() == Material.WOOL) {
+                if (sheep.isSheared()) {
+                    return;
+                }
+
+                sheep.setSheared(true);
+                drop.setAmount(Misc.getRandom().nextInt(6) + 1);
+                ((Wool) drop.getData()).setColor(sheep.getColor());
+            }
+            break;
+
+        case SKELETON:
+            Skeleton skeleton = (Skeleton) mob;
+
+            if (skeleton.getSkeletonType() == SkeletonType.WITHER) {
+                switch (drop.getType()) {
+                case SKULL_ITEM:
+                    drop.setDurability((short) 1);
+                    break;
+                case ARROW:
+                    drop.setType(Material.COAL);
+                    break;
+                default:
+                    break;
+                }
+            }
+
+        default:
+            break;
+        }
+
+        Misc.dropItem(mob.getLocation(), drop);
+        Combat.dealDamage(mob, 1); // We may want to base the damage on the entity max health
+    }
+
+    /**
+     * Finds the possible drops of an entity
+     *
+     * @param mob Targeted entity
+     * @param possibleDrops List of ItemStack that can be dropped
+     */
+    private static void findPossibleDrops(LivingEntity mob, Map<ItemStack, Integer> possibleDrops) {
+        switch (mob.getType()) {
+        case BLAZE:
+            possibleDrops.put(new ItemStack(Material.BLAZE_ROD), 100);
+            break;
+        case CAVE_SPIDER:
+        case SPIDER:
+            possibleDrops.put(new ItemStack(Material.SPIDER_EYE), 50);
+            possibleDrops.put(new ItemStack(Material.STRING), 50);
+            break;
+        case CHICKEN:
+            possibleDrops.put(new ItemStack(Material.FEATHER), 34);
+            possibleDrops.put(new ItemStack(Material.RAW_CHICKEN), 33);
+            possibleDrops.put(new ItemStack(Material.EGG), 33);
+            break;
+        case COW:
+            possibleDrops.put(new ItemStack(Material.MILK_BUCKET), 2);
+            possibleDrops.put(new ItemStack(Material.LEATHER), 49);
+            possibleDrops.put(new ItemStack(Material.RAW_BEEF), 49);
+            break;
+        case CREEPER:
+            possibleDrops.put(new ItemStack(Material.SKULL_ITEM, (byte) 0x4), 1);
+            possibleDrops.put(new ItemStack(Material.SULPHUR), 99);
+            break;
+        case ENDERMAN:
+            possibleDrops.put(new ItemStack(Material.ENDER_PEARL), 100);
+            break;
+        case GHAST:
+            possibleDrops.put(new ItemStack(Material.SULPHUR), 50);
+            possibleDrops.put(new ItemStack(Material.GHAST_TEAR), 50);
+            break;
+        case IRON_GOLEM:
+            possibleDrops.put(new ItemStack(Material.PUMPKIN), 3);
+            possibleDrops.put(new ItemStack(Material.IRON_INGOT), 12);
+            possibleDrops.put(new ItemStack(Material.RED_ROSE), 85);
+            break;
+        case MAGMA_CUBE:
+            possibleDrops.put(new ItemStack(Material.MAGMA_CREAM), 3);
+            break;
+        case MUSHROOM_COW:
+            possibleDrops.put(new ItemStack(Material.MILK_BUCKET), 5);
+            possibleDrops.put(new ItemStack(Material.MUSHROOM_SOUP), 5);
+            possibleDrops.put(new ItemStack(Material.LEATHER), 30);
+            possibleDrops.put(new ItemStack(Material.RAW_BEEF), 30);
+            possibleDrops.put(new ItemStack(Material.RED_MUSHROOM, Misc.getRandom().nextInt(3) + 1), 30);
+            break;
+        case PIG:
+            possibleDrops.put(new ItemStack(Material.PORK), 3);
+            break;
+        case PIG_ZOMBIE:
+            possibleDrops.put(new ItemStack(Material.ROTTEN_FLESH), 50);
+            possibleDrops.put(new ItemStack(Material.GOLD_NUGGET), 50);
+            break;
+        case SHEEP:
+            possibleDrops.put(new ItemStack(Material.WOOL), 100);
+            break;
+        case SKELETON:
+            possibleDrops.put(new ItemStack(Material.SKULL_ITEM), 2);
+            possibleDrops.put(new ItemStack(Material.BONE), 49);
+            possibleDrops.put(new ItemStack(Material.ARROW, Misc.getRandom().nextInt(3) + 1), 49);
+            break;
+        case SLIME:
+            possibleDrops.put(new ItemStack(Material.SLIME_BALL), 100);
+            break;
+        case SNOWMAN:
+            possibleDrops.put(new ItemStack(Material.PUMPKIN), 3);
+            possibleDrops.put(new ItemStack(Material.SNOW_BALL, Misc.getRandom().nextInt(4) + 1), 97);
+            break;
+        case SQUID:
+            possibleDrops.put(new ItemStack(Material.INK_SACK, (byte) 0x0), 100);
+            break;
+        case WITCH:
+            possibleDrops.put(new Potion(PotionType.INSTANT_HEAL).toItemStack(1), 1);
+            possibleDrops.put(new Potion(PotionType.FIRE_RESISTANCE).toItemStack(1), 1);
+            possibleDrops.put(new Potion(PotionType.SPEED).toItemStack(1), 1);
+            possibleDrops.put(new ItemStack(Material.GLASS_BOTTLE), 9);
+            possibleDrops.put(new ItemStack(Material.GLOWSTONE_DUST), 13);
+            possibleDrops.put(new ItemStack(Material.SULPHUR), 12);
+            possibleDrops.put(new ItemStack(Material.REDSTONE), 13);
+            possibleDrops.put(new ItemStack(Material.SPIDER_EYE), 12);
+            possibleDrops.put(new ItemStack(Material.STICK), 13);
+            possibleDrops.put(new ItemStack(Material.SUGAR), 12);
+            possibleDrops.put(new ItemStack(Material.POTION), 13);
+            break;
+        case ZOMBIE:
+            possibleDrops.put(new ItemStack(Material.SKULL_ITEM, (byte) 0x2), 2);
+            possibleDrops.put(new ItemStack(Material.ROTTEN_FLESH), 98);
+            break;
+        default:
+            return;
+        }
+    }
+
+    /**
+     * Randomly chooses a drop among the list
+     *
+     * @param possibleDrops List of ItemStack that can be dropped
+     * @return Chosen ItemStack
+     */
+    private static ItemStack chooseDrop(Map<ItemStack, Integer> possibleDrops) {
+        int dropProbability = Misc.getRandom().nextInt(100);
+        int cumulatedProbability = 0;
+
+        for (Entry<ItemStack, Integer> entry : possibleDrops.entrySet()) {
+            cumulatedProbability += entry.getValue();
+
+            if (dropProbability < cumulatedProbability) {
+                return entry.getKey();
+            }
+        }
+
+        return null;
+    }
+
+    /**
+     * Gets the Shake Mob probability for a given skill level
+     *
+     * @param skillLevel Fishing skill level
+     * @return Shake Mob probability
+     */
+    public static int getShakeProbability(int skillLevel) {
+        for (Tier tier : Tier.values()) {
+            if (skillLevel >= tier.getLevel()) {
+                return tier.getShakeChance();
+            }
+        }
+
+        return 0;
+    }
+}

src/main/java/com/gmail/nossr50/skills/woodcutting/TreeFeller.java

@@ -9,7 +9,6 @@ import org.bukkit.World;
 import org.bukkit.block.Block;
 import org.bukkit.enchantments.Enchantment;
 import org.bukkit.entity.Player;
-import org.bukkit.event.block.BlockBreakEvent;
 import org.bukkit.inventory.ItemStack;
 
 import com.gmail.nossr50.mcMMO;
@@ -22,22 +21,23 @@ import com.gmail.nossr50.skills.woodcutting.Woodcutting.ExperienceGainMethod;
 import com.gmail.nossr50.util.BlockChecks;
 import com.gmail.nossr50.util.Misc;
 import com.gmail.nossr50.util.ModChecks;
-import com.gmail.nossr50.util.Permissions;
 import com.gmail.nossr50.util.Users;
 
-public abstract class TreeFeller {
+public final class TreeFeller {
     private static boolean treeFellerReachedThreshold = false;
 
+    private TreeFeller() {}
+
     /**
      * Begins Tree Feller
      *
-     * @param event Event to process
+     * @param player Player using Tree Feller
+     * @param block Block being broken
      */
-    public static void process(BlockBreakEvent event) {
+    public static void process(Player player, Block block) {
         List<Block> treeFellerBlocks = new ArrayList<Block>();
-        Player player = event.getPlayer();
 
-        processRecursively(event.getBlock(), treeFellerBlocks);
+        processRecursively(block, treeFellerBlocks);
 
         // If the player is trying to break to many block
         if (treeFellerReachedThreshold) {
@@ -66,7 +66,7 @@ public abstract class TreeFeller {
     /**
      * Processes Tree Feller
      *
-     * @param block Point of origin of the layer
+     * @param block Block being checked
      * @param treeFellerBlocks List of blocks to be removed
      */
     private static void processRecursively(Block block, List<Block> treeFellerBlocks) {
@@ -176,7 +176,7 @@ public abstract class TreeFeller {
 
             switch (block.getType()) {
             case LOG:
-                Woodcutting.checkDoubleDrop(player, block);
+                Woodcutting.checkForDoubleDrop(player, block);
 
                 try {
                     xp += Woodcutting.getExperienceFromLog(block, ExperienceGainMethod.TREE_FELLER);
@@ -192,7 +192,7 @@ public abstract class TreeFeller {
                 break;
             default:
                 if (ModChecks.isCustomLogBlock(block)) {
-                    Woodcutting.checkDoubleDrop(player, block);
+                    Woodcutting.checkForDoubleDrop(player, block);
 
                     CustomBlock customBlock = ModChecks.getCustomBlock(block);
                     xp = customBlock.getXpGain();
@@ -220,9 +220,6 @@ public abstract class TreeFeller {
             block.setType(Material.AIR);
         }
 
-        // Do we really have to check the permission here?
-        if (Permissions.woodcutting(player)) {
-            SkillTools.xpProcessing(player, Users.getProfile(player), SkillType.WOODCUTTING, xp);
-        }
+        SkillTools.xpProcessing(player, Users.getProfile(player), SkillType.WOODCUTTING, xp);
     }
 }

src/main/java/com/gmail/nossr50/skills/woodcutting/Woodcutting.java

@@ -42,7 +42,7 @@ public final class Woodcutting {
      * @param event Event to process
      */
     public static void beginTreeFeller(BlockBreakEvent event) {
-        TreeFeller.process(event);
+        TreeFeller.process(event.getPlayer(), event.getBlock());
     }
 
     /**
@@ -78,7 +78,10 @@ public final class Woodcutting {
             }
         }
 
-        checkDoubleDrop(player, block);
+        if (Permissions.woodcuttingDoubleDrops(player)) {
+            checkForDoubleDrop(player, block);
+        }
+
         SkillTools.xpProcessing(player,  Users.getProfile(player), SkillType.WOODCUTTING, xp);
     }
 
@@ -126,11 +129,7 @@ public final class Woodcutting {
      * @param player Player breaking the block
      * @param block Block being broken
      */
-    protected static void checkDoubleDrop(Player player, Block block) {
-        if (!Permissions.woodcuttingDoubleDrops(player)) {
-            return;
-        }
-
+    protected static void checkForDoubleDrop(Player player, Block block) {
         int chance = (int) ((DOUBLE_DROP_CHANCE / DOUBLE_DROP_MAX_LEVEL) * Users.getProfile(player).getSkillLevel(SkillType.WOODCUTTING));
         int activationChance = Misc.calculateActivationChance(Permissions.luckyWoodcutting(player));