Correct display error when successfully salvaging enchantments

Changelog.txt

@@ -7,6 +7,9 @@ Key:
   ! Change
   - Removal
 
+Version 2.1.43
+    Fixed a bug that would result in players being told they they failed to salvage enchantments when in reality they succeeded.
+
 Version 2.1.42
     Fixed McMMOPlayerNotFoundException being thrown instead of null
     (API) UserManager.getPlayer() returns null again (oopsie)

pom.xml

@@ -2,7 +2,7 @@
     <modelVersion>4.0.0</modelVersion>
     <groupId>com.gmail.nossr50.mcMMO</groupId>
     <artifactId>mcMMO</artifactId>
-    <version>2.1.42</version>
+    <version>2.1.43-SNAPSHOT</version>
     <name>mcMMO</name>
     <url>https://github.com/mcMMO-Dev/mcMMO</url>
     <scm>

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

@@ -65,11 +65,13 @@ public class BlockListener implements Listener {
             if(is.getAmount() <= 0)
                 continue;
 
+            //TODO: Ignore this abomination its rewritten in 2.2
             if(!Config.getInstance().getDoubleDropsEnabled(PrimarySkillType.MINING, is.getType())
                     && !Config.getInstance().getDoubleDropsEnabled(PrimarySkillType.HERBALISM, is.getType())
                         && !Config.getInstance().getDoubleDropsEnabled(PrimarySkillType.WOODCUTTING, is.getType()))
                 continue;
 
+            //TODO: Should just store the amount of drops in the metadata itself and use a loop
             if(event.getBlock().getState().getMetadata(mcMMO.doubleDrops).size() > 0)
             {
                 event.getBlock().getState().getWorld().dropItemNaturally(event.getBlockState().getLocation(), is);

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

@@ -204,7 +204,7 @@ public class SalvageManager extends SkillManager {
         EnchantmentStorageMeta enchantMeta = (EnchantmentStorageMeta) book.getItemMeta();
 
         boolean downgraded = false;
-        boolean arcaneFailure = false;
+        int arcaneFailureCount = 0;
 
         for (Entry<Enchantment, Integer> enchant : enchants.entrySet()) {
             if (!Salvage.arcaneSalvageEnchantLoss
@@ -217,36 +217,28 @@ public class SalvageManager extends SkillManager {
                     && RandomChanceUtil.checkRandomChanceExecutionSuccess(new RandomChanceSkillStatic(getExtractPartialEnchantChance(), getPlayer(), SubSkillType.SALVAGE_ARCANE_SALVAGE))) {
                 enchantMeta.addStoredEnchant(enchant.getKey(), enchant.getValue() - 1, true);
                 downgraded = true;
-            }
+            } else {
-            else {
+                arcaneFailureCount++;
-                arcaneFailure = true;
-                downgraded = true;
             }
         }
 
-        if(!arcaneFailure)
+        if(failedAllEnchants(arcaneFailureCount, enchants.entrySet().size()))
         {
-            Map<Enchantment, Integer> newEnchants = enchantMeta.getStoredEnchants();
+            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Salvage.Skills.ArcaneFailed");
-
-            if (downgraded || newEnchants.size() < enchants.size()) {
-                NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Salvage.Skills.ArcanePartial");
-            }
-            else {
-                NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Salvage.Skills.ArcanePartial");
-            }
-
-            book.setItemMeta(enchantMeta);
-        } else {
-            if(enchantMeta.getStoredEnchants().size() > 0)
-            {
-                NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Salvage.Skills.ArcaneFailed");
-            }
             return null;
+        } else if(downgraded)
+        {
+            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Salvage.Skills.ArcanePartial");
         }
 
+        book.setItemMeta(enchantMeta);
         return book;
     }
 
+    private boolean failedAllEnchants(int arcaneFailureCount, int size) {
+        return arcaneFailureCount == size;
+    }
+
     /**
      * Check if the player has tried to use an Anvil before.
      * @param actualize