From 1f08f5dc344cfd5b5a78200822edba2c622a10b0 Mon Sep 17 00:00:00 2001 From: TheMode Date: Mon, 19 Apr 2021 07:42:48 +0200 Subject: [PATCH 1/7] Use a parallel stream Signed-off-by: TheMode --- src/main/java/net/minestom/server/UpdateManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/minestom/server/UpdateManager.java b/src/main/java/net/minestom/server/UpdateManager.java index 23b0020b0..39c50cc2c 100644 --- a/src/main/java/net/minestom/server/UpdateManager.java +++ b/src/main/java/net/minestom/server/UpdateManager.java @@ -91,7 +91,7 @@ public final class UpdateManager { } // Flush all waiting packets - AsyncUtils.runAsync(() -> connectionManager.getOnlinePlayers().stream() + AsyncUtils.runAsync(() -> connectionManager.getOnlinePlayers().parallelStream() .filter(player -> player.getPlayerConnection() instanceof NettyPlayerConnection) .map(player -> (NettyPlayerConnection) player.getPlayerConnection()) .forEach(NettyPlayerConnection::flush)); From 1daaeda63f1cc906af3af8f35e4452491c729a1a Mon Sep 17 00:00:00 2001 From: TheMode Date: Mon, 19 Apr 2021 20:09:42 +0200 Subject: [PATCH 2/7] Temporary fix for shift click --- src/main/java/net/minestom/server/inventory/Inventory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/minestom/server/inventory/Inventory.java b/src/main/java/net/minestom/server/inventory/Inventory.java index 9935653eb..ba114aa83 100644 --- a/src/main/java/net/minestom/server/inventory/Inventory.java +++ b/src/main/java/net/minestom/server/inventory/Inventory.java @@ -344,7 +344,7 @@ public class Inventory extends AbstractInventory implements Viewable { if (isInWindow) { clickResult = clickProcessor.shiftClick(this, player, slot, clicked, cursor, // Player inventory loop - new InventoryClickLoopHandler(0, PlayerInventory.INVENTORY_SIZE, 1, + new InventoryClickLoopHandler(offset, PlayerInventory.INVENTORY_SIZE + offset, 1, PlayerInventoryUtils::convertToPacketSlot, index -> isClickInWindow(index) ? getItemStack(index) : From e9b5779b24f50c052d0a318f45155063f399831f Mon Sep 17 00:00:00 2001 From: TheMode Date: Tue, 20 Apr 2021 06:41:55 +0200 Subject: [PATCH 3/7] Simplify shift click handling, fix click processor ignoring non-air slot --- .../minestom/server/inventory/Inventory.java | 46 ++------- .../server/inventory/PlayerInventory.java | 23 ++--- .../server/inventory/TransactionType.java | 46 +++++++-- .../click/InventoryClickProcessor.java | 97 +++++++------------ 4 files changed, 90 insertions(+), 122 deletions(-) diff --git a/src/main/java/net/minestom/server/inventory/Inventory.java b/src/main/java/net/minestom/server/inventory/Inventory.java index ba114aa83..aa9be6aaf 100644 --- a/src/main/java/net/minestom/server/inventory/Inventory.java +++ b/src/main/java/net/minestom/server/inventory/Inventory.java @@ -338,51 +338,21 @@ public class Inventory extends AbstractInventory implements Viewable { final ItemStack clicked = isInWindow ? getItemStack(slot) : playerInventory.getItemStack(clickSlot); final ItemStack cursor = getCursorItem(player); // Isn't used in the algorithm - - final InventoryClickResult clickResult; - - if (isInWindow) { - clickResult = clickProcessor.shiftClick(this, player, slot, clicked, cursor, - // Player inventory loop - new InventoryClickLoopHandler(offset, PlayerInventory.INVENTORY_SIZE + offset, 1, - PlayerInventoryUtils::convertToPacketSlot, - index -> isClickInWindow(index) ? - getItemStack(index) : - playerInventory.getItemStack(PlayerInventoryUtils.convertSlot(index, offset)), - (index, itemStack) -> { - if (isClickInWindow(index)) { - setItemStack(index, itemStack); - } else { - playerInventory.setItemStack(PlayerInventoryUtils.convertSlot(index, offset), itemStack); - } - })); - } else { - clickResult = clickProcessor.shiftClick(null, player, slot, clicked, cursor, - // Window loop - new InventoryClickLoopHandler(0, getSize(), 1, - i -> i, - index -> isClickInWindow(index) ? - getItemStack(index) : - playerInventory.getItemStack(PlayerInventoryUtils.convertSlot(index, offset)), - (index, itemStack) -> { - if (isClickInWindow(index)) { - setItemStack(index, itemStack); - } else { - playerInventory.setItemStack(PlayerInventoryUtils.convertSlot(index, offset), itemStack); - } - })); - } + final InventoryClickResult clickResult = clickProcessor.shiftClick( + isInWindow ? playerInventory : this, + isInWindow ? this : null, + player, slot, clicked, cursor); if (clickResult == null) return false; - if (clickResult.doRefresh()) { - updateFromClick(clickResult, player); + if (isInWindow) { + setItemStack(slot, clickResult.getClicked()); + } else { + playerInventory.setItemStack(clickSlot, clickResult.getClicked()); } refreshPlayerCursorItem(player, clickResult.getCursor()); - playerInventory.update(); - update(); return !clickResult.isCancel(); } diff --git a/src/main/java/net/minestom/server/inventory/PlayerInventory.java b/src/main/java/net/minestom/server/inventory/PlayerInventory.java index bdc3975e0..5cd377e92 100644 --- a/src/main/java/net/minestom/server/inventory/PlayerInventory.java +++ b/src/main/java/net/minestom/server/inventory/PlayerInventory.java @@ -339,26 +339,17 @@ public class PlayerInventory extends AbstractInventory implements EquipmentHandl public boolean shiftClick(@NotNull Player player, int slot) { final ItemStack cursor = getCursorItem(); final ItemStack clicked = getItemStack(slot, OFFSET); - - final boolean hotBarClick = convertToPacketSlot(slot) < 9; - final InventoryClickResult clickResult = clickProcessor.shiftClick(null, player, slot, clicked, cursor, - new InventoryClickLoopHandler(0, itemStacks.length, 1, - i -> { - if (hotBarClick) { - return i < 9 ? i + 9 : i - 9; - } else { - return convertPlayerInventorySlot(i, OFFSET); - } - }, - index -> getItemStack(index, OFFSET), - (index, itemStack) -> setItemStack(index, OFFSET, itemStack))); + final boolean hotBarClick = convertSlot(slot, OFFSET) < 9; + final int start = hotBarClick ? 9 : 0; + final int end = hotBarClick ? getSize() - 9 : 8; + final InventoryClickResult clickResult = clickProcessor.shiftClick(this, + start, end, 1, + player, slot, clicked, cursor); if (clickResult == null) return false; - if (clickResult.doRefresh()) - update(); - + setItemStack(slot, OFFSET, clickResult.getClicked()); setCursorItem(clickResult.getCursor()); return !clickResult.isCancel(); diff --git a/src/main/java/net/minestom/server/inventory/TransactionType.java b/src/main/java/net/minestom/server/inventory/TransactionType.java index fa0bafc67..8cdf04075 100644 --- a/src/main/java/net/minestom/server/inventory/TransactionType.java +++ b/src/main/java/net/minestom/server/inventory/TransactionType.java @@ -19,13 +19,13 @@ public interface TransactionType { * Adds an item to the inventory. * Can either take an air slot or be stacked. */ - TransactionType ADD = (inventory, itemStack) -> { + TransactionType ADD = (inventory, itemStack, slotPredicate, start, end, step) -> { Int2ObjectMap itemChangesMap = new Int2ObjectOpenHashMap<>(); final StackingRule stackingRule = itemStack.getStackingRule(); // Check filled slot (not air) - for (int i = 0; i < inventory.getInnerSize(); i++) { + for (int i = start; i < end; i += step) { ItemStack inventoryItem = inventory.getItemStack(i); if (inventoryItem.isAir()) { continue; @@ -34,6 +34,12 @@ public interface TransactionType { final int itemAmount = stackingRule.getAmount(inventoryItem); if (itemAmount == stackingRule.getMaxSize()) continue; + + if (!slotPredicate.test(i, inventoryItem)) { + // Cancelled transaction + continue; + } + final int itemStackAmount = stackingRule.getAmount(itemStack); final int totalAmount = itemStackAmount + itemAmount; if (!stackingRule.canApply(itemStack, totalAmount)) { @@ -50,11 +56,17 @@ public interface TransactionType { } // Check air slot to fill - for (int i = 0; i < inventory.getInnerSize(); i++) { + for (int i = start; i < end; i += step) { ItemStack inventoryItem = inventory.getItemStack(i); if (!inventoryItem.isAir()) { continue; } + + if (!slotPredicate.test(i, inventoryItem)) { + // Cancelled transaction + continue; + } + // Fill the slot itemChangesMap.put(i, itemStack); itemStack = stackingRule.apply(itemStack, 0); @@ -68,15 +80,20 @@ public interface TransactionType { * Takes an item from the inventory. * Can either transform items to air or reduce their amount. */ - TransactionType TAKE = (inventory, itemStack) -> { + TransactionType TAKE = (inventory, itemStack, slotPredicate, start, end, step) -> { Int2ObjectMap itemChangesMap = new Int2ObjectOpenHashMap<>(); final StackingRule stackingRule = itemStack.getStackingRule(); - for (int i = 0; i < inventory.getInnerSize(); i++) { + for (int i = start; i < end; i += step) { ItemStack inventoryItem = inventory.getItemStack(i); if (inventoryItem.isAir()) { continue; } if (stackingRule.canBeStacked(itemStack, inventoryItem)) { + if (!slotPredicate.test(i, inventoryItem)) { + // Cancelled transaction + continue; + } + final int itemAmount = stackingRule.getAmount(inventoryItem); final int itemStackAmount = stackingRule.getAmount(itemStack); if (itemStackAmount < itemAmount) { @@ -97,6 +114,23 @@ public interface TransactionType { }; @NotNull Pair> process(@NotNull AbstractInventory inventory, - @NotNull ItemStack itemStack); + @NotNull ItemStack itemStack, + @NotNull SlotPredicate slotPredicate, + int start, int end, int step); + default @NotNull Pair> process(@NotNull AbstractInventory inventory, + @NotNull ItemStack itemStack, + @NotNull SlotPredicate slotPredicate) { + return process(inventory, itemStack, slotPredicate, 0, inventory.getInnerSize(), 1); + } + + default @NotNull Pair> process(@NotNull AbstractInventory inventory, + @NotNull ItemStack itemStack) { + return process(inventory, itemStack, (slot, itemStack1) -> true); + } + + @FunctionalInterface + interface SlotPredicate { + boolean test(int slot, @NotNull ItemStack itemStack); + } } diff --git a/src/main/java/net/minestom/server/inventory/click/InventoryClickProcessor.java b/src/main/java/net/minestom/server/inventory/click/InventoryClickProcessor.java index b026b9134..b4c4b735b 100644 --- a/src/main/java/net/minestom/server/inventory/click/InventoryClickProcessor.java +++ b/src/main/java/net/minestom/server/inventory/click/InventoryClickProcessor.java @@ -7,7 +7,9 @@ import it.unimi.dsi.fastutil.ints.IntSet; import net.minestom.server.entity.Player; import net.minestom.server.event.inventory.InventoryClickEvent; import net.minestom.server.event.inventory.InventoryPreClickEvent; +import net.minestom.server.inventory.AbstractInventory; import net.minestom.server.inventory.Inventory; +import net.minestom.server.inventory.TransactionType; import net.minestom.server.inventory.condition.InventoryCondition; import net.minestom.server.inventory.condition.InventoryConditionResult; import net.minestom.server.item.ItemStack; @@ -167,9 +169,8 @@ public class InventoryClickProcessor { return clickResult; } - @Nullable - public InventoryClickResult shiftClick(@Nullable Inventory inventory, @NotNull Player player, int slot, - @NotNull ItemStack clicked, @NotNull ItemStack cursor, @NotNull InventoryClickLoopHandler... loopHandlers) { + public @Nullable InventoryClickResult shiftClick(AbstractInventory targetInventory, @Nullable Inventory inventory, @NotNull Player player, int slot, + @NotNull ItemStack clicked, @NotNull ItemStack cursor) { InventoryClickResult clickResult = startCondition(inventory, player, slot, ClickType.START_SHIFT_CLICK, clicked, cursor); if (clickResult.isCancel()) { @@ -179,72 +180,44 @@ public class InventoryClickProcessor { if (clicked.isAir()) return null; - final StackingRule clickedRule = clicked.getStackingRule(); + var pair = TransactionType.ADD.process(targetInventory, clicked, (index, itemStack) -> { + InventoryClickResult result = startCondition(inventory, player, index, ClickType.SHIFT_CLICK, itemStack, cursor); + return !result.isCancel(); + }); - boolean filled = false; - ItemStack resultClicked = clicked; + var itemResult = pair.left(); + var map = pair.right(); + map.forEach(targetInventory::setItemStack); - for (InventoryClickLoopHandler loopHandler : loopHandlers) { - final Int2IntFunction indexModifier = loopHandler.getIndexModifier(); - final Int2ObjectFunction itemGetter = loopHandler.getItemGetter(); - final BiConsumer itemSetter = loopHandler.getItemSetter(); + clickResult.setClicked(itemResult); - for (int i = loopHandler.getStart(); i < loopHandler.getEnd(); i += loopHandler.getStep()) { - final int index = indexModifier.apply(i); - if (index == slot) - continue; + return clickResult; + } - ItemStack item = itemGetter.apply(index); - final StackingRule itemRule = item.getStackingRule(); - if (itemRule.canBeStacked(item, clicked)) { + public @Nullable InventoryClickResult shiftClick(@NotNull AbstractInventory targetInventory, int start, int end, int step, @NotNull Player player, int slot, + @NotNull ItemStack clicked, @NotNull ItemStack cursor) { + InventoryClickResult clickResult = startCondition(null, player, slot, ClickType.START_SHIFT_CLICK, clicked, cursor); - clickResult = startCondition(inventory, player, index, ClickType.SHIFT_CLICK, item, cursor); - if (clickResult.isCancel()) - break; - - final int amount = itemRule.getAmount(item); - if (!clickedRule.canApply(clicked, amount + 1)) - continue; - - final int totalAmount = clickedRule.getAmount(resultClicked) + amount; - if (!clickedRule.canApply(clicked, totalAmount)) { - item = itemRule.apply(item, itemRule.getMaxSize()); - itemSetter.accept(index, item); - - resultClicked = clickedRule.apply(resultClicked, totalAmount - clickedRule.getMaxSize()); - filled = false; - - callClickEvent(player, inventory, index, ClickType.SHIFT_CLICK, item, cursor); - continue; - } else { - resultClicked = clickedRule.apply(resultClicked, totalAmount); - itemSetter.accept(index, resultClicked); - - item = itemRule.apply(item, 0); - itemSetter.accept(slot, item); - filled = true; - - callClickEvent(player, inventory, index, ClickType.SHIFT_CLICK, item, cursor); - break; - } - } else if (item.isAir()) { - - clickResult = startCondition(inventory, player, index, ClickType.SHIFT_CLICK, item, cursor); - if (clickResult.isCancel()) - break; - - // Switch - itemSetter.accept(index, resultClicked); - itemSetter.accept(slot, ItemStack.AIR); - filled = true; - break; - } - } - if (!filled) { - itemSetter.accept(slot, resultClicked); - } + if (clickResult.isCancel()) { + return clickResult; } + if (clicked.isAir()) + return null; + + var pair = TransactionType.ADD.process(targetInventory, clicked, (index, itemStack) -> { + if (index == slot) // Prevent item lose/duplication + return false; + InventoryClickResult result = startCondition(null, player, index, ClickType.SHIFT_CLICK, itemStack, cursor); + return !result.isCancel(); + }, start, end, step); + + var itemResult = pair.left(); + var map = pair.right(); + map.forEach(targetInventory::setItemStack); + + clickResult.setClicked(itemResult); + return clickResult; } From 5390cd14f0eb417b0a9e84731a59da78a5b1228a Mon Sep 17 00:00:00 2001 From: TheMode Date: Tue, 20 Apr 2021 07:29:01 +0200 Subject: [PATCH 4/7] Simplify double click handling --- .../server/inventory/AbstractInventory.java | 3 +- .../minestom/server/inventory/Inventory.java | 14 +--- .../server/inventory/PlayerInventory.java | 9 +-- .../server/inventory/TransactionOption.java | 7 ++ .../click/InventoryClickProcessor.java | 75 +++++++++---------- 5 files changed, 49 insertions(+), 59 deletions(-) diff --git a/src/main/java/net/minestom/server/inventory/AbstractInventory.java b/src/main/java/net/minestom/server/inventory/AbstractInventory.java index a95114d5b..b4cc7a641 100644 --- a/src/main/java/net/minestom/server/inventory/AbstractInventory.java +++ b/src/main/java/net/minestom/server/inventory/AbstractInventory.java @@ -55,8 +55,7 @@ public abstract class AbstractInventory implements InventoryClickHandler, DataCo public synchronized @NotNull T processItemStack(@NotNull ItemStack itemStack, @NotNull TransactionType type, @NotNull TransactionOption option) { - var pair = type.process(this, itemStack); - return option.fill(this, pair.left(), pair.right()); + return option.fill(type, this, itemStack); } public synchronized @NotNull List<@NotNull T> processItemStacks(@NotNull List<@NotNull ItemStack> itemStacks, diff --git a/src/main/java/net/minestom/server/inventory/Inventory.java b/src/main/java/net/minestom/server/inventory/Inventory.java index aa9be6aaf..c61aeb7c9 100644 --- a/src/main/java/net/minestom/server/inventory/Inventory.java +++ b/src/main/java/net/minestom/server/inventory/Inventory.java @@ -4,7 +4,6 @@ import net.kyori.adventure.text.Component; import net.minestom.server.Viewable; import net.minestom.server.entity.Player; import net.minestom.server.inventory.click.ClickType; -import net.minestom.server.inventory.click.InventoryClickLoopHandler; import net.minestom.server.inventory.click.InventoryClickResult; import net.minestom.server.item.ItemStack; import net.minestom.server.network.packet.server.play.OpenWindowPacket; @@ -468,17 +467,8 @@ public class Inventory extends AbstractInventory implements Viewable { final ItemStack cursor = getCursorItem(player); final boolean isInWindow = isClickInWindow(slot); - final InventoryClickResult clickResult = clickProcessor.doubleClick(isInWindow ? this : null, player, slot, cursor, - // Start by looping through the opened inventory - new InventoryClickLoopHandler(0, getSize(), 1, - i -> i, - this::getItemStack, - this::setItemStack), - // Looping through player inventory - new InventoryClickLoopHandler(0, PlayerInventory.INVENTORY_SIZE, 1, - PlayerInventoryUtils::convertToPacketSlot, - index -> playerInventory.getItemStack(index, PlayerInventoryUtils.OFFSET), - (index, itemStack) -> playerInventory.setItemStack(index, PlayerInventoryUtils.OFFSET, itemStack))); + final InventoryClickResult clickResult = clickProcessor.doubleClick(isInWindow ? this : playerInventory, + this, player, slot, cursor); if (clickResult == null) return false; diff --git a/src/main/java/net/minestom/server/inventory/PlayerInventory.java b/src/main/java/net/minestom/server/inventory/PlayerInventory.java index 5cd377e92..1dfa63a6f 100644 --- a/src/main/java/net/minestom/server/inventory/PlayerInventory.java +++ b/src/main/java/net/minestom/server/inventory/PlayerInventory.java @@ -3,7 +3,6 @@ package net.minestom.server.inventory; import net.minestom.server.entity.Player; import net.minestom.server.event.item.ArmorEquipEvent; import net.minestom.server.inventory.click.ClickType; -import net.minestom.server.inventory.click.InventoryClickLoopHandler; import net.minestom.server.inventory.click.InventoryClickResult; import net.minestom.server.inventory.condition.InventoryCondition; import net.minestom.server.item.ItemStack; @@ -406,12 +405,7 @@ public class PlayerInventory extends AbstractInventory implements EquipmentHandl @Override public boolean doubleClick(@NotNull Player player, int slot) { final ItemStack cursor = getCursorItem(); - - final InventoryClickResult clickResult = clickProcessor.doubleClick(null, player, slot, cursor, - new InventoryClickLoopHandler(0, itemStacks.length, 1, - i -> i < 9 ? i + 9 : i - 9, - index -> itemStacks[index], - this::setItemStack)); + final InventoryClickResult clickResult = clickProcessor.doubleClick(this, null, player, slot, cursor); if (clickResult == null) return false; @@ -419,6 +413,7 @@ public class PlayerInventory extends AbstractInventory implements EquipmentHandl if (clickResult.doRefresh()) update(); + setItemStack(slot, OFFSET, clickResult.getClicked()); setCursorItem(clickResult.getCursor()); return !clickResult.isCancel(); diff --git a/src/main/java/net/minestom/server/inventory/TransactionOption.java b/src/main/java/net/minestom/server/inventory/TransactionOption.java index 0fa871d87..577784ebd 100644 --- a/src/main/java/net/minestom/server/inventory/TransactionOption.java +++ b/src/main/java/net/minestom/server/inventory/TransactionOption.java @@ -44,4 +44,11 @@ public interface TransactionOption { @NotNull T fill(@NotNull AbstractInventory inventory, @NotNull ItemStack result, @NotNull Map<@NotNull Integer, @NotNull ItemStack> itemChangesMap); + + default @NotNull T fill(@NotNull TransactionType type, + @NotNull AbstractInventory inventory, + @NotNull ItemStack itemStack) { + var pair = type.process(inventory, itemStack); + return fill(inventory, pair.left(), pair.right()); + } } diff --git a/src/main/java/net/minestom/server/inventory/click/InventoryClickProcessor.java b/src/main/java/net/minestom/server/inventory/click/InventoryClickProcessor.java index b4c4b735b..ffa056637 100644 --- a/src/main/java/net/minestom/server/inventory/click/InventoryClickProcessor.java +++ b/src/main/java/net/minestom/server/inventory/click/InventoryClickProcessor.java @@ -1,6 +1,5 @@ package net.minestom.server.inventory.click; -import it.unimi.dsi.fastutil.ints.Int2IntFunction; import it.unimi.dsi.fastutil.ints.Int2ObjectFunction; import it.unimi.dsi.fastutil.ints.IntOpenHashSet; import it.unimi.dsi.fastutil.ints.IntSet; @@ -9,6 +8,7 @@ import net.minestom.server.event.inventory.InventoryClickEvent; import net.minestom.server.event.inventory.InventoryPreClickEvent; import net.minestom.server.inventory.AbstractInventory; import net.minestom.server.inventory.Inventory; +import net.minestom.server.inventory.TransactionOption; import net.minestom.server.inventory.TransactionType; import net.minestom.server.inventory.condition.InventoryCondition; import net.minestom.server.inventory.condition.InventoryConditionResult; @@ -21,7 +21,9 @@ import org.jetbrains.annotations.Nullable; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.function.BiConsumer; +import java.util.function.BiFunction; public class InventoryClickProcessor { @@ -342,8 +344,8 @@ public class InventoryClickProcessor { } @Nullable - public InventoryClickResult doubleClick(@Nullable Inventory inventory, @NotNull Player player, int slot, - @NotNull ItemStack cursor, @NotNull InventoryClickLoopHandler... loopHandlers) { + public InventoryClickResult doubleClick(@NotNull AbstractInventory clickedInventory, @Nullable Inventory inventory, @NotNull Player player, int slot, + @NotNull final ItemStack cursor) { InventoryClickResult clickResult = startCondition(inventory, player, slot, ClickType.START_DOUBLE_CLICK, ItemStack.AIR, cursor); if (clickResult.isCancel()) { @@ -354,49 +356,46 @@ public class InventoryClickProcessor { return null; final StackingRule cursorRule = cursor.getStackingRule(); - int amount = cursorRule.getAmount(cursor); + final int amount = cursorRule.getAmount(cursor); + final int remainingAmount = cursorRule.getMaxSize() - amount; - if (!cursorRule.canApply(cursor, amount + 1)) - return null; + ItemStack remain = cursorRule.apply(cursor, remainingAmount); - for (InventoryClickLoopHandler loopHandler : loopHandlers) { - final Int2IntFunction indexModifier = loopHandler.getIndexModifier(); - final Int2ObjectFunction itemGetter = loopHandler.getItemGetter(); - final BiConsumer itemSetter = loopHandler.getItemSetter(); + BiFunction func = (inv, rest) -> { + var pair = TransactionType.TAKE.process(inv, rest, (index, itemStack) -> { + if (index == slot) // Prevent item lose/duplication + return false; + InventoryClickResult result = startCondition(inventory, player, index, ClickType.DOUBLE_CLICK, itemStack, cursor); + return !result.isCancel(); + }); + var itemResult = pair.left(); + var map = pair.right(); + return TransactionOption.ALL.fill(inv, itemResult, map); + }; - for (int i = loopHandler.getStart(); i < loopHandler.getEnd(); i += loopHandler.getStep()) { - final int index = indexModifier.apply(i); - if (index == slot) - continue; + var playerInventory = player.getInventory(); - ItemStack item = itemGetter.apply(index); - final StackingRule itemRule = item.getStackingRule(); - if (!cursorRule.canApply(cursor, amount + 1)) - break; - if (cursorRule.canBeStacked(cursor, item)) { - clickResult = startCondition(inventory, player, index, ClickType.DOUBLE_CLICK, item, cursor); - if (clickResult.isCancel()) - break; - - final int totalAmount = amount + cursorRule.getAmount(item); - if (!cursorRule.canApply(cursor, totalAmount)) { - cursor = cursorRule.apply(cursor, cursorRule.getMaxSize()); - - item = itemRule.apply(item, totalAmount - itemRule.getMaxSize()); - } else { - cursor = cursorRule.apply(cursor, totalAmount); - item = itemRule.apply(item, 0); - } - itemSetter.accept(index, item); - amount = cursorRule.getAmount(cursor); - - callClickEvent(player, inventory, index, ClickType.DOUBLE_CLICK, item, cursor); - } + if (Objects.equals(clickedInventory, inventory)) { + // Clicked inside inventory + remain = func.apply(inventory, remain); + if (!remain.isAir()) { + remain = func.apply(playerInventory, remain); } + } else if (inventory != null && clickedInventory == playerInventory) { + // Clicked inside player inventory, but with another inventory open + remain = func.apply(playerInventory, remain); + if (!remain.isAir()) { + remain = func.apply(inventory, remain); + } + } else { + // Clicked inside player inventory + remain = func.apply(playerInventory, remain); } - clickResult.setCursor(cursor); + final int tookAmount = remainingAmount - cursorRule.getAmount(remain); + ItemStack resultCursor = cursorRule.apply(cursor, amount + tookAmount); + clickResult.setCursor(resultCursor); return clickResult; } From 359d18e8a50cbd956fec97fec47a41c2727c9707 Mon Sep 17 00:00:00 2001 From: TheMode Date: Tue, 20 Apr 2021 07:30:09 +0200 Subject: [PATCH 5/7] Remove unused class --- .../click/InventoryClickLoopHandler.java | 53 ------------------- 1 file changed, 53 deletions(-) delete mode 100644 src/main/java/net/minestom/server/inventory/click/InventoryClickLoopHandler.java diff --git a/src/main/java/net/minestom/server/inventory/click/InventoryClickLoopHandler.java b/src/main/java/net/minestom/server/inventory/click/InventoryClickLoopHandler.java deleted file mode 100644 index 54e183581..000000000 --- a/src/main/java/net/minestom/server/inventory/click/InventoryClickLoopHandler.java +++ /dev/null @@ -1,53 +0,0 @@ -package net.minestom.server.inventory.click; - -import it.unimi.dsi.fastutil.ints.Int2IntFunction; -import it.unimi.dsi.fastutil.ints.Int2ObjectFunction; -import net.minestom.server.item.ItemStack; - -import java.util.function.BiConsumer; - -public class InventoryClickLoopHandler { - - private final int start; - private final int end; - private final int step; - private final Int2IntFunction indexModifier; - private final Int2ObjectFunction itemGetter; - private final BiConsumer itemSetter; - - public InventoryClickLoopHandler(int start, int end, int step, - Int2IntFunction indexModifier, - Int2ObjectFunction itemGetter, - BiConsumer itemSetter) { - this.start = start; - this.end = end; - this.step = step; - this.indexModifier = indexModifier; - this.itemGetter = itemGetter; - this.itemSetter = itemSetter; - } - - public int getStart() { - return start; - } - - public int getEnd() { - return end; - } - - public int getStep() { - return step; - } - - public Int2IntFunction getIndexModifier() { - return indexModifier; - } - - public Int2ObjectFunction getItemGetter() { - return itemGetter; - } - - public BiConsumer getItemSetter() { - return itemSetter; - } -} From 66513025c76173c43d4e32139bc789ee3690de3a Mon Sep 17 00:00:00 2001 From: TheMode Date: Tue, 20 Apr 2021 07:32:53 +0200 Subject: [PATCH 6/7] Fix thread safety with inventory click, add Internal annotation --- .../server/inventory/click/InventoryClickProcessor.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/minestom/server/inventory/click/InventoryClickProcessor.java b/src/main/java/net/minestom/server/inventory/click/InventoryClickProcessor.java index ffa056637..3b969f408 100644 --- a/src/main/java/net/minestom/server/inventory/click/InventoryClickProcessor.java +++ b/src/main/java/net/minestom/server/inventory/click/InventoryClickProcessor.java @@ -15,21 +15,23 @@ import net.minestom.server.inventory.condition.InventoryConditionResult; import net.minestom.server.item.ItemStack; import net.minestom.server.item.StackingRule; import net.minestom.server.utils.inventory.PlayerInventoryUtils; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.BiConsumer; import java.util.function.BiFunction; +@ApiStatus.Internal public class InventoryClickProcessor { // Dragging maps - private final Map leftDraggingMap = new HashMap<>(); - private final Map rightDraggingMap = new HashMap<>(); + private final Map leftDraggingMap = new ConcurrentHashMap<>(); + private final Map rightDraggingMap = new ConcurrentHashMap<>(); @NotNull public InventoryClickResult leftClick(@Nullable Inventory inventory, @NotNull Player player, int slot, From 3fa89737d927a197c1b99a5f5b78e49c24d32e79 Mon Sep 17 00:00:00 2001 From: TheMode Date: Tue, 20 Apr 2021 08:54:33 +0200 Subject: [PATCH 7/7] Reduce tick buffer copy overhead --- .../network/netty/channel/ClientChannel.java | 5 +--- .../network/player/NettyPlayerConnection.java | 25 ++++++++++--------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/main/java/net/minestom/server/network/netty/channel/ClientChannel.java b/src/main/java/net/minestom/server/network/netty/channel/ClientChannel.java index f915afccf..48bc2e0e1 100644 --- a/src/main/java/net/minestom/server/network/netty/channel/ClientChannel.java +++ b/src/main/java/net/minestom/server/network/netty/channel/ClientChannel.java @@ -71,10 +71,7 @@ public class ClientChannel extends SimpleChannelInboundHandler { // Release tick buffer if (playerConnection instanceof NettyPlayerConnection) { - final ByteBuf tickBuffer = ((NettyPlayerConnection) playerConnection).getTickBuffer(); - synchronized (tickBuffer) { - tickBuffer.release(); - } + ((NettyPlayerConnection) playerConnection).releaseTickBuffer(); } } } diff --git a/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java b/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java index 6c1d796ea..063b9a3b4 100644 --- a/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java +++ b/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java @@ -62,7 +62,8 @@ public class NettyPlayerConnection extends PlayerConnection { private PlayerSkin bungeeSkin; private final static int INITIAL_BUFFER_SIZE = 65_535; // 2^16-1 - private final ByteBuf tickBuffer = BufUtils.getBuffer(true); + private final Object tickBufferLock = new Object(); + private volatile ByteBuf tickBuffer = BufUtils.getBuffer(true); public NettyPlayerConnection(@NotNull SocketChannel channel) { super(); @@ -119,7 +120,7 @@ public class NettyPlayerConnection extends PlayerConnection { if (getPlayer() != null) { // Flush happen during #update() if (serverPacket instanceof CacheablePacket && MinecraftServer.hasPacketCaching()) { - synchronized (tickBuffer) { + synchronized (tickBufferLock) { if (tickBuffer.refCnt() == 0) return; CacheablePacket.writeCache(tickBuffer, serverPacket); @@ -141,7 +142,7 @@ public class NettyPlayerConnection extends PlayerConnection { public void write(@NotNull Object message, boolean skipTranslating) { if (message instanceof FramedPacket) { final FramedPacket framedPacket = (FramedPacket) message; - synchronized (tickBuffer) { + synchronized (tickBufferLock) { if (tickBuffer.refCnt() == 0) return; final ByteBuf body = framedPacket.getBody(); @@ -155,14 +156,14 @@ public class NettyPlayerConnection extends PlayerConnection { serverPacket = ((ComponentHoldingServerPacket) serverPacket).copyWithOperator(component -> AdventureSerializer.translate(component, getPlayer())); } - synchronized (tickBuffer) { + synchronized (tickBufferLock) { if (tickBuffer.refCnt() == 0) return; PacketUtils.writeFramedPacket(tickBuffer, serverPacket); } return; } else if (message instanceof ByteBuf) { - synchronized (tickBuffer) { + synchronized (tickBufferLock) { if (tickBuffer.refCnt() == 0) return; tickBuffer.writeBytes((ByteBuf) message); @@ -193,11 +194,11 @@ public class NettyPlayerConnection extends PlayerConnection { // Retrieve safe copy final ByteBuf copy; - synchronized (tickBuffer) { + synchronized (tickBufferLock) { if (tickBuffer.refCnt() == 0) return; - copy = tickBuffer.copy(); - tickBuffer.clear(); + copy = tickBuffer; + tickBuffer = tickBuffer.alloc().buffer(tickBuffer.writerIndex()); } // Write copied buffer to netty @@ -379,10 +380,10 @@ public class NettyPlayerConnection extends PlayerConnection { } } - - @NotNull - public ByteBuf getTickBuffer() { - return tickBuffer; + public void releaseTickBuffer() { + synchronized (tickBufferLock) { + tickBuffer.release(); + } } public byte[] getNonce() {