From f849a5b9f99f38073d775cf85957c93eb094bb0e Mon Sep 17 00:00:00 2001 From: Camotoy <20743703+Camotoy@users.noreply.github.com> Date: Sat, 20 Feb 2021 23:52:49 -0500 Subject: [PATCH] A bunch of fixes --- .../connector/inventory/Inventory.java | 15 ++++++- .../network/UpstreamPacketHandler.java | 1 + .../network/session/GeyserSession.java | 2 +- .../inventory/InventoryTranslator.java | 41 +++++++++---------- .../holder/BlockInventoryHolder.java | 19 +++++++-- .../chest/SingleChestInventoryTranslator.java | 13 +++++- .../window/JavaCloseWindowTranslator.java | 5 ++- .../java/window/JavaOpenWindowTranslator.java | 6 +-- 8 files changed, 69 insertions(+), 33 deletions(-) diff --git a/connector/src/main/java/org/geysermc/connector/inventory/Inventory.java b/connector/src/main/java/org/geysermc/connector/inventory/Inventory.java index 7048ce808..535a9077a 100644 --- a/connector/src/main/java/org/geysermc/connector/inventory/Inventory.java +++ b/connector/src/main/java/org/geysermc/connector/inventory/Inventory.java @@ -36,7 +36,7 @@ import java.util.Arrays; public class Inventory { @Getter - protected int id; + protected final int id; @Getter protected final int size; @@ -96,4 +96,17 @@ public class Inventory { public short getNextTransactionId() { return ++transactionId; } + + @Override + public String toString() { + return "Inventory{" + + "id=" + id + + ", size=" + size + + ", title='" + title + '\'' + + ", items=" + Arrays.toString(items) + + ", holderPosition=" + holderPosition + + ", holderId=" + holderId + + ", transactionId=" + transactionId + + '}'; + } } diff --git a/connector/src/main/java/org/geysermc/connector/network/UpstreamPacketHandler.java b/connector/src/main/java/org/geysermc/connector/network/UpstreamPacketHandler.java index 7ebfaeda5..1245fe558 100644 --- a/connector/src/main/java/org/geysermc/connector/network/UpstreamPacketHandler.java +++ b/connector/src/main/java/org/geysermc/connector/network/UpstreamPacketHandler.java @@ -47,6 +47,7 @@ public class UpstreamPacketHandler extends LoggingPacketHandler { } private boolean translateAndDefault(BedrockPacket packet) { + System.out.println(packet.toString()); return PacketTranslatorRegistry.BEDROCK_TRANSLATOR.translate(packet.getClass(), packet, session); } diff --git a/connector/src/main/java/org/geysermc/connector/network/session/GeyserSession.java b/connector/src/main/java/org/geysermc/connector/network/session/GeyserSession.java index 080e8bf02..41269ebf1 100644 --- a/connector/src/main/java/org/geysermc/connector/network/session/GeyserSession.java +++ b/connector/src/main/java/org/geysermc/connector/network/session/GeyserSession.java @@ -144,7 +144,7 @@ public class GeyserSession implements CommandSender { * Use {@link #getNextItemNetId()} instead for consistency */ @Getter(AccessLevel.NONE) - private final AtomicInteger itemNetId = new AtomicInteger(1); + private final AtomicInteger itemNetId = new AtomicInteger(2); @Getter(AccessLevel.NONE) private final Object inventoryLock = new Object(); diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/inventory/InventoryTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/inventory/InventoryTranslator.java index 7a9b890b3..d924c1b88 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/inventory/InventoryTranslator.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/inventory/InventoryTranslator.java @@ -85,6 +85,8 @@ public abstract class InventoryTranslator { put(WindowType.CARTOGRAPHY, new CartographyInventoryTranslator()); put(WindowType.CRAFTING, new CraftingInventoryTranslator()); put(WindowType.ENCHANTMENT, new EnchantingInventoryTranslator()); + put(WindowType.HOPPER, new HopperInventoryTranslator()); + put(WindowType.GENERIC_3X3, new Generic3X3InventoryTranslator()); put(WindowType.GRINDSTONE, new GrindstoneInventoryTranslator()); put(WindowType.LOOM, new LoomInventoryTranslator()); put(WindowType.MERCHANT, new MerchantInventoryTranslator()); @@ -92,10 +94,6 @@ public abstract class InventoryTranslator { put(WindowType.SMITHING, new SmithingInventoryTranslator()); put(WindowType.STONECUTTER, new StonecutterInventoryTranslator()); - /* Generics */ - put(WindowType.GENERIC_3X3, new Generic3X3InventoryTranslator()); - put(WindowType.HOPPER, new HopperInventoryTranslator()); - /* Lectern */ put(WindowType.LECTERN, new LecternInventoryTranslator()); } @@ -195,9 +193,11 @@ public abstract class InventoryTranslator { transferAction.getSource().getSlot() >= 28 && transferAction.getSource().getSlot() <= 31) { return rejectRequest(request, false); } - session.getConnector().getLogger().error("DEBUG: About to reject request made by " + session.getName()); + session.getConnector().getLogger().error("DEBUG: About to reject TAKE/PLACE request made by " + session.getName()); session.getConnector().getLogger().error("Source: " + transferAction.getSource().toString() + " Result: " + checkNetId(session, inventory, transferAction.getSource())); session.getConnector().getLogger().error("Destination: " + transferAction.getDestination().toString() + " Result: " + checkNetId(session, inventory, transferAction.getDestination())); + session.getConnector().getLogger().error("Geyser's record of source slot: " + inventory.getItem(bedrockSlotToJava(transferAction.getSource()))); + session.getConnector().getLogger().error("Geyser's record of destination slot: " + inventory.getItem(bedrockSlotToJava(transferAction.getDestination()))); return rejectRequest(request); } @@ -278,8 +278,14 @@ public abstract class InventoryTranslator { } case SWAP: { //TODO SwapStackRequestActionData swapAction = (SwapStackRequestActionData) action; - if (!(checkNetId(session, inventory, swapAction.getSource()) && checkNetId(session, inventory, swapAction.getDestination()))) + if (!(checkNetId(session, inventory, swapAction.getSource()) && checkNetId(session, inventory, swapAction.getDestination()))) { + session.getConnector().getLogger().error("DEBUG: About to reject SWAP request made by " + session.getName()); + session.getConnector().getLogger().error("Source: " + swapAction.getSource().toString() + " Result: " + checkNetId(session, inventory, swapAction.getSource())); + session.getConnector().getLogger().error("Destination: " + swapAction.getDestination().toString() + " Result: " + checkNetId(session, inventory, swapAction.getDestination())); + session.getConnector().getLogger().error("Geyser's record of source slot: " + inventory.getItem(bedrockSlotToJava(swapAction.getSource()))); + session.getConnector().getLogger().error("Geyser's record of destination slot: " + inventory.getItem(bedrockSlotToJava(swapAction.getDestination()))); return rejectRequest(request); + } if (isCursor(swapAction.getSource()) && isCursor(swapAction.getDestination())) { //??? return rejectRequest(request); @@ -361,14 +367,9 @@ public abstract class InventoryTranslator { } break; } - case CRAFT_NON_IMPLEMENTED_DEPRECATED: { - break; - } - case CRAFT_RESULTS_DEPRECATED: { - break; - } - case CRAFT_RECIPE_OPTIONAL: { - // Anvils and cartography tables will handle this + case CRAFT_NON_IMPLEMENTED_DEPRECATED: + case CRAFT_RESULTS_DEPRECATED: + case CRAFT_RECIPE_OPTIONAL: { // Anvils and cartography tables will handle this break; } default: @@ -381,9 +382,8 @@ public abstract class InventoryTranslator { } public ItemStackResponsePacket.Response translateCraftingRequest(GeyserSession session, Inventory inventory, ItemStackRequest request) { - int recipeId = 0; int resultSize = 0; - int timesCrafted = 0; + int timesCrafted; CraftState craftState = CraftState.START; int leftover = 0; @@ -396,7 +396,6 @@ public abstract class InventoryTranslator { return rejectRequest(request); } craftState = CraftState.RECIPE_ID; - recipeId = craftAction.getRecipeNetworkId(); break; } case CRAFT_RESULTS_DEPRECATED: { @@ -417,7 +416,6 @@ public abstract class InventoryTranslator { break; } case CONSUME: { - ConsumeStackRequestActionData consumeAction = (ConsumeStackRequestActionData) action; if (craftState != CraftState.DEPRECATED && craftState != CraftState.INGREDIENTS) { return rejectRequest(request); } @@ -507,9 +505,7 @@ public abstract class InventoryTranslator { Int2IntMap consumedSlots = new Int2IntOpenHashMap(); int prioritySlot = -1; - int secondarySlot = -1; - int tempSlot = -1; - boolean intoCursor = false; + int tempSlot; int resultSize; int timesCrafted = 0; @@ -626,7 +622,6 @@ public abstract class InventoryTranslator { int javaSlot = bedrockSlotToJava(transferAction.getDestination()); if (isCursor(transferAction.getDestination())) { //TODO - intoCursor = true; if (timesCrafted > 1) { tempSlot = findTempSlot(inventory, GeyserItemStack.from(output), true); if (tempSlot == -1) { @@ -735,6 +730,8 @@ public abstract class InventoryTranslator { public boolean checkNetId(GeyserSession session, Inventory inventory, StackRequestSlotInfoData slotInfoData) { int netId = slotInfoData.getStackNetworkId(); + // "In my testing, sometimes the client thinks the netId of an item in the crafting grid is 1, even though we never said it was. + // I think it only happens when we manually set the grid but that was my quick fix" if (netId < 0 || netId == 1) return true; diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/inventory/holder/BlockInventoryHolder.java b/connector/src/main/java/org/geysermc/connector/network/translators/inventory/holder/BlockInventoryHolder.java index 24be49144..3186a85e9 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/inventory/holder/BlockInventoryHolder.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/inventory/holder/BlockInventoryHolder.java @@ -31,6 +31,7 @@ import com.nukkitx.math.vector.Vector3i; import com.nukkitx.nbt.NbtMap; import com.nukkitx.protocol.bedrock.data.inventory.ContainerType; import com.nukkitx.protocol.bedrock.packet.BlockEntityDataPacket; +import com.nukkitx.protocol.bedrock.packet.ContainerClosePacket; import com.nukkitx.protocol.bedrock.packet.ContainerOpenPacket; import com.nukkitx.protocol.bedrock.packet.UpdateBlockPacket; import org.geysermc.connector.inventory.Container; @@ -76,11 +77,11 @@ public class BlockInventoryHolder extends InventoryHolder { if (session.getLastInteractionPlayerPosition().equals(session.getPlayerEntity().getPosition())) { // Then, check to see if the interacted block is valid for this inventory by ensuring the block state identifier is valid int javaBlockId = session.getConnector().getWorldManager().getBlockAt(session, session.getLastInteractionBlockPosition()); - String javaBlockString = BlockTranslator.getJavaIdBlockMap().inverse().getOrDefault(javaBlockId, "minecraft:air").split("\\[")[0]; - if (this.validBlocks.contains(javaBlockString)) { + String[] javaBlockString = BlockTranslator.getJavaIdBlockMap().inverse().getOrDefault(javaBlockId, "minecraft:air").split("\\["); + if (isValidBlock(javaBlockString)) { // We can safely use this block inventory.setHolderPosition(session.getLastInteractionBlockPosition()); - ((Container) inventory).setUsingRealBlock(true, javaBlockString); + ((Container) inventory).setUsingRealBlock(true, javaBlockString[0]); setCustomName(session, session.getLastInteractionBlockPosition(), inventory); return; } @@ -100,6 +101,13 @@ public class BlockInventoryHolder extends InventoryHolder { setCustomName(session, position, inventory); } + /** + * @return true if this Java block ID can be used for player inventory. + */ + protected boolean isValidBlock(String[] javaBlockString) { + return this.validBlocks.contains(javaBlockString[0]); + } + protected void setCustomName(GeyserSession session, Vector3i position, Inventory inventory) { NbtMap tag = NbtMap.builder() .putInt("x", position.getX()) @@ -126,6 +134,11 @@ public class BlockInventoryHolder extends InventoryHolder { public void closeInventory(InventoryTranslator translator, GeyserSession session, Inventory inventory) { if (((Container) inventory).isUsingRealBlock()) { // No need to reset a block since we didn't change any blocks + // But send a container close packet because we aren't destroying the original. + ContainerClosePacket packet = new ContainerClosePacket(); + packet.setId((byte) inventory.getId()); + packet.setUnknownBool0(true); //TODO needs to be changed in Protocol to "server-side" or something + session.sendUpstreamPacket(packet); return; } diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/inventory/translators/chest/SingleChestInventoryTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/inventory/translators/chest/SingleChestInventoryTranslator.java index 7b9b12c4f..42b23d5b4 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/inventory/translators/chest/SingleChestInventoryTranslator.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/inventory/translators/chest/SingleChestInventoryTranslator.java @@ -37,7 +37,18 @@ public class SingleChestInventoryTranslator extends ChestInventoryTranslator { public SingleChestInventoryTranslator(int size) { super(size, 27); this.holder = new BlockInventoryHolder("minecraft:chest[facing=north,type=single,waterlogged=false]", ContainerType.CONTAINER, - "minecraft:ender_chest", "minecraft:trapped_chest"); + "minecraft:ender_chest", "minecraft:trapped_chest") { + @Override + protected boolean isValidBlock(String[] javaBlockString) { + if (javaBlockString[0].equals("minecraft:ender_chest")) { + // Can't have double ender chests + return true; + } + + // Add provision to ensure this isn't a double chest + return super.isValidBlock(javaBlockString) && (javaBlockString.length > 1 && javaBlockString[1].contains("type=single")); + } + }; } @Override diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaCloseWindowTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaCloseWindowTranslator.java index b78dc32f2..6d3ae0ab1 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaCloseWindowTranslator.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaCloseWindowTranslator.java @@ -36,6 +36,9 @@ public class JavaCloseWindowTranslator extends PacketTranslator InventoryUtils.closeInventory(session, packet.getWindowId())); + session.addInventoryTask(() -> { + session.getConnector().getLogger().info("Closing window ID " + packet.getWindowId()); + InventoryUtils.closeInventory(session, packet.getWindowId()); + }); } } diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaOpenWindowTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaOpenWindowTranslator.java index 99af2e73e..fa83de7c2 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaOpenWindowTranslator.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaOpenWindowTranslator.java @@ -45,6 +45,7 @@ public class JavaOpenWindowTranslator extends PacketTranslator