From c5e6704530d55ea5793981c60d76714d6b854355 Mon Sep 17 00:00:00 2001
From: Paul J Thordarson <kapunga@gmail.com>
Date: Thu, 9 May 2013 09:01:19 -0400
Subject: [PATCH] Magic Hunter fix and optimization

There was a small bug in magic hunter, where it tried to apply an
enchantment level between 1 and the Enchantment.getMaxLevel(), however
it didn't take into account Enchantment.getStartLevel(), so when it
tried to apply an enchantment level below the start level, an
IllegalArgumentException was being thrown and the magic hunter event
wasn't happening.

Also, it's potentially inefficient to recalculate which enchantments
are possible for each ItemStack every time, so I added a HashMap to
cache the possibleEnchantments for each material type, then check this
cache instead of just looping to regenerate the list each time.
---
 .../skills/fishing/FishingManager.java        | 25 +++++++++++++++----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/src/main/java/com/gmail/nossr50/skills/fishing/FishingManager.java b/src/main/java/com/gmail/nossr50/skills/fishing/FishingManager.java
index 35b6caf29..5d94cc0eb 100644
--- a/src/main/java/com/gmail/nossr50/skills/fishing/FishingManager.java
+++ b/src/main/java/com/gmail/nossr50/skills/fishing/FishingManager.java
@@ -2,6 +2,7 @@ package com.gmail.nossr50.skills.fishing;
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 
 import org.bukkit.ChatColor;
@@ -48,6 +49,7 @@ import com.gmail.nossr50.util.skills.CombatUtils;
 import com.gmail.nossr50.util.skills.SkillUtils;
 
 public class FishingManager extends SkillManager {
+    private static HashMap<Material, List<Enchantment>> enchantableCache = new HashMap<Material, List<Enchantment>>();
     private final long FISHING_COOLDOWN_SECONDS = 1000L;
 
     private int fishingTries = 0;
@@ -416,12 +418,22 @@ public class FishingManager extends SkillManager {
             return false;
         }
 
-        List<Enchantment> possibleEnchantments = new ArrayList<Enchantment>();
+        // When calculating the possible enchantments, we should cache the possible enchantments to minimize
+        // looping every time someone fishes.
+        List<Enchantment> possibleEnchantments;
 
-        for (Enchantment enchantment : Enchantment.values()) {
-            if (enchantment.canEnchantItem(treasureDrop)) {
-                possibleEnchantments.add(enchantment);
+        if (enchantableCache.containsKey(treasureDrop.getType())) { // Check if possible enchantments is already cached for this item.
+            possibleEnchantments = enchantableCache.get(treasureDrop.getType());
+        } else { // If not, check which enchantments are possible
+            possibleEnchantments = new ArrayList<Enchantment>();
+
+            for (Enchantment enchantment : Enchantment.values()) {
+                if (enchantment.canEnchantItem(treasureDrop)) {
+                    possibleEnchantments.add(enchantment);
+                }
             }
+        	
+            enchantableCache.put(treasureDrop.getType(), possibleEnchantments); // Cache these enchantments.
         }
 
         // 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
@@ -432,7 +444,10 @@ public class FishingManager extends SkillManager {
 
         for (Enchantment possibleEnchantment : possibleEnchantments) {
             if (!treasureDrop.getItemMeta().hasConflictingEnchant(possibleEnchantment) && Misc.getRandom().nextInt(specificChance) == 0) {
-                treasureDrop.addEnchantment(possibleEnchantment, Misc.getRandom().nextInt(possibleEnchantment.getMaxLevel()) + 1);
+                // We need our random enchantment level to fall in the range between getStartLevel() and getMaxLevel()
+                // so we take a random number in the range of their difference, then add the start level.
+                final int levelDiff = possibleEnchantment.getMaxLevel() - possibleEnchantment.getStartLevel();
+                treasureDrop.addEnchantment(possibleEnchantment, Misc.getRandom().nextInt(levelDiff + 1) + possibleEnchantment.getStartLevel());
 
                 specificChance++;
                 enchanted = true;