Many Herbalism bug fixes

Changelog.txt

@@ -1,6 +1,17 @@
 Version 2.1.95
+    Added missing Chorus_Fruit & Chorus_Plant entries to Herbalism's Bonus Drops in config.yml (See notes)
+    Added 'Carrots, Cocoa, Potatoes, Wheat, Beetroots, Nether_Wart' to Herbalism in experience.yml (See notes)
     Fixed a bug preventing Wandering Traders from granting XP
+    Fixed a bug that prevented Chorus Tree's from giving full XP if you broke anything other than the bottom block
+    Fixed a bug which could cause Large Fern's to reward less XP
+    Fixed a bug where certain herbalism crops could have fewer than intended bonus drops
     Added missing 'Chorus_Flower' entry to herbalism in experience.yml (update your config manually or delete the file to regenerate it)
+    Added some debug messages about XP gains if you are in debug mode
+    Added some debug messages for Acrobatics if you are in debug mode
+
+    NOTES:
+    Add 'Chorus_Fruit' and 'Chorus_Plant' under Bonus_Drops.Herbalism in config.yml or you will not be getting double drops for Chorus Fruit.
+    You shouldn't need to add "Carrots, Cocoa, Potatoes, Wheat, Beetroots, Nether_Wart" to your experience file, it seems that file updates automatically for missing entries.
 
 Version 2.1.94
     2 new devs have joined the mcMMO team (electronicboy, kashike), bringing the active dev team to 3 including myself! Strings relating to authors of mcMMO have been updated to reflect this

src/main/java/com/gmail/nossr50/datatypes/BlockSnapshot.java

@@ -0,0 +1,30 @@
+package com.gmail.nossr50.datatypes;
+
+import org.bukkit.Material;
+import org.bukkit.block.Block;
+
+/**
+ * Contains a snapshot of a block at a specific moment in time
+ * Used to check before/after type stuff
+ */
+public class BlockSnapshot {
+    private final Material oldType;
+    private Block blockRef;
+
+    public BlockSnapshot(Material oldType, Block blockRef) {
+        this.oldType = oldType;
+        this.blockRef = blockRef;
+    }
+
+    public Material getOldType() {
+        return oldType;
+    }
+
+    public Block getBlockRef() {
+        return blockRef;
+    }
+
+    public boolean hasChangedType() {
+        return oldType != blockRef.getState().getType();
+    }
+}

src/main/java/com/gmail/nossr50/datatypes/skills/subskills/acrobatics/Roll.java

@@ -274,12 +274,23 @@ public class Roll extends AcrobaticsSubSkill {
             return false;
         }
 
+        McMMOPlayer mcMMOPlayer = UserManager.getPlayer(player);
+
         if (player.getInventory().getItemInMainHand().getType() == Material.ENDER_PEARL || player.isInsideVehicle()) {
+            if(mcMMOPlayer.isDebugMode()) {
+                mcMMOPlayer.getPlayer().sendMessage("Acrobatics XP Prevented: Ender Pearl or Inside Vehicle");
+            }
             return true;
         }
 
         if(UserManager.getPlayer(player).getAcrobaticsManager().hasFallenInLocationBefore(getBlockLocation(player)))
+        {
+            if(mcMMOPlayer.isDebugMode()) {
+                mcMMOPlayer.getPlayer().sendMessage("Acrobatics XP Prevented: Fallen in location before");
+            }
+
             return true;
+        }
 
         return false; //NOT EXPLOITING
     }

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

@@ -343,7 +343,7 @@ public class BlockListener implements Listener {
              * Instead, we check it inside the drops handler.
              */
             if (PrimarySkillType.HERBALISM.getPermissions(player)) {
-                herbalismManager.herbalismBlockCheck(blockState);
+                herbalismManager.processHerbalismBlockBreakEvent(event);
             }
         }
 
@@ -574,7 +574,7 @@ public class BlockListener implements Listener {
          * We don't need to check permissions here because they've already been checked for the ability to even activate.
          */
         if (mcMMOPlayer.getAbilityMode(SuperAbilityType.GREEN_TERRA) && BlockUtils.canMakeMossy(blockState)) {
-            if (mcMMOPlayer.getHerbalismManager().processGreenTerra(blockState)) {
+            if (mcMMOPlayer.getHerbalismManager().processGreenTerraBlockConversion(blockState)) {
                 blockState.update(true);
             }
         }

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

@@ -75,6 +75,11 @@ public class SelfListener implements Listener {
         McMMOPlayer mcMMOPlayer = UserManager.getPlayer(player);
         PrimarySkillType primarySkillType = event.getSkill();
 
+        if(mcMMOPlayer.isDebugMode()) {
+            mcMMOPlayer.getPlayer().sendMessage(event.getSkill().toString() + " XP Gained");
+            mcMMOPlayer.getPlayer().sendMessage("Incoming Raw XP: "+event.getRawXpGained());
+        }
+
         //WorldGuard XP Check
         if(event.getXpGainReason() == XPGainReason.PVE ||
                 event.getXpGainReason() == XPGainReason.PVP ||
@@ -87,6 +92,10 @@ public class SelfListener implements Listener {
                 {
                     event.setRawXpGained(0);
                     event.setCancelled(true);
+
+                    if(mcMMOPlayer.isDebugMode()) {
+                        mcMMOPlayer.getPlayer().sendMessage("No WG XP Flag - New Raw XP: "+event.getRawXpGained());
+                    }
                 }
             }
         }
@@ -112,6 +121,9 @@ public class SelfListener implements Listener {
         int threshold = ExperienceConfig.getInstance().getDiminishedReturnsThreshold(primarySkillType);
 
         if (threshold <= 0 || !ExperienceConfig.getInstance().getDiminishedReturnsEnabled()) {
+            if(mcMMOPlayer.isDebugMode()) {
+                mcMMOPlayer.getPlayer().sendMessage("Final Raw XP: "+event.getRawXpGained());
+            }
             // Diminished returns is turned off
             return;
         }
@@ -156,6 +168,10 @@ public class SelfListener implements Listener {
             }
 
         }
+
+        if(mcMMOPlayer.isDebugMode()) {
+            mcMMOPlayer.getPlayer().sendMessage("Final Raw XP: "+event.getRawXpGained());
+        }
     }
 
 

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

@@ -0,0 +1,23 @@
+package com.gmail.nossr50.runnables.skills;
+
+import com.gmail.nossr50.datatypes.BlockSnapshot;
+import com.gmail.nossr50.datatypes.player.McMMOPlayer;
+import org.bukkit.scheduler.BukkitRunnable;
+
+import java.util.ArrayList;
+
+public class DelayedHerbalismXPCheckTask extends BukkitRunnable {
+
+    private final McMMOPlayer mcMMOPlayer;
+    private final ArrayList<BlockSnapshot> chorusBlocks;
+
+    public DelayedHerbalismXPCheckTask(McMMOPlayer mcMMOPlayer, ArrayList<BlockSnapshot> chorusBlocks) {
+        this.mcMMOPlayer = mcMMOPlayer;
+        this.chorusBlocks = chorusBlocks;
+    }
+
+    @Override
+    public void run() {
+        mcMMOPlayer.getHerbalismManager().awardXPForBlockSnapshots(chorusBlocks);
+    }
+}

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

@@ -1,15 +1,10 @@
 package com.gmail.nossr50.skills.herbalism;
 
 import com.gmail.nossr50.mcMMO;
-import com.gmail.nossr50.util.BlockUtils;
 import com.gmail.nossr50.util.skills.SkillUtils;
 import org.bukkit.Material;
-import org.bukkit.block.Block;
-import org.bukkit.block.BlockFace;
 import org.bukkit.block.BlockState;
 
-import java.util.HashSet;
-
 public class Herbalism {
 
     /**
@@ -43,145 +38,6 @@ public class Herbalism {
         }
     }
 
-    private static int calculateChorusPlantDrops(Block target, boolean triple, HerbalismManager herbalismManager) {
-        return calculateChorusPlantDropsRecursive(target, new HashSet<>(), triple, herbalismManager);
-    }
-
-    private static int calculateChorusPlantDropsRecursive(Block target, HashSet<Block> traversed, boolean triple, HerbalismManager herbalismManager) {
-        if (target.getType() != Material.CHORUS_PLANT)
-            return 0;
-
-        // Prevent any infinite loops, who needs more than 64 chorus anyways
-        if (traversed.size() > 64)
-            return 0;
-
-        if (!traversed.add(target))
-            return 0;
-
-        int dropAmount = 0;
-
-        if (mcMMO.getPlaceStore().isTrue(target))
-            mcMMO.getPlaceStore().setFalse(target);
-        else
-        {
-            dropAmount++;
-
-            if(herbalismManager.checkDoubleDrop(target.getState()))
-                BlockUtils.markDropsAsBonus(target.getState(), triple);
-        }
-
-        for (BlockFace blockFace : new BlockFace[] { BlockFace.UP, BlockFace.NORTH, BlockFace.SOUTH, BlockFace.EAST ,BlockFace.WEST})
-            dropAmount += calculateChorusPlantDropsRecursive(target.getRelative(blockFace, 1), traversed, triple, herbalismManager);
-
-        return dropAmount;
-    }
-
-    /**
-     * Calculate the drop amounts for multi block plants based on the blocks
-     * relative to them.
-     *
-     * @param blockState
-     *            The {@link BlockState} of the bottom block of the plant
-     * @return the number of bonus drops to award from the blocks in this plant
-     */
-    protected static int countAndMarkDoubleDropsMultiBlockPlant(BlockState blockState, boolean triple, HerbalismManager herbalismManager) {
-        Block block = blockState.getBlock();
-        Material blockType = blockState.getType();
-        int dropAmount = 0;
-        int bonusDropAmount = 0;
-        int bonusAdd = triple ? 2 : 1;
-
-        if (blockType == Material.CHORUS_PLANT) {
-            dropAmount = 1;
-
-            if (block.getRelative(BlockFace.DOWN, 1).getType() == Material.END_STONE) {
-                dropAmount = calculateChorusPlantDrops(block, triple, herbalismManager);
-            }
-        } else {
-            //Check the block itself first
-            if(!mcMMO.getPlaceStore().isTrue(block))
-            {
-                dropAmount++;
-
-                if(herbalismManager.checkDoubleDrop(blockState))
-                    bonusDropAmount+=bonusAdd;
-            } else {
-                mcMMO.getPlaceStore().setFalse(blockState);
-            }
-
-            // Handle the two blocks above it - cacti & sugar cane can only grow 3 high naturally
-            for (int y = 1; y < 255; y++) {
-                Block relativeBlock = block.getRelative(BlockFace.UP, y);
-
-                if (relativeBlock.getType() != blockType) {
-                    break;
-                }
-
-                if (mcMMO.getPlaceStore().isTrue(relativeBlock)) {
-                    mcMMO.getPlaceStore().setFalse(relativeBlock);
-                } else {
-                    dropAmount++;
-
-                    if(herbalismManager.checkDoubleDrop(relativeBlock.getState()))
-                        bonusDropAmount+=bonusAdd;
-                }
-            }
-        }
-
-        //Mark the original block for bonus drops
-        BlockUtils.markDropsAsBonus(blockState, bonusDropAmount);
-
-        return dropAmount;
-    }
-
-    /**
-     * Calculate the drop amounts for kelp plants based on the blocks
-     * relative to them.
-     *
-     * @param blockState
-     *            The {@link BlockState} of the bottom block of the plant
-     * @return the number of bonus drops to award from the blocks in this plant
-     */
-    protected static int countAndMarkDoubleDropsKelp(BlockState blockState, boolean triple, HerbalismManager herbalismManager) {
-        Block block = blockState.getBlock();
-
-        int kelpMaxHeight = 255;
-        int amount = 1;
-
-        // Handle the two blocks above it - cacti & sugar cane can only grow 3 high naturally
-        for (int y = 1; y < kelpMaxHeight; y++) {
-            Block relativeUpBlock = block.getRelative(BlockFace.UP, y);
-
-            if(!isKelp(relativeUpBlock))
-                break;
-
-            amount += 1;
-
-            if(herbalismManager.checkDoubleDrop(relativeUpBlock.getState()))
-                BlockUtils.markDropsAsBonus(relativeUpBlock.getState(), triple);
-
-        }
-
-        return amount;
-    }
-
-    private static int addKelpDrops(int dropAmount, Block relativeBlock) {
-        if (isKelp(relativeBlock) && !mcMMO.getPlaceStore().isTrue(relativeBlock)) {
-            dropAmount++;
-        } else {
-            mcMMO.getPlaceStore().setFalse(relativeBlock);
-        }
-
-        return dropAmount;
-    }
-
-    private static boolean isKelp(Block relativeBlock) {
-        Material kelptype_1 = Material.KELP_PLANT;
-        Material kelptype_2 = Material.KELP;
-
-        return relativeBlock.getType() == kelptype_1 || relativeBlock.getType() == kelptype_2;
-    }
-
     /**
      * Convert blocks affected by the Green Thumb & Green Terra abilities.
      *

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

@@ -3,9 +3,10 @@ package com.gmail.nossr50.skills.herbalism;
 import com.gmail.nossr50.config.Config;
 import com.gmail.nossr50.config.experience.ExperienceConfig;
 import com.gmail.nossr50.config.treasure.TreasureConfig;
+import com.gmail.nossr50.datatypes.BlockSnapshot;
 import com.gmail.nossr50.datatypes.experience.XPGainReason;
+import com.gmail.nossr50.datatypes.experience.XPGainSource;
 import com.gmail.nossr50.datatypes.interactions.NotificationType;
-import com.gmail.nossr50.datatypes.mods.CustomBlock;
 import com.gmail.nossr50.datatypes.player.McMMOPlayer;
 import com.gmail.nossr50.datatypes.skills.PrimarySkillType;
 import com.gmail.nossr50.datatypes.skills.SubSkillType;
@@ -13,6 +14,7 @@ import com.gmail.nossr50.datatypes.skills.SuperAbilityType;
 import com.gmail.nossr50.datatypes.skills.ToolType;
 import com.gmail.nossr50.datatypes.treasure.HylianTreasure;
 import com.gmail.nossr50.mcMMO;
+import com.gmail.nossr50.runnables.skills.DelayedHerbalismXPCheckTask;
 import com.gmail.nossr50.runnables.skills.HerbalismBlockUpdaterTask;
 import com.gmail.nossr50.skills.SkillManager;
 import com.gmail.nossr50.util.*;
@@ -24,13 +26,20 @@ import com.gmail.nossr50.util.skills.SkillActivationType;
 import com.gmail.nossr50.util.skills.SkillUtils;
 import org.bukkit.Location;
 import org.bukkit.Material;
+import org.bukkit.block.Block;
+import org.bukkit.block.BlockFace;
 import org.bukkit.block.BlockState;
 import org.bukkit.block.data.Ageable;
+import org.bukkit.block.data.BlockData;
 import org.bukkit.entity.Player;
+import org.bukkit.event.block.BlockBreakEvent;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.inventory.PlayerInventory;
 import org.bukkit.metadata.FixedMetadataValue;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
 import java.util.List;
 
 public class HerbalismManager extends SkillManager {
@@ -38,10 +47,6 @@ public class HerbalismManager extends SkillManager {
         super(mcMMOPlayer, PrimarySkillType.HERBALISM);
     }
 
-    public boolean canBlockCheck() {
-        return !(Config.getInstance().getHerbalismPreventAFK() && getPlayer().isInsideVehicle());
-    }
-
     public boolean canGreenThumbBlock(BlockState blockState) {
         if(!RankUtils.hasUnlockedSubskill(getPlayer(), SubSkillType.HERBALISM_GREEN_THUMB))
             return false;
@@ -78,7 +83,7 @@ public class HerbalismManager extends SkillManager {
         return mcMMOPlayer.getToolPreparationMode(ToolType.HOE) && Permissions.greenTerra(getPlayer());
     }
 
-    public boolean canGreenTerraPlant() {
+    public boolean isGreenTerraActive() {
         return mcMMOPlayer.getAbilityMode(SuperAbilityType.GREEN_TERRA);
     }
 
@@ -98,7 +103,7 @@ public class HerbalismManager extends SkillManager {
      * @param blockState The {@link BlockState} to check ability activation for
      * @return true if the ability was successful, false otherwise
      */
-    public boolean processGreenTerra(BlockState blockState) {
+    public boolean processGreenTerraBlockConversion(BlockState blockState) {
         Player player = getPlayer();
 
         if (!Permissions.greenThumbBlock(player, blockState.getType())) {
@@ -120,63 +125,398 @@ public class HerbalismManager extends SkillManager {
     }
 
     /**
-     * @param blockState The {@link BlockState} to check ability activation for
+     * Handles herbalism abilities and XP rewards from a BlockBreakEvent
+     * @param blockBreakEvent The Block Break Event to process
      */
-    public void herbalismBlockCheck(BlockState blockState) {
+    public void processHerbalismBlockBreakEvent(BlockBreakEvent blockBreakEvent) {
         Player player = getPlayer();
-        Material material = blockState.getType();
-        boolean oneBlockPlant = isOneBlockPlant(material);
 
-        // Prevents placing and immediately breaking blocks for exp
-        if (oneBlockPlant && mcMMO.getPlaceStore().isTrue(blockState)) {
+        if (Config.getInstance().getHerbalismPreventAFK() && player.isInsideVehicle()) {
             return;
         }
 
-        if (!canBlockCheck()) {
-            return;
+        /*
+         * There are single-block plants and multi-block plants in Minecraft
+         * In order to give out proper rewards, we need to collect all blocks that would be broken from this event
+         */
+
+        //Grab all broken blocks
+        HashSet<Block> brokenBlocks = getBrokenHerbalismBlocks(blockBreakEvent);
+
+        //Handle rewards, xp, ability interactions, etc
+        processHerbalismOnBlocksBroken(blockBreakEvent, brokenBlocks);
+    }
+
+    /**
+     * Process rewards for a set of plant blocks for Herbalism
+     * @param blockBreakEvent the block break event
+     * @param brokenPlants plant blocks to process
+     */
+    private void processHerbalismOnBlocksBroken(BlockBreakEvent blockBreakEvent, HashSet<Block> brokenPlants) {
+        BlockState originalBreak = blockBreakEvent.getBlock().getState();
+
+        //TODO: The design of Green Terra needs to change, this is a mess
+        if(Permissions.greenThumbPlant(getPlayer(), originalBreak.getType())) {
+            processGreenThumbPlants(originalBreak, isGreenTerraActive());
         }
 
-        int amount;
-        int xp;
-        boolean greenTerra = mcMMOPlayer.getAbilityMode(skill.getAbility());
+        /*
+         * Mark blocks for double drops
+         * Be aware of the hacky interactions we are doing with Chorus Plants
+         */
+        checkDoubleDropsOnBrokenPlants(brokenPlants);
+
+        //It would take an expensive algorithm to predict which parts of a Chorus Tree will break as a result of root break
+        //So this hacky method is used instead
+        ArrayList<BlockSnapshot> delayedChorusBlocks = new ArrayList<>(); //Blocks that will be checked in future ticks
+        HashSet<Block> noDelayPlantBlocks = new HashSet<>(); //Blocks that will be checked immediately
+
+        for(Block brokenPlant : brokenPlants) {
+            /*
+             * This check is to make XP bars appear to work properly with Chorus Trees by giving XP for the originalBreak immediately instead of later
+             */
+            if(brokenPlant.getLocation().equals(originalBreak.getBlock().getLocation())) {
+                //If its the same block as the original, we are going to directly check it for being a valid XP gain and add it to the nonChorusBlocks list even if its a chorus block
+                //This stops a delay from happening when bringing up the XP bar for chorus trees
+                if(!mcMMO.getPlaceStore().isTrue(originalBreak)) {
+                    //Even if its a chorus block, the original break will be moved to nonChorusBlocks for immediate XP rewards
+                    noDelayPlantBlocks.add(brokenPlant);
+                } else {
+                    if(isChorusTree(brokenPlant.getType())) {
+                        //If its a chorus tree AND it was marked as true in the placestore then we add this block to the list of chorus blocks
+                        delayedChorusBlocks.add(new BlockSnapshot(brokenPlant.getType(), brokenPlant));
+                    } else {
+                        noDelayPlantBlocks.add(brokenPlant); //If its not a chorus plant that was marked as unnatural but it was marked unnatural, put it in the nodelay list to be handled
+                    }
+                }
+            } else if(isChorusTree(brokenPlant.getType())) {
+                //Chorus Blocks get checked for XP several ticks later to avoid expensive calculations
+                delayedChorusBlocks.add(new BlockSnapshot(brokenPlant.getType(), brokenPlant));
+            } else {
+                noDelayPlantBlocks.add(brokenPlant);
+            }
+        }
+
+        //Give out XP to the non-chorus blocks
+        if(noDelayPlantBlocks.size() > 0) {
+            //Note: Will contain 1 chorus block if the original block was a chorus block, this is to prevent delays for the XP bar
+            awardXPForPlantBlocks(noDelayPlantBlocks);
+        }
 
-        if (mcMMO.getModManager().isCustomHerbalismBlock(blockState)) {
-            CustomBlock customBlock = mcMMO.getModManager().getBlock(blockState);
-            xp = customBlock.getXpGain();
+        if(delayedChorusBlocks.size() > 0) {
+            //Check XP for chorus blocks
+            DelayedHerbalismXPCheckTask delayedHerbalismXPCheckTask = new DelayedHerbalismXPCheckTask(mcMMOPlayer, delayedChorusBlocks);
+
+            //Large delay because the tree takes a while to break
+            delayedHerbalismXPCheckTask.runTaskLater(mcMMO.p, 20); //Calculate Chorus XP + Bonus Drops 1 tick later
+        }
+    }
+
+    public void checkDoubleDropsOnBrokenPlants(Collection<Block> brokenPlants) {
+        for(Block brokenPlant : brokenPlants) {
+            BlockState brokenPlantState = brokenPlant.getState();
+            BlockData plantData = brokenPlantState.getBlockData();
+
+            //Check for double drops
+            if(!mcMMO.getPlaceStore().isTrue(brokenPlant)) {
+
+                /*
+                 *
+                 * Natural Blocks
+                 *
+                 *
+                 *
+                 */
+
+                //Not all things that are natural should give double drops, make sure its fully mature as well
+                if(plantData instanceof Ageable) {
+                    Ageable ageable = (Ageable) plantData;
+
+                    if(isAgeableMature(ageable) || isBizarreAgeable(plantData)) {
+                        markForBonusDrops(brokenPlantState);
+                    }
+                } else if(checkDoubleDrop(brokenPlantState)) {
+                    //Add metadata to mark this block for double or triple drops
+                    markForBonusDrops(brokenPlantState);
+                }
+            } else {
+
+                /*
+                 *
+                 * Unnatural Blocks
+                 *
+                 */
+
+                //If its a Crop we need to reward XP when its fully grown
+                if(isAgeableAndFullyMature(plantData) && !isBizarreAgeable(plantData)) {
+                    //Add metadata to mark this block for double or triple drops
+                    markForBonusDrops(brokenPlantState);
+                }
+            }
+        }
+    }
 
-            if (Permissions.isSubSkillEnabled(player, SubSkillType.HERBALISM_DOUBLE_DROPS) && customBlock.isDoubleDropEnabled()) {
-                if(checkDoubleDrop(blockState))
-                    BlockUtils.markDropsAsBonus(blockState, greenTerra);
+    /**
+     * Checks if BlockData is ageable and we can trust that age for Herbalism rewards/XP reasons
+     * @param blockData target BlockData
+     * @return returns true if the ageable is trustworthy for Herbalism XP / Rewards
+     */
+    public boolean isBizarreAgeable(BlockData blockData) {
+        if(blockData instanceof Ageable) {
+            //Catcus and Sugar Canes cannot be trusted
+            switch(blockData.getMaterial()) {
+                case CACTUS:
+                case SUGAR_CANE:
+                    return true;
+                default:
+                    return false;
             }
         }
-        else {
-            xp = ExperienceConfig.getInstance().getXp(skill, blockState.getBlockData());
 
-            if (!oneBlockPlant) {
-                //Kelp is actually two blocks mixed together
-                if(material == Material.KELP_PLANT || material == Material.KELP) {
-                    amount = Herbalism.countAndMarkDoubleDropsKelp(blockState, greenTerra,this);
-                } else {
-                    amount = Herbalism.countAndMarkDoubleDropsMultiBlockPlant(blockState, greenTerra, this);
+        return false;
+    }
+
+    public void markForBonusDrops(BlockState brokenPlantState) {
+        //Add metadata to mark this block for double or triple drops
+        boolean awardTriple = mcMMOPlayer.getAbilityMode(SuperAbilityType.GREEN_TERRA);
+        BlockUtils.markDropsAsBonus(brokenPlantState, awardTriple);
+    }
+
+    /**
+     * Checks if a block is an ageable and if that ageable is fully mature
+     * @param plantData target plant
+     * @return returns true if the block is both an ageable and fully mature
+     */
+    public boolean isAgeableAndFullyMature(BlockData plantData) {
+        return plantData instanceof Ageable && isAgeableMature((Ageable) plantData);
+    }
+
+    public void awardXPForPlantBlocks(HashSet<Block> brokenPlants) {
+        int xpToReward = 0;
+
+        for(Block brokenPlantBlock : brokenPlants) {
+            BlockState brokenBlockNewState = brokenPlantBlock.getState();
+            BlockData plantData = brokenBlockNewState.getBlockData();
+
+            if(mcMMO.getPlaceStore().isTrue(brokenBlockNewState)) {
+                /*
+                 *
+                 * Unnatural Blocks
+                 *
+                 *
+                 */
+
+                //If its a Crop we need to reward XP when its fully grown
+                if(isAgeableAndFullyMature(plantData) && !isBizarreAgeable(plantData)) {
+                    xpToReward += ExperienceConfig.getInstance().getXp(PrimarySkillType.HERBALISM, brokenBlockNewState.getType());
                 }
 
-                xp *= amount;
+                //Mark it as natural again as it is being broken
+                mcMMO.getPlaceStore().setFalse(brokenBlockNewState);
             } else {
-                /* MARK SINGLE BLOCK CROP FOR DOUBLE DROP */
-                if(checkDoubleDrop(blockState))
-                    BlockUtils.markDropsAsBonus(blockState, greenTerra);
+                /*
+                 *
+                 * Natural Blocks
+                 *
+                 *
+                 */
+
+                //Calculate XP
+                if(plantData instanceof Ageable) {
+                    Ageable plantAgeable = (Ageable) plantData;
+                    if(isAgeableMature(plantAgeable) || isBizarreAgeable(plantData)) {
+                        xpToReward += ExperienceConfig.getInstance().getXp(PrimarySkillType.HERBALISM, brokenBlockNewState.getType());
+                    }
+                } else {
+                    xpToReward += ExperienceConfig.getInstance().getXp(PrimarySkillType.HERBALISM, brokenPlantBlock.getType());
+                }
             }
+        }
+
+        if(mcMMOPlayer.isDebugMode()) {
+            mcMMOPlayer.getPlayer().sendMessage("Plants processed: "+brokenPlants.size());
+        }
+
+        //Reward XP
+        if(xpToReward > 0) {
+            applyXpGain(xpToReward, XPGainReason.PVE, XPGainSource.SELF);
+        }
+    }
 
-            if (Permissions.greenThumbPlant(player, material)) {
-                processGreenThumbPlants(blockState, greenTerra);
+    public boolean isAgeableMature(Ageable ageable) {
+        return ageable.getAge() == ageable.getMaximumAge()
+                && ageable.getAge() != 0;
+    }
+
+    /**
+     * Award XP for any blocks that used to be something else but are now AIR
+     * @param brokenPlants snapshot of broken blocks
+     */
+    public void awardXPForBlockSnapshots(ArrayList<BlockSnapshot> brokenPlants) {
+        /*
+         * This handles XP for blocks that we need to check are broken after the fact
+         * This only applies to chorus trees right now
+         */
+        int xpToReward = 0;
+        int blocksGivingXP = 0;
+
+        for(BlockSnapshot blockSnapshot : brokenPlants) {
+            BlockState brokenBlockNewState = blockSnapshot.getBlockRef().getState();
+
+            //Remove metadata from the snapshot of blocks
+            if(brokenBlockNewState.hasMetadata(mcMMO.BONUS_DROPS_METAKEY)) {
+                brokenBlockNewState.removeMetadata(mcMMO.BONUS_DROPS_METAKEY, mcMMO.p);
+            }
+
+            //If the block is not AIR that means it wasn't broken
+            if(brokenBlockNewState.getType() != Material.AIR) {
+                continue;
+            }
+
+            if(mcMMO.getPlaceStore().isTrue(brokenBlockNewState)) {
+                //Mark it as natural again as it is being broken
+                mcMMO.getPlaceStore().setFalse(brokenBlockNewState);
+            } else {
+                //TODO: Do we care about chorus flower age?
+                //Calculate XP for the old type
+                xpToReward += ExperienceConfig.getInstance().getXp(PrimarySkillType.HERBALISM, blockSnapshot.getOldType());
+                blocksGivingXP++;
             }
         }
 
-        applyXpGain(xp, XPGainReason.PVE);
+        if(mcMMOPlayer.isDebugMode()) {
+            mcMMOPlayer.getPlayer().sendMessage("Chorus Plants checked for XP: "+brokenPlants.size());
+            mcMMOPlayer.getPlayer().sendMessage("Valid Chorus Plant XP Gains: "+blocksGivingXP);
+        }
+
+        //Reward XP
+        if(xpToReward > 0) {
+            applyXpGain(xpToReward, XPGainReason.PVE, XPGainSource.SELF);
+        }
+    }
+
+    /**
+     * Process and return plant blocks from a BlockBreakEvent
+     * @param blockBreakEvent target event
+     * @return a set of plant-blocks that were broken as a result of this event
+     */
+    private HashSet<Block> getBrokenHerbalismBlocks(BlockBreakEvent blockBreakEvent) {
+        //Get an updated capture of this block
+        BlockState originalBlockBlockState = blockBreakEvent.getBlock().getState();
+        Material originalBlockMaterial = originalBlockBlockState.getType();
+        HashSet<Block> blocksBroken = new HashSet<>(); //Blocks broken
+
+        //Check if this block is a one block plant or not
+        boolean oneBlockPlant = isOneBlockPlant(originalBlockMaterial);
+
+        if(oneBlockPlant) {
+            //If the block is a one-block plant return only that
+            blocksBroken.add(originalBlockBlockState.getBlock());
+        } else {
+            //If the block is a multi-block structure, capture a set of all blocks broken and return that
+            blocksBroken = getBrokenBlocksMultiBlockPlants(originalBlockBlockState, blockBreakEvent);
+        }
+
+        //Return all broken plant-blocks
+        return blocksBroken;
+    }
+
+    private HashSet<Block> getBrokenChorusBlocks(BlockState originalBreak) {
+        HashSet<Block> traversedBlocks = grabChorusTreeBrokenBlocksRecursive(originalBreak.getBlock(), new HashSet<>());
+        return traversedBlocks;
+    }
+
+    private HashSet<Block> grabChorusTreeBrokenBlocksRecursive(Block currentBlock, HashSet<Block> traversed) {
+        if (!isChorusTree(currentBlock.getType()))
+            return traversed;
+
+        // Prevent any infinite loops, who needs more than 256 chorus anyways
+        if (traversed.size() > 256)
+            return traversed;
+
+        if (!traversed.add(currentBlock))
+            return traversed;
+
+        //Grab all Blocks in the Tree
+        for (BlockFace blockFace : new BlockFace[] { BlockFace.UP, BlockFace.NORTH, BlockFace.SOUTH, BlockFace.EAST ,BlockFace.WEST})
+            grabChorusTreeBrokenBlocksRecursive(currentBlock.getRelative(blockFace, 1), traversed);
+
+        traversed.add(currentBlock);
+
+        return traversed;
     }
 
-    public boolean isOneBlockPlant(Material material) {
-        return !mcMMO.getMaterialMapStore().isMultiBlock(material);
+    /**
+     * Grab a set of all plant blocks that are broken as a result of this event
+     * The method to grab these blocks is a bit hacky and does not hook into the API
+     * Basically we expect the blocks to be broken if this event is not cancelled and we determine which block are broken on our end rather than any event state captures
+     *
+     * @param blockBreakEvent target event
+     * @return a set of plant-blocks broken from this event
+     */
+    protected HashSet<Block> getBrokenBlocksMultiBlockPlants(BlockState originalBlockBroken, BlockBreakEvent blockBreakEvent) {
+        //Track the broken blocks
+        HashSet<Block> brokenBlocks;
+
+        if (isChorusBranch(originalBlockBroken.getType())) {
+            brokenBlocks = getBrokenChorusBlocks(originalBlockBroken);
+        } else {
+            brokenBlocks = getBlocksBrokenAbove(originalBlockBroken);
+        }
+
+        return brokenBlocks;
+    }
+
+    private boolean isChorusBranch(Material blockType) {
+        return blockType == Material.CHORUS_PLANT;
+    }
+
+    private boolean isChorusTree(Material blockType) {
+        return blockType == Material.CHORUS_PLANT || blockType == Material.CHORUS_FLOWER;
+    }
+
+    /**
+     * Grabs blocks upwards from a target block
+     * A lot of Plants/Crops in Herbalism only break vertically from a broken block
+     * The vertical search returns early if it runs into anything that is not a multi-block plant
+     * Multi-block plants are hard-coded and kept in {@link MaterialMapStore}
+     *
+     * @param breakPointBlockState The point of the "break"
+     * @return A set of blocks above the target block which can be assumed to be broken
+     */
+    private HashSet<Block> getBlocksBrokenAbove(BlockState breakPointBlockState) {
+        HashSet<Block> brokenBlocks = new HashSet<>();
+        Block block = breakPointBlockState.getBlock();
+
+        //Add the initial block to the set
+        brokenBlocks.add(block);
+
+        //Limit our search
+        int maxHeight = 255;
+
+        // Search vertically for multi-block plants, exit early if any non-multi block plants
+        for (int y = 1; y < maxHeight; y++) {
+            //TODO: Should this grab state? It would be more expensive..
+            Block relativeUpBlock = block.getRelative(BlockFace.UP, y);
+
+            //Abandon our search if the block isn't multi
+            if(!mcMMO.getMaterialMapStore().isMultiBlockPlant(relativeUpBlock.getType()))
+                break;
+
+            brokenBlocks.add(relativeUpBlock);
+        }
+
+        return brokenBlocks;
+    }
+
+    /**
+     * If the plant is considered a one block plant
+     * This is determined by seeing if it exists in a hard-coded collection of Multi-Block plants
+     * @param material target plant material
+     * @return true if the block is not contained in the collection of multi-block plants
+     */
+    private boolean isOneBlockPlant(Material material) {
+        return !mcMMO.getMaterialMapStore().isMultiBlockPlant(material);
     }
 
     /**
@@ -184,7 +524,7 @@ public class HerbalismManager extends SkillManager {
      * @param blockState target block state
      * @return true if double drop succeeds
      */
-    public boolean checkDoubleDrop(BlockState blockState)
+    private boolean checkDoubleDrop(BlockState blockState)
     {
         return BlockUtils.checkDoubleDrops(getPlayer(), blockState, skill, SubSkillType.HERBALISM_DOUBLE_DROPS);
     }
@@ -324,7 +664,7 @@ public class HerbalismManager extends SkillManager {
             return;
         }
 
-        if (!handleBlockState(blockState, greenTerra)) {
+        if (!processGrowingPlants(blockState, greenTerra)) {
             return;
         }
 
@@ -341,7 +681,7 @@ public class HerbalismManager extends SkillManager {
         new HerbalismBlockUpdaterTask(blockState).runTaskLater(mcMMO.p, 0);
     }
 
-    private boolean handleBlockState(BlockState blockState, boolean greenTerra) {
+    private boolean processGrowingPlants(BlockState blockState, boolean greenTerra) {
         int greenThumbStage = getGreenThumbStage();
 
         blockState.setMetadata(mcMMO.greenThumbDataKey, new FixedMetadataValue(mcMMO.p, (int) (System.currentTimeMillis() / Misc.TIME_CONVERSION_FACTOR)));

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

@@ -11,6 +11,7 @@ import com.gmail.nossr50.skills.salvage.Salvage;
 import com.gmail.nossr50.util.random.RandomChanceSkill;
 import com.gmail.nossr50.util.random.RandomChanceUtil;
 import org.bukkit.Material;
+import org.bukkit.block.BlockFace;
 import org.bukkit.block.BlockState;
 import org.bukkit.block.data.Ageable;
 import org.bukkit.block.data.BlockData;
@@ -261,8 +262,9 @@ public final class BlockUtils {
 
     public static boolean isFullyGrown(BlockState blockState) {
         BlockData data = blockState.getBlockData();
-        if (data.getMaterial() == Material.CACTUS || data.getMaterial() == Material.SUGAR_CANE)
+        if (data.getMaterial() == Material.CACTUS || data.getMaterial() == Material.SUGAR_CANE) {
             return true;
+        }
         if (data instanceof Ageable) {
             Ageable ageable = (Ageable) data;
             return ageable.getAge() == ageable.getMaximumAge();

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

@@ -20,7 +20,7 @@ public class MaterialMapStore {
     private HashSet<String> herbalismAbilityBlackList;
     private HashSet<String> blockCrackerWhiteList;
     private HashSet<String> canMakeShroomyWhiteList;
-    private HashSet<String> multiBlockEntities;
+    private HashSet<String> multiBlockPlant;
     private HashSet<String> foodItemWhiteList;
 
     public MaterialMapStore()
@@ -32,15 +32,15 @@ public class MaterialMapStore {
         herbalismAbilityBlackList = new HashSet<>();
         blockCrackerWhiteList = new HashSet<>();
         canMakeShroomyWhiteList = new HashSet<>();
-        multiBlockEntities = new HashSet<>();
+        multiBlockPlant = new HashSet<>();
         foodItemWhiteList = new HashSet<>();
 
         fillHardcodedHashSets();
     }
 
-    public boolean isMultiBlock(Material material)
+    public boolean isMultiBlockPlant(Material material)
     {
-        return multiBlockEntities.contains(material.getKey().getKey());
+        return multiBlockPlant.contains(material.getKey().getKey());
     }
 
     public boolean isAbilityActivationBlackListed(Material material)
@@ -81,13 +81,13 @@ public class MaterialMapStore {
     private void fillHardcodedHashSets()
     {
         fillAbilityBlackList();
-        filltoolBlackList();
+        fillToolBlackList();
         fillMossyWhiteList();
         fillLeavesWhiteList();
         fillHerbalismAbilityBlackList();
         fillBlockCrackerWhiteList();
         fillShroomyWhiteList();
-        fillMultiBlockEntitiesList();
+        fillMultiBlockPlantSet();
         fillFoodWhiteList();
     }
 
@@ -134,16 +134,30 @@ public class MaterialMapStore {
         return foodItemWhiteList.contains(material.getKey().getKey());
     }
 
-    private void fillMultiBlockEntitiesList()
+    private void fillMultiBlockPlantSet()
     {
-        multiBlockEntities.add("cactus");
-        multiBlockEntities.add("chorus_plant");
-        multiBlockEntities.add("sugar_cane");
-        multiBlockEntities.add("kelp_plant");
-        multiBlockEntities.add("kelp");
-        multiBlockEntities.add("tall_seagrass");
-        multiBlockEntities.add("tall_grass");
-        multiBlockEntities.add("bamboo");
+        //Single Block Plants
+//        plantBlockSet.add("melon");
+//        plantBlockSet.add("pumpkin");
+//        plantBlockSet.add("potatoes");
+//        plantBlockSet.add("carrots");
+//        plantBlockSet.add("beetroots");
+//        plantBlockSet.add("nether_wart");
+//        plantBlockSet.add("grass");
+//        plantBlockSet.add("fern");
+//        plantBlockSet.add("large_fern");
+
+        //Multi-Block Plants
+        multiBlockPlant.add("cactus");
+        multiBlockPlant.add("chorus_plant");
+        multiBlockPlant.add("chorus_flower");
+        multiBlockPlant.add("sugar_cane");
+        multiBlockPlant.add("kelp_plant");
+        multiBlockPlant.add("kelp");
+        multiBlockPlant.add("tall_seagrass");
+        multiBlockPlant.add("large_fern");
+        multiBlockPlant.add("tall_grass");
+        multiBlockPlant.add("bamboo");
     }
 
     private void fillShroomyWhiteList()
@@ -287,7 +301,7 @@ public class MaterialMapStore {
         abilityBlackList.add("sign"); //1.13 and lower?
     }
     
-    private void filltoolBlackList()
+    private void fillToolBlackList()
     {
         //TODO: Add anvils / missing logs
         toolBlackList.add("black_bed");

src/main/resources/config.yml

@@ -426,6 +426,8 @@ Skills:
 ###
 Bonus_Drops:
     Herbalism:
+        Chorus_Fruit: true
+        Chorus_Plant: true
         Beetroots: true
         Beetroot: true
         Brown_Mushroom: true

src/main/resources/experience.yml

@@ -311,26 +311,25 @@ Experience_Values:
         Dead_Horn_Coral_Wall_Fan: 10
         Allium: 300
         Azure_Bluet: 150
-        Beetroots_Ripe: 50
         Blue_Orchid: 150
         Brown_Mushroom: 150
         Cactus: 30
-        Carrots_Ripe: 50
-        Chorus_Flower_Ripe: 25
         Chorus_Flower: 25
         Chorus_Plant: 1
-        Cocoa_Ripe: 30
-        Wheat_Ripe: 50
+        Carrots: 50
+        Cocoa: 30
+        Potatoes: 50
+        Wheat: 50
+        Beetroots: 50
+        Nether_Wart: 50
         Dead_Bush: 30
         Lilac: 50
         Melon: 20
-        Nether_Wart_Ripe: 50
         Orange_Tulip: 150
         Oxeye_Daisy: 150
         Peony: 50
         Pink_Tulip: 150
         Poppy: 100
-        Potatoes_Ripe: 50
         Pumpkin: 20
         Red_Mushroom: 150
         Red_Tulip: 150
@@ -347,8 +346,8 @@ Experience_Values:
         Dandelion: 100
         Bamboo: 10
         Cornflower: 150
-        Lily_of_the_valley: 150
-        Wither_rose: 500
+        Lily_Of_The_Valley: 150
+        Wither_Rose: 500
     Mining:
         Magma_Block: 30
         Tube_Coral_Block: 75