Fix maps not loading in sometimes (#758)

* Fix maps not loading in sometimes
Adds a default map ID so the map item isnt invisible on bedrock.
Respond to the MapInfoRequestPacket so the image loads on first join.

* Remove debug log

* Add comments

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

@@ -47,6 +47,8 @@ import com.nukkitx.protocol.bedrock.BedrockPacket;
 import com.nukkitx.protocol.bedrock.BedrockServerSession;
 import com.nukkitx.protocol.bedrock.data.*;
 import com.nukkitx.protocol.bedrock.packet.*;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap;
 import it.unimi.dsi.fastutil.objects.Object2LongMap;
 import it.unimi.dsi.fastutil.objects.Object2LongOpenHashMap;
 import lombok.Getter;
@@ -57,7 +59,6 @@ import org.geysermc.connector.GeyserConnector;
 import org.geysermc.connector.command.CommandSender;
 import org.geysermc.connector.entity.Entity;
 import org.geysermc.connector.entity.PlayerEntity;
-import org.geysermc.connector.entity.attribute.AttributeType;
 import org.geysermc.connector.inventory.PlayerInventory;
 import org.geysermc.connector.network.remote.RemoteServer;
 import org.geysermc.connector.network.session.auth.AuthData;
@@ -108,6 +109,9 @@ public class GeyserSession implements CommandSender {
     @Setter
     private TeleportCache teleportCache;
 
+    @Getter
+    private final Long2ObjectMap<ClientboundMapItemDataPacket> storedMaps = new Long2ObjectOpenHashMap<>();
+
     /**
      * A map of Vector3i positions to Java entity IDs.
      * Used for translating Bedrock block actions to Java entity actions.

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

@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2019-2020 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.bedrock;
+
+import com.nukkitx.protocol.bedrock.packet.MapInfoRequestPacket;
+import org.geysermc.connector.GeyserConnector;
+import org.geysermc.connector.network.session.GeyserSession;
+import org.geysermc.connector.network.translators.PacketTranslator;
+import org.geysermc.connector.network.translators.Translator;
+
+import java.util.concurrent.TimeUnit;
+
+@Translator(packet = MapInfoRequestPacket.class)
+public class BedrockMapInfoRequestTranslator extends PacketTranslator<MapInfoRequestPacket> {
+
+    @Override
+    public void translate(MapInfoRequestPacket packet, GeyserSession session) {
+        long mapID = packet.getUniqueMapId();
+
+        if (session.getStoredMaps().containsKey(mapID)) {
+            // Delay the packet 100ms to prevent the client from ignoring the packet
+            GeyserConnector.getInstance().getGeneralThreadPool().schedule(() -> {
+                session.sendUpstreamPacket(session.getStoredMaps().get(mapID));
+                session.getStoredMaps().remove(mapID);
+            }, 100, TimeUnit.MILLISECONDS);
+        }
+    }
+}

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

@@ -28,8 +28,8 @@ package org.geysermc.connector.network.translators.item;
 
 import com.github.steveice10.mc.protocol.data.game.entity.metadata.ItemStack;
 import com.github.steveice10.mc.protocol.data.message.Message;
-import com.nukkitx.nbt.CompoundTagBuilder;
 import com.github.steveice10.opennbt.tag.builtin.*;
+import com.nukkitx.nbt.CompoundTagBuilder;
 import com.nukkitx.nbt.tag.CompoundTag;
 import com.nukkitx.nbt.tag.Tag;
 import com.nukkitx.protocol.bedrock.data.ItemData;
@@ -41,11 +41,7 @@ import org.geysermc.connector.network.translators.ItemRemapper;
 import org.geysermc.connector.utils.MessageUtils;
 import org.reflections.Reflections;
 
-import java.util.ArrayList;
-import java.util.Comparator;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 import java.util.stream.Collectors;
 
 public abstract class ItemTranslator {
@@ -126,12 +122,20 @@ public abstract class ItemTranslator {
 
         ItemEntry bedrockItem = ItemRegistry.getItem(stack);
 
-        ItemStack itemStack = new ItemStack(stack.getId(), stack.getAmount(), stack.getNbt() != null ? stack.getNbt().clone() : null);
+        com.github.steveice10.opennbt.tag.builtin.CompoundTag nbt = stack.getNbt() != null ? stack.getNbt().clone() : null;
 
-        if (itemStack.getNbt() != null) {
+        // This is a fallback for maps with no nbt
+        if (nbt == null && bedrockItem.getJavaIdentifier().equals("minecraft:filled_map")) {
+            nbt = new com.github.steveice10.opennbt.tag.builtin.CompoundTag("");
+            nbt.put(new IntTag("map", 0));
+        }
+
+        ItemStack itemStack = new ItemStack(stack.getId(), stack.getAmount(), nbt);
+
+        if (nbt != null) {
             for (NbtItemStackTranslator translator : NBT_TRANSLATORS) {
                 if (translator.acceptItem(bedrockItem)) {
-                    translator.translateToBedrock(itemStack.getNbt(), bedrockItem);
+                    translator.translateToBedrock(nbt, bedrockItem);
                 }
             }
         }

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

@@ -57,4 +57,9 @@ public class MapItemTranslator extends NbtItemStackTranslator {
             itemTag.remove("map_uuid");
         }
     }
+
+    @Override
+    public boolean acceptItem(ItemEntry itemEntry) {
+        return itemEntry.getJavaIdentifier().equals("minecraft:filled_map");
+    }
 }

connector/src/main/java/org/geysermc/connector/network/translators/java/world/JavaMapDataTranslator.java

@@ -42,6 +42,7 @@ public class JavaMapDataTranslator extends PacketTranslator<ServerMapDataPacket>
     @Override
     public void translate(ServerMapDataPacket packet, GeyserSession session) {
         ClientboundMapItemDataPacket mapItemDataPacket = new ClientboundMapItemDataPacket();
+        boolean shouldStore = false;
 
         mapItemDataPacket.setUniqueMapId(packet.getMapId());
         mapItemDataPacket.setDimensionId(session.getPlayerEntity().getDimension());
@@ -55,6 +56,11 @@ public class JavaMapDataTranslator extends PacketTranslator<ServerMapDataPacket>
             mapItemDataPacket.setWidth(data.getColumns());
             mapItemDataPacket.setHeight(data.getRows());
 
+            // We have a full map image, this usually only happens on spawn for the initial image
+            if (mapItemDataPacket.getWidth() == 128 && mapItemDataPacket.getHeight() == 128) {
+                shouldStore = true;
+            }
+
             // Every int entry is an ABGR color
             int[] colors = new int[data.getData().length];
 
@@ -76,6 +82,12 @@ public class JavaMapDataTranslator extends PacketTranslator<ServerMapDataPacket>
             id++;
         }
 
-        session.getUpstream().getSession().sendPacket(mapItemDataPacket);
+        // Store the map to send when the client requests it, as bedrock expects the data after a MapInfoRequestPacket
+        if (shouldStore) {
+            session.getStoredMaps().put(mapItemDataPacket.getUniqueMapId(), mapItemDataPacket);
+        }
+
+        // Send anyway just in case
+        session.sendUpstreamPacket(mapItemDataPacket);
     }
 }