Use a server impl for hopper event to track get/setItem calls (#9905)

* Use a server impl for hopper event to track getItem/setItem calls

* Rebase

* Comments
This commit is contained in:
Jake Potrebic 2023-11-04 12:58:40 -07:00
parent 2cef1cc67e
commit 0609eeadec
2 changed files with 40 additions and 23 deletions

View file

@ -3,37 +3,17 @@ From: Aikar <aikar@aikar.co>
Date: Thu, 18 Jan 2018 01:00:27 -0500 Date: Thu, 18 Jan 2018 01:00:27 -0500
Subject: [PATCH] Optimize Hoppers Subject: [PATCH] Optimize Hoppers
Adds data about what Item related methods were used in InventoryMoveItem event
so that the server can improve the performance of this event.
diff --git a/src/main/java/org/bukkit/event/inventory/InventoryMoveItemEvent.java b/src/main/java/org/bukkit/event/inventory/InventoryMoveItemEvent.java diff --git a/src/main/java/org/bukkit/event/inventory/InventoryMoveItemEvent.java b/src/main/java/org/bukkit/event/inventory/InventoryMoveItemEvent.java
index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
--- a/src/main/java/org/bukkit/event/inventory/InventoryMoveItemEvent.java --- a/src/main/java/org/bukkit/event/inventory/InventoryMoveItemEvent.java
+++ b/src/main/java/org/bukkit/event/inventory/InventoryMoveItemEvent.java +++ b/src/main/java/org/bukkit/event/inventory/InventoryMoveItemEvent.java
@@ -0,0 +0,0 @@ public class InventoryMoveItemEvent extends Event implements Cancellable {
private final Inventory destinationInventory;
private ItemStack itemStack;
private final boolean didSourceInitiate;
+ public boolean calledGetItem; // Paper
+ public boolean calledSetItem; // Paper
public InventoryMoveItemEvent(@NotNull final Inventory sourceInventory, @NotNull final ItemStack itemStack, @NotNull final Inventory destinationInventory, final boolean didSourceInitiate) {
Preconditions.checkArgument(itemStack != null, "ItemStack cannot be null");
@@ -0,0 +0,0 @@ public class InventoryMoveItemEvent extends Event implements Cancellable { @@ -0,0 +0,0 @@ public class InventoryMoveItemEvent extends Event implements Cancellable {
*/ */
@NotNull @NotNull
public ItemStack getItem() { public ItemStack getItem() {
- return itemStack.clone(); - return itemStack.clone();
+ calledGetItem = true; // Paper - record this method was used for auto detection of mode
+ return itemStack; // Paper - Removed clone, handled better in Server + return itemStack; // Paper - Removed clone, handled better in Server
} }
/** /**
@@ -0,0 +0,0 @@ public class InventoryMoveItemEvent extends Event implements Cancellable {
*/
public void setItem(@NotNull ItemStack itemStack) {
Preconditions.checkArgument(itemStack != null, "ItemStack cannot be null. Cancel the event if you want nothing to be transferred.");
+ calledSetItem = true; // Paper - record this method was used for auto detection of mode
this.itemStack = itemStack.clone();
}

View file

@ -8,10 +8,47 @@ Subject: [PATCH] Optimize Hoppers
* Return true when a plugin cancels inventory move item event instead of false, as false causes pulls to cycle through all items. * Return true when a plugin cancels inventory move item event instead of false, as false causes pulls to cycle through all items.
However, pushes do not exhibit the same behavior, so this is not something plugins could of been relying on. However, pushes do not exhibit the same behavior, so this is not something plugins could of been relying on.
* Add option (Default on) to cooldown hoppers when they fail to move an item due to full inventory * Add option (Default on) to cooldown hoppers when they fail to move an item due to full inventory
* Skip subsequent InventoryMoveItemEvents if a plugin does not use the item after first event fire for an iteration * Skip subsequent InventoryMoveItemEvents if a plugin does not use the item after first event fire for an iteration by tracking changes to the event via an internal event implementation.
* Don't check for Entities with Inventories if the block above us is also occluding (not just Inventoried) * Don't check for Entities with Inventories if the block above us is also occluding (not just Inventoried)
* Remove Streams from Item Suck In and restore restore 1.12 AABB checks which is simpler and no voxel allocations (was doing TWO Item Suck ins) * Remove Streams from Item Suck In and restore restore 1.12 AABB checks which is simpler and no voxel allocations (was doing TWO Item Suck ins)
diff --git a/src/main/java/io/papermc/paper/event/inventory/PaperInventoryMoveItemEvent.java b/src/main/java/io/papermc/paper/event/inventory/PaperInventoryMoveItemEvent.java
new file mode 100644
index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000
--- /dev/null
+++ b/src/main/java/io/papermc/paper/event/inventory/PaperInventoryMoveItemEvent.java
@@ -0,0 +0,0 @@
+package io.papermc.paper.event.inventory;
+
+import org.bukkit.event.inventory.InventoryMoveItemEvent;
+import org.bukkit.inventory.Inventory;
+import org.bukkit.inventory.ItemStack;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.framework.qual.DefaultQualifier;
+import org.jetbrains.annotations.NotNull;
+
+@DefaultQualifier(NonNull.class)
+public class PaperInventoryMoveItemEvent extends InventoryMoveItemEvent {
+
+ public boolean calledSetItem;
+ public boolean calledGetItem;
+
+ public PaperInventoryMoveItemEvent(final @NotNull Inventory sourceInventory, final @NotNull ItemStack itemStack, final @NotNull Inventory destinationInventory, final boolean didSourceInitiate) {
+ super(sourceInventory, itemStack, destinationInventory, didSourceInitiate);
+ }
+
+ @Override
+ public ItemStack getItem() {
+ this.calledGetItem = true;
+ return super.getItem();
+ }
+
+ @Override
+ public void setItem(final ItemStack itemStack) {
+ super.setItem(itemStack);
+ this.calledSetItem = true;
+ }
+}
diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java
index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
--- a/src/main/java/net/minecraft/server/MinecraftServer.java --- a/src/main/java/net/minecraft/server/MinecraftServer.java
@ -230,7 +267,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
+ @Nullable + @Nullable
+ private static ItemStack callPushMoveEvent(Container iinventory, ItemStack itemstack, HopperBlockEntity hopper) { + private static ItemStack callPushMoveEvent(Container iinventory, ItemStack itemstack, HopperBlockEntity hopper) {
+ final Inventory destinationInventory = getInventory(iinventory); + final Inventory destinationInventory = getInventory(iinventory);
+ final InventoryMoveItemEvent event = new InventoryMoveItemEvent(hopper.getOwner(false).getInventory(), + final io.papermc.paper.event.inventory.PaperInventoryMoveItemEvent event = new io.papermc.paper.event.inventory.PaperInventoryMoveItemEvent(hopper.getOwner(false).getInventory(),
+ CraftItemStack.asCraftMirror(itemstack), destinationInventory, true); + CraftItemStack.asCraftMirror(itemstack), destinationInventory, true);
+ final boolean result = event.callEvent(); + final boolean result = event.callEvent();
+ if (!event.calledGetItem && !event.calledSetItem) { + if (!event.calledGetItem && !event.calledSetItem) {
@ -254,7 +291,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
+ final Inventory destination = getInventory(hopper); + final Inventory destination = getInventory(hopper);
+ +
+ // Mirror is safe as no plugins ever use this item + // Mirror is safe as no plugins ever use this item
+ final InventoryMoveItemEvent event = new InventoryMoveItemEvent(sourceInventory, CraftItemStack.asCraftMirror(itemstack), destination, false); + final io.papermc.paper.event.inventory.PaperInventoryMoveItemEvent event = new io.papermc.paper.event.inventory.PaperInventoryMoveItemEvent(sourceInventory, CraftItemStack.asCraftMirror(itemstack), destination, false);
+ final boolean result = event.callEvent(); + final boolean result = event.callEvent();
+ if (!event.calledGetItem && !event.calledSetItem) { + if (!event.calledGetItem && !event.calledSetItem) {
+ skipPullModeEventFire = true; + skipPullModeEventFire = true;