From 14882534c0f14be29c05c0cafb593889a2e0a550 Mon Sep 17 00:00:00 2001 From: Camotoy <20743703+Camotoy@users.noreply.github.com> Date: Sun, 30 Jan 2022 11:05:29 -0500 Subject: [PATCH] Don't fully translate item data to compare net IDs Just compare the item mappings of the two Java items. This should shave some NBT and display conversion processing time down. --- .../geysermc/geyser/inventory/Inventory.java | 6 +++- .../geyser/session/cache/LodestoneCache.java | 28 +++++++--------- .../inventory/item/BannerTranslator.java | 6 ++-- .../inventory/item/CompassTranslator.java | 33 +++++++++++++------ .../inventory/item/ItemTranslator.java | 31 ++++++++++++----- .../inventory/item/PotionTranslator.java | 2 +- .../inventory/item/TippedArrowTranslator.java | 2 +- ...tionTrackingDBClientRequestTranslator.java | 4 +-- 8 files changed, 68 insertions(+), 44 deletions(-) diff --git a/core/src/main/java/org/geysermc/geyser/inventory/Inventory.java b/core/src/main/java/org/geysermc/geyser/inventory/Inventory.java index 3b307ba8d..26dc261a0 100644 --- a/core/src/main/java/org/geysermc/geyser/inventory/Inventory.java +++ b/core/src/main/java/org/geysermc/geyser/inventory/Inventory.java @@ -35,7 +35,9 @@ import lombok.NonNull; import lombok.Setter; import lombok.ToString; import org.geysermc.geyser.GeyserImpl; +import org.geysermc.geyser.registry.type.ItemMapping; import org.geysermc.geyser.session.GeyserSession; +import org.geysermc.geyser.translator.inventory.item.ItemTranslator; import org.jetbrains.annotations.Range; import java.util.Arrays; @@ -136,7 +138,9 @@ public abstract class Inventory { protected void updateItemNetId(GeyserItemStack oldItem, GeyserItemStack newItem, GeyserSession session) { if (!newItem.isEmpty()) { - if (newItem.getItemData(session).equals(oldItem.getItemData(session), false, false, false)) { + ItemMapping oldMapping = ItemTranslator.getBedrockItemMapping(session, oldItem); + ItemMapping newMapping = ItemTranslator.getBedrockItemMapping(session, newItem); + if (oldMapping.getBedrockId() == newMapping.getBedrockId()) { newItem.setNetId(oldItem.getNetId()); } else { newItem.setNetId(session.getNextItemNetId()); diff --git a/core/src/main/java/org/geysermc/geyser/session/cache/LodestoneCache.java b/core/src/main/java/org/geysermc/geyser/session/cache/LodestoneCache.java index f0cbbb189..05c2628df 100644 --- a/core/src/main/java/org/geysermc/geyser/session/cache/LodestoneCache.java +++ b/core/src/main/java/org/geysermc/geyser/session/cache/LodestoneCache.java @@ -30,9 +30,6 @@ import com.github.steveice10.opennbt.tag.builtin.IntTag; import com.github.steveice10.opennbt.tag.builtin.StringTag; import it.unimi.dsi.fastutil.ints.Int2ObjectMap; import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap; -import lombok.AllArgsConstructor; -import lombok.EqualsAndHashCode; -import lombok.Getter; import org.geysermc.geyser.inventory.GeyserItemStack; import javax.annotation.Nullable; @@ -43,7 +40,7 @@ import java.util.WeakHashMap; * A temporary cache for lodestone information. * Bedrock requests the lodestone position information separately from the item. */ -public class LodestoneCache { +public final class LodestoneCache { /** * A list of any GeyserItemStacks that are lodestones. Used mainly to minimize Bedrock's "pop-in" effect * when a new item has been created; instead we can re-use already existing IDs @@ -121,8 +118,16 @@ public class LodestoneCache { } public @Nullable LodestonePos getPos(int id) { - // We should not need to check the activeLodestones map as Bedrock should already be aware of this ID - return this.lodestones.remove(id); + LodestonePos pos = this.lodestones.remove(id); + if (pos != null) { + return pos; + } + for (LodestonePos activePos : this.activeLodestones.values()) { + if (activePos.id == id) { + return activePos; + } + } + return null; } public void clear() { @@ -131,16 +136,7 @@ public class LodestoneCache { this.lodestones.clear(); } - @Getter - @AllArgsConstructor - @EqualsAndHashCode - public static class LodestonePos { - private final int id; - private final int x; - private final int y; - private final int z; - private final String dimension; - + public record LodestonePos(int id, int x, int y, int z, String dimension) { boolean equals(int x, int y, int z, String dimension) { return this.x == x && this.y == y && this.z == z && this.dimension.equals(dimension); } diff --git a/core/src/main/java/org/geysermc/geyser/translator/inventory/item/BannerTranslator.java b/core/src/main/java/org/geysermc/geyser/translator/inventory/item/BannerTranslator.java index 3c566e76c..a5c3235a2 100644 --- a/core/src/main/java/org/geysermc/geyser/translator/inventory/item/BannerTranslator.java +++ b/core/src/main/java/org/geysermc/geyser/translator/inventory/item/BannerTranslator.java @@ -155,7 +155,7 @@ public class BannerTranslator extends ItemTranslator { } @Override - public ItemData.Builder translateToBedrock(ItemStack itemStack, ItemMapping mapping, ItemMappings mappings) { + protected ItemData.Builder translateToBedrock(ItemStack itemStack, ItemMapping mapping, ItemMappings mappings) { if (itemStack.getNbt() == null) { return super.translateToBedrock(itemStack, mapping, mappings); } @@ -163,9 +163,7 @@ public class BannerTranslator extends ItemTranslator { ItemData.Builder builder = super.translateToBedrock(itemStack, mapping, mappings); CompoundTag blockEntityTag = itemStack.getNbt().get("BlockEntityTag"); - if (blockEntityTag != null && blockEntityTag.contains("Patterns")) { - ListTag patterns = blockEntityTag.get("Patterns"); - + if (blockEntityTag != null && blockEntityTag.get("Patterns") instanceof ListTag patterns) { NbtMapBuilder nbtBuilder = builder.build().getTag().toBuilder(); //TODO fix ugly hack if (patterns.equals(OMINOUS_BANNER_PATTERN)) { // Remove the current patterns and set the ominous banner type diff --git a/core/src/main/java/org/geysermc/geyser/translator/inventory/item/CompassTranslator.java b/core/src/main/java/org/geysermc/geyser/translator/inventory/item/CompassTranslator.java index 65f26542f..9637f1aa9 100644 --- a/core/src/main/java/org/geysermc/geyser/translator/inventory/item/CompassTranslator.java +++ b/core/src/main/java/org/geysermc/geyser/translator/inventory/item/CompassTranslator.java @@ -26,7 +26,9 @@ package org.geysermc.geyser.translator.inventory.item; import com.github.steveice10.mc.protocol.data.game.entity.metadata.ItemStack; -import com.github.steveice10.opennbt.tag.builtin.*; +import com.github.steveice10.opennbt.tag.builtin.ByteTag; +import com.github.steveice10.opennbt.tag.builtin.CompoundTag; +import com.github.steveice10.opennbt.tag.builtin.Tag; import com.nukkitx.protocol.bedrock.data.inventory.ItemData; import org.geysermc.geyser.network.MinecraftProtocol; import org.geysermc.geyser.registry.Registries; @@ -51,19 +53,30 @@ public class CompassTranslator extends ItemTranslator { } @Override - public ItemData.Builder translateToBedrock(ItemStack itemStack, ItemMapping mapping, ItemMappings mappings) { - if (itemStack.getNbt() == null) return super.translateToBedrock(itemStack, mapping, mappings); - - Tag lodestoneTag = itemStack.getNbt().get("LodestoneTracked"); - if (lodestoneTag instanceof ByteTag) { - // Get the fake lodestonecompass entry - mapping = mappings.getStoredItems().lodestoneCompass(); - // NBT will be translated in nbt/LodestoneCompassTranslator + protected ItemData.Builder translateToBedrock(ItemStack itemStack, ItemMapping mapping, ItemMappings mappings) { + if (isLodestoneCompass(itemStack.getNbt())) { + // NBT will be translated in nbt/LodestoneCompassTranslator if applicable + return super.translateToBedrock(itemStack, mappings.getStoredItems().lodestoneCompass(), mappings); } - return super.translateToBedrock(itemStack, mapping, mappings); } + @Override + protected ItemMapping getItemMapping(int javaId, CompoundTag nbt, ItemMappings mappings) { + if (isLodestoneCompass(nbt)) { + return mappings.getStoredItems().lodestoneCompass(); + } + return super.getItemMapping(javaId, nbt, mappings); + } + + private boolean isLodestoneCompass(CompoundTag nbt) { + if (nbt != null) { + Tag lodestoneTag = nbt.get("LodestoneTracked"); + return lodestoneTag instanceof ByteTag; + } + return false; + } + @Override public ItemStack translateToJava(ItemData itemData, ItemMapping mapping, ItemMappings mappings) { if (mapping.getBedrockIdentifier().equals("minecraft:lodestone_compass")) { diff --git a/core/src/main/java/org/geysermc/geyser/translator/inventory/item/ItemTranslator.java b/core/src/main/java/org/geysermc/geyser/translator/inventory/item/ItemTranslator.java index 5014969e1..b8a7b60e2 100644 --- a/core/src/main/java/org/geysermc/geyser/translator/inventory/item/ItemTranslator.java +++ b/core/src/main/java/org/geysermc/geyser/translator/inventory/item/ItemTranslator.java @@ -37,6 +37,7 @@ import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.format.NamedTextColor; import org.geysermc.geyser.GeyserImpl; +import org.geysermc.geyser.inventory.GeyserItemStack; import org.geysermc.geyser.registry.BlockRegistries; import org.geysermc.geyser.registry.type.ItemMapping; import org.geysermc.geyser.registry.type.ItemMappings; @@ -157,18 +158,13 @@ public abstract class ItemTranslator { nbt = translateDisplayProperties(session, nbt, bedrockItem); if (session.isAdvancedTooltips()) { - nbt = addAdvancedTooltips(nbt, session.getItemMappings().getMapping(stack), session.getLocale()); + nbt = addAdvancedTooltips(nbt, bedrockItem, session.getLocale()); } ItemStack itemStack = new ItemStack(stack.getId(), stack.getAmount(), nbt); - ItemData.Builder builder; - ItemTranslator itemStackTranslator = ITEM_STACK_TRANSLATORS.get(bedrockItem.getJavaId()); - if (itemStackTranslator != null) { - builder = itemStackTranslator.translateToBedrock(itemStack, bedrockItem, session.getItemMappings()); - } else { - builder = DEFAULT_TRANSLATOR.translateToBedrock(itemStack, bedrockItem, session.getItemMappings()); - } + ItemTranslator itemStackTranslator = ITEM_STACK_TRANSLATORS.getOrDefault(bedrockItem.getJavaId(), DEFAULT_TRANSLATOR); + ItemData.Builder builder = itemStackTranslator.translateToBedrock(itemStack, bedrockItem, session.getItemMappings()); if (bedrockItem.isBlock()) { builder.blockRuntimeId(bedrockItem.getBedrockBlockId()); } @@ -263,6 +259,19 @@ public abstract class ItemTranslator { return canModifyBedrock; } + /** + * Given an item stack, determine the item mapping that should be applied to Bedrock players. + */ + @Nonnull + public static ItemMapping getBedrockItemMapping(GeyserSession session, @Nonnull GeyserItemStack itemStack) { + if (itemStack.isEmpty()) { + return ItemMapping.AIR; + } + int javaId = itemStack.getJavaId(); + return ITEM_STACK_TRANSLATORS.getOrDefault(javaId, DEFAULT_TRANSLATOR) + .getItemMapping(javaId, itemStack.getNbt(), session.getItemMappings()); + } + private static final ItemTranslator DEFAULT_TRANSLATOR = new ItemTranslator() { @Override public List getAppliedItems() { @@ -270,7 +279,7 @@ public abstract class ItemTranslator { } }; - public ItemData.Builder translateToBedrock(ItemStack itemStack, ItemMapping mapping, ItemMappings mappings) { + protected ItemData.Builder translateToBedrock(ItemStack itemStack, ItemMapping mapping, ItemMappings mappings) { if (itemStack == null) { // Return, essentially, air return ItemData.builder(); @@ -295,6 +304,10 @@ public abstract class ItemTranslator { public abstract List getAppliedItems(); + protected ItemMapping getItemMapping(int javaId, CompoundTag nbt, ItemMappings mappings) { + return mappings.getMapping(javaId); + } + public NbtMap translateNbtToBedrock(CompoundTag tag) { NbtMapBuilder builder = NbtMap.builder(); if (tag.getValue() != null && !tag.getValue().isEmpty()) { diff --git a/core/src/main/java/org/geysermc/geyser/translator/inventory/item/PotionTranslator.java b/core/src/main/java/org/geysermc/geyser/translator/inventory/item/PotionTranslator.java index 272092da6..54a6deadb 100644 --- a/core/src/main/java/org/geysermc/geyser/translator/inventory/item/PotionTranslator.java +++ b/core/src/main/java/org/geysermc/geyser/translator/inventory/item/PotionTranslator.java @@ -54,7 +54,7 @@ public class PotionTranslator extends ItemTranslator { } @Override - public ItemData.Builder translateToBedrock(ItemStack itemStack, ItemMapping mapping, ItemMappings mappings) { + protected ItemData.Builder translateToBedrock(ItemStack itemStack, ItemMapping mapping, ItemMappings mappings) { if (itemStack.getNbt() == null) return super.translateToBedrock(itemStack, mapping, mappings); Tag potionTag = itemStack.getNbt().get("Potion"); if (potionTag instanceof StringTag) { diff --git a/core/src/main/java/org/geysermc/geyser/translator/inventory/item/TippedArrowTranslator.java b/core/src/main/java/org/geysermc/geyser/translator/inventory/item/TippedArrowTranslator.java index 4925d3e69..35e8baa07 100644 --- a/core/src/main/java/org/geysermc/geyser/translator/inventory/item/TippedArrowTranslator.java +++ b/core/src/main/java/org/geysermc/geyser/translator/inventory/item/TippedArrowTranslator.java @@ -59,7 +59,7 @@ public class TippedArrowTranslator extends ItemTranslator { } @Override - public ItemData.Builder translateToBedrock(ItemStack itemStack, ItemMapping mapping, ItemMappings mappings) { + protected ItemData.Builder translateToBedrock(ItemStack itemStack, ItemMapping mapping, ItemMappings mappings) { if (!mapping.getJavaIdentifier().equals("minecraft:tipped_arrow") || itemStack.getNbt() == null) { // We're only concerned about minecraft:arrow when translating Bedrock -> Java return super.translateToBedrock(itemStack, mapping, mappings); diff --git a/core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/BedrockPositionTrackingDBClientRequestTranslator.java b/core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/BedrockPositionTrackingDBClientRequestTranslator.java index a6551afbd..f3d0ff344 100644 --- a/core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/BedrockPositionTrackingDBClientRequestTranslator.java +++ b/core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/BedrockPositionTrackingDBClientRequestTranslator.java @@ -58,14 +58,14 @@ public class BedrockPositionTrackingDBClientRequestTranslator extends PacketTran // Build the NBT data for the update NbtMapBuilder builder = NbtMap.builder(); - builder.putInt("dim", DimensionUtils.javaToBedrock(pos.getDimension())); + builder.putInt("dim", DimensionUtils.javaToBedrock(pos.dimension())); builder.putString("id", "0x" + String.format("%08X", packet.getTrackingId())); builder.putByte("version", (byte) 1); // Not sure what this is for builder.putByte("status", (byte) 0); // Not sure what this is for // Build the position for the update - builder.putList("pos", NbtType.INT, pos.getX(), pos.getY(), pos.getZ()); + builder.putList("pos", NbtType.INT, pos.x(), pos.y(), pos.z()); broadcastPacket.setTag(builder.build()); session.sendUpstreamPacket(broadcastPacket);