Fix milking task type checking only main hand

Fix interact and bucket empty task types throwing exceptions on 1.8
(unfortunately we can't get bucket NBT on versions lower than 1.19.2)

9 files changed, 248 insertions, 14 deletions

diff --git a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler.java b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler.java
index bdfa513e..c9d16be5 100644
--- a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler.java
+++ b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler.java
@@ -1,53 +1,216 @@
 package com.leonardobishop.quests.bukkit.hook.versionspecific;
 
 import org.bukkit.block.data.BlockData;
+import org.bukkit.block.data.type.CaveVinesPlant;
+import org.bukkit.entity.AbstractHorse;
+import org.bukkit.entity.Camel;
+import org.bukkit.entity.Donkey;
 import org.bukkit.entity.Entity;
+import org.bukkit.entity.Goat;
+import org.bukkit.entity.Horse;
+import org.bukkit.entity.Llama;
+import org.bukkit.entity.Mule;
 import org.bukkit.entity.Player;
+import org.bukkit.entity.SkeletonHorse;
+import org.bukkit.entity.Strider;
+import org.bukkit.entity.ZombieHorse;
 import org.bukkit.event.inventory.ClickType;
 import org.bukkit.event.inventory.SmithItemEvent;
+import org.bukkit.event.player.PlayerBucketEmptyEvent;
+import org.bukkit.event.player.PlayerInteractEntityEvent;
+import org.bukkit.event.player.PlayerInteractEvent;
+import org.bukkit.inventory.EquipmentSlot;
 import org.bukkit.inventory.ItemStack;
+import org.bukkit.inventory.PlayerInventory;
+import org.bukkit.inventory.SmithingInventory;
+import org.bukkit.inventory.SmithingTransformRecipe;
+import org.bukkit.inventory.SmithingTrimRecipe;
 
-//TODO move titles, itemgetter, other version specific shite in here
+/**
+ * Interface used for implementing version-specific features.
+ * All information about changes in the API should be documented HERE in the method docs.
+ */
+@SuppressWarnings({"deprecation", "BooleanMethodIsAlwaysInverted"})
 public interface VersionSpecificHandler {
 
     @SuppressWarnings("unused")
     int getMinecraftVersion();
 
+    /**
+     * Elytra were introduced in {@code 1.9}.
+     *
+     * @see Player#isGliding()
+     */
     boolean isPlayerGliding(Player player);
 
+    /**
+     * Camels were introduced in {@code 1.20}.
+     *
+     * @see Camel
+     */
     boolean isPlayerOnCamel(Player player);
 
+    /**
+     * Donkeys were introduced in {@code 1.6.1}.
+     *
+     * <p>
+     * Each horse variant got its own interface in {@code 1.11}. {@link Horse#getVariant()} and {@link Horse.Variant}
+     * have been deprecated. {@link AbstractHorse} superseded {@link Horse} (interface used before to represent all
+     * the horses) and the old one represents now an actual horse. {@link Horse.Variant#DONKEY} corresponding interface
+     * in modern versions is {@link Donkey}.
+     * </p>
+     */
     boolean isPlayerOnDonkey(Player player);
 
+    /**
+     * Horses were introduced in {@code 1.6.1}.
+     *
+     * <p>
+     * Each horse variant got its own interface in {@code 1.11}. {@link Horse#getVariant()} and {@link Horse.Variant}
+     * have been deprecated. {@link AbstractHorse} superseded {@link Horse} (interface used before to represent all
+     * the horses) and the old one represents now an actual horse. {@link Horse.Variant#HORSE} corresponding interface
+     * in modern versions is {@link Horse}.
+     * </p>
+     */
     boolean isPlayerOnHorse(Player player);
 
+    /**
+     * Llamas were introduced in {@code 1.11}.
+     *
+     * @see Llama
+     */
     boolean isPlayerOnLlama(Player player);
 
+    /**
+     * Mules were introduced in {@code 1.6.1}.
+     *
+     * <p>
+     * Each horse variant got its own interface in {@code 1.11}. {@link Horse#getVariant()} and {@link Horse.Variant}
+     * have been deprecated. {@link AbstractHorse} superseded {@link Horse} (interface used before to represent all
+     * the horses) and the old one represents now an actual horse. {@link Horse.Variant#MULE} corresponding interface
+     * in modern versions is {@link Mule}.
+     * </p>
+     */
     boolean isPlayerOnMule(Player player);
 
+    /**
+     * Skeleton horses were introduced in {@code 1.6.1}.
+     *
+     * <p>
+     * Each horse variant got its own interface in {@code 1.11}. {@link Horse#getVariant()} and {@link Horse.Variant}
+     * have been deprecated. {@link AbstractHorse} superseded {@link Horse} (interface used before to represent all
+     * the horses) and the old one represents now an actual horse. {@link Horse.Variant#SKELETON_HORSE} corresponding
+     * interface in modern versions is {@link SkeletonHorse}.
+     * </p>
+     */
     boolean isPlayerOnSkeletonHorse(Player player);
 
+    /**
+     * Striders were introduced in {@code 1.16}.
+     *
+     * @see Strider
+     */
     boolean isPlayerOnStrider(Player player);
 
+    /**
+     * Zombie (undead) horses were introduced in {@code 1.6.1}.
+     *
+     * <p>
+     * Each horse variant got its own interface in {@code 1.11}. {@link Horse#getVariant()} and {@link Horse.Variant}
+     * have been deprecated. {@link AbstractHorse} superseded {@link Horse} (interface used before to represent all
+     * the horses) and the old one represents now an actual horse. {@link Horse.Variant#UNDEAD_HORSE} corresponding
+     * interface in modern versions is {@link ZombieHorse}.
+     * </p>
+     */
     boolean isPlayerOnZombieHorse(Player player);
 
+    /**
+     * Ability to swap item in hand while hovering over an item was introduced in {@code 1.16}.
+     *
+     * @see ClickType#SWAP_OFFHAND
+     */
     boolean isOffHandSwap(ClickType clickType);
 
-    @SuppressWarnings("BooleanMethodIsAlwaysInverted")
+    /**
+     * Dual-wielding system was introduced in {@code 1.9}.
+     */
     boolean isOffHandEmpty(Player player);
 
+    /**
+     * Initially, the proper method to get an inventory contents except armor and other extra slots
+     * (not allowing player to store results of crafting actions) was {@link PlayerInventory#getContents()}.
+     * In {@code 1.9} {@link PlayerInventory#getStorageContents()} method was introduced superseding the old
+     * one. In newer versions {@link PlayerInventory#getContents()} method returns all the items including
+     * the extra slots of player inventories.
+     *
+     * @apiNote This method is intended to be used as a check for item crafting related task types.
+     */
     int getAvailableSpace(Player player, ItemStack newItemStack);
 
-    @SuppressWarnings("BooleanMethodIsAlwaysInverted")
+    /**
+     * Initially, clicking with a number key on a crafting result made the item go to the selected slot.
+     * Starting with {@code 1.9} clicking it is no longer effective.
+     *
+     * @apiNote This method is intended to be used as a check for item crafting related task types.
+     */
     boolean isHotbarMoveAndReaddSupported();
 
+    /**
+     * Cave vines plants were introduced in {@code 1.17}.
+     *
+     * @see CaveVinesPlant#isBerries()
+     */
     boolean isCaveVinesPlantWithBerries(BlockData blockData);
 
+    /**
+     * Dual-wielding system was introduced in {@code 1.9}.
+     */
     ItemStack getItemInMainHand(Player player);
 
+    /**
+     * Initially there was no {@link PlayerInventory#getItem(EquipmentSlot)} method. Possible enum values were:
+     * {@link EquipmentSlot#CHEST}, {@link EquipmentSlot#FEET}, {@link EquipmentSlot#HAND}, {@link EquipmentSlot#HEAD}
+     * and {@link EquipmentSlot#LEGS}. In {@code 1.9} {@link EquipmentSlot#OFF_HAND} was introduced, however a method
+     * to get specified equipment slot item still hasn't existed. In {@code 1.15.2} the method was finally introduced
+     * {@link PlayerInventory#getItem(EquipmentSlot)} making us able to no longer maintain this one.
+     */
+    ItemStack getItemInEquipmentSlot(PlayerInventory inventory, EquipmentSlot slot);
+
+    /**
+     * Dual-wielding system was introduced in {@code 1.9}.
+     * Unfortunately {@link PlayerBucketEmptyEvent#getHand()} method was added later in {@code 1.19.2}.
+     * {@link PlayerBucketEmptyEvent#getItemStack()} returns the empty action result (empty bucket).
+     */
+    ItemStack getItem(PlayerBucketEmptyEvent event);
+
+    /**
+     * Dual-wielding system was introduced in {@code 1.9}.
+     */
+    EquipmentSlot getHand(PlayerInteractEvent event);
+
+    /**
+     * Dual-wielding system was introduced in {@code 1.9}.
+     */
+    EquipmentSlot getHand(PlayerInteractEntityEvent event);
+
+    /**
+     * Items smithing system was introduced in {@code 1.16} with {@link SmithingInventory#getInputEquipment()}
+     * and {@link SmithingInventory#getInputMineral()}. In {@code 1.20} the feature was extended to support templates.
+     * Due to the following reason, new method was added: {@link SmithingInventory#getInputTemplate()}.
+     */
     ItemStack[] getSmithItems(SmithItemEvent event);
 
+    /**
+     * Items smithing system was introduced in {@code 1.16} with {@link SmithingTransformRecipe}.
+     * In {@code 1.20} the feature was extended to support templates. Due to the following reason,
+     * new class has been added {@link SmithingTrimRecipe}.
+     */
     String getSmithMode(SmithItemEvent event);
 
+    /**
+     * Goats were introduced in {@code 1.17}.
+     *
+     * @see Goat
+     */
     boolean isGoat(Entity entity);
 }
diff --git a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler16.java b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler16.java
index 97f51047..9865a3d2 100644
--- a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler16.java
+++ b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler16.java
@@ -4,7 +4,9 @@ import org.bukkit.entity.EntityType;
 import org.bukkit.entity.Player;
 import org.bukkit.event.inventory.ClickType;
 import org.bukkit.event.inventory.SmithItemEvent;
+import org.bukkit.inventory.EquipmentSlot;
 import org.bukkit.inventory.ItemStack;
+import org.bukkit.inventory.PlayerInventory;
 
 public class VersionSpecificHandler16 extends VersionSpecificHandler11 implements VersionSpecificHandler {
 
@@ -29,6 +31,11 @@ public class VersionSpecificHandler16 extends VersionSpecificHandler11 implement
     }
 
     @Override
+    public ItemStack getItemInEquipmentSlot(PlayerInventory inventory, EquipmentSlot slot) {
+        return inventory.getItem(slot);
+    }
+
+    @Override
     public ItemStack[] getSmithItems(SmithItemEvent event) {
         return new ItemStack[]{
                 event.getInventory().getInputEquipment(),
diff --git a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler20.java b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler20.java
index 9976afa7..d9553694 100644
--- a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler20.java
+++ b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler20.java
@@ -3,6 +3,7 @@ package com.leonardobishop.quests.bukkit.hook.versionspecific;
 import org.bukkit.entity.Camel;
 import org.bukkit.entity.Player;
 import org.bukkit.event.inventory.SmithItemEvent;
+import org.bukkit.event.player.PlayerBucketEmptyEvent;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.inventory.Recipe;
 import org.bukkit.inventory.SmithingTransformRecipe;
@@ -21,6 +22,11 @@ public class VersionSpecificHandler20 extends VersionSpecificHandler17 implement
     }
 
     @Override
+    public ItemStack getItem(PlayerBucketEmptyEvent event) {
+        return event.getPlayer().getInventory().getItem(event.getHand());
+    }
+
+    @Override
     public ItemStack[] getSmithItems(SmithItemEvent event) {
         return new ItemStack[]{
                 event.getInventory().getInputEquipment(),
diff --git a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler8.java b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler8.java
index 6d08140d..9846a025 100644
--- a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler8.java
+++ b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler8.java
@@ -6,6 +6,10 @@ import org.bukkit.entity.Horse;
 import org.bukkit.entity.Player;
 import org.bukkit.event.inventory.ClickType;
 import org.bukkit.event.inventory.SmithItemEvent;
+import org.bukkit.event.player.PlayerBucketEmptyEvent;
+import org.bukkit.event.player.PlayerInteractEntityEvent;
+import org.bukkit.event.player.PlayerInteractEvent;
+import org.bukkit.inventory.EquipmentSlot;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.inventory.PlayerInventory;
 
@@ -114,6 +118,36 @@ public class VersionSpecificHandler8 implements VersionSpecificHandler {
         return player.getItemInHand();
     }
 
+    @SuppressWarnings("deprecation")
+    @Override
+    public ItemStack getItemInEquipmentSlot(PlayerInventory inventory, EquipmentSlot slot) {
+        return switch (slot) {
+            case CHEST -> inventory.getChestplate();
+            case FEET -> inventory.getBoots();
+            case HAND -> inventory.getItemInHand();
+            case HEAD -> inventory.getHelmet();
+            case LEGS -> inventory.getLeggings();
+
+            // there are 5 equipment slots on 1.8
+            default -> null;
+        };
+    }
+
+    @Override
+    public ItemStack getItem(PlayerBucketEmptyEvent event) {
+        return new ItemStack(event.getBucket(), 1);
+    }
+
+    @Override
+    public EquipmentSlot getHand(PlayerInteractEvent event) {
+        return EquipmentSlot.HAND;
+    }
+
+    @Override
+    public EquipmentSlot getHand(PlayerInteractEntityEvent event) {
+        return EquipmentSlot.HAND;
+    }
+
     @Override
     public ItemStack[] getSmithItems(SmithItemEvent event) {
         return new ItemStack[0];
diff --git a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler9.java b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler9.java
index b00fdb22..cf2f0a9c 100644
--- a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler9.java
+++ b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler9.java
@@ -1,6 +1,9 @@
 package com.leonardobishop.quests.bukkit.hook.versionspecific;
 
 import org.bukkit.entity.Player;
+import org.bukkit.event.player.PlayerInteractEntityEvent;
+import org.bukkit.event.player.PlayerInteractEvent;
+import org.bukkit.inventory.EquipmentSlot;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.inventory.PlayerInventory;
 
@@ -47,4 +50,30 @@ public class VersionSpecificHandler9 extends VersionSpecificHandler8 implements
     public ItemStack getItemInMainHand(Player player) {
         return player.getInventory().getItemInMainHand();
     }
+
+    @SuppressWarnings("deprecation")
+    @Override
+    public ItemStack getItemInEquipmentSlot(PlayerInventory inventory, EquipmentSlot slot) {
+        return switch (slot) {
+            case CHEST -> inventory.getChestplate();
+            case FEET -> inventory.getBoots();
+            case HAND -> inventory.getItemInHand();
+            case HEAD -> inventory.getHelmet();
+            case LEGS -> inventory.getLeggings();
+            case OFF_HAND -> inventory.getItemInOffHand();
+
+            // there are 6 equipment slots on 1.9
+            default -> null;
+        };
+    }
+
+    @Override
+    public EquipmentSlot getHand(PlayerInteractEvent event) {
+        return event.getHand();
+    }
+
+    @Override
+    public EquipmentSlot getHand(PlayerInteractEntityEvent event) {
+        return event.getHand();
+    }
 }
diff --git a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/tasktype/type/BucketEmptyTaskType.java b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/tasktype/type/BucketEmptyTaskType.java
index 61708243..1860d92d 100644
--- a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/tasktype/type/BucketEmptyTaskType.java
+++ b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/tasktype/type/BucketEmptyTaskType.java
@@ -2,12 +2,9 @@ package com.leonardobishop.quests.bukkit.tasktype.type;
 
 import com.leonardobishop.quests.bukkit.BukkitQuestsPlugin;
 import com.leonardobishop.quests.bukkit.util.TaskUtils;
-import org.bukkit.entity.Player;
 import org.bukkit.event.EventHandler;
 import org.bukkit.event.EventPriority;
 import org.bukkit.event.player.PlayerBucketEmptyEvent;
-import org.bukkit.inventory.EquipmentSlot;
-import org.bukkit.inventory.PlayerInventory;
 
 public final class BucketEmptyTaskType extends BucketInteractionTaskType {
 
@@ -17,10 +14,6 @@ public final class BucketEmptyTaskType extends BucketInteractionTaskType {
 
     @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true)
     public void onPlayerBucketEmpty(PlayerBucketEmptyEvent event) {
-        Player player = event.getPlayer();
-        PlayerInventory inventory = player.getInventory();
-        EquipmentSlot slot = event.getHand();
-
-        handle(player, inventory.getItem(slot));
+        handle(event.getPlayer(), plugin.getVersionSpecificHandler().getItem(event));
     }
 }
diff --git a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/tasktype/type/BucketInteractionTaskType.java b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/tasktype/type/BucketInteractionTaskType.java
index b4f0191a..a497db19 100644
--- a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/tasktype/type/BucketInteractionTaskType.java
+++ b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/tasktype/type/BucketInteractionTaskType.java
@@ -16,7 +16,7 @@ import org.bukkit.inventory.ItemStack;
 
 public abstract class BucketInteractionTaskType extends BukkitTaskType {
 
-    private final BukkitQuestsPlugin plugin;
+    protected final BukkitQuestsPlugin plugin;
     private final Table<String, String, QuestItem> fixedQuestItemCache = HashBasedTable.create();
 
     public BucketInteractionTaskType(BukkitQuestsPlugin plugin, String type, String author, String description) {
diff --git a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/tasktype/type/InteractTaskType.java b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/tasktype/type/InteractTaskType.java
index f337c3f6..18113eaa 100644
--- a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/tasktype/type/InteractTaskType.java
+++ b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/tasktype/type/InteractTaskType.java
@@ -62,7 +62,7 @@ public final class InteractTaskType extends BukkitTaskType {
         Block block = event.getClickedBlock();
         ItemStack item = event.getItem();
         Action action = event.getAction();
-        EquipmentSlot hand = event.getHand();
+        EquipmentSlot hand = plugin.getVersionSpecificHandler().getHand(event);
         Event.Result useInteractedBlock = event.useInteractedBlock();
         Event.Result useItemInHand = event.useItemInHand();
 
diff --git a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/tasktype/type/MilkingTaskType.java b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/tasktype/type/MilkingTaskType.java
index 20c27fb6..ffdd39c9 100644
--- a/bukkit/src/main/java/com/leonardobishop/quests/bukkit/tasktype/type/MilkingTaskType.java
+++ b/bukkit/src/main/java/com/leonardobishop/quests/bukkit/tasktype/type/MilkingTaskType.java
@@ -16,6 +16,7 @@ import org.bukkit.entity.Player;
 import org.bukkit.event.EventHandler;
 import org.bukkit.event.EventPriority;
 import org.bukkit.event.player.PlayerInteractEntityEvent;
+import org.bukkit.inventory.EquipmentSlot;
 import org.bukkit.inventory.ItemStack;
 
 public final class MilkingTaskType extends BukkitTaskType {
@@ -49,7 +50,8 @@ public final class MilkingTaskType extends BukkitTaskType {
             return;
         }
 
-        ItemStack item = plugin.getVersionSpecificHandler().getItemInMainHand(player);
+        EquipmentSlot slot = plugin.getVersionSpecificHandler().getHand(event);
+        ItemStack item = plugin.getVersionSpecificHandler().getItemInEquipmentSlot(player.getInventory(), slot);
         if (item.getType() != Material.BUCKET) {
             return;
         }