-
-
Notifications
You must be signed in to change notification settings - Fork 674
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Book cloning fix #4784
base: master
Are you sure you want to change the base?
Book cloning fix #4784
Conversation
/** | ||
* Book cloning recipe ID; stored separately as its recipe works differently from others. | ||
*/ | ||
public static final int BOOK_CLONING_RECIPE_ID = 278; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can't be hardcoded, as it could differ between two sessions on standalone working with different recipes.
@@ -376,7 +382,7 @@ private void reduceCraftingGrid(boolean makeAll) { | |||
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())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The second comparison can now be item.asItem() == Items.WRITTEN_BOOK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean the third comparison? And !=
instead of ==
?
If the clone recipe is occurring, and then we should only sub items that are not the written book
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just committed to make the changes the way Konicai described, if that's not what Camotoy intended I can change it back
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
side note, InventoryTranslator is now over 1k lines 🥴
core/src/main/java/org/geysermc/geyser/inventory/click/ClickPlan.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/geysermc/geyser/translator/inventory/InventoryTranslator.java
Show resolved
Hide resolved
core/src/main/java/org/geysermc/geyser/translator/inventory/InventoryTranslator.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Konicai <71294714+Konicai@users.noreply.github.com>
…into fix-books-camo
Did some tests. When cloning multiple books at once the request is rejected because I had to comment out |
This is a bug fix to allow copying books to work 100% of the time and closes #3682. This is an adaption of a patch Camotoy wrote a few years ago, I changed it to work with the modern codebase and fixed an issue or two. This needs more testing to ensure that it works 100% of the time and that it doesn't break any existing crafting recipes. The code can likely be rewritten to be neater and more modular but I did not get around to that yet.