Add InventoryDragEvent #5474
Add InventoryDragEvent #5474
Conversation
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
Outdated
Show resolved
Hide resolved
Co-authored-by: LimeGlass <16087552+TheLimeGlass@users.noreply.github.com>
Co-authored-by: Ayham Al Ali <20037329+AyhamAl-Ali@users.noreply.github.com>
| return event.getWhoClicked().getWorld(); | ||
| } | ||
| }, EventValues.TIME_NOW); | ||
| EventValues.registerEventValue(InventoryDragEvent.class, ItemStack[].class, new Getter<ItemStack[], InventoryDragEvent>() { |
There was a problem hiding this comment.
Is there a use for getOldCursor() here? Could it be the past timestate of event-itemstack?
| @Override | ||
| @Nullable | ||
| public Inventory[] get(InventoryDragEvent event) { | ||
| Set<Inventory> inventories = new HashSet<>(2); |
There was a problem hiding this comment.
use an array here instead of a set when we know the length of the return array
There was a problem hiding this comment.
That won't work because I'm dynamically checking the size of the set, with an array it's always a fixed size. Technically it can be done with arrays but it will just make code uglier
There was a problem hiding this comment.
It just occurred to me that this is not future proof, Mojang could add some sort of view where more than 2 inventories could be returned from this.
I don't believe it should be capped off at 2, just return however many inventories could be found.
| int slot = view.convertSlot(rawSlot); | ||
| if (inventory == null) | ||
| continue; | ||
| if (inventory instanceof PlayerInventory && slot >= 36) { |
There was a problem hiding this comment.
Maybe add notes why using the inventory size (36) for future contributions
| @Override | ||
| @Nullable | ||
| public Inventory[] get(InventoryDragEvent event) { | ||
| Set<Inventory> inventories = new HashSet<>(2); |
There was a problem hiding this comment.
It just occurred to me that this is not future proof, Mojang could add some sort of view where more than 2 inventories could be returned from this.
I don't believe it should be capped off at 2, just return however many inventories could be found.
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/classes/data/BukkitEventValues.java
Outdated
Show resolved
Hide resolved
Co-authored-by: LimeGlass <16087552+TheLimeGlass@users.noreply.github.com>
(cherry picked from commit 3dd5394)
Description
This PR adds the inventory drag event.
Target Minecraft Versions: any
Requirements: none
Related Issues: #3744