Use server data to pick block on banners

bootstrap/fabric/src/main/java/org/geysermc/geyser/platform/fabric/world/GeyserFabricWorldManager.java

@@ -31,12 +31,13 @@ import com.nukkitx.nbt.NbtMapBuilder;
 import com.nukkitx.nbt.NbtType;
 import me.lucko.fabric.api.permissions.v0.Permissions;
 import net.minecraft.core.BlockPos;
-import net.minecraft.nbt.ListTag;
+import net.minecraft.nbt.*;
 import net.minecraft.server.MinecraftServer;
 import net.minecraft.server.level.ServerPlayer;
 import net.minecraft.world.item.ItemStack;
 import net.minecraft.world.item.WritableBookItem;
 import net.minecraft.world.item.WrittenBookItem;
+import net.minecraft.world.level.block.entity.BannerBlockEntity;
 import net.minecraft.world.level.block.entity.BlockEntity;
 import net.minecraft.world.level.block.entity.LecternBlockEntity;
 import org.geysermc.geyser.level.GeyserWorldManager;
@@ -44,8 +45,10 @@ import org.geysermc.geyser.session.GeyserSession;
 import org.geysermc.geyser.translator.inventory.LecternInventoryTranslator;
 import org.geysermc.geyser.util.BlockEntityUtils;
 
+import javax.annotation.Nonnull;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.TimeUnit;
 
 public class GeyserFabricWorldManager extends GeyserWorldManager {
@@ -127,7 +130,127 @@ public class GeyserFabricWorldManager extends GeyserWorldManager {
         return Permissions.check(player, permission);
     }
 
+    @Nonnull
+    @Override
+    public CompletableFuture<com.github.steveice10.opennbt.tag.builtin.CompoundTag> getPickItemNbt(GeyserSession session, int x, int y, int z, boolean addNbtData) {
+        CompletableFuture<com.github.steveice10.opennbt.tag.builtin.CompoundTag> future = new CompletableFuture<>();
+        server.execute(() -> {
+            ServerPlayer player = getPlayer(session);
+            if (player == null) {
+                future.complete(null);
+                return;
+            }
+
+            BlockPos pos = new BlockPos(x, y, z);
+            // Don't create a new block entity if invalid
+            BlockEntity blockEntity = player.level.getChunkAt(pos).getBlockEntity(pos);
+            if (blockEntity instanceof BannerBlockEntity banner) {
+                // Potentially exposes other NBT data? But we need to get the NBT data for the banner patterns *and*
+                // the banner might have a custom name, both of which a Java client knows and caches
+                ItemStack itemStack = banner.getItem();
+                var tag = OpenNbtTagVisitor.convert("", itemStack.getOrCreateTag());
+
+                future.complete(tag);
+                return;
+            }
+            future.complete(null);
+        });
+        return future;
+    }
+
     private ServerPlayer getPlayer(GeyserSession session) {
         return server.getPlayerList().getPlayer(session.getPlayerEntity().getUuid());
     }
+
+    // Future considerations: option to clone; would affect arrays
+    private static class OpenNbtTagVisitor implements TagVisitor {
+        private String currentKey;
+        private final com.github.steveice10.opennbt.tag.builtin.CompoundTag root;
+        private com.github.steveice10.opennbt.tag.builtin.Tag currentTag;
+
+        OpenNbtTagVisitor(String key) {
+            root = new com.github.steveice10.opennbt.tag.builtin.CompoundTag(key);
+        }
+
+        @Override
+        public void visitString(StringTag stringTag) {
+            currentTag = new com.github.steveice10.opennbt.tag.builtin.StringTag(currentKey, stringTag.getAsString());
+        }
+
+        @Override
+        public void visitByte(ByteTag byteTag) {
+            currentTag = new com.github.steveice10.opennbt.tag.builtin.ByteTag(currentKey, byteTag.getAsByte());
+        }
+
+        @Override
+        public void visitShort(ShortTag shortTag) {
+            currentTag = new com.github.steveice10.opennbt.tag.builtin.ShortTag(currentKey, shortTag.getAsShort());
+        }
+
+        @Override
+        public void visitInt(IntTag intTag) {
+            currentTag = new com.github.steveice10.opennbt.tag.builtin.IntTag(currentKey, intTag.getAsInt());
+        }
+
+        @Override
+        public void visitLong(LongTag longTag) {
+            currentTag = new com.github.steveice10.opennbt.tag.builtin.LongTag(currentKey, longTag.getAsLong());
+        }
+
+        @Override
+        public void visitFloat(FloatTag floatTag) {
+            currentTag = new com.github.steveice10.opennbt.tag.builtin.FloatTag(currentKey, floatTag.getAsFloat());
+        }
+
+        @Override
+        public void visitDouble(DoubleTag doubleTag) {
+            currentTag = new com.github.steveice10.opennbt.tag.builtin.DoubleTag(currentKey, doubleTag.getAsDouble());
+        }
+
+        @Override
+        public void visitByteArray(ByteArrayTag byteArrayTag) {
+            currentTag = new com.github.steveice10.opennbt.tag.builtin.ByteArrayTag(currentKey, byteArrayTag.getAsByteArray());
+        }
+
+        @Override
+        public void visitIntArray(IntArrayTag intArrayTag) {
+            currentTag = new com.github.steveice10.opennbt.tag.builtin.IntArrayTag(currentKey, intArrayTag.getAsIntArray());
+        }
+
+        @Override
+        public void visitLongArray(LongArrayTag longArrayTag) {
+            currentTag = new com.github.steveice10.opennbt.tag.builtin.LongArrayTag(currentKey, longArrayTag.getAsLongArray());
+        }
+
+        @Override
+        public void visitList(ListTag listTag) {
+            var newList = new com.github.steveice10.opennbt.tag.builtin.ListTag(currentKey);
+            for (Tag tag : listTag) {
+                currentKey = "";
+                tag.accept(this);
+                newList.add(currentTag);
+            }
+            currentTag = newList;
+        }
+
+        @Override
+        public void visitCompound(CompoundTag compoundTag) {
+            currentTag = convert(currentKey, compoundTag);
+        }
+
+        private static com.github.steveice10.opennbt.tag.builtin.CompoundTag convert(String name, CompoundTag compoundTag) {
+            OpenNbtTagVisitor visitor = new OpenNbtTagVisitor(name);
+            for (String key : compoundTag.getAllKeys()) {
+                visitor.currentKey = key;
+                Tag tag = compoundTag.get(key);
+                tag.accept(visitor);
+                visitor.root.put(visitor.currentTag);
+            }
+            return visitor.root;
+        }
+
+        @Override
+        public void visitEnd(EndTag endTag) {
+        }
+    }
 }

bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/world/manager/GeyserSpigotWorldManager.java

@@ -25,14 +25,17 @@
 
 package org.geysermc.geyser.platform.spigot.world.manager;
 
+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.math.vector.Vector3i;
 import com.nukkitx.nbt.NbtMap;
 import com.nukkitx.nbt.NbtMapBuilder;
 import com.nukkitx.nbt.NbtType;
 import org.bukkit.Bukkit;
 import org.bukkit.World;
-import org.bukkit.block.Block;
+import org.bukkit.block.*;
-import org.bukkit.block.Lectern;
+import org.bukkit.block.banner.Pattern;
 import org.bukkit.entity.Player;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.inventory.meta.BookMeta;
@@ -43,10 +46,14 @@ import org.geysermc.geyser.level.block.BlockStateValues;
 import org.geysermc.geyser.registry.BlockRegistries;
 import org.geysermc.geyser.session.GeyserSession;
 import org.geysermc.geyser.translator.inventory.LecternInventoryTranslator;
+import org.geysermc.geyser.translator.inventory.item.nbt.BannerTranslator;
 import org.geysermc.geyser.util.BlockEntityUtils;
+import org.jetbrains.annotations.Nullable;
 
+import javax.annotation.Nonnull;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.CompletableFuture;
 
 /**
  * The base world manager to use when there is no supported NMS revision
@@ -173,6 +180,46 @@ public class GeyserSpigotWorldManager extends WorldManager {
         return Bukkit.getPlayer(session.getPlayerEntity().getUsername()).hasPermission(permission);
     }
 
+    @Nonnull
+    @Override
+    public CompletableFuture<@Nullable CompoundTag> getPickItemNbt(GeyserSession session, int x, int y, int z, boolean addNbtData) {
+        CompletableFuture<@Nullable CompoundTag> future = new CompletableFuture<>();
+        // Paper 1.19.3 complains about async access otherwise.
+        // java.lang.IllegalStateException: Tile is null, asynchronous access?
+        Bukkit.getScheduler().runTask(this.plugin, () -> {
+            Player bukkitPlayer;
+            if ((bukkitPlayer = Bukkit.getPlayer(session.getPlayerEntity().getUuid())) == null) {
+                future.complete(null);
+                return;
+            }
+
+            Block block = bukkitPlayer.getWorld().getBlockAt(x, y, z);
+            BlockState state = block.getState();
+            if (state instanceof Banner banner) {
+                ListTag list = new ListTag("Patterns");
+                for (int i = 0; i < banner.numberOfPatterns(); i++) {
+                    Pattern pattern = banner.getPattern(i);
+                    list.add(BannerTranslator.getJavaPatternTag(pattern.getPattern().getIdentifier(), pattern.getColor().ordinal()));
+                }
+
+                CompoundTag root = addToBlockEntityTag(list);
+
+                future.complete(root);
+                return;
+            }
+            future.complete(null);
+        });
+        return future;
+    }
+
+    private CompoundTag addToBlockEntityTag(Tag tag) {
+        CompoundTag compoundTag = new CompoundTag("");
+        CompoundTag blockEntityTag = new CompoundTag("BlockEntityTag");
+        blockEntityTag.put(tag);
+        compoundTag.put(blockEntityTag);
+        return compoundTag;
+    }
+
     /**
      * This should be set to true if we are post-1.13 but before the latest version, and we should convert the old block state id
      * to the current one.

core/src/main/java/org/geysermc/geyser/level/WorldManager.java

@@ -27,12 +27,15 @@ package org.geysermc.geyser.level;
 
 import com.github.steveice10.mc.protocol.data.game.entity.player.GameMode;
 import com.github.steveice10.mc.protocol.data.game.setting.Difficulty;
+import com.github.steveice10.opennbt.tag.builtin.CompoundTag;
 import com.nukkitx.math.vector.Vector3i;
 import com.nukkitx.nbt.NbtMap;
 import org.geysermc.geyser.session.GeyserSession;
+import org.jetbrains.annotations.Nullable;
 
-import javax.annotation.Nullable;
+import javax.annotation.Nonnull;
 import java.util.Locale;
+import java.util.concurrent.CompletableFuture;
 
 /**
  * Class that manages or retrieves various information
@@ -166,4 +169,14 @@ public abstract class WorldManager {
     public String[] getBiomeIdentifiers(boolean withTags) {
         return null;
     }
+
+    /**
+     * Used for pick block, so we don't need to cache more data than necessary.
+     *
+     * @return expected NBT for this item.
+     */
+    @Nonnull
+    public CompletableFuture<@Nullable CompoundTag> getPickItemNbt(GeyserSession session, int x, int y, int z, boolean addNbtData) {
+        return CompletableFuture.completedFuture(null);
+    }
 }

core/src/main/java/org/geysermc/geyser/session/GeyserSession.java

@@ -1073,6 +1073,17 @@ public class GeyserSession implements GeyserConnection, GeyserCommandSource {
         closed = true;
     }
 
+    /**
+     * Moves task to the session event loop if already not in it. Otherwise, the task is automatically ran.
+     */
+    public void ensureInEventLoop(Runnable runnable) {
+        if (eventLoop.inEventLoop()) {
+            runnable.run();
+            return;
+        }
+        executeInEventLoop(runnable);
+    }
+
     /**
      * Executes a task and prints a stack trace if an error occurs.
      */

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

@@ -56,17 +56,17 @@ public class BannerTranslator extends NbtItemStackTranslator {
     static {
         OMINOUS_BANNER_PATTERN = new ListTag("Patterns");
         // Construct what an ominous banner is supposed to look like
-        OMINOUS_BANNER_PATTERN.add(getPatternTag("mr", 9));
+        OMINOUS_BANNER_PATTERN.add(getJavaPatternTag("mr", 9));
-        OMINOUS_BANNER_PATTERN.add(getPatternTag("bs", 8));
+        OMINOUS_BANNER_PATTERN.add(getJavaPatternTag("bs", 8));
-        OMINOUS_BANNER_PATTERN.add(getPatternTag("cs", 7));
+        OMINOUS_BANNER_PATTERN.add(getJavaPatternTag("cs", 7));
-        OMINOUS_BANNER_PATTERN.add(getPatternTag("bo", 8));
+        OMINOUS_BANNER_PATTERN.add(getJavaPatternTag("bo", 8));
-        OMINOUS_BANNER_PATTERN.add(getPatternTag("ms", 15));
+        OMINOUS_BANNER_PATTERN.add(getJavaPatternTag("ms", 15));
-        OMINOUS_BANNER_PATTERN.add(getPatternTag("hh", 8));
+        OMINOUS_BANNER_PATTERN.add(getJavaPatternTag("hh", 8));
-        OMINOUS_BANNER_PATTERN.add(getPatternTag("mc", 8));
+        OMINOUS_BANNER_PATTERN.add(getJavaPatternTag("mc", 8));
-        OMINOUS_BANNER_PATTERN.add(getPatternTag("bo", 15));
+        OMINOUS_BANNER_PATTERN.add(getJavaPatternTag("bo", 15));
     }
 
-    private static CompoundTag getPatternTag(String pattern, int color) {
+    public static CompoundTag getJavaPatternTag(String pattern, int color) {
         StringTag patternType = new StringTag("Pattern", pattern);
         IntTag colorTag = new IntTag("Color", color);
         CompoundTag tag = new CompoundTag("");
@@ -117,11 +117,7 @@ public class BannerTranslator extends NbtItemStackTranslator {
      * @return The Java edition format pattern nbt
      */
     public static CompoundTag getJavaBannerPattern(NbtMap pattern) {
-        Map<String, Tag> tags = new HashMap<>();
+        return BannerTranslator.getJavaPatternTag(pattern.getString("Pattern"), 15 - pattern.getInt("Color"));
-        tags.put("Color", new IntTag("Color", 15 - pattern.getInt("Color")));
-        tags.put("Pattern", new StringTag("Pattern", pattern.getString("Pattern")));
-
-        return new CompoundTag("", tags);
     }
 
     /**

core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/BedrockBlockPickRequestTranslator.java

@@ -25,12 +25,18 @@
 
 package org.geysermc.geyser.translator.protocol.bedrock;
 
+import com.github.steveice10.mc.protocol.data.game.entity.metadata.ItemStack;
+import com.github.steveice10.opennbt.tag.builtin.CompoundTag;
+import com.github.steveice10.opennbt.tag.builtin.ListTag;
+import com.github.steveice10.opennbt.tag.builtin.StringTag;
 import com.nukkitx.math.vector.Vector3i;
 import com.nukkitx.protocol.bedrock.packet.BlockPickRequestPacket;
 import org.geysermc.geyser.entity.EntityDefinitions;
 import org.geysermc.geyser.entity.type.ItemFrameEntity;
 import org.geysermc.geyser.level.block.BlockStateValues;
 import org.geysermc.geyser.registry.BlockRegistries;
+import org.geysermc.geyser.registry.type.BlockMapping;
+import org.geysermc.geyser.registry.type.ItemMapping;
 import org.geysermc.geyser.session.GeyserSession;
 import org.geysermc.geyser.translator.protocol.PacketTranslator;
 import org.geysermc.geyser.translator.protocol.Translator;
@@ -61,6 +67,41 @@ public class BedrockBlockPickRequestTranslator extends PacketTranslator<BlockPic
             return;
         }
 
-        InventoryUtils.findOrCreateItem(session, BlockRegistries.JAVA_BLOCKS.get(blockToPick).getPickItem());
+        BlockMapping blockMapping = BlockRegistries.JAVA_BLOCKS.getOrDefault(blockToPick, BlockMapping.AIR);
+        boolean addNbtData = packet.isAddUserData() && blockMapping.isBlockEntity(); // Holding down CTRL
+        if (BlockStateValues.getBannerColor(blockToPick) != -1 || addNbtData) {
+            session.getGeyser().getWorldManager().getPickItemNbt(session, vector.getX(), vector.getY(), vector.getZ(), addNbtData)
+                    .whenComplete((tag, ex) -> {
+                        if (tag == null) {
+                            pickItem(session, blockMapping);
+                            return;
+                        }
+
+                        session.ensureInEventLoop(() -> {
+                            if (addNbtData) {
+                                ListTag lore = new ListTag("Lore");
+                                lore.add(new StringTag("", "\"(+NBT)\""));
+                                CompoundTag display = tag.get("display");
+                                if (display == null) {
+                                    display = new CompoundTag("display");
+                                    tag.put(display);
+                                }
+                                display.put(lore);
+                            }
+                            // I don't really like this... I'd rather get an ID from the block mapping I think
+                            ItemMapping mapping = session.getItemMappings().getMapping(blockMapping.getPickItem());
+
+                            ItemStack itemStack = new ItemStack(mapping.getJavaId(), 1, tag);
+                            InventoryUtils.findOrCreateItem(session, itemStack);
+                        });
+                    });
+            return;
+        }
+
+        pickItem(session, blockMapping);
+    }
+
+    private void pickItem(GeyserSession session, BlockMapping blockToPick) {
+        InventoryUtils.findOrCreateItem(session, blockToPick.getPickItem());
     }
 }

core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/BedrockEntityPickRequestTranslator.java

@@ -25,7 +25,6 @@
 
 package org.geysermc.geyser.translator.protocol.bedrock;
 
-import com.github.steveice10.mc.protocol.data.game.entity.player.GameMode;
 import com.nukkitx.protocol.bedrock.packet.EntityPickRequestPacket;
 import org.geysermc.geyser.entity.type.BoatEntity;
 import org.geysermc.geyser.entity.type.Entity;
@@ -45,7 +44,10 @@ public class BedrockEntityPickRequestTranslator extends PacketTranslator<EntityP
 
     @Override
     public void translate(GeyserSession session, EntityPickRequestPacket packet) {
-        if (session.getGameMode() != GameMode.CREATIVE) return; // Apparently Java behavior
+        if (!session.isInstabuild()) {
+            // As of Java Edition 1.19.3
+            return;
+        }
         Entity entity = session.getEntityCache().getEntityByGeyserId(packet.getRuntimeEntityId());
         if (entity == null) return;