Fixed a bug where Arcane Salvage could never fail

SubSkillFailure renamed to SubSkillFailed in config.yml
SubSkillFailed now sends to chat by default and no longer sends to the action bar

Changelog.txt

@@ -8,7 +8,11 @@ Key:
   - Removal
 
 Version 2.1.6
+    Fixed a bug where Arcane Salvage could never fail
     Fixed a bug where Salvage was giving back too many resources
+    SubSkillFailure renamed to SubSkillFailed in config.yml
+    SubSkillFailed now sends copies to chat by default
+    SubSkillFailed no longer sends messages to the action bar by default
 
 Version 2.1.5
     mcMMO no longer ignores cancelled events relating to the player left or right clicking

src/main/java/com/gmail/nossr50/datatypes/interactions/NotificationType.java

@@ -11,7 +11,7 @@ public enum NotificationType {
     LEVEL_UP_MESSAGE("LevelUps"),
     HOLIDAY("Holiday"),
     SUBSKILL_MESSAGE("SubSkillInteraction"),
-    SUBSKILL_MESSAGE_FAILURE("SubSkillFailure"),
+    SUBSKILL_MESSAGE_FAILED("SubSkillFailed"),
     TOOL("ToolReady"),
     REQUIREMENTS_NOT_MET("RequirementsNotMet"),
     ABILITY_OFF("AbilityOff"),

src/main/java/com/gmail/nossr50/skills/herbalism/HerbalismManager.java

@@ -190,7 +190,7 @@ public class HerbalismManager extends SkillManager {
      */
     public boolean processGreenThumbBlocks(BlockState blockState) {
         if (!RandomChanceUtil.isActivationSuccessful(SkillActivationType.RANDOM_LINEAR_100_SCALE_WITH_CAP, SubSkillType.HERBALISM_GREEN_THUMB, getPlayer())) {
-            NotificationManager.sendPlayerInformation(getPlayer(), NotificationType.SUBSKILL_MESSAGE_FAILURE, "Herbalism.Ability.GTh.Fail");
+            NotificationManager.sendPlayerInformation(getPlayer(), NotificationType.SUBSKILL_MESSAGE_FAILED, "Herbalism.Ability.GTh.Fail");
             return false;
         }
 
@@ -261,7 +261,7 @@ public class HerbalismManager extends SkillManager {
         player.updateInventory();
 
         if (!RandomChanceUtil.isActivationSuccessful(SkillActivationType.RANDOM_LINEAR_100_SCALE_WITH_CAP, SubSkillType.HERBALISM_SHROOM_THUMB, player)) {
-            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILURE, "Herbalism.Ability.ShroomThumb.Fail");
+            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Herbalism.Ability.ShroomThumb.Fail");
             return false;
         }
 

src/main/java/com/gmail/nossr50/skills/repair/RepairManager.java

@@ -67,7 +67,7 @@ public class RepairManager extends SkillManager {
         Repairable repairable = mcMMO.getRepairableManager().getRepairable(item.getType());
 
         if (item.getItemMeta().isUnbreakable()) {
-            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILURE, "Anvil.Unbreakable");
+            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Anvil.Unbreakable");
             return;
         }
 
@@ -87,7 +87,7 @@ public class RepairManager extends SkillManager {
 
         // Level check
         if (skillLevel < minimumRepairableLevel) {
-            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILURE, "Repair.Skills.Adept", String.valueOf(minimumRepairableLevel), StringUtils.getPrettyItemString(item.getType()));
+            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Repair.Skills.Adept", String.valueOf(minimumRepairableLevel), StringUtils.getPrettyItemString(item.getType()));
             return;
         }
 
@@ -101,7 +101,7 @@ public class RepairManager extends SkillManager {
 
         // Do not repair if at full durability
         if (startDurability <= 0) {
-            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILURE, "Repair.Skills.FullDurability");
+            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Repair.Skills.FullDurability");
             return;
         }
 
@@ -115,13 +115,13 @@ public class RepairManager extends SkillManager {
                 materialsNeeded += ":" + repairMaterialMetadata;
             }
 
-            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILURE, "Skills.NeedMore.Extra", prettyName, materialsNeeded);
+            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Skills.NeedMore.Extra", prettyName, materialsNeeded);
             return;
         }
 
         // Do not repair stacked items
         if (item.getAmount() != 1) {
-            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILURE, "Repair.Skills.StackedItems");
+            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Repair.Skills.StackedItems");
             return;
         }
 
@@ -319,7 +319,7 @@ public class RepairManager extends SkillManager {
                 item.removeEnchantment(enchant);
             }
 
-            NotificationManager.sendPlayerInformation(getPlayer(), NotificationType.SUBSKILL_MESSAGE_FAILURE, "Repair.Arcane.Lost");
+            NotificationManager.sendPlayerInformation(getPlayer(), NotificationType.SUBSKILL_MESSAGE_FAILED, "Repair.Arcane.Lost");
             return;
         }
 
@@ -345,10 +345,10 @@ public class RepairManager extends SkillManager {
         Map<Enchantment, Integer> newEnchants = item.getEnchantments();
 
         if (newEnchants.isEmpty()) {
-            NotificationManager.sendPlayerInformation(getPlayer(), NotificationType.SUBSKILL_MESSAGE_FAILURE, "Repair.Arcane.Fail");
+            NotificationManager.sendPlayerInformation(getPlayer(), NotificationType.SUBSKILL_MESSAGE_FAILED, "Repair.Arcane.Fail");
         }
         else if (downgraded || newEnchants.size() < enchants.size()) {
-            NotificationManager.sendPlayerInformation(getPlayer(), NotificationType.SUBSKILL_MESSAGE_FAILURE, "Repair.Arcane.Downgrade");
+            NotificationManager.sendPlayerInformation(getPlayer(), NotificationType.SUBSKILL_MESSAGE_FAILED, "Repair.Arcane.Downgrade");
         }
         else {
             NotificationManager.sendPlayerInformation(getPlayer(), NotificationType.SUBSKILL_MESSAGE, "Repair.Arcane.Perfect");

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

@@ -66,7 +66,7 @@ public class SalvageManager extends SkillManager {
         Salvageable salvageable = mcMMO.getSalvageableManager().getSalvageable(item.getType());
 
         if (item.getItemMeta().isUnbreakable()) {
-            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILURE, "Anvil.Unbreakable");
+            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Anvil.Unbreakable");
             return;
         }
 
@@ -91,14 +91,14 @@ public class SalvageManager extends SkillManager {
         }
 
         if (item.getDurability() != 0 && (!RankUtils.hasUnlockedSubskill(player, SubSkillType.SALVAGE_ADVANCED_SALVAGE) || !Permissions.advancedSalvage(player))) {
-            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILURE, "Salvage.Skills.Adept.Damaged");
+            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Salvage.Skills.Adept.Damaged");
             return;
         }
 
         int salvageableAmount = Salvage.calculateSalvageableAmount(item.getDurability(), salvageable.getMaximumDurability(), salvageable.getMaximumQuantity());
 
         if (salvageableAmount == 0) {
-            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILURE, "Salvage.Skills.TooDamaged");
+            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Salvage.Skills.TooDamaged");
             player.sendMessage(LocaleLoader.getString("Salvage.Skills.TooDamaged"));
             return;
         }
@@ -193,7 +193,7 @@ public class SalvageManager extends SkillManager {
         Player player = getPlayer();
 
         if (!RankUtils.hasUnlockedSubskill(player, SubSkillType.SALVAGE_ARCANE_SALVAGE) || !Permissions.arcaneSalvage(player)) {
-            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILURE, "Salvage.Skills.ArcaneFailed");
+            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Salvage.Skills.ArcaneFailed");
             return null;
         }
 
@@ -201,6 +201,7 @@ public class SalvageManager extends SkillManager {
         EnchantmentStorageMeta enchantMeta = (EnchantmentStorageMeta) book.getItemMeta();
 
         boolean downgraded = false;
+        boolean arcaneFailure = false;
 
         for (Entry<Enchantment, Integer> enchant : enchants.entrySet()) {
             if (!Salvage.arcaneSalvageEnchantLoss
@@ -210,30 +211,33 @@ public class SalvageManager extends SkillManager {
             }
             else if (enchant.getValue() > 1
                     && Salvage.arcaneSalvageDowngrades
-                    && !RandomChanceUtil.checkRandomChanceExecutionSuccess(new RandomChanceSkillStatic(getExtractPartialEnchantChance(), getPlayer(), SubSkillType.SALVAGE_ARCANE_SALVAGE))) {
+                    && RandomChanceUtil.checkRandomChanceExecutionSuccess(new RandomChanceSkillStatic(getExtractPartialEnchantChance(), getPlayer(), SubSkillType.SALVAGE_ARCANE_SALVAGE))) {
                 enchantMeta.addStoredEnchant(enchant.getKey(), enchant.getValue() - 1, true);
                 downgraded = true;
             }
             else {
+                arcaneFailure = true;
                 downgraded = true;
             }
         }
 
-        Map<Enchantment, Integer> newEnchants = enchantMeta.getStoredEnchants();
+        if(!arcaneFailure)
+        {
+            Map<Enchantment, Integer> newEnchants = enchantMeta.getStoredEnchants();
 
-        if (newEnchants.isEmpty()) {
-            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILURE, "Salvage.Skills.ArcaneFailed");
-            return null;
-        }
+            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");
+            }
 
-        if (downgraded || newEnchants.size() < enchants.size()) {
-            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILURE, "Salvage.Skills.ArcanePartial");
-        }
-        else {
-            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE, "Salvage.Skills.ArcanePartial");
+            book.setItemMeta(enchantMeta);
+        } else {
+            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Salvage.Skills.ArcaneFailed");
+            return null;
         }
 
-        book.setItemMeta(enchantMeta);
         return book;
     }
 

src/main/java/com/gmail/nossr50/skills/taming/TamingManager.java

@@ -351,7 +351,7 @@ public class TamingManager extends SkillManager {
 
         for (Entity entity : player.getNearbyEntities(range, range, range)) {
             if (entity.getType() == type) {
-                NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILURE, Taming.getCallOfTheWildFailureMessage(type));
+                NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, Taming.getCallOfTheWildFailureMessage(type));
                 return false;
             }
         }
@@ -372,7 +372,7 @@ public class TamingManager extends SkillManager {
         int summonAmount = trackedEntities == null ? 0 : trackedEntities.size();
 
         if (summonAmount >= maxAmountSummons) {
-            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILURE, "Taming.Summon.Fail.TooMany", String.valueOf(maxAmountSummons));
+            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Taming.Summon.Fail.TooMany", String.valueOf(maxAmountSummons));
             return false;
         }
 

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

@@ -86,13 +86,13 @@ public class WoodcuttingManager extends SkillManager {
         if (Woodcutting.treeFellerReachedThreshold) {
             Woodcutting.treeFellerReachedThreshold = false;
 
-            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILURE, "Woodcutting.Skills.TreeFeller.Threshold");
+            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Woodcutting.Skills.TreeFeller.Threshold");
             return;
         }
 
         // If the tool can't sustain the durability loss
         if (!Woodcutting.handleDurabilityLoss(treeFellerBlocks, player.getInventory().getItemInMainHand())) {
-            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILURE, "Woodcutting.Skills.TreeFeller.Splinter");
+            NotificationManager.sendPlayerInformation(player, NotificationType.SUBSKILL_MESSAGE_FAILED, "Woodcutting.Skills.TreeFeller.Splinter");
 
             double health = player.getHealth();
 

src/main/resources/advanced.yml

@@ -50,9 +50,9 @@ Feedback:
         SubSkillInteraction:
             Enabled: true
             SendCopyOfMessageToChat: false
-        SubSkillFailure:
-            Enabled: true
-            SendCopyOfMessageToChat: false
+        SubSkillFailed:
+            Enabled: false
+            SendCopyOfMessageToChat: true
         SubSkillUnlocked:
             Enabled: true
             SendCopyOfMessageToChat: true