Change banner item translator into NBT-specific translator

Since we don't need to change any other item properties, this removes a builder hack that had to be implemented.

core/src/main/java/org/geysermc/geyser/translator/inventory/LoomInventoryTranslator.java

@@ -41,13 +41,13 @@ import com.nukkitx.protocol.bedrock.data.inventory.stackrequestactions.StackRequ
 import com.nukkitx.protocol.bedrock.packet.ItemStackResponsePacket;
 import it.unimi.dsi.fastutil.objects.Object2IntMap;
 import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap;
+import org.geysermc.geyser.inventory.BedrockContainerSlot;
 import org.geysermc.geyser.inventory.GeyserItemStack;
 import org.geysermc.geyser.inventory.Inventory;
-import org.geysermc.geyser.session.GeyserSession;
-import org.geysermc.geyser.inventory.BedrockContainerSlot;
 import org.geysermc.geyser.inventory.SlotType;
 import org.geysermc.geyser.inventory.updater.UIInventoryUpdater;
-import org.geysermc.geyser.translator.inventory.item.BannerTranslator;
+import org.geysermc.geyser.session.GeyserSession;
+import org.geysermc.geyser.translator.inventory.item.nbt.BannerTranslator;
 
 import java.util.Collections;
 import java.util.List;

core/src/main/java/org/geysermc/geyser/translator/inventory/item/ItemTranslator.java

@@ -47,6 +47,7 @@ import org.geysermc.geyser.translator.text.MessageTranslator;
 import org.geysermc.geyser.util.FileUtils;
 
 import javax.annotation.Nonnull;
+import java.lang.reflect.InvocationTargetException;
 import java.util.*;
 import java.util.stream.Collectors;
 
@@ -71,11 +72,11 @@ public abstract class ItemTranslator {
 
             try {
                 if (NbtItemStackTranslator.class.isAssignableFrom(clazz)) {
-                    NbtItemStackTranslator nbtItemTranslator = (NbtItemStackTranslator) clazz.newInstance();
+                    NbtItemStackTranslator nbtItemTranslator = (NbtItemStackTranslator) clazz.getDeclaredConstructor().newInstance();
                     loadedNbtItemTranslators.put(nbtItemTranslator, priority);
                     continue;
                 }
-                ItemTranslator itemStackTranslator = (ItemTranslator) clazz.newInstance();
+                ItemTranslator itemStackTranslator = (ItemTranslator) clazz.getDeclaredConstructor().newInstance();
                 List<ItemMapping> appliedItems = itemStackTranslator.getAppliedItems();
                 for (ItemMapping item : appliedItems) {
                     ItemTranslator registered = ITEM_STACK_TRANSLATORS.get(item.getJavaId());
@@ -87,7 +88,7 @@ public abstract class ItemTranslator {
                     }
                     ITEM_STACK_TRANSLATORS.put(item.getJavaId(), itemStackTranslator);
                 }
-            } catch (InstantiationException | IllegalAccessException e) {
+            } catch (InstantiationException | InvocationTargetException | IllegalAccessException | NoSuchMethodException e) {
                 GeyserImpl.getInstance().getLogger().error("Could not instantiate annotated item translator " + clazz.getCanonicalName());
             }
         }

core/src/main/java/org/geysermc/geyser/translator/inventory/item/NbtItemStackTranslator.java

@@ -29,12 +29,12 @@ import com.github.steveice10.opennbt.tag.builtin.CompoundTag;
 import org.geysermc.geyser.session.GeyserSession;
 import org.geysermc.geyser.registry.type.ItemMapping;
 
-public class NbtItemStackTranslator {
+public abstract class NbtItemStackTranslator {
 
     /**
      * Translate the item NBT to Bedrock
      * @param session the client's current session
-     * @param itemTag the item's CompoundTag
+     * @param itemTag the item's CompoundTag (cloned from Geyser's cached copy)
      * @param mapping Geyser's item mapping
      */
     public void translateToBedrock(GeyserSession session, CompoundTag itemTag, ItemMapping mapping) {

core/src/main/java/org/geysermc/geyser/translator/inventory/item/nbt/BannerTranslator.java

@@ -23,19 +23,18 @@
  * @link https://github.com/GeyserMC/Geyser
  */
 
-package org.geysermc.geyser.translator.inventory.item;
+package org.geysermc.geyser.translator.inventory.item.nbt;
 
-import com.github.steveice10.mc.protocol.data.game.entity.metadata.ItemStack;
 import com.github.steveice10.opennbt.tag.builtin.*;
 import com.nukkitx.nbt.NbtList;
 import com.nukkitx.nbt.NbtMap;
-import com.nukkitx.nbt.NbtMapBuilder;
 import com.nukkitx.nbt.NbtType;
-import com.nukkitx.protocol.bedrock.data.inventory.ItemData;
 import org.geysermc.geyser.network.MinecraftProtocol;
 import org.geysermc.geyser.registry.Registries;
 import org.geysermc.geyser.registry.type.ItemMapping;
-import org.geysermc.geyser.registry.type.ItemMappings;
+import org.geysermc.geyser.session.GeyserSession;
+import org.geysermc.geyser.translator.inventory.item.ItemRemapper;
+import org.geysermc.geyser.translator.inventory.item.NbtItemStackTranslator;
 
 import javax.annotation.Nonnull;
 import java.util.ArrayList;
@@ -45,7 +44,7 @@ import java.util.Map;
 import java.util.stream.Collectors;
 
 @ItemRemapper
-public class BannerTranslator extends ItemTranslator {
+public class BannerTranslator extends NbtItemStackTranslator {
     /**
      * Holds what a Java ominous banner pattern looks like.
      *
@@ -117,21 +116,6 @@ public class BannerTranslator extends ItemTranslator {
                 .build();
     }
 
-    /**
-     * Convert a list of patterns from Bedrock nbt to Java nbt
-     *
-     * @param patterns The patterns to convert
-     * @return The new converted patterns
-     */
-    public static ListTag convertBannerPattern(List<NbtMap> patterns) {
-        List<Tag> tagsList = new ArrayList<>();
-        for (NbtMap patternTag : patterns) {
-            tagsList.add(getJavaBannerPattern(patternTag));
-        }
-
-        return new ListTag("Patterns", tagsList);
-    }
-
     /**
      * Convert the Bedrock edition banner pattern nbt to Java edition
      *
@@ -146,62 +130,54 @@ public class BannerTranslator extends ItemTranslator {
         return new CompoundTag("", tags);
     }
 
-    @Override
+    /**
-    protected ItemData.Builder translateToBedrock(ItemStack itemStack, ItemMapping mapping, ItemMappings mappings) {
+     * Convert a list of patterns from Java nbt to Bedrock nbt, or vice versa (we just need to invert the color)
-        if (itemStack.getNbt() == null) {
+     *
-            return super.translateToBedrock(itemStack, mapping, mappings);
+     * @param patterns The patterns to convert
+     */
+    private void invertBannerColors(ListTag patterns) {
+        for (Tag patternTag : patterns.getValue()) {
+            IntTag color = ((CompoundTag) patternTag).get("Color");
+            color.setValue(15 - color.getValue());
         }
-
-        ItemData.Builder builder = super.translateToBedrock(itemStack, mapping, mappings);
-
-        CompoundTag blockEntityTag = itemStack.getNbt().get("BlockEntityTag");
-        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
-                nbtBuilder.remove("Patterns");
-                nbtBuilder.putInt("Type", 1);
-            } else {
-                nbtBuilder.put("Patterns", convertBannerPattern(patterns));
-            }
-
-            builder.tag(nbtBuilder.build());
-        }
-
-        return builder;
     }
 
     @Override
-    public ItemStack translateToJava(ItemData itemData, ItemMapping mapping, ItemMappings mappings) {
+    public void translateToBedrock(GeyserSession session, CompoundTag itemTag, ItemMapping mapping) {
-        if (itemData.getTag() == null) {
+        CompoundTag blockEntityTag = itemTag.get("BlockEntityTag");
-            return super.translateToJava(itemData, mapping, mappings);
+        if (blockEntityTag != null && blockEntityTag.get("Patterns") instanceof ListTag patterns) {
+            if (patterns.equals(OMINOUS_BANNER_PATTERN)) {
+                // Remove the current patterns and set the ominous banner type
+                itemTag.put(new IntTag("Type", 1));
+            } else {
+                invertBannerColors(patterns);
+                itemTag.put(patterns);
+            }
+            itemTag.remove("BlockEntityTag");
         }
+    }
 
-        ItemStack itemStack = super.translateToJava(itemData, mapping, mappings);
+    @Override
-
+    public void translateToJava(CompoundTag itemTag, ItemMapping mapping) {
-        NbtMap nbtTag = itemData.getTag();
+        if (itemTag.get("Type") instanceof IntTag type && type.getValue() == 1) {
-        if (nbtTag.containsKey("Type", NbtType.INT) && nbtTag.getInt("Type") == 1) {
             // Ominous banner pattern
-            itemStack.getNbt().remove("Type");
+            itemTag.remove("Type");
             CompoundTag blockEntityTag = new CompoundTag("BlockEntityTag");
             blockEntityTag.put(OMINOUS_BANNER_PATTERN);
 
-            itemStack.getNbt().put(blockEntityTag);
+            itemTag.put(blockEntityTag);
-        } else if (nbtTag.containsKey("Patterns", NbtType.LIST)) {
+        } else if (itemTag.get("Patterns") instanceof ListTag patterns) {
-            List<NbtMap> patterns = nbtTag.getList("Patterns", NbtType.COMPOUND);
-
             CompoundTag blockEntityTag = new CompoundTag("BlockEntityTag");
-            blockEntityTag.put(convertBannerPattern(patterns));
+            invertBannerColors(patterns);
+            blockEntityTag.put(patterns);
 
-            itemStack.getNbt().put(blockEntityTag);
+            itemTag.put(blockEntityTag);
-            itemStack.getNbt().remove("Patterns"); // Remove the old Bedrock patterns list
+            itemTag.remove("Patterns"); // Remove the old Bedrock patterns list
         }
-
-        return itemStack;
     }
 
     @Override
-    public List<ItemMapping> getAppliedItems() {
+    public boolean acceptItem(ItemMapping mapping) {
-        return appliedItems;
+        return appliedItems.contains(mapping);
     }
 }

core/src/main/java/org/geysermc/geyser/translator/level/block/entity/BannerBlockEntityTranslator.java

@@ -28,9 +28,10 @@ package org.geysermc.geyser.translator.level.block.entity;
 import com.github.steveice10.mc.protocol.data.game.level.block.BlockEntityType;
 import com.github.steveice10.opennbt.tag.builtin.CompoundTag;
 import com.github.steveice10.opennbt.tag.builtin.ListTag;
+import com.github.steveice10.opennbt.tag.builtin.Tag;
 import com.nukkitx.nbt.NbtMapBuilder;
-import org.geysermc.geyser.translator.inventory.item.BannerTranslator;
 import org.geysermc.geyser.level.block.BlockStateValues;
+import org.geysermc.geyser.translator.inventory.item.nbt.BannerTranslator;
 
 @BlockEntity(type = BlockEntityType.BANNER)
 public class BannerBlockEntityTranslator extends BlockEntityTranslator implements RequiresBlockState {
@@ -45,8 +46,7 @@ public class BannerBlockEntityTranslator extends BlockEntityTranslator implement
             return;
         }
 
-        if (tag.contains("Patterns")) {
+        if (tag.get("Patterns") instanceof ListTag patterns) {
-            ListTag patterns = tag.get("Patterns");
             if (patterns.equals(BannerTranslator.OMINOUS_BANNER_PATTERN)) {
                 // This is an ominous banner; don't try to translate the raw patterns (it doesn't translate correctly)
                 // and tell the Bedrock client that this is an ominous banner
@@ -56,8 +56,9 @@ public class BannerBlockEntityTranslator extends BlockEntityTranslator implement
             }
         }
 
-        if (tag.contains("CustomName")) {
+        Tag customName = tag.get("CustomName");
-            builder.put("CustomName", tag.get("CustomName").getValue());
+        if (customName != null) {
+            builder.put("CustomName", customName.getValue());
         }
     }
 }