Some small optimizations (#4305)

some improvements to itemmeta calls and misc changes

pom.xml

@@ -228,6 +228,14 @@
             <groupId>com.sk89q.worldguard</groupId>
             <artifactId>worldguard-core</artifactId>
             <version>7.0.1-SNAPSHOT</version>
+            <exclusions>
+                <exclusion>
+                    <!-- We use jetbrains instead. Excluding this -->
+                    <!-- prevents us from using inconsistent annotations -->
+                    <groupId>com.google.code.findbugs</groupId>
+                    <artifactId>jsr305</artifactId>
+                </exclusion>
+            </exclusions>
         </dependency>
         <dependency>
             <groupId>com.sk89q.worldguard</groupId>

src/main/java/com/gmail/nossr50/api/exceptions/IncompleteNamespacedKeyRegister.java

@@ -1,7 +1,11 @@
 package com.gmail.nossr50.api.exceptions;
 
+import org.jetbrains.annotations.NotNull;
+
 public class IncompleteNamespacedKeyRegister extends RuntimeException {
-    public IncompleteNamespacedKeyRegister(String message) {
+    private static final long serialVersionUID = -6905157273569301219L;
+
+    public IncompleteNamespacedKeyRegister(@NotNull String message) {
         super(message);
     }
 }

src/main/java/com/gmail/nossr50/api/exceptions/McMMOPlayerNotFoundException.java

@@ -1,11 +1,12 @@
 package com.gmail.nossr50.api.exceptions;
 
 import org.bukkit.entity.Player;
+import org.jetbrains.annotations.NotNull;
 
 public class McMMOPlayerNotFoundException extends RuntimeException {
     private static final long serialVersionUID = 761917904993202836L;
 
-    public McMMOPlayerNotFoundException(Player player) {
+    public McMMOPlayerNotFoundException(@NotNull Player player) {
         super("McMMOPlayer object was not found for [NOTE: This can mean the profile is not loaded yet!] : " + player.getName() + " " + player.getUniqueId());
     }
 }

src/main/java/com/gmail/nossr50/chat/ChatManagerFactory.java

@@ -1,15 +1,26 @@
 package com.gmail.nossr50.chat;
 
-import com.gmail.nossr50.datatypes.chat.ChatMode;
-import org.bukkit.plugin.Plugin;
-
 import java.util.HashMap;
+import java.util.Map;
 
-public class ChatManagerFactory {
-    private static final HashMap<Plugin, AdminChatManager> adminChatManagers = new HashMap<>();
-    private static final HashMap<Plugin, PartyChatManager> partyChatManagers = new HashMap<>();
+import org.bukkit.plugin.Plugin;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
-    public static ChatManager getChatManager(Plugin plugin, ChatMode mode) {
+import com.gmail.nossr50.datatypes.chat.ChatMode;
+
+public final class ChatManagerFactory {
+    private static final Map<Plugin, AdminChatManager> adminChatManagers = new HashMap<>();
+    private static final Map<Plugin, PartyChatManager> partyChatManagers = new HashMap<>();
+
+    /**
+     * This is a static utility class, therefore we don't want any instances of
+     * this class. Making the constructor private prevents accidents like that.
+     */
+    private ChatManagerFactory() {}
+    
+    @Nullable
+    public static ChatManager getChatManager(@NotNull Plugin plugin, @NotNull ChatMode mode) {
         switch (mode) {
             case ADMIN:
                 if (!adminChatManagers.containsKey(plugin)) {

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

@@ -42,6 +42,7 @@ import org.bukkit.event.Listener;
 import org.bukkit.event.entity.*;
 import org.bukkit.event.entity.EntityDamageEvent.DamageCause;
 import org.bukkit.inventory.ItemStack;
+import org.bukkit.inventory.meta.ItemMeta;
 import org.bukkit.inventory.meta.PotionMeta;
 import org.bukkit.metadata.FixedMetadataValue;
 import org.bukkit.potion.PotionEffect;
@@ -1068,10 +1069,13 @@ public class EntityListener implements Listener {
         if(WorldBlacklist.isWorldBlacklisted(event.getEntity().getWorld()))
             return;
 
-        if(event.getPotion().getItem().getItemMeta() == null)
-            return;
+        ItemMeta meta = event.getPotion().getItem().getItemMeta();
 
-        for (PotionEffect effect : ((PotionMeta) event.getPotion().getItem().getItemMeta()).getCustomEffects()) {
+        if (meta == null) {
+            return;
+        }
+
+        for (PotionEffect effect : ((PotionMeta) meta).getCustomEffects()) {
             if (!effect.getType().equals(PotionEffectType.SATURATION)) {
                 return;
             }

src/main/java/com/gmail/nossr50/skills/salvage/Salvage.java

@@ -4,7 +4,14 @@ import com.gmail.nossr50.config.AdvancedConfig;
 import com.gmail.nossr50.config.Config;
 import org.bukkit.Material;
 
-public class Salvage {
+public final class Salvage {
+
+    /**
+     * This is a static utility class, therefore we don't want any instances of
+     * this class. Making the constructor private prevents accidents like that.
+     */
+    private Salvage() {}
+    
     public static Material anvilMaterial = Config.getInstance().getSalvageAnvilMaterial();
 
     /*public static int    salvageMaxPercentageLevel = AdvancedConfig.getInstance().getSalvageMaxPercentageLevel();
@@ -15,7 +22,7 @@ public class Salvage {
     public static boolean arcaneSalvageDowngrades  = AdvancedConfig.getInstance().getArcaneSalvageEnchantDowngradeEnabled();
     public static boolean arcaneSalvageEnchantLoss = AdvancedConfig.getInstance().getArcaneSalvageEnchantLossEnabled();
 
-    protected static int calculateSalvageableAmount(short currentDurability, short maxDurability, int baseAmount) {
+    static int calculateSalvageableAmount(int currentDurability, short maxDurability, int baseAmount) {
         double percentDamaged = (maxDurability <= 0) ? 1D : (double) (maxDurability - currentDurability) / maxDurability;
 
         return (int) Math.floor(baseAmount * percentDamaged);

src/main/java/com/gmail/nossr50/skills/salvage/SalvageManager.java

@@ -1,5 +1,18 @@
 package com.gmail.nossr50.skills.salvage;
 
+import java.util.Map;
+import java.util.Map.Entry;
+
+import org.bukkit.Location;
+import org.bukkit.Material;
+import org.bukkit.enchantments.Enchantment;
+import org.bukkit.entity.Player;
+import org.bukkit.inventory.ItemStack;
+import org.bukkit.inventory.meta.Damageable;
+import org.bukkit.inventory.meta.EnchantmentStorageMeta;
+import org.bukkit.inventory.meta.ItemMeta;
+
+import com.gmail.nossr50.mcMMO;
 import com.gmail.nossr50.config.AdvancedConfig;
 import com.gmail.nossr50.config.Config;
 import com.gmail.nossr50.config.experience.ExperienceConfig;
@@ -8,7 +21,6 @@ import com.gmail.nossr50.datatypes.player.McMMOPlayer;
 import com.gmail.nossr50.datatypes.skills.PrimarySkillType;
 import com.gmail.nossr50.datatypes.skills.SubSkillType;
 import com.gmail.nossr50.locale.LocaleLoader;
-import com.gmail.nossr50.mcMMO;
 import com.gmail.nossr50.skills.SkillManager;
 import com.gmail.nossr50.skills.salvage.salvageables.Salvageable;
 import com.gmail.nossr50.util.EventUtils;
@@ -22,15 +34,6 @@ import com.gmail.nossr50.util.skills.RankUtils;
 import com.gmail.nossr50.util.skills.SkillUtils;
 import com.gmail.nossr50.util.sounds.SoundManager;
 import com.gmail.nossr50.util.sounds.SoundType;
-import org.bukkit.Location;
-import org.bukkit.Material;
-import org.bukkit.enchantments.Enchantment;
-import org.bukkit.entity.Player;
-import org.bukkit.inventory.ItemStack;
-import org.bukkit.inventory.meta.EnchantmentStorageMeta;
-
-import java.util.Map;
-import java.util.Map.Entry;
 
 public class SalvageManager extends SkillManager {
     private boolean placedAnvil;
@@ -65,8 +68,9 @@ public class SalvageManager extends SkillManager {
         Player player = getPlayer();
 
         Salvageable salvageable = mcMMO.getSalvageableManager().getSalvageable(item.getType());
-
-        if (item.getItemMeta() != null && item.getItemMeta().isUnbreakable()) {
+        ItemMeta meta = item.getItemMeta();
+        
+        if (meta != null && meta.isUnbreakable()) {
             NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Anvil.Unbreakable");
             return;
         }
@@ -91,7 +95,8 @@ public class SalvageManager extends SkillManager {
             return;
         }
 
-        int potentialSalvageYield = Salvage.calculateSalvageableAmount(item.getDurability(), salvageable.getMaximumDurability(), salvageable.getMaximumQuantity());
+        int durability = meta instanceof Damageable ? ((Damageable) meta).getDamage(): 0;
+        int potentialSalvageYield = Salvage.calculateSalvageableAmount(durability, salvageable.getMaximumDurability(), salvageable.getMaximumQuantity());
 
         if (potentialSalvageYield <= 0) {
             NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Salvage.Skills.TooDamaged");

src/main/java/com/gmail/nossr50/skills/salvage/salvageables/SalvageableFactory.java

@@ -4,7 +4,13 @@ import com.gmail.nossr50.datatypes.skills.ItemType;
 import com.gmail.nossr50.datatypes.skills.MaterialType;
 import org.bukkit.Material;
 
-public class SalvageableFactory {
+public final class SalvageableFactory {
+    /**
+     * This is a static utility class, therefore we don't want any instances of
+     * this class. Making the constructor private prevents accidents like that.
+     */
+    private SalvageableFactory() {}
+    
     public static Salvageable getSalvageable(Material itemMaterial, Material recipeMaterial, int maximumQuantity, short maximumDurability) {
         return getSalvageable(itemMaterial, recipeMaterial, 0, maximumQuantity, maximumDurability, ItemType.OTHER, MaterialType.OTHER, 1);
     }

src/main/java/com/gmail/nossr50/skills/salvage/salvageables/SimpleSalvageableManager.java

@@ -1,14 +1,14 @@
 package com.gmail.nossr50.skills.salvage.salvageables;
 
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 import org.bukkit.Material;
 import org.bukkit.inventory.ItemStack;
 
-import java.util.HashMap;
-import java.util.List;
-
-
 public class SimpleSalvageableManager implements SalvageableManager {
-    private final HashMap<Material, Salvageable> salvageables;
+    private final Map<Material, Salvageable> salvageables;
 
     public SimpleSalvageableManager() {
         this(55);

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

@@ -25,6 +25,8 @@ import org.bukkit.block.BlockState;
 import org.bukkit.entity.Player;
 import org.bukkit.event.player.PlayerItemDamageEvent;
 import org.bukkit.inventory.ItemStack;
+import org.bukkit.inventory.meta.Damageable;
+import org.bukkit.inventory.meta.ItemMeta;
 
 import java.util.ArrayList;
 import java.util.HashSet;
@@ -32,7 +34,6 @@ import java.util.List;
 import java.util.Set;
 
 public class WoodcuttingManager extends SkillManager {
-
     private boolean treeFellerReachedThreshold = false;
     private static int treeFellerThreshold; //TODO: Shared setting, will be removed in 2.2
 
@@ -207,11 +208,13 @@ public class WoodcuttingManager extends SkillManager {
      */
     private static boolean handleDurabilityLoss(Set<BlockState> treeFellerBlocks, ItemStack inHand, Player player) {
         //Treat the NBT tag for unbreakable and the durability enchant differently
-        if(inHand.getItemMeta() != null && inHand.getItemMeta().isUnbreakable()) {
+        ItemMeta meta = inHand.getItemMeta();
+
+        if (meta != null && meta.isUnbreakable()) {
             return true;
         }
 
-        short durabilityLoss = 0;
+        int durabilityLoss = 0;
         Material type = inHand.getType();
 
         for (BlockState blockState : treeFellerBlocks) {
@@ -230,7 +233,8 @@ public class WoodcuttingManager extends SkillManager {
         }
 
         SkillUtils.handleDurabilityChange(inHand, durabilityLoss);
-        return (inHand.getDurability() < (mcMMO.getRepairableManager().isRepairable(type) ? mcMMO.getRepairableManager().getRepairable(type).getMaximumDurability() : type.getMaxDurability()));
+        int durability = meta instanceof Damageable ? ((Damageable) meta).getDamage(): 0;
+        return (durability < (mcMMO.getRepairableManager().isRepairable(type) ? mcMMO.getRepairableManager().getRepairable(type).getMaximumDurability() : type.getMaxDurability()));
     }
 
     /**

src/main/java/com/gmail/nossr50/util/ItemUtils.java

@@ -18,6 +18,10 @@ import org.jetbrains.annotations.NotNull;
 import java.util.List;
 
 public final class ItemUtils {
+    /**
+     * This is a static utility class, therefore we don't want any instances of
+     * this class. Making the constructor private prevents accidents like that.
+     */
     private ItemUtils() {}
 
     /**

src/main/java/com/gmail/nossr50/util/skills/SkillUtils.java

@@ -35,7 +35,12 @@ import org.jetbrains.annotations.Nullable;
 
 import java.util.Iterator;
 
-public class SkillUtils {
+public final class SkillUtils {
+    /**
+     * This is a static utility class, therefore we don't want any instances of
+     * this class. Making the constructor private prevents accidents like that.
+     */
+    private SkillUtils() {}
 
     public static void applyXpGain(McMMOPlayer mcMMOPlayer, PrimarySkillType skill, float xp, XPGainReason xpGainReason) {
         mcMMOPlayer.beginXpGain(skill, xp, xpGainReason, XPGainSource.SELF);
@@ -217,10 +222,8 @@ public class SkillUtils {
         if(compatLayer.isLegacyAbilityTool(itemStack)) {
             ItemMeta itemMeta = itemStack.getItemMeta();
 
-            //TODO: can be optimized
-            if(itemMeta.hasEnchant(Enchantment.DIG_SPEED)) {
-                itemMeta.removeEnchant(Enchantment.DIG_SPEED);
-            }
+            // This is safe to call without prior checks.
+            itemMeta.removeEnchant(Enchantment.DIG_SPEED);
 
             itemStack.setItemMeta(itemMeta);
             ItemUtils.removeAbilityLore(itemStack);
@@ -264,7 +267,8 @@ public class SkillUtils {
         return false;
     }
 
-    protected static Material getRepairAndSalvageItem(ItemStack inHand) {
+    @Nullable
+    public static Material getRepairAndSalvageItem(@NotNull ItemStack inHand) {
         if (ItemUtils.isDiamondTool(inHand) || ItemUtils.isDiamondArmor(inHand)) {
             return Material.DIAMOND;
         }