From 6543f17d4cc43546622a0b1d8ca0436e2db94ea2 Mon Sep 17 00:00:00 2001 From: Felix Cravic Date: Fri, 29 May 2020 19:41:08 +0200 Subject: [PATCH] Remove useless synchronization blocks and fix Inventory#addItemStack --- .../net/minestom/server/instance/Chunk.java | 40 +++--- .../server/instance/batch/BlockBatch.java | 26 ++-- .../minestom/server/inventory/Inventory.java | 54 ++++++-- .../server/inventory/PlayerInventory.java | 120 +++++++++--------- .../network/player/FakePlayerConnection.java | 2 +- 5 files changed, 131 insertions(+), 111 deletions(-) diff --git a/src/main/java/net/minestom/server/instance/Chunk.java b/src/main/java/net/minestom/server/instance/Chunk.java index c529675c7..a6b79aa48 100644 --- a/src/main/java/net/minestom/server/instance/Chunk.java +++ b/src/main/java/net/minestom/server/instance/Chunk.java @@ -223,35 +223,33 @@ public class Chunk implements Viewable { return blocksData.get(index); } - public void updateBlocks(long time, Instance instance) { + public synchronized void updateBlocks(long time, Instance instance) { if (updatableBlocks.isEmpty()) return; // Block all chunk operation during the update - synchronized (this) { - IntIterator iterator = new IntOpenHashSet(updatableBlocks).iterator(); - while (iterator.hasNext()) { - int index = iterator.nextInt(); - CustomBlock customBlock = getCustomBlock(index); + IntIterator iterator = new IntOpenHashSet(updatableBlocks).iterator(); + while (iterator.hasNext()) { + int index = iterator.nextInt(); + CustomBlock customBlock = getCustomBlock(index); - // Update cooldown - UpdateOption updateOption = customBlock.getUpdateOption(); - long lastUpdate = updatableBlocksLastUpdate.get(index); - boolean hasCooldown = CooldownUtils.hasCooldown(time, lastUpdate, updateOption.getTimeUnit(), updateOption.getValue()); - if (hasCooldown) - continue; + // Update cooldown + UpdateOption updateOption = customBlock.getUpdateOption(); + long lastUpdate = updatableBlocksLastUpdate.get(index); + boolean hasCooldown = CooldownUtils.hasCooldown(time, lastUpdate, updateOption.getTimeUnit(), updateOption.getValue()); + if (hasCooldown) + continue; - this.updatableBlocksLastUpdate.put(index, time); // Refresh last update time + this.updatableBlocksLastUpdate.put(index, time); // Refresh last update time - int[] blockPos = ChunkUtils.indexToPosition(index, chunkX, chunkZ); - int x = blockPos[0]; - int y = blockPos[1]; - int z = blockPos[2]; + int[] blockPos = ChunkUtils.indexToPosition(index, chunkX, chunkZ); + int x = blockPos[0]; + int y = blockPos[1]; + int z = blockPos[2]; - BlockPosition blockPosition = new BlockPosition(x, y, z); - Data data = getData(index); - customBlock.update(instance, blockPosition, data); - } + BlockPosition blockPosition = new BlockPosition(x, y, z); + Data data = getData(index); + customBlock.update(instance, blockPosition, data); } } diff --git a/src/main/java/net/minestom/server/instance/batch/BlockBatch.java b/src/main/java/net/minestom/server/instance/batch/BlockBatch.java index 42f3e6f07..215ef7cef 100644 --- a/src/main/java/net/minestom/server/instance/batch/BlockBatch.java +++ b/src/main/java/net/minestom/server/instance/batch/BlockBatch.java @@ -21,28 +21,22 @@ public class BlockBatch implements InstanceBatch { } @Override - public void setBlock(int x, int y, int z, short blockId, Data data) { - synchronized (this) { - Chunk chunk = this.instance.getChunkAt(x, z); - addBlockData(chunk, x, y, z, false, blockId, (short) 0, data); - } + public synchronized void setBlock(int x, int y, int z, short blockId, Data data) { + Chunk chunk = this.instance.getChunkAt(x, z); + addBlockData(chunk, x, y, z, false, blockId, (short) 0, data); } @Override - public void setCustomBlock(int x, int y, int z, short blockId, Data data) { - synchronized (this) { - Chunk chunk = this.instance.getChunkAt(x, z); - CustomBlock customBlock = BLOCK_MANAGER.getCustomBlock(blockId); - addBlockData(chunk, x, y, z, true, customBlock.getBlockId(), blockId, data); - } + public synchronized void setCustomBlock(int x, int y, int z, short blockId, Data data) { + Chunk chunk = this.instance.getChunkAt(x, z); + CustomBlock customBlock = BLOCK_MANAGER.getCustomBlock(blockId); + addBlockData(chunk, x, y, z, true, customBlock.getBlockId(), blockId, data); } @Override - public void setSeparateBlocks(int x, int y, int z, short blockId, short customBlockId, Data data) { - synchronized (this) { - Chunk chunk = this.instance.getChunkAt(x, z); - addBlockData(chunk, x, y, z, true, blockId, customBlockId, data); - } + public synchronized void setSeparateBlocks(int x, int y, int z, short blockId, short customBlockId, Data data) { + Chunk chunk = this.instance.getChunkAt(x, z); + addBlockData(chunk, x, y, z, true, blockId, customBlockId, data); } private void addBlockData(Chunk chunk, int x, int y, int z, boolean customBlock, short blockId, short customBlockId, Data data) { diff --git a/src/main/java/net/minestom/server/inventory/Inventory.java b/src/main/java/net/minestom/server/inventory/Inventory.java index df2a23ed8..d7ec08281 100644 --- a/src/main/java/net/minestom/server/inventory/Inventory.java +++ b/src/main/java/net/minestom/server/inventory/Inventory.java @@ -8,6 +8,7 @@ import net.minestom.server.inventory.click.InventoryClickProcessor; import net.minestom.server.inventory.click.InventoryClickResult; import net.minestom.server.inventory.condition.InventoryCondition; import net.minestom.server.item.ItemStack; +import net.minestom.server.item.StackingRule; import net.minestom.server.network.PacketWriterUtils; import net.minestom.server.network.packet.server.play.SetSlotPacket; import net.minestom.server.network.packet.server.play.WindowItemsPacket; @@ -88,7 +89,32 @@ public class Inventory implements InventoryModifier, InventoryClickHandler, View } @Override - public boolean addItemStack(ItemStack itemStack) { + public synchronized boolean addItemStack(ItemStack itemStack) { + StackingRule stackingRule = itemStack.getStackingRule(); + for (int i = 0; i < getItemStacks().length; i++) { + ItemStack item = getItemStacks()[i]; + StackingRule itemStackingRule = item.getStackingRule(); + if (itemStackingRule.canBeStacked(itemStack, item)) { + int itemAmount = itemStackingRule.getAmount(item); + if (itemAmount == stackingRule.getMaxSize()) + continue; + int itemStackAmount = itemStackingRule.getAmount(itemStack); + int totalAmount = itemStackAmount + itemAmount; + if (!stackingRule.canApply(itemStack, totalAmount)) { + item = itemStackingRule.apply(item, itemStackingRule.getMaxSize()); + + sendSlotRefresh((short) i, item); + itemStack = stackingRule.apply(itemStack, totalAmount - stackingRule.getMaxSize()); + } else { + item.setAmount((byte) totalAmount); + sendSlotRefresh((short) i, item); + return true; + } + } else if (item.isAir()) { + setItemStack(i, itemStack); + return true; + } + } return false; } @@ -161,16 +187,14 @@ public class Inventory implements InventoryModifier, InventoryClickHandler, View return cursorPlayersItem.getOrDefault(player, ItemStack.getAirItem()); } - private void safeItemInsert(int slot, ItemStack itemStack) { - synchronized (this) { - itemStack = ItemStackUtils.notNull(itemStack); - this.itemStacks[slot] = itemStack; - SetSlotPacket setSlotPacket = new SetSlotPacket(); - setSlotPacket.windowId = getWindowId(); - setSlotPacket.slot = (short) slot; - setSlotPacket.itemStack = itemStack; - sendPacketToViewers(setSlotPacket); - } + private synchronized void safeItemInsert(int slot, ItemStack itemStack) { + itemStack = ItemStackUtils.notNull(itemStack); + this.itemStacks[slot] = itemStack; + SetSlotPacket setSlotPacket = new SetSlotPacket(); + setSlotPacket.windowId = getWindowId(); + setSlotPacket.slot = (short) slot; + setSlotPacket.itemStack = itemStack; + sendPacketToViewers(setSlotPacket); } private WindowItemsPacket createWindowItemsPacket() { @@ -440,6 +464,14 @@ public class Inventory implements InventoryModifier, InventoryClickHandler, View return !clickResult.isCancel(); } + private void sendSlotRefresh(short slot, ItemStack itemStack) { + SetSlotPacket setSlotPacket = new SetSlotPacket(); + setSlotPacket.windowId = getWindowId(); + setSlotPacket.slot = slot; + setSlotPacket.itemStack = itemStack; + sendPacketToViewers(setSlotPacket); + } + /** * Used to update the inventory for a specific player in order to fix his cancelled actions * diff --git a/src/main/java/net/minestom/server/inventory/PlayerInventory.java b/src/main/java/net/minestom/server/inventory/PlayerInventory.java index 86141ca4b..0b9c6d500 100644 --- a/src/main/java/net/minestom/server/inventory/PlayerInventory.java +++ b/src/main/java/net/minestom/server/inventory/PlayerInventory.java @@ -72,32 +72,30 @@ public class PlayerInventory implements InventoryModifier, InventoryClickHandler } @Override - public boolean addItemStack(ItemStack itemStack) { - synchronized (this) { - StackingRule stackingRule = itemStack.getStackingRule(); - for (int i = 0; i < items.length - 10; i++) { - ItemStack item = items[i]; - StackingRule itemStackingRule = item.getStackingRule(); - if (itemStackingRule.canBeStacked(itemStack, item)) { - int itemAmount = itemStackingRule.getAmount(item); - if (itemAmount == stackingRule.getMaxSize()) - continue; - int itemStackAmount = itemStackingRule.getAmount(itemStack); - int totalAmount = itemStackAmount + itemAmount; - if (!stackingRule.canApply(itemStack, totalAmount)) { - item = itemStackingRule.apply(item, itemStackingRule.getMaxSize()); + public synchronized boolean addItemStack(ItemStack itemStack) { + StackingRule stackingRule = itemStack.getStackingRule(); + for (int i = 0; i < items.length - 10; i++) { + ItemStack item = items[i]; + StackingRule itemStackingRule = item.getStackingRule(); + if (itemStackingRule.canBeStacked(itemStack, item)) { + int itemAmount = itemStackingRule.getAmount(item); + if (itemAmount == stackingRule.getMaxSize()) + continue; + int itemStackAmount = itemStackingRule.getAmount(itemStack); + int totalAmount = itemStackAmount + itemAmount; + if (!stackingRule.canApply(itemStack, totalAmount)) { + item = itemStackingRule.apply(item, itemStackingRule.getMaxSize()); - sendSlotRefresh((short) convertToPacketSlot(i), item); - itemStack = stackingRule.apply(itemStack, totalAmount - stackingRule.getMaxSize()); - } else { - item.setAmount((byte) totalAmount); - sendSlotRefresh((short) convertToPacketSlot(i), item); - return true; - } - } else if (item.isAir()) { - setItemStack(i, itemStack); + sendSlotRefresh((short) convertToPacketSlot(i), item); + itemStack = stackingRule.apply(itemStack, totalAmount - stackingRule.getMaxSize()); + } else { + item.setAmount((byte) totalAmount); + sendSlotRefresh((short) convertToPacketSlot(i), item); return true; } + } else if (item.isAir()) { + setItemStack(i, itemStack); + return true; } } return false; @@ -184,50 +182,48 @@ public class PlayerInventory implements InventoryModifier, InventoryClickHandler this.cursorItem = ItemStackUtils.notNull(cursorItem); } - private void safeItemInsert(int slot, ItemStack itemStack) { - synchronized (this) { - itemStack = ItemStackUtils.notNull(itemStack); + private synchronized void safeItemInsert(int slot, ItemStack itemStack) { + itemStack = ItemStackUtils.notNull(itemStack); - EntityEquipmentPacket.Slot equipmentSlot; + EntityEquipmentPacket.Slot equipmentSlot; - if (slot == player.getHeldSlot()) { - equipmentSlot = EntityEquipmentPacket.Slot.MAIN_HAND; - } else if (slot == OFFHAND_SLOT) { - equipmentSlot = EntityEquipmentPacket.Slot.OFF_HAND; + if (slot == player.getHeldSlot()) { + equipmentSlot = EntityEquipmentPacket.Slot.MAIN_HAND; + } else if (slot == OFFHAND_SLOT) { + equipmentSlot = EntityEquipmentPacket.Slot.OFF_HAND; + } else { + ArmorEquipEvent armorEquipEvent = null; + + if (slot == HELMET_SLOT) { + armorEquipEvent = new ArmorEquipEvent(player, itemStack, ArmorEquipEvent.ArmorSlot.HELMET); + } else if (slot == CHESTPLATE_SLOT) { + armorEquipEvent = new ArmorEquipEvent(player, itemStack, ArmorEquipEvent.ArmorSlot.CHESTPLATE); + } else if (slot == LEGGINGS_SLOT) { + armorEquipEvent = new ArmorEquipEvent(player, itemStack, ArmorEquipEvent.ArmorSlot.LEGGINGS); + } else if (slot == BOOTS_SLOT) { + armorEquipEvent = new ArmorEquipEvent(player, itemStack, ArmorEquipEvent.ArmorSlot.BOOTS); + } + + if (armorEquipEvent != null) { + ArmorEquipEvent.ArmorSlot armorSlot = armorEquipEvent.getArmorSlot(); + equipmentSlot = EntityEquipmentPacket.Slot.fromArmorSlot(armorSlot); + player.callEvent(ArmorEquipEvent.class, armorEquipEvent); + itemStack = armorEquipEvent.getArmorItem(); } else { - ArmorEquipEvent armorEquipEvent = null; - - if (slot == HELMET_SLOT) { - armorEquipEvent = new ArmorEquipEvent(player, itemStack, ArmorEquipEvent.ArmorSlot.HELMET); - } else if (slot == CHESTPLATE_SLOT) { - armorEquipEvent = new ArmorEquipEvent(player, itemStack, ArmorEquipEvent.ArmorSlot.CHESTPLATE); - } else if (slot == LEGGINGS_SLOT) { - armorEquipEvent = new ArmorEquipEvent(player, itemStack, ArmorEquipEvent.ArmorSlot.LEGGINGS); - } else if (slot == BOOTS_SLOT) { - armorEquipEvent = new ArmorEquipEvent(player, itemStack, ArmorEquipEvent.ArmorSlot.BOOTS); - } - - if (armorEquipEvent != null) { - ArmorEquipEvent.ArmorSlot armorSlot = armorEquipEvent.getArmorSlot(); - equipmentSlot = EntityEquipmentPacket.Slot.fromArmorSlot(armorSlot); - player.callEvent(ArmorEquipEvent.class, armorEquipEvent); - itemStack = armorEquipEvent.getArmorItem(); - } else { - equipmentSlot = null; - } + equipmentSlot = null; } - - this.items[slot] = itemStack; - - // Sync equipment - if (equipmentSlot != null) { - player.syncEquipment(equipmentSlot); - } - - // Refresh slot - update(); - //refreshSlot(slot); seems to break things concerning +64 stacks } + + this.items[slot] = itemStack; + + // Sync equipment + if (equipmentSlot != null) { + player.syncEquipment(equipmentSlot); + } + + // Refresh slot + update(); + //refreshSlot(slot); seems to break things concerning +64 stacks } protected void setItemStack(int slot, int offset, ItemStack itemStack) { diff --git a/src/main/java/net/minestom/server/network/player/FakePlayerConnection.java b/src/main/java/net/minestom/server/network/player/FakePlayerConnection.java index 72c5f54f7..f3be6ccea 100644 --- a/src/main/java/net/minestom/server/network/player/FakePlayerConnection.java +++ b/src/main/java/net/minestom/server/network/player/FakePlayerConnection.java @@ -29,7 +29,7 @@ public class FakePlayerConnection extends PlayerConnection { @Override public void flush() { - throw new UnsupportedOperationException("FakePlayer does not have anything to flush"); + // Does nothing } @Override