From 8738f991ff6532416054cfef5b19918b9f7fa4d4 Mon Sep 17 00:00:00 2001 From: YHDiamond Date: Wed, 19 Jun 2024 20:15:27 -0400 Subject: [PATCH 1/5] Proof of concept for book cloning fix --- .../geyser/inventory/click/ClickPlan.java | 8 +- .../inventory/InventoryTranslator.java | 125 ++++++++++++++++-- .../geysermc/geyser/util/InventoryUtils.java | 4 + 3 files changed, 125 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/org/geysermc/geyser/inventory/click/ClickPlan.java b/core/src/main/java/org/geysermc/geyser/inventory/click/ClickPlan.java index 53b02ef88..e54c63976 100644 --- a/core/src/main/java/org/geysermc/geyser/inventory/click/ClickPlan.java +++ b/core/src/main/java/org/geysermc/geyser/inventory/click/ClickPlan.java @@ -58,11 +58,17 @@ public final class ClickPlan { private final InventoryTranslator translator; private final Inventory inventory; private final int gridSize; + private final boolean handleBookRecipe; public ClickPlan(GeyserSession session, InventoryTranslator translator, Inventory inventory) { + this(session, translator, inventory, false); + } + + public ClickPlan(GeyserSession session, InventoryTranslator translator, Inventory inventory, boolean handleBookRecipe) { this.session = session; this.translator = translator; this.inventory = inventory; + this.handleBookRecipe = handleBookRecipe; this.simulatedItems = new Int2ObjectOpenHashMap<>(inventory.getSize()); this.changedItems = null; @@ -376,7 +382,7 @@ public final class ClickPlan { for (int i = 0; i < gridSize; i++) { final int slot = i + 1; GeyserItemStack item = getItem(slot); - if (!item.isEmpty()) { + if (!item.isEmpty() && (!handleBookRecipe || item.getJavaId() != session.getItemMappings().getStoredItems().writtenBook().getJavaItem().javaId())) { // These changes should be broadcasted to the server sub(slot, item, crafted); } diff --git a/core/src/main/java/org/geysermc/geyser/translator/inventory/InventoryTranslator.java b/core/src/main/java/org/geysermc/geyser/translator/inventory/InventoryTranslator.java index 4c426b410..5db3c3b8d 100644 --- a/core/src/main/java/org/geysermc/geyser/translator/inventory/InventoryTranslator.java +++ b/core/src/main/java/org/geysermc/geyser/translator/inventory/InventoryTranslator.java @@ -453,35 +453,46 @@ public abstract class InventoryTranslator { ClickPlan plan = new ClickPlan(session, this, inventory); // Track all the crafting table slots to report back the contents of the slots after crafting IntSet affectedSlots = new IntOpenHashSet(); + boolean reject = false; for (ItemStackRequestAction action : request.getActions()) { switch (action.getType()) { case CRAFT_RECIPE: { if (craftState != CraftState.START) { - return rejectRequest(request); + reject = true; + break; } craftState = CraftState.RECIPE_ID; + + if (((RecipeItemStackRequestAction) action).getRecipeNetworkId() == InventoryUtils.BOOK_CLONING_RECIPE_ID) { + // Book copying needs to be handled differently + // There's a leftover item + return translateBookCopyCraftingRequest(session, inventory, request); + } break; } case CRAFT_RESULTS_DEPRECATED: { CraftResultsDeprecatedAction deprecatedCraftAction = (CraftResultsDeprecatedAction) action; if (craftState != CraftState.RECIPE_ID) { - return rejectRequest(request); + reject = true; + break; } craftState = CraftState.DEPRECATED; if (deprecatedCraftAction.getResultItems().length != 1) { - return rejectRequest(request); + reject = true; + break; } resultSize = deprecatedCraftAction.getResultItems()[0].getCount(); timesCrafted = deprecatedCraftAction.getTimesCrafted(); if (resultSize <= 0 || timesCrafted <= 0) { - return rejectRequest(request); + reject = true; } break; } case CONSUME: { if (craftState != CraftState.DEPRECATED && craftState != CraftState.INGREDIENTS) { - return rejectRequest(request); + reject = true; + break; } craftState = CraftState.INGREDIENTS; affectedSlots.add(bedrockSlotToJava(((ConsumeAction) action).getSource())); @@ -491,15 +502,18 @@ public abstract class InventoryTranslator { case PLACE: { TransferItemStackRequestAction transferAction = (TransferItemStackRequestAction) action; if (craftState != CraftState.INGREDIENTS && craftState != CraftState.TRANSFER) { - return rejectRequest(request); + reject = true; + break; } craftState = CraftState.TRANSFER; if (transferAction.getSource().getContainer() != ContainerSlotType.CREATED_OUTPUT) { - return rejectRequest(request); + reject = true; + break; } if (transferAction.getCount() <= 0) { - return rejectRequest(request); + reject = true; + break; } int sourceSlot = bedrockSlotToJava(transferAction.getSource()); @@ -511,7 +525,8 @@ public abstract class InventoryTranslator { } else { if (leftover != 0) { if (transferAction.getCount() > leftover) { - return rejectRequest(request); + reject = true; + break; } if (transferAction.getCount() == leftover) { plan.add(Click.LEFT, destSlot); @@ -537,7 +552,8 @@ public abstract class InventoryTranslator { GeyserItemStack cursor = session.getPlayerInventory().getCursor(); int tempSlot = findTempSlot(inventory, cursor, true, sourceSlot, destSlot); if (tempSlot == -1) { - return rejectRequest(request); + reject = true; + break; } plan.add(Click.LEFT, tempSlot); //place cursor into temp slot @@ -559,14 +575,101 @@ public abstract class InventoryTranslator { break; } default: - return rejectRequest(request); + reject = true; } + + } + if (reject) { + return rejectRequest(request); } plan.execute(false); affectedSlots.addAll(plan.getAffectedSlots()); return acceptRequest(request, makeContainerEntries(session, inventory, affectedSlots)); } + /** + * Book copying is unique in that there is an item remaining in the crafting table when done. + */ + public ItemStackResponse translateBookCopyCraftingRequest(GeyserSession session, Inventory inventory, ItemStackRequest request) { + CraftState craftState = CraftState.START; + boolean newBookHandled = false; + + ClickPlan plan = new ClickPlan(session, this, inventory, true); + for (ItemStackRequestAction action : request.getActions()) { + switch (action.getType()) { + case CRAFT_RECIPE -> { + if (craftState != CraftState.START) { + return rejectRequest(request); + } + craftState = CraftState.RECIPE_ID; + } + case CRAFT_RESULTS_DEPRECATED -> { + CraftResultsDeprecatedAction deprecatedCraftAction = (CraftResultsDeprecatedAction) action; + if (craftState != CraftState.RECIPE_ID) { + return rejectRequest(request); + } + craftState = CraftState.DEPRECATED; + + if (deprecatedCraftAction.getResultItems().length != 2) { + // Crafted item and old book + return rejectRequest(request); + } + int resultSize = deprecatedCraftAction.getResultItems()[0].getCount(); + int timesCrafted = deprecatedCraftAction.getTimesCrafted(); + if (resultSize != 1 || timesCrafted != 1) { + return rejectRequest(request); + } + } + case CONSUME -> { + // Ignore I guess + } + case CREATE -> { + // After the proper book is created this is called + } + case TAKE, PLACE -> { + TransferItemStackRequestAction transferAction = (TransferItemStackRequestAction) action; + if (craftState != CraftState.DEPRECATED) { + return rejectRequest(request); + } + + if (newBookHandled) { + // Don't let this execute for the old book and keep it in its old slot + // Bedrock wants to move it to the inventory; don't let it + continue; + } + + if (transferAction.getSource().getContainer() != ContainerSlotType.CREATED_OUTPUT) { + return rejectRequest(request); + } + if (transferAction.getCount() != 1) { + return rejectRequest(request); + } + + int sourceSlot = bedrockSlotToJava(transferAction.getSource()); + int destSlot = bedrockSlotToJava(transferAction.getDestination()); + + // Books are pretty simple in this regard - we'll yeet the written book when we execute + // the click plan, but otherwise a book isn't stackable so there aren't many options for it + if (isCursor(transferAction.getDestination())) { + plan.add(Click.LEFT, sourceSlot); + } else { + plan.add(Click.LEFT, sourceSlot); + plan.add(Click.LEFT, destSlot); + } + + newBookHandled = true; + } + default -> { + return rejectRequest(request); + } + } + } + + plan.execute(false); + return acceptRequest(request, makeContainerEntries(session, inventory, plan.getAffectedSlots())); + } + + public ItemStackResponse translateAutoCraftingRequest(GeyserSession session, Inventory inventory, ItemStackRequest request) { final int gridSize = getGridSize(); if (gridSize == -1) { diff --git a/core/src/main/java/org/geysermc/geyser/util/InventoryUtils.java b/core/src/main/java/org/geysermc/geyser/util/InventoryUtils.java index 48ade52e2..0286461a5 100644 --- a/core/src/main/java/org/geysermc/geyser/util/InventoryUtils.java +++ b/core/src/main/java/org/geysermc/geyser/util/InventoryUtils.java @@ -74,6 +74,10 @@ public class InventoryUtils { * each recipe needs a unique network ID (or else in .200 the client crashes). */ public static int LAST_RECIPE_NET_ID; + /** + * Book cloning recipe ID; stored separately as its recipe works differently from others. + */ + public static final int BOOK_CLONING_RECIPE_ID = 278; public static final ItemStack REFRESH_ITEM = new ItemStack(1, 127, new DataComponents(new HashMap<>())); From 634c0976fec4e3237b9a1efd4b39a719644a785f Mon Sep 17 00:00:00 2001 From: YHDiamond Date: Mon, 8 Jul 2024 21:00:15 -0400 Subject: [PATCH 2/5] Shorten second comparison --- .../java/org/geysermc/geyser/inventory/click/ClickPlan.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/geysermc/geyser/inventory/click/ClickPlan.java b/core/src/main/java/org/geysermc/geyser/inventory/click/ClickPlan.java index e54c63976..953b6cedc 100644 --- a/core/src/main/java/org/geysermc/geyser/inventory/click/ClickPlan.java +++ b/core/src/main/java/org/geysermc/geyser/inventory/click/ClickPlan.java @@ -25,6 +25,7 @@ package org.geysermc.geyser.inventory.click; +import org.geysermc.geyser.item.Items; import org.geysermc.mcprotocollib.protocol.data.game.item.ItemStack; import org.geysermc.mcprotocollib.protocol.data.game.inventory.ContainerActionType; import org.geysermc.mcprotocollib.protocol.data.game.inventory.ContainerType; @@ -382,7 +383,7 @@ public final class ClickPlan { for (int i = 0; i < gridSize; i++) { final int slot = i + 1; GeyserItemStack item = getItem(slot); - if (!item.isEmpty() && (!handleBookRecipe || item.getJavaId() != session.getItemMappings().getStoredItems().writtenBook().getJavaItem().javaId())) { + if (!item.isEmpty() && item.asItem() == Items.WRITTEN_BOOK) { // These changes should be broadcasted to the server sub(slot, item, crafted); } From 7b8a423367f59ca581e94f7ffe1e8b445bb101e2 Mon Sep 17 00:00:00 2001 From: YHDiamond <47502993+YHDiamond@users.noreply.github.com> Date: Mon, 8 Jul 2024 22:53:31 -0400 Subject: [PATCH 3/5] Change comment to be more descriptive Co-authored-by: Konicai <71294714+Konicai@users.noreply.github.com> --- .../geyser/translator/inventory/InventoryTranslator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/geysermc/geyser/translator/inventory/InventoryTranslator.java b/core/src/main/java/org/geysermc/geyser/translator/inventory/InventoryTranslator.java index 5db3c3b8d..aed16d022 100644 --- a/core/src/main/java/org/geysermc/geyser/translator/inventory/InventoryTranslator.java +++ b/core/src/main/java/org/geysermc/geyser/translator/inventory/InventoryTranslator.java @@ -465,7 +465,7 @@ public abstract class InventoryTranslator { if (((RecipeItemStackRequestAction) action).getRecipeNetworkId() == InventoryUtils.BOOK_CLONING_RECIPE_ID) { // Book copying needs to be handled differently - // There's a leftover item + // The original written book is leftover in the crafting grid return translateBookCopyCraftingRequest(session, inventory, request); } break; From f1b91b954606d42d2827379e5543eda28968bdd4 Mon Sep 17 00:00:00 2001 From: YHDiamond Date: Mon, 8 Jul 2024 22:56:23 -0400 Subject: [PATCH 4/5] rename variable and add comment --- .../org/geysermc/geyser/inventory/click/ClickPlan.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/geysermc/geyser/inventory/click/ClickPlan.java b/core/src/main/java/org/geysermc/geyser/inventory/click/ClickPlan.java index 953b6cedc..c43bf7b16 100644 --- a/core/src/main/java/org/geysermc/geyser/inventory/click/ClickPlan.java +++ b/core/src/main/java/org/geysermc/geyser/inventory/click/ClickPlan.java @@ -59,17 +59,20 @@ public final class ClickPlan { private final InventoryTranslator translator; private final Inventory inventory; private final int gridSize; - private final boolean handleBookRecipe; + /** + * The recipe for cloning books requires special handling, this dictates whether that handling should be performed + */ + private final boolean cloneBookRecipe; public ClickPlan(GeyserSession session, InventoryTranslator translator, Inventory inventory) { this(session, translator, inventory, false); } - public ClickPlan(GeyserSession session, InventoryTranslator translator, Inventory inventory, boolean handleBookRecipe) { + public ClickPlan(GeyserSession session, InventoryTranslator translator, Inventory inventory, boolean cloneBookRecipe) { this.session = session; this.translator = translator; this.inventory = inventory; - this.handleBookRecipe = handleBookRecipe; + this.cloneBookRecipe = cloneBookRecipe; this.simulatedItems = new Int2ObjectOpenHashMap<>(inventory.getSize()); this.changedItems = null; From 9ff6c7da5dedc634178733b6c0fac6a05430e526 Mon Sep 17 00:00:00 2001 From: YHDiamond Date: Tue, 9 Jul 2024 08:46:28 -0400 Subject: [PATCH 5/5] Change book clone ID to be dynamic, rename boolean --- .../org/geysermc/geyser/inventory/click/ClickPlan.java | 8 ++++---- .../java/org/geysermc/geyser/session/GeyserSession.java | 7 +++++++ .../geyser/translator/inventory/InventoryTranslator.java | 2 +- .../protocol/java/JavaUpdateRecipesTranslator.java | 4 +++- .../java/org/geysermc/geyser/util/InventoryUtils.java | 4 ---- 5 files changed, 15 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/org/geysermc/geyser/inventory/click/ClickPlan.java b/core/src/main/java/org/geysermc/geyser/inventory/click/ClickPlan.java index c43bf7b16..2955e69fc 100644 --- a/core/src/main/java/org/geysermc/geyser/inventory/click/ClickPlan.java +++ b/core/src/main/java/org/geysermc/geyser/inventory/click/ClickPlan.java @@ -62,17 +62,17 @@ public final class ClickPlan { /** * The recipe for cloning books requires special handling, this dictates whether that handling should be performed */ - private final boolean cloneBookRecipe; + private final boolean handleBookCloneRecipe; public ClickPlan(GeyserSession session, InventoryTranslator translator, Inventory inventory) { this(session, translator, inventory, false); } - public ClickPlan(GeyserSession session, InventoryTranslator translator, Inventory inventory, boolean cloneBookRecipe) { + public ClickPlan(GeyserSession session, InventoryTranslator translator, Inventory inventory, boolean handleBookCloneRecipe) { this.session = session; this.translator = translator; this.inventory = inventory; - this.cloneBookRecipe = cloneBookRecipe; + this.handleBookCloneRecipe = handleBookCloneRecipe; this.simulatedItems = new Int2ObjectOpenHashMap<>(inventory.getSize()); this.changedItems = null; @@ -386,7 +386,7 @@ public final class ClickPlan { for (int i = 0; i < gridSize; i++) { final int slot = i + 1; GeyserItemStack item = getItem(slot); - if (!item.isEmpty() && item.asItem() == Items.WRITTEN_BOOK) { + if (!item.isEmpty() && (!handleBookCloneRecipe || item.asItem() == Items.WRITTEN_BOOK)) { // These changes should be broadcasted to the server sub(slot, item, crafted); } diff --git a/core/src/main/java/org/geysermc/geyser/session/GeyserSession.java b/core/src/main/java/org/geysermc/geyser/session/GeyserSession.java index e228fc02f..60dafce8c 100644 --- a/core/src/main/java/org/geysermc/geyser/session/GeyserSession.java +++ b/core/src/main/java/org/geysermc/geyser/session/GeyserSession.java @@ -370,6 +370,13 @@ public class GeyserSession implements GeyserConnection, GeyserCommandSource { @Setter private Int2ObjectMap stonecutterRecipes; + /** + * Saves the ID for cloning books through the crafting table, as these need different handling + */ + @Setter + private int bookCloningID; + + /** * Whether to work around 1.13's different behavior in villager trading menus. */ diff --git a/core/src/main/java/org/geysermc/geyser/translator/inventory/InventoryTranslator.java b/core/src/main/java/org/geysermc/geyser/translator/inventory/InventoryTranslator.java index aed16d022..d0c9e3d6b 100644 --- a/core/src/main/java/org/geysermc/geyser/translator/inventory/InventoryTranslator.java +++ b/core/src/main/java/org/geysermc/geyser/translator/inventory/InventoryTranslator.java @@ -463,7 +463,7 @@ public abstract class InventoryTranslator { } craftState = CraftState.RECIPE_ID; - if (((RecipeItemStackRequestAction) action).getRecipeNetworkId() == InventoryUtils.BOOK_CLONING_RECIPE_ID) { + if (((RecipeItemStackRequestAction) action).getRecipeNetworkId() == session.getBookCloningID()) { // Book copying needs to be handled differently // The original written book is leftover in the crafting grid return translateBookCopyCraftingRequest(session, inventory, request); diff --git a/core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaUpdateRecipesTranslator.java b/core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaUpdateRecipesTranslator.java index f9b840dd9..4a3b77f0c 100644 --- a/core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaUpdateRecipesTranslator.java +++ b/core/src/main/java/org/geysermc/geyser/translator/protocol/java/JavaUpdateRecipesTranslator.java @@ -180,7 +180,9 @@ public class JavaUpdateRecipesTranslator extends PacketTranslator { - craftingDataPacket.getCraftingData().add(MultiRecipeData.of(UUID.fromString("d1ca6b84-338e-4f2f-9c6b-76cc8b4bd98d"), context.getAndIncrementNetId())); + int bookCloningID = context.getAndIncrementNetId(); + session.setBookCloningID(bookCloningID); + craftingDataPacket.getCraftingData().add(MultiRecipeData.of(UUID.fromString("d1ca6b84-338e-4f2f-9c6b-76cc8b4bd98d"), bookCloningID)); } case CRAFTING_SPECIAL_REPAIRITEM -> { craftingDataPacket.getCraftingData().add(MultiRecipeData.of(UUID.fromString("00000000-0000-0000-0000-000000000001"), context.getAndIncrementNetId())); diff --git a/core/src/main/java/org/geysermc/geyser/util/InventoryUtils.java b/core/src/main/java/org/geysermc/geyser/util/InventoryUtils.java index 0286461a5..48ade52e2 100644 --- a/core/src/main/java/org/geysermc/geyser/util/InventoryUtils.java +++ b/core/src/main/java/org/geysermc/geyser/util/InventoryUtils.java @@ -74,10 +74,6 @@ public class InventoryUtils { * each recipe needs a unique network ID (or else in .200 the client crashes). */ public static int LAST_RECIPE_NET_ID; - /** - * Book cloning recipe ID; stored separately as its recipe works differently from others. - */ - public static final int BOOK_CLONING_RECIPE_ID = 278; public static final ItemStack REFRESH_ITEM = new ItemStack(1, 127, new DataComponents(new HashMap<>()));