Refactor lodestones to be more memory efficient

- Most importantly, redesign lodestone caches to be per-player.
- Redesign lodestone caches with the expectation that a client will never re-request the same value.
- Re-use lodestone IDs in a WeakHashMap to be re-used but successfully garbage-collected.

connector/src/main/java/org/geysermc/connector/inventory/GeyserItemStack.java

@@ -33,6 +33,8 @@ import org.geysermc.connector.network.session.GeyserSession;
 import org.geysermc.connector.network.translators.item.ItemTranslator;
 import org.geysermc.connector.registry.type.ItemMapping;
 
+import javax.annotation.Nonnull;
+
 @Data
 public class GeyserItemStack {
     public static final GeyserItemStack EMPTY = new GeyserItemStack(0, 0, null);
@@ -42,26 +44,18 @@ public class GeyserItemStack {
     private CompoundTag nbt;
     private int netId;
 
-    public GeyserItemStack(int javaId) {
+    private GeyserItemStack(int javaId, int amount, CompoundTag nbt) {
-        this(javaId, 1);
-    }
-
-    public GeyserItemStack(int javaId, int amount) {
-        this(javaId, amount, null);
-    }
-
-    public GeyserItemStack(int javaId, int amount, CompoundTag nbt) {
         this(javaId, amount, nbt, 1);
     }
 
-    public GeyserItemStack(int javaId, int amount, CompoundTag nbt, int netId) {
+    private GeyserItemStack(int javaId, int amount, CompoundTag nbt, int netId) {
         this.javaId = javaId;
         this.amount = amount;
         this.nbt = nbt;
         this.netId = netId;
     }
 
-    public static GeyserItemStack from(ItemStack itemStack) {
+    public static @Nonnull GeyserItemStack from(ItemStack itemStack) {
         return itemStack == null ? EMPTY : new GeyserItemStack(itemStack.getId(), itemStack.getAmount(), itemStack.getNbt());
     }
 

connector/src/main/java/org/geysermc/connector/inventory/Inventory.java

@@ -26,6 +26,9 @@
 package org.geysermc.connector.inventory;
 
 import com.github.steveice10.mc.protocol.data.game.window.WindowType;
+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.math.vector.Vector3i;
 import lombok.Getter;
 import lombok.NonNull;
@@ -93,7 +96,7 @@ public class Inventory {
 
     public GeyserItemStack getItem(int slot) {
         if (slot > this.size) {
-            GeyserConnector.getInstance().getLogger().debug("Tried to get an item out of bounds! " + this.toString());
+            GeyserConnector.getInstance().getLogger().debug("Tried to get an item out of bounds! " + this);
             return GeyserItemStack.EMPTY;
         }
         return items[slot];
@@ -101,12 +104,23 @@ public class Inventory {
 
     public void setItem(int slot, @NonNull GeyserItemStack newItem, GeyserSession session) {
         if (slot > this.size) {
-            session.getConnector().getLogger().debug("Tried to set an item out of bounds! " + this.toString());
+            session.getConnector().getLogger().debug("Tried to set an item out of bounds! " + this);
             return;
         }
         GeyserItemStack oldItem = items[slot];
         updateItemNetId(oldItem, newItem, session);
         items[slot] = newItem;
+
+        // Lodestone caching
+        if (newItem.getJavaId() == session.getItemMappings().getStoredItems().compass().getJavaId()) {
+            CompoundTag nbt = newItem.getNbt();
+            if (nbt != null) {
+                Tag lodestoneTag = nbt.get("LodestoneTracked");
+                if (lodestoneTag instanceof ByteTag) {
+                    session.getLodestoneCache().cacheInventoryItem(newItem);
+                }
+            }
+        }
     }
 
     protected static void updateItemNetId(GeyserItemStack oldItem, GeyserItemStack newItem, GeyserSession session) {

connector/src/main/java/org/geysermc/connector/network/session/GeyserSession.java

@@ -146,6 +146,7 @@ public class GeyserSession implements CommandSender {
     private EntityCache entityCache;
     private EntityEffectCache effectCache;
     private final FormCache formCache;
+    private final LodestoneCache lodestoneCache;
     private final PreferencesCache preferencesCache;
     private final TagCache tagCache;
     private WorldCache worldCache;
@@ -444,6 +445,7 @@ public class GeyserSession implements CommandSender {
         this.entityCache = new EntityCache(this);
         this.effectCache = new EntityEffectCache();
         this.formCache = new FormCache(this);
+        this.lodestoneCache = new LodestoneCache();
         this.preferencesCache = new PreferencesCache(this);
         this.tagCache = new TagCache();
         this.worldCache = new WorldCache(this);

connector/src/main/java/org/geysermc/connector/network/session/cache/LodestoneCache.java

@@ -0,0 +1,148 @@
+/*
+ * Copyright (c) 2019-2021 GeyserMC. http://geysermc.org
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * @author GeyserMC
+ * @link https://github.com/GeyserMC/Geyser
+ */
+
+package org.geysermc.connector.network.session.cache;
+
+import com.github.steveice10.opennbt.tag.builtin.CompoundTag;
+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.connector.inventory.GeyserItemStack;
+
+import javax.annotation.Nullable;
+import java.util.Map;
+import java.util.WeakHashMap;
+
+/**
+ * A temporary cache for lodestone information.
+ * Bedrock requests the lodestone position information separately from the item.
+ */
+public 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
+     */
+    private final Map<GeyserItemStack, LodestonePos> activeLodestones = new WeakHashMap<>();
+    private final Int2ObjectMap<LodestonePos> lodestones = new Int2ObjectOpenHashMap<>();
+    /**
+     * An ID to increment for each lodestone
+     */
+    private int id = 1;
+
+    public void cacheInventoryItem(GeyserItemStack itemStack) {
+        CompoundTag tag = itemStack.getNbt();
+        CompoundTag lodestonePos = tag.get("LodestonePos");
+        if (lodestonePos == null) {
+            // invalid
+            return;
+        }
+
+        // Get all info needed for tracking
+        int x = ((IntTag) lodestonePos.get("X")).getValue();
+        int y = ((IntTag) lodestonePos.get("Y")).getValue();
+        int z = ((IntTag) lodestonePos.get("Z")).getValue();
+        String dim = ((StringTag) tag.get("LodestoneDimension")).getValue();
+
+        for (LodestonePos pos : this.activeLodestones.values()) {
+            if (pos.equals(x, y, z, dim)) {
+                this.activeLodestones.put(itemStack, pos);
+                return;
+            }
+        }
+
+        for (Int2ObjectMap.Entry<LodestonePos> entry : this.lodestones.int2ObjectEntrySet()) {
+            LodestonePos pos = entry.getValue();
+            if (pos.equals(x, y, z, dim)) {
+                // Use this existing position instead
+                this.activeLodestones.put(itemStack, pos);
+                return;
+            }
+        }
+
+        this.activeLodestones.put(itemStack, new LodestonePos(id++, x, y, z, dim));
+    }
+
+    public int store(CompoundTag tag) {
+        CompoundTag lodestonePos = tag.get("LodestonePos");
+        if (lodestonePos == null) {
+            // invalid
+            return 0;
+        }
+
+        // Get all info needed for tracking
+        int x = ((IntTag) lodestonePos.get("X")).getValue();
+        int y = ((IntTag) lodestonePos.get("Y")).getValue();
+        int z = ((IntTag) lodestonePos.get("Z")).getValue();
+        String dim = ((StringTag) tag.get("LodestoneDimension")).getValue();
+
+        for (LodestonePos pos : this.activeLodestones.values()) {
+            if (pos.equals(x, y, z, dim)) {
+                // No need to add this into the lodestones map as it should not be re-requested
+                return pos.id;
+            }
+        }
+
+        for (Int2ObjectMap.Entry<LodestonePos> entry : this.lodestones.int2ObjectEntrySet()) {
+            if (entry.getValue().equals(x, y, z, dim)) {
+                // Use this existing position instead
+                return entry.getIntKey();
+            }
+        }
+
+        // Start at 1 as 0 does not work
+        this.lodestones.put(id, new LodestonePos(id, x, y, z, dim));
+        return id++;
+    }
+
+    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);
+    }
+
+    public void clear() {
+        // Just in case...
+        this.activeLodestones.clear();
+        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;
+
+        boolean equals(int x, int y, int z, String dimension) {
+            return this.x == x && this.y == y && this.z == z && this.dimension.equals(dimension);
+        }
+    }
+}

connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockPositionTrackingDBClientRequestTranslator.java

@@ -30,24 +30,24 @@ import com.nukkitx.nbt.NbtMapBuilder;
 import com.nukkitx.nbt.NbtType;
 import com.nukkitx.protocol.bedrock.packet.PositionTrackingDBClientRequestPacket;
 import com.nukkitx.protocol.bedrock.packet.PositionTrackingDBServerBroadcastPacket;
-import it.unimi.dsi.fastutil.ints.IntArrayList;
-import it.unimi.dsi.fastutil.ints.IntList;
 import org.geysermc.connector.network.session.GeyserSession;
+import org.geysermc.connector.network.session.cache.LodestoneCache;
 import org.geysermc.connector.network.translators.PacketTranslator;
 import org.geysermc.connector.network.translators.Translator;
 import org.geysermc.connector.utils.DimensionUtils;
-import org.geysermc.connector.utils.LoadstoneTracker;
 
 @Translator(packet = PositionTrackingDBClientRequestPacket.class)
 public class BedrockPositionTrackingDBClientRequestTranslator extends PacketTranslator<PositionTrackingDBClientRequestPacket> {
 
     @Override
     public void translate(GeyserSession session, PositionTrackingDBClientRequestPacket packet) {
+        System.out.println(packet.toString());
         PositionTrackingDBServerBroadcastPacket broadcastPacket = new PositionTrackingDBServerBroadcastPacket();
         broadcastPacket.setTrackingId(packet.getTrackingId());
 
-        // Fetch the stored Loadstone
+        // Fetch the stored lodestone
-        LoadstoneTracker.LoadstonePos pos = LoadstoneTracker.getPos(packet.getTrackingId());
+        LodestoneCache.LodestonePos pos = session.getLodestoneCache().getPos(packet.getTrackingId());
+        System.out.println(pos);
 
         // If we don't have data for that ID tell the client its not found
         if (pos == null) {
@@ -58,22 +58,20 @@ public class BedrockPositionTrackingDBClientRequestTranslator extends PacketTran
 
         broadcastPacket.setAction(PositionTrackingDBServerBroadcastPacket.Action.UPDATE);
 
-        // Build the nbt data for the update
+        // Build the NBT data for the update
         NbtMapBuilder builder = NbtMap.builder();
         builder.putInt("dim", DimensionUtils.javaToBedrock(pos.getDimension()));
-        builder.putString("id", String.format("%08X", packet.getTrackingId()));
+        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
-        IntList posList = new IntArrayList();
+        builder.putList("pos", NbtType.INT, pos.getX(), pos.getY(), pos.getZ());
-        posList.add(pos.getX());
-        posList.add(pos.getY());
-        posList.add(pos.getZ());
-        builder.putList("pos", NbtType.INT, posList);
         broadcastPacket.setTag(builder.build());
 
+        System.out.println(broadcastPacket);
+
         session.sendUpstreamPacket(broadcastPacket);
     }
 }

connector/src/main/java/org/geysermc/connector/network/translators/item/translators/CompassTranslator.java

@@ -34,7 +34,6 @@ import org.geysermc.connector.network.translators.item.ItemTranslator;
 import org.geysermc.connector.registry.Registries;
 import org.geysermc.connector.registry.type.ItemMapping;
 import org.geysermc.connector.registry.type.ItemMappings;
-import org.geysermc.connector.utils.LoadstoneTracker;
 
 import java.util.List;
 import java.util.stream.Collectors;
@@ -61,25 +60,7 @@ public class CompassTranslator extends ItemTranslator {
         if (lodestoneTag instanceof ByteTag) {
             // Get the fake lodestonecompass entry
             mapping = mappings.getStoredItems().lodestoneCompass();
-
+            // NBT will be translated in nbt/LodestoneCompassTranslator
-            // Get the loadstone pos
-            CompoundTag loadstonePos = itemStack.getNbt().get("LodestonePos");
-            if (loadstonePos != null) {
-                // Get all info needed for tracking
-                int x = ((IntTag) loadstonePos.get("X")).getValue();
-                int y = ((IntTag) loadstonePos.get("Y")).getValue();
-                int z = ((IntTag) loadstonePos.get("Z")).getValue();
-                String dim = ((StringTag) itemStack.getNbt().get("LodestoneDimension")).getValue();
-
-                // Store the info
-                int trackID = LoadstoneTracker.store(x, y, z, dim);
-
-                // Set the bedrock tracking id
-                itemStack.getNbt().put(new IntTag("trackingHandle", trackID));
-            } else {
-                // The loadstone was removed just set the tracking id to 0
-                itemStack.getNbt().put(new IntTag("trackingHandle", 0));
-            }
         }
 
         return super.translateToBedrock(itemStack, mapping, mappings);
@@ -87,36 +68,12 @@ public class CompassTranslator extends ItemTranslator {
 
     @Override
     public ItemStack translateToJava(ItemData itemData, ItemMapping mapping, ItemMappings mappings) {
-        boolean isLoadstone = false;
         if (mapping.getBedrockIdentifier().equals("minecraft:lodestone_compass")) {
             // Revert the entry back to the compass
             mapping = mappings.getStoredItems().compass();
-
-            isLoadstone = true;
         }
 
-        ItemStack itemStack = super.translateToJava(itemData, mapping, mappings);
+        return super.translateToJava(itemData, mapping, mappings);
-
-        if (isLoadstone) {
-            // Get the tracking id
-            int trackingID = ((IntTag) itemStack.getNbt().get("trackingHandle")).getValue();
-
-            // Fetch the tracking info from the id
-            LoadstoneTracker.LoadstonePos pos = LoadstoneTracker.getPos(trackingID);
-            if (pos != null) {
-                // Build the new NBT data for the fetched tracking info
-                itemStack.getNbt().put(new StringTag("LodestoneDimension", pos.getDimension()));
-
-                CompoundTag posTag = new CompoundTag("LodestonePos");
-                posTag.put(new IntTag("X", pos.getX()));
-                posTag.put(new IntTag("Y", pos.getY()));
-                posTag.put(new IntTag("Z", pos.getZ()));
-
-                itemStack.getNbt().put(posTag);
-            }
-        }
-
-        return itemStack;
     }
 
     @Override

connector/src/main/java/org/geysermc/connector/network/translators/item/translators/nbt/LodestoneCompassTranslator.java

@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) 2019-2021 GeyserMC. http://geysermc.org
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * @author GeyserMC
+ * @link https://github.com/GeyserMC/Geyser
+ */
+
+package org.geysermc.connector.network.translators.item.translators.nbt;
+
+import com.github.steveice10.opennbt.tag.builtin.*;
+import org.geysermc.connector.network.session.GeyserSession;
+import org.geysermc.connector.network.translators.ItemRemapper;
+import org.geysermc.connector.network.translators.item.NbtItemStackTranslator;
+import org.geysermc.connector.registry.type.ItemMapping;
+
+@ItemRemapper
+public class LodestoneCompassTranslator extends NbtItemStackTranslator {
+
+    @Override
+    public void translateToBedrock(GeyserSession session, CompoundTag itemTag, ItemMapping mapping) {
+        Tag lodestoneTag = itemTag.get("LodestoneTracked");
+        if (lodestoneTag instanceof ByteTag) {
+            int trackId = session.getLodestoneCache().store(itemTag);
+            // Set the bedrock tracking id - will return 0 if invalid
+            itemTag.put(new IntTag("trackingHandle", trackId));
+        }
+    }
+
+    // NBT does not need to be translated from Bedrock Edition to Java Edition.
+    // translateToJava is called in three places: extra recipe loading, creative menu, and stonecutters
+    // Lodestone compasses cannot be touched in any of those places.
+
+    @Override
+    public boolean acceptItem(ItemMapping mapping) {
+        return mapping.getJavaIdentifier().equals("minecraft:compass");
+    }
+}

connector/src/main/java/org/geysermc/connector/utils/DimensionUtils.java

@@ -64,6 +64,7 @@ public class DimensionUtils {
         session.getEntityCache().removeAllEntities();
         session.getItemFrameCache().clear();
         session.getLecternCache().clear();
+        session.getLodestoneCache().clear();
         session.getSkullCache().clear();
 
         Vector3f pos = Vector3f.from(0, Short.MAX_VALUE, 0);

connector/src/main/java/org/geysermc/connector/utils/LoadstoneTracker.java

@@ -1,83 +0,0 @@
-/*
- * Copyright (c) 2019-2021 GeyserMC. http://geysermc.org
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- *
- * @author GeyserMC
- * @link https://github.com/GeyserMC/Geyser
- */
-
-package org.geysermc.connector.utils;
-
-import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
-import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
-import lombok.AllArgsConstructor;
-import lombok.EqualsAndHashCode;
-import lombok.Getter;
-
-public class LoadstoneTracker {
-
-    private static final Int2ObjectMap<LoadstonePos> LOADSTONES = new Int2ObjectOpenHashMap<>();
-
-    /**
-     * Store the given coordinates and dimensions
-     *
-     * @param x The X position of the Loadstone
-     * @param y The Y position of the Loadstone
-     * @param z The Z position of the Loadstone
-     * @param dim The dimension containing of the Loadstone
-     * @return The id in the Map
-     */
-    public static int store(int x, int y, int z, String dim) {
-        LoadstonePos pos = new LoadstonePos(x, y, z, dim);
-
-        if (!LOADSTONES.containsValue(pos)) {
-            // Start at 1 as 0 seems to not work
-            LOADSTONES.put(LOADSTONES.size() + 1, pos);
-        }
-
-        for (Int2ObjectMap.Entry<LoadstonePos> loadstone : LOADSTONES.int2ObjectEntrySet()) {
-            if (loadstone.getValue().equals(pos)) {
-                return loadstone.getIntKey();
-            }
-        }
-
-        return 0;
-    }
-
-    /**
-     * Get the loadstone data
-     *
-     * @param id The ID to get the data for
-     * @return The stored data
-     */
-    public static LoadstonePos getPos(int id) {
-        return LOADSTONES.get(id);
-    }
-
-    @Getter
-    @AllArgsConstructor
-    @EqualsAndHashCode
-    public static class LoadstonePos {
-        int x;
-        int y;
-        int z;
-        String dimension;
-    }
-}

connector/src/main/resources/mappings

@@ -1 +1 @@
-Subproject commit 2efdb453e4e76992d63824b5c8b551bebec67b71
+Subproject commit 3a2f75a2760923ec1aa7aaf70a2f00d566ef069e