Reworked some methods related to Herbalism

Fixes double seed consumption for Green Terra when used on crops
Fixes wheat consumption instead of seed for Green Thumb
Fixes XP and double drop exploit with some blocks (flowers...)

Changelog.txt

@@ -9,6 +9,9 @@ Key:
 
 Version 1.4.03-dev
  + Added option to advanced.yml to determine the # of enchant levels used when buffing Super Breaker & Giga Drill Breaker
+ = Fixed bug where Green Thumb would consume wheat instead of seeds
+ = Fixed bug where Green Terra would consume twice the amount of seed when used on crops
+ = Fixed bug where experience would be awarded in Herbalism for some player-placed blocks
  = Fixed bug where players were unable to salvage leather armor
  = Fixed bug with repairing using materials with byte metadata
  = Fixed bug where Fishing was becoming less successful at higher levels

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

@@ -154,14 +154,7 @@ public class BlockListener implements Listener {
              * Instead, we check it inside the drops handler.
              */
             if (Permissions.skillEnabled(player, SkillType.HERBALISM)) {
-
-                // Double drops
                 herbalismManager.herbalismBlockCheck(blockState);
-
-                // Triple drops
-                if (herbalismManager.canGreenTerraPlant()) {
-                    herbalismManager.herbalismBlockCheck(blockState);
-                }
             }
         }
 

src/main/java/com/gmail/nossr50/runnables/skills/HerbalismBlockUpdaterTask.java

@@ -0,0 +1,16 @@
+package com.gmail.nossr50.runnables.skills;
+
+import org.bukkit.block.BlockState;
+
+public class HerbalismBlockUpdaterTask implements Runnable {
+    private BlockState blockState;
+
+    public HerbalismBlockUpdaterTask(BlockState blockState) {
+        this.blockState = blockState;
+    }
+
+    @Override
+    public void run() {
+        blockState.update(true);
+    }
+}

src/main/java/com/gmail/nossr50/runnables/skills/herbalism/GreenTerraTimerTask.java

@@ -1,46 +0,0 @@
-package com.gmail.nossr50.runnables.skills.herbalism;
-
-import org.bukkit.CropState;
-import org.bukkit.block.BlockState;
-import org.bukkit.material.CocoaPlant;
-import org.bukkit.material.CocoaPlant.CocoaPlantSize;
-
-public class GreenTerraTimerTask implements Runnable {
-    private BlockState blockState;
-
-    /**
-     * Convert plants affected by the Green Terra ability.
-     *
-     * @param blockState The {@link BlockState} to check ability activation for
-     */
-    public GreenTerraTimerTask(BlockState blockState) {
-        this.blockState = blockState;
-    }
-
-    @Override
-    public void run() {
-        switch (blockState.getType()) {
-            case CROPS:
-            case CARROT:
-            case POTATO:
-                blockState.setRawData(CropState.MEDIUM.getData());
-                blockState.update(true);
-                return;
-
-            case NETHER_WARTS:
-                blockState.setRawData((byte) 0x2);
-                blockState.update(true);
-                return;
-
-            case COCOA:
-                CocoaPlant plant = (CocoaPlant) blockState.getData();
-                plant.setSize(CocoaPlantSize.MEDIUM);
-                blockState.setData(plant);
-                blockState.update(true);
-                return;
-
-            default:
-                return;
-        }
-    }
-}

src/main/java/com/gmail/nossr50/runnables/skills/herbalism/GreenThumbTimerTask.java

@@ -1,66 +0,0 @@
-package com.gmail.nossr50.runnables.skills.herbalism;
-
-import org.bukkit.block.BlockState;
-import org.bukkit.material.CocoaPlant;
-import org.bukkit.material.CocoaPlant.CocoaPlantSize;
-
-import com.gmail.nossr50.skills.herbalism.Herbalism;
-
-public class GreenThumbTimerTask implements Runnable {
-    private BlockState blockState;
-    private int skillLevel;
-
-    /**
-     * Convert plants affected by the Green Thumb ability.
-     *
-     * @param blockState The {@link BlockState} to check ability activation for
-     * @param skillLevel The player's Herbalism skill level
-     */
-    public GreenThumbTimerTask(BlockState blockState, int skillLevel) {
-        this.blockState = blockState;
-        this.skillLevel = skillLevel;
-    }
-
-    @Override
-    public void run() {
-        int greenThumbStage = Math.min(Math.min(skillLevel, Herbalism.greenThumbStageMaxLevel) / Herbalism.greenThumbStageChangeLevel, 4);
-
-        switch (blockState.getType()) {
-            case CROPS:
-            case CARROT:
-            case POTATO:
-                blockState.setRawData((byte) greenThumbStage);
-                blockState.update(true);
-                return;
-
-            case NETHER_WARTS:
-                if (greenThumbStage > 2) {
-                    blockState.setRawData((byte) 0x2);
-                }
-                else if (greenThumbStage == 2) {
-                    blockState.setRawData((byte) 0x1);
-                }
-                else {
-                    blockState.setRawData((byte) 0x0);
-                }
-                blockState.update(true);
-                return;
-
-            case COCOA:
-                CocoaPlant plant = (CocoaPlant) blockState.getData();
-
-                if (greenThumbStage > 1) {
-                    plant.setSize(CocoaPlantSize.MEDIUM);
-                }
-                else {
-                    plant.setSize(CocoaPlantSize.SMALL);
-                }
-                blockState.setData(plant);
-                blockState.update(true);
-                return;
-
-            default:
-                return;
-        }
-    }
-}

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

@@ -63,24 +63,17 @@ public class Herbalism {
     protected static int calculateCatciAndSugarDrops(BlockState blockState) {
         Block block = blockState.getBlock();
         Material blockType = blockState.getType();
-        int dropAmount = 0;
-
-        // Handle the original block
-        if (!mcMMO.placeStore.isTrue(blockState)) {
-            dropAmount++;
-        }
+        int dropAmount = 1;
 
         // Handle the two blocks above it - cacti & sugar cane can only grow 3 high naturally
         for (int y = 1; y < 3; y++) {
             Block relativeBlock = block.getRelative(BlockFace.UP, y);
-            Material relativeBlockType = relativeBlock.getType();
 
-            // If the first one is air, so is the next one
-            if (relativeBlockType == Material.AIR) {
+            if (relativeBlock.getType() != blockType) {
                 break;
             }
 
-            if (relativeBlockType == blockType && !mcMMO.placeStore.isTrue(relativeBlock)) {
+            if (!mcMMO.placeStore.isTrue(relativeBlock)) {
                 dropAmount++;
             }
         }

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

@@ -3,12 +3,14 @@ package com.gmail.nossr50.skills.herbalism;
 import java.util.ArrayList;
 import java.util.List;
 
-import org.bukkit.Location;
+import org.bukkit.CropState;
 import org.bukkit.Material;
 import org.bukkit.block.BlockState;
 import org.bukkit.entity.Player;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.inventory.PlayerInventory;
+import org.bukkit.material.CocoaPlant;
+import org.bukkit.material.CocoaPlant.CocoaPlantSize;
 
 import com.gmail.nossr50.mcMMO;
 import com.gmail.nossr50.config.Config;
@@ -20,8 +22,7 @@ import com.gmail.nossr50.datatypes.skills.SkillType;
 import com.gmail.nossr50.datatypes.skills.ToolType;
 import com.gmail.nossr50.datatypes.treasure.HylianTreasure;
 import com.gmail.nossr50.locale.LocaleLoader;
-import com.gmail.nossr50.runnables.skills.herbalism.GreenTerraTimerTask;
-import com.gmail.nossr50.runnables.skills.herbalism.GreenThumbTimerTask;
+import com.gmail.nossr50.runnables.skills.HerbalismBlockUpdaterTask;
 import com.gmail.nossr50.skills.SkillManager;
 import com.gmail.nossr50.util.BlockUtils;
 import com.gmail.nossr50.util.Misc;
@@ -107,65 +108,61 @@ public class HerbalismManager extends SkillManager {
     }
 
     /**
-     * Process double drops & XP gain for Herbalism.
+     * 
      *
      * @param blockState The {@link BlockState} to check ability activation for
-     * @return true if the ability was successful, false otherwise
      */
     public void herbalismBlockCheck(BlockState blockState) {
-        Player player = getPlayer();
-        Material blockType = blockState.getType();
-
-        HerbalismBlock herbalismBlock = HerbalismBlock.getHerbalismBlock(blockType);
-        CustomBlock customBlock = null;
+        if (mcMMO.placeStore.isTrue(blockState)) {
+            return;
+        }
 
+        Material material = blockState.getType();
+        HerbalismBlock herbalismBlock = HerbalismBlock.getHerbalismBlock(material);
+        ItemStack drop = null;
+        int amount = 1;
         int xp = 0;
-        int dropAmount = 1;
-        ItemStack dropItem = null;
+        boolean greenTerra = mcMMOPlayer.getAbilityMode(AbilityType.GREEN_TERRA);
 
         if (herbalismBlock != null) {
-            if (blockType == Material.CACTUS || blockType == Material.SUGAR_CANE_BLOCK) {
-                dropItem = herbalismBlock.getDropItem();
-                dropAmount = Herbalism.calculateCatciAndSugarDrops(blockState);
-                xp = herbalismBlock.getXpGain() * dropAmount;
+            if (herbalismBlock.hasGreenThumbPermission(getPlayer())) {
+                processGreenThumbPlants(blockState, greenTerra);
             }
-            else if (herbalismBlock.hasGreenThumbPermission(player)) {
-                dropItem = herbalismBlock.getDropItem();
-                xp = herbalismBlock.getXpGain();
-                processGreenThumbPlants(blockState);
+
+            xp = herbalismBlock.getXpGain();
+
+            if (herbalismBlock.canDoubleDrop() && Permissions.doubleDrops(getPlayer(), skill)) {
+                drop = herbalismBlock.getDropItem();
             }
-            else {
-                if (!mcMMO.placeStore.isTrue(blockState)) {
-                    dropItem = herbalismBlock.getDropItem();
-                    xp = herbalismBlock.getXpGain();
-                }
+
+            if (material == Material.CACTUS || material == Material.SUGAR_CANE_BLOCK) {
+                amount = Herbalism.calculateCatciAndSugarDrops(blockState);
+                xp *= amount;
             }
         }
         else {
-            customBlock = ModUtils.getCustomBlock(blockState);
-            dropItem = customBlock.getItemDrop();
+            CustomBlock customBlock = ModUtils.getCustomBlock(blockState);
             xp = customBlock.getXpGain();
-        }
-
-        if (Permissions.doubleDrops(player, skill) && SkillUtils.activationSuccessful(getSkillLevel(), getActivationChance(), Herbalism.doubleDropsMaxChance, Herbalism.doubleDropsMaxLevel)) {
-            Location location = blockState.getLocation();
 
-            if (dropItem != null && herbalismBlock != null && herbalismBlock.canDoubleDrop()) {
-                Misc.dropItems(location, dropItem, dropAmount);
-            }
-            else if (customBlock != null) {
+            if (Permissions.doubleDrops(getPlayer(), skill)) {
                 int minimumDropAmount = customBlock.getMinimumDropAmount();
                 int maximumDropAmount = customBlock.getMaximumDropAmount();
-
-                if (minimumDropAmount != maximumDropAmount) {
-                    Misc.randomDropItems(location, dropItem, maximumDropAmount - minimumDropAmount);
-                }
-
-                Misc.dropItems(location, dropItem, minimumDropAmount);
+                drop = customBlock.getItemDrop();
+                amount = Misc.getRandom().nextInt(maximumDropAmount - minimumDropAmount + 1) + minimumDropAmount;
             }
         }
 
         applyXpGain(xp);
+
+        if (drop == null) {
+            return;
+        }
+
+        for (int i = greenTerra ? 2 : 1; i != 0; i--) {
+            if (SkillUtils.activationSuccessful(getSkillLevel(), getActivationChance(), Herbalism.doubleDropsMaxChance, Herbalism.doubleDropsMaxLevel)) {
+                Misc.dropItems(blockState.getLocation(), drop, amount);
+            }
+        }
     }
 
     /**
@@ -269,29 +266,82 @@ public class HerbalismManager extends SkillManager {
      * Process the Green Thumb ability for plants.
      *
      * @param blockState The {@link BlockState} to check ability activation for
+     * @param greenTerra
      */
-    private void processGreenThumbPlants(BlockState blockState) {
+    private void processGreenThumbPlants(BlockState blockState, boolean greenTerra) {
         Player player = getPlayer();
         PlayerInventory playerInventory = player.getInventory();
-        ItemStack seed = HerbalismBlock.getHerbalismBlock(blockState.getType()).getDropItem();
+        ItemStack seed = (blockState.getType() == Material.CROPS) ? new ItemStack(Material.SEEDS) : HerbalismBlock.getHerbalismBlock(blockState.getType()).getDropItem();
 
         if (!playerInventory.containsAtLeast(seed, 1)) {
             return;
         }
 
-        if (mcMMOPlayer.getAbilityMode(AbilityType.GREEN_TERRA)) {
-            playerInventory.removeItem(seed);
-            player.updateInventory(); // Needed until replacement available
-
-            mcMMO.p.getServer().getScheduler().scheduleSyncDelayedTask(mcMMO.p, new GreenTerraTimerTask(blockState), 0);
+        if (!greenTerra && !SkillUtils.activationSuccessful(getSkillLevel(), getActivationChance(), Herbalism.greenThumbMaxChance, Herbalism.greenThumbMaxLevel)) {
             return;
         }
-        else if (SkillUtils.activationSuccessful(getSkillLevel(), getActivationChance(), Herbalism.greenThumbMaxChance, Herbalism.greenThumbMaxLevel)) {
-            playerInventory.removeItem(seed);
-            player.updateInventory(); // Needed until replacement available
 
-            mcMMO.p.getServer().getScheduler().scheduleSyncDelayedTask(mcMMO.p, new GreenThumbTimerTask(blockState, getSkillLevel()), 0);
+        if (!handleBlockState(blockState, greenTerra)) {
             return;
         }
+
+        playerInventory.removeItem(seed);
+        player.updateInventory(); // Needed until replacement available
+        mcMMO.p.getServer().getScheduler().runTaskLater(mcMMO.p, new HerbalismBlockUpdaterTask(blockState), 0);
+    }
+
+    private boolean handleBlockState(BlockState blockState, boolean greenTerra) {
+        switch (blockState.getType()) {
+            case CROPS:
+            case CARROT:
+            case POTATO:
+                if (greenTerra) {
+                    blockState.setRawData(CropState.MEDIUM.getData()); // 2
+                }
+                else {
+                    blockState.setRawData(getGreenThumbStage());
+                }
+
+                return true;
+
+            case NETHER_WARTS:
+                if (greenTerra) {
+                    blockState.setRawData((byte) 2);
+                }
+                else {
+                    int greenThumbStage = getGreenThumbStage();
+
+                    if (greenThumbStage > 2) {
+                        blockState.setRawData((byte) 2);
+                    }
+                    else if (greenThumbStage == 2) {
+                        blockState.setRawData((byte) 1);
+                    }
+                    else {
+                        blockState.setRawData((byte) 0);
+                    }
+                }
+
+                return true;
+
+            case COCOA:
+                CocoaPlant plant = (CocoaPlant) blockState.getData();
+
+                if (greenTerra || getGreenThumbStage() > 1) {
+                    plant.setSize(CocoaPlantSize.MEDIUM);
+                }
+                else {
+                    plant.setSize(CocoaPlantSize.SMALL);
+                }
+
+                return true;
+
+            default:
+                return false;
+        }
+    }
+
+    private byte getGreenThumbStage() {
+        return (byte) Math.min(Math.min(getProfile().getSkillLevel(SkillType.HERBALISM), Herbalism.greenThumbStageMaxLevel) / Herbalism.greenThumbStageChangeLevel, 4);
     }
 }