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)

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;
-
-//TODO move titles, itemgetter, other version specific shite in here
+import org.bukkit.inventory.PlayerInventory;
+import org.bukkit.inventory.SmithingInventory;
+import org.bukkit.inventory.SmithingTransformRecipe;
+import org.bukkit.inventory.SmithingTrimRecipe;
+
+/**
+ * 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);
 }

bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler16.java

@@ -4,7 +4,9 @@
 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 {
 
@@ -28,6 +30,11 @@ public boolean isOffHandEmpty(Player player) {
         return player.getInventory().getItemInOffHand().getAmount() == 0;
     }
 
+    @Override
+    public ItemStack getItemInEquipmentSlot(PlayerInventory inventory, EquipmentSlot slot) {
+        return inventory.getItem(slot);
+    }
+
     @Override
     public ItemStack[] getSmithItems(SmithItemEvent event) {
         return new ItemStack[]{

bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler20.java

@@ -3,6 +3,7 @@
 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;
@@ -20,6 +21,11 @@ public boolean isPlayerOnCamel(Player player) {
         return player.getVehicle() instanceof Camel;
     }
 
+    @Override
+    public ItemStack getItem(PlayerBucketEmptyEvent event) {
+        return event.getPlayer().getInventory().getItem(event.getHand());
+    }
+
     @Override
     public ItemStack[] getSmithItems(SmithItemEvent event) {
         return new ItemStack[]{

bukkit/src/main/java/com/leonardobishop/quests/bukkit/hook/versionspecific/VersionSpecificHandler8.java

@@ -6,6 +6,10 @@
 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 ItemStack getItemInMainHand(Player player) {
         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];

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 boolean isHotbarMoveAndReaddSupported() {
     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();
+    }
 }

bukkit/src/main/java/com/leonardobishop/quests/bukkit/tasktype/type/BucketEmptyTaskType.java

@@ -2,12 +2,9 @@
 
 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 BucketEmptyTaskType(BukkitQuestsPlugin plugin) {
 
     @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));
     }
 }

bukkit/src/main/java/com/leonardobishop/quests/bukkit/tasktype/type/BucketInteractionTaskType.java

@@ -16,7 +16,7 @@
 
 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) {

bukkit/src/main/java/com/leonardobishop/quests/bukkit/tasktype/type/InteractTaskType.java

@@ -62,7 +62,7 @@ public void onPlayerInteract(PlayerInteractEvent event) {
         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();