From 852d5b050dde8e3aedf6eef80891640d916f39b8 Mon Sep 17 00:00:00 2001 From: Camotoy <20743703+Camotoy@users.noreply.github.com> Date: Fri, 16 Apr 2021 11:42:03 -0400 Subject: [PATCH] Fix item enchanting pre-1.14 (#2127) If the server spams the window property update packet, then the network ID assigned for each enchanting slot will update too quickly, essentially disabling enchanting. This commit remedies this by only updating the network ID of each slot if a property changed. --- .../inventory/GeyserEnchantOption.java | 35 +++++++++++++++++-- .../EnchantingInventoryTranslator.java | 22 +++++++----- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/connector/src/main/java/org/geysermc/connector/inventory/GeyserEnchantOption.java b/connector/src/main/java/org/geysermc/connector/inventory/GeyserEnchantOption.java index e9ad81a6a..68f65f9af 100644 --- a/connector/src/main/java/org/geysermc/connector/inventory/GeyserEnchantOption.java +++ b/connector/src/main/java/org/geysermc/connector/inventory/GeyserEnchantOption.java @@ -28,7 +28,6 @@ package org.geysermc.connector.inventory; import com.nukkitx.protocol.bedrock.data.inventory.EnchantData; import com.nukkitx.protocol.bedrock.data.inventory.EnchantOptionData; import lombok.Getter; -import lombok.Setter; import org.geysermc.connector.network.session.GeyserSession; import java.util.Arrays; @@ -38,7 +37,6 @@ import java.util.List; /** * A mutable "wrapper" around {@link EnchantOptionData} */ -@Setter public class GeyserEnchantOption { private static final List EMPTY = Collections.emptyList(); /** @@ -57,6 +55,12 @@ public class GeyserEnchantOption { @Getter private final int javaIndex; + /** + * Whether the enchantment details have actually changed. + * Used to mitigate weird packet spamming pre-1.14, causing the net ID to always update. + */ + private boolean hasChanged; + private int xpCost = 0; private int javaEnchantIndex = -1; private int bedrockEnchantIndex = -1; @@ -67,8 +71,35 @@ public class GeyserEnchantOption { } public EnchantOptionData build(GeyserSession session) { + this.hasChanged = false; return new EnchantOptionData(xpCost, javaIndex + 16, EMPTY, enchantLevel == -1 ? EMPTY : Collections.singletonList(new EnchantData(bedrockEnchantIndex, enchantLevel)), EMPTY, javaEnchantIndex == -1 ? "unknown" : ENCHANT_NAMES.get(javaEnchantIndex), enchantLevel == -1 ? 0 : session.getNextItemNetId()); } + + public boolean hasChanged() { + return hasChanged; + } + + public void setXpCost(int xpCost) { + if (this.xpCost != xpCost) { + hasChanged = true; + this.xpCost = xpCost; + } + } + + public void setEnchantIndex(int javaEnchantIndex, int bedrockEnchantIndex) { + if (this.javaEnchantIndex != javaEnchantIndex) { + hasChanged = true; + this.javaEnchantIndex = javaEnchantIndex; + this.bedrockEnchantIndex = bedrockEnchantIndex; + } + } + + public void setEnchantLevel(int enchantLevel) { + if (this.enchantLevel != enchantLevel) { + hasChanged = true; + this.enchantLevel = enchantLevel; + } + } } diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/inventory/translators/EnchantingInventoryTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/inventory/translators/EnchantingInventoryTranslator.java index 03f8bb104..6e03c7df3 100644 --- a/connector/src/main/java/org/geysermc/connector/network/translators/inventory/translators/EnchantingInventoryTranslator.java +++ b/connector/src/main/java/org/geysermc/connector/network/translators/inventory/translators/EnchantingInventoryTranslator.java @@ -34,6 +34,7 @@ import com.nukkitx.protocol.bedrock.data.inventory.stackrequestactions.StackRequ import com.nukkitx.protocol.bedrock.packet.ItemStackResponsePacket; import com.nukkitx.protocol.bedrock.packet.PlayerEnchantOptionsPacket; import org.geysermc.connector.inventory.EnchantingContainer; +import org.geysermc.connector.inventory.GeyserEnchantOption; import org.geysermc.connector.inventory.Inventory; import org.geysermc.connector.inventory.PlayerInventory; import org.geysermc.connector.network.session.GeyserSession; @@ -67,18 +68,20 @@ public class EnchantingInventoryTranslator extends AbstractBlockInventoryTransla case 6: // Enchantment type slotToUpdate = key - 4; - int index = value; - if (index != -1) { - Enchantment enchantment = Enchantment.getByJavaIdentifier("minecraft:" + JavaEnchantment.values()[index].name().toLowerCase()); + // "value" here is the Java enchant ordinal, so that does not need to be changed + // The Bedrock index might need changed, so let's look it up and see. + int bedrockIndex = value; + if (bedrockIndex != -1) { + Enchantment enchantment = Enchantment.getByJavaIdentifier("minecraft:" + JavaEnchantment.values()[bedrockIndex].name().toLowerCase()); if (enchantment != null) { // Convert the Java enchantment index to Bedrock's - index = enchantment.ordinal(); + bedrockIndex = enchantment.ordinal(); } else { - index = -1; + // There is no Bedrock enchantment equivalent + bedrockIndex = -1; } } - enchantingInventory.getGeyserEnchantOptions()[slotToUpdate].setJavaEnchantIndex(value); - enchantingInventory.getGeyserEnchantOptions()[slotToUpdate].setBedrockEnchantIndex(index); + enchantingInventory.getGeyserEnchantOptions()[slotToUpdate].setEnchantIndex(value, bedrockIndex); break; case 7: case 8: @@ -91,8 +94,9 @@ public class EnchantingInventoryTranslator extends AbstractBlockInventoryTransla default: return; } - if (shouldUpdate) { - enchantingInventory.getEnchantOptions()[slotToUpdate] = enchantingInventory.getGeyserEnchantOptions()[slotToUpdate].build(session); + GeyserEnchantOption enchantOption = enchantingInventory.getGeyserEnchantOptions()[slotToUpdate]; + if (shouldUpdate && enchantOption.hasChanged()) { + enchantingInventory.getEnchantOptions()[slotToUpdate] = enchantOption.build(session); PlayerEnchantOptionsPacket packet = new PlayerEnchantOptionsPacket(); packet.getOptions().addAll(Arrays.asList(enchantingInventory.getEnchantOptions())); session.sendUpstreamPacket(packet);