Fix Skin Caching and Fix Skin Restorer (#680)

* Fix Skin Caching

Changes:
* Instead of caching a skin based upon the player we cached it based upon the textureURL. This means multiple players with the same skin will benefit from the cache and more importantly will mean a player changing their skin will not get a false cache hit.
* This should fix all issues with SkinRestorer and will now correctly show the skin both to the player themselves and to other players

Closes #518

* Remove duplicated code

* Minimize playerlist updates

Changes:
* All async skin stuff will now just update skins and not be involved with sending the session to the player. This eliminates issues where the player list changes whilst an async task is occuring plus it means no invisible players while retrieving skin.
* Fix bug when retrieving cached skin

* When sending PlayerList packets ensure the skins have appropriate skinIds so the Bedrock client will cache hit/miss as needed

* Make sure to add and remove player when setting skin if they do not belong on the playerlist

* Make use of AuthData UUID when removing the player

* Revert removal of checking if entity is valid when initialized

This section is supposed to send all spawned entities in the java world to a player only after they've initialized. By removing this check it would also be sending entities that exist but are not spawned.

* Optimizations

Changes:
* Check for duplicate requests based on textureURL instead of player ID
* Don't use the PlayerSkinPacket. It duplicates the data sent in the PlayerListPacket and without it the players still get skin updates.

* Support caching of skins to disk based on configuration variable

If a skin is downloaded it will be saved to `cache/skins` using a base64 encoded filename of the textureUrl, if allowed by setting a non 0 value for the configuration variable `cache-skins`

When reading a skin we try load it from a cache file first before trying to download it.

We don't yet expire them but do update their last modification so we know which ones have been accessed.

* Update `config.yml` with cache-skins directive, defaulting to disabled

* Merge Fixes

* Cache all images instead of just skins

Changes:
* Move the image caching from skins to where images may get downloaded so this also covers capes and anything else that uses the same method of image retrieval
* Updated config value from `cache-skins` to `cache-images`
* Updated cache location from `cache/skins` to `cache/images`
* Images are stored in png format with a uuid. This may make debugging easier as they can be directly opened.

* Implement cached image expiry

If `cache-images` is set to a value greater than 0 then a scheduled task will occur once a day that will remove images with a modification date older than the value in days.

* Force skin changes as trusted

* Resolve PR queries

* Fix signed int causing issues calculating expiry time for images

* Reset Defaults to 0 and implement Google Timed Eviction cache for Images

* Add memory cache for Capes

Co-authored-by: Brendan Grieve <brendan.grieve@zepli.com.au>
Co-authored-by: bundabrg <bundabrg@grieve.com.au>

.gitignore

@@ -241,3 +241,4 @@ config.yml
 logs/
 public-key.pem
 locales/
+cache/

bootstrap/sponge/src/main/java/org/geysermc/platform/sponge/GeyserSpongeConfiguration.java

@@ -149,6 +149,11 @@ public class GeyserSpongeConfiguration implements GeyserConfiguration {
         return node.getNode("cache-chunks").getBoolean(false);
     }
 
+    @Override
+    public int getCacheImages() {
+        return node.getNode("cache-skins").getInt(0);
+    }
+
     @Override
     public boolean isAboveBedrockNetherBuilding() {
         return node.getNode("above-bedrock-nether-building").getBoolean(false);

connector/src/main/java/org/geysermc/connector/configuration/GeyserConfiguration.java

@@ -77,6 +77,8 @@ public interface GeyserConfiguration {
 
     boolean isCacheChunks();
 
+    int getCacheImages();
+
     IMetricsInfo getMetrics();
 
     interface IBedrockConfiguration {

connector/src/main/java/org/geysermc/connector/configuration/GeyserJacksonConfiguration.java

@@ -87,6 +87,9 @@ public abstract class GeyserJacksonConfiguration implements GeyserConfiguration
     @JsonProperty("cache-chunks")
     private boolean cacheChunks;
 
+    @JsonProperty("cache-images")
+    private int cacheImages = 0;
+
     @JsonProperty("above-bedrock-nether-building")
     private boolean aboveBedrockNetherBuilding;
 

connector/src/main/java/org/geysermc/connector/entity/PlayerEntity.java

@@ -38,7 +38,6 @@ import com.nukkitx.protocol.bedrock.data.entity.EntityLinkData;
 import com.nukkitx.protocol.bedrock.packet.*;
 import lombok.Getter;
 import lombok.Setter;
-import org.geysermc.connector.GeyserConnector;
 import org.geysermc.connector.entity.attribute.Attribute;
 import org.geysermc.connector.entity.attribute.AttributeType;
 import org.geysermc.connector.entity.type.EntityType;
@@ -47,7 +46,6 @@ import org.geysermc.connector.network.session.cache.EntityEffectCache;
 import org.geysermc.connector.scoreboard.Team;
 import org.geysermc.connector.utils.AttributeUtils;
 import org.geysermc.connector.utils.MessageUtils;
-import org.geysermc.connector.utils.SkinUtils;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -61,7 +59,7 @@ public class PlayerEntity extends LivingEntity {
     private UUID uuid;
     private String username;
     private long lastSkinUpdate = -1;
-    private boolean playerList = true;
+    private boolean playerList = true;  // Player is in the player list
     private final EntityEffectCache effectCache;
 
     private Entity leftParrot;
@@ -117,30 +115,12 @@ public class PlayerEntity extends LivingEntity {
     public void sendPlayer(GeyserSession session) {
         if(session.getEntityCache().getPlayerEntity(uuid) == null)
             return;
-        if (getLastSkinUpdate() == -1) {
-            if (playerList) {
-                PlayerListPacket playerList = new PlayerListPacket();
-                playerList.setAction(PlayerListPacket.Action.ADD);
-                playerList.getEntries().add(SkinUtils.buildDefaultEntry(profile, geyserId));
-                session.sendUpstreamPacket(playerList);
-            }
-        }
 
         if (session.getUpstream().isInitialized() && session.getEntityCache().getEntityByGeyserId(geyserId) == null) {
             session.getEntityCache().spawnEntity(this);
         } else {
             spawnEntity(session);
         }
-
-        if (!playerList) {
-            // remove from playerlist if player isn't on playerlist
-            GeyserConnector.getInstance().getGeneralThreadPool().execute(() -> {
-                PlayerListPacket playerList = new PlayerListPacket();
-                playerList.setAction(PlayerListPacket.Action.REMOVE);
-                playerList.getEntries().add(new PlayerListPacket.Entry(uuid));
-                session.sendUpstreamPacket(playerList);
-            });
-        }
     }
 
     @Override

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

@@ -251,17 +251,6 @@ public class GeyserSession implements CommandSender {
         upstream.sendPacket(attributesPacket);
     }
 
-    public void fetchOurSkin(PlayerListPacket.Entry entry) {
-        PlayerSkinPacket playerSkinPacket = new PlayerSkinPacket();
-        playerSkinPacket.setUuid(authData.getUUID());
-        playerSkinPacket.setSkin(entry.getSkin());
-        playerSkinPacket.setOldSkinName("OldName");
-        playerSkinPacket.setNewSkinName("NewName");
-        playerSkinPacket.setTrustedSkin(true);
-        upstream.sendPacket(playerSkinPacket);
-        getConnector().getLogger().debug("Sending skin for " + playerEntity.getUsername() + " " + authData.getUUID());
-    }
-
     public void login() {
         if (connector.getAuthType() != AuthType.ONLINE) {
             if (connector.getAuthType() == AuthType.OFFLINE) {

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

@@ -44,8 +44,8 @@ public class BedrockSetLocalPlayerAsInitializedTranslator extends PacketTranslat
 
                 for (PlayerEntity entity : session.getEntityCache().getEntitiesByType(PlayerEntity.class)) {
                     if (!entity.isValid()) {
-                        // async skin loading
-                        SkinUtils.requestAndHandleSkinAndCape(entity, session, skinAndCape -> entity.sendPlayer(session));
+                        SkinUtils.requestAndHandleSkinAndCape(entity, session, null);
+                        entity.sendPlayer(session);
                     }
                 }
             }

connector/src/main/java/org/geysermc/connector/network/translators/java/entity/player/JavaPlayerListEntryTranslator.java

@@ -82,18 +82,7 @@ public class JavaPlayerListEntryTranslator extends PacketTranslator<ServerPlayer
                     playerEntity.setPlayerList(true);
                     playerEntity.setValid(true);
 
-                    PlayerListPacket.Entry playerListEntry = SkinUtils.buildCachedEntry(entry.getProfile(), playerEntity.getGeyserId());
-                    if (self) {
-                        // Copy the entry with our identity instead.
-                        PlayerListPacket.Entry copy = new PlayerListPacket.Entry(session.getAuthData().getUUID());
-                        copy.setName(playerListEntry.getName());
-                        copy.setEntityId(playerListEntry.getEntityId());
-                        copy.setSkin(playerListEntry.getSkin());
-                        copy.setXuid(playerListEntry.getXuid());
-                        copy.setPlatformChatId(playerListEntry.getPlatformChatId());
-                        copy.setTeacher(playerListEntry.isTeacher());
-                        playerListEntry = copy;
-                    }
+                    PlayerListPacket.Entry playerListEntry = SkinUtils.buildCachedEntry(session, entry.getProfile(), playerEntity.getGeyserId());
 
                     translate.getEntries().add(playerListEntry);
                     break;
@@ -103,15 +92,20 @@ public class JavaPlayerListEntryTranslator extends PacketTranslator<ServerPlayer
                         // remove from tablist but player entity is still there
                         entity.setPlayerList(false);
                     } else {
-                        // just remove it from caching
                         if (entity == null) {
+                            // just remove it from caching
                             session.getEntityCache().removePlayerEntity(entry.getProfile().getId());
                         } else {
                             entity.setPlayerList(false);
                             session.getEntityCache().removeEntity(entity, false);
                         }
                     }
-                    translate.getEntries().add(new PlayerListPacket.Entry(entry.getProfile().getId()));
+                    if (entity == session.getPlayerEntity()) {
+                        // If removing ourself we use our AuthData UUID
+                        translate.getEntries().add(new PlayerListPacket.Entry(session.getAuthData().getUUID()));
+                    } else {
+                        translate.getEntries().add(new PlayerListPacket.Entry(entry.getProfile().getId()));
+                    }
                     break;
             }
         }

connector/src/main/java/org/geysermc/connector/network/translators/java/entity/spawn/JavaSpawnPlayerTranslator.java

@@ -54,9 +54,9 @@ public class JavaSpawnPlayerTranslator extends PacketTranslator<ServerSpawnPlaye
         entity.setRotation(rotation);
         session.getEntityCache().cacheEntity(entity);
 
-        // async skin loading
         if (session.getUpstream().isInitialized()) {
-            SkinUtils.requestAndHandleSkinAndCape(entity, session, skinAndCape -> entity.sendPlayer(session));
+            entity.sendPlayer(session);
+            SkinUtils.requestAndHandleSkinAndCape(entity, session, null);
         }
     }
 }

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

@@ -27,6 +27,8 @@ package org.geysermc.connector.utils;
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
 import lombok.AllArgsConstructor;
 import lombok.Getter;
 import lombok.NoArgsConstructor;
@@ -40,9 +42,11 @@ import java.net.HttpURLConnection;
 import java.net.URL;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.Base64;
 import java.util.Map;
+import java.util.Objects;
 import java.util.UUID;
 import java.util.concurrent.*;
 
@@ -54,22 +58,26 @@ public class SkinProvider {
     public static final Skin EMPTY_SKIN = new Skin(-1, "steve", STEVE_SKIN);
     public static final byte[] ALEX_SKIN = new ProvidedSkin("bedrock/skin/skin_alex.png").getSkin();
     public static final Skin EMPTY_SKIN_ALEX = new Skin(-1, "alex", ALEX_SKIN);
-    private static Map<UUID, Skin> cachedSkins = new ConcurrentHashMap<>();
-    private static Map<UUID, CompletableFuture<Skin>> requestedSkins = new ConcurrentHashMap<>();
+    private static final Cache<String, Skin> cachedSkins = CacheBuilder.newBuilder()
+            .expireAfterAccess(1, TimeUnit.HOURS)
+            .build();
+
+    private static final Map<String, CompletableFuture<Skin>> requestedSkins = new ConcurrentHashMap<>();
 
     public static final Cape EMPTY_CAPE = new Cape("", "no-cape", new byte[0], -1, true);
-    private static Map<String, Cape> cachedCapes = new ConcurrentHashMap<>();
-    private static Map<String, CompletableFuture<Cape>> requestedCapes = new ConcurrentHashMap<>();
+    private static final Cache<String, Cape> cachedCapes = CacheBuilder.newBuilder()
+            .expireAfterAccess(1, TimeUnit.HOURS)
+            .build();
+    private static final Map<String, CompletableFuture<Cape>> requestedCapes = new ConcurrentHashMap<>();
 
     public static final SkinGeometry EMPTY_GEOMETRY = SkinProvider.SkinGeometry.getLegacy(false);
-    private static Map<UUID, SkinGeometry> cachedGeometry = new ConcurrentHashMap<>();
+    private static final Map<UUID, SkinGeometry> cachedGeometry = new ConcurrentHashMap<>();
 
     public static final boolean ALLOW_THIRD_PARTY_EARS = GeyserConnector.getInstance().getConfig().isAllowThirdPartyEars();
     public static String EARS_GEOMETRY;
     public static String EARS_GEOMETRY_SLIM;
 
     private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
-    private static final int CACHE_INTERVAL = 8 * 60 * 1000; // 8 minutes
 
     static {
         /* Load in the normal ears geometry */
@@ -102,22 +110,44 @@ public class SkinProvider {
         }
 
         EARS_GEOMETRY_SLIM = earsDataBuilder.toString();
-    }
 
-    public static boolean hasSkinCached(UUID uuid) {
-        return cachedSkins.containsKey(uuid);
+        // Schedule Daily Image Expiry if we are caching them
+        if (GeyserConnector.getInstance().getConfig().getCacheImages() > 0) {
+            GeyserConnector.getInstance().getGeneralThreadPool().scheduleAtFixedRate(() -> {
+                File cacheFolder = Paths.get("cache", "images").toFile();
+                if (!cacheFolder.exists()) {
+                    return;
+                }
+
+                int count = 0;
+                final long expireTime = ((long)GeyserConnector.getInstance().getConfig().getCacheImages()) * ((long)1000 * 60 * 60 * 24);
+                for (File imageFile : Objects.requireNonNull(cacheFolder.listFiles())) {
+                    if (imageFile.lastModified() < System.currentTimeMillis() - expireTime) {
+                        //noinspection ResultOfMethodCallIgnored
+                        imageFile.delete();
+                        count++;
+                    }
+                }
+
+                if (count > 0) {
+                    GeyserConnector.getInstance().getLogger().debug(String.format("Removed %d cached image files as they have expired", count));
+                }
+            }, 10, 1440, TimeUnit.MINUTES);
+        }
     }
 
     public static boolean hasCapeCached(String capeUrl) {
-        return cachedCapes.containsKey(capeUrl);
+        return cachedCapes.getIfPresent(capeUrl) != null;
     }
 
-    public static Skin getCachedSkin(UUID uuid) {
-        return cachedSkins.getOrDefault(uuid, EMPTY_SKIN);
+    public static Skin getCachedSkin(String skinUrl) {
+        Skin skin = cachedSkins.getIfPresent(skinUrl);
+        return skin != null ? skin : EMPTY_SKIN;
     }
 
     public static Cape getCachedCape(String capeUrl) {
-        return capeUrl != null ? cachedCapes.getOrDefault(capeUrl, EMPTY_CAPE) : EMPTY_CAPE;
+        Cape cape = capeUrl != null ? cachedCapes.getIfPresent(capeUrl) : EMPTY_CAPE;
+        return cape != null ? cape : EMPTY_CAPE;
     }
 
     public static CompletableFuture<SkinAndCape> requestSkinAndCape(UUID playerId, String skinUrl, String capeUrl) {
@@ -137,28 +167,26 @@ public class SkinProvider {
 
     public static CompletableFuture<Skin> requestSkin(UUID playerId, String textureUrl, boolean newThread) {
         if (textureUrl == null || textureUrl.isEmpty()) return CompletableFuture.completedFuture(EMPTY_SKIN);
-        if (requestedSkins.containsKey(playerId)) return requestedSkins.get(playerId); // already requested
+        if (requestedSkins.containsKey(textureUrl)) return requestedSkins.get(textureUrl); // already requested
 
-        if ((System.currentTimeMillis() - CACHE_INTERVAL) < cachedSkins.getOrDefault(playerId, EMPTY_SKIN).getRequestedOn()) {
-            // no need to update, still cached
-            return CompletableFuture.completedFuture(cachedSkins.get(playerId));
+        Skin cachedSkin = cachedSkins.getIfPresent(textureUrl);
+        if (cachedSkin != null) {
+            return CompletableFuture.completedFuture(cachedSkin);
         }
 
         CompletableFuture<Skin> future;
         if (newThread) {
             future = CompletableFuture.supplyAsync(() -> supplySkin(playerId, textureUrl), EXECUTOR_SERVICE)
                     .whenCompleteAsync((skin, throwable) -> {
-                        if (!cachedSkins.getOrDefault(playerId, EMPTY_SKIN).getTextureUrl().equals(textureUrl)) {
-                            skin.updated = true;
-                            cachedSkins.put(playerId, skin);
-                        }
-                        requestedSkins.remove(skin.getSkinOwner());
+                        skin.updated = true;
+                        cachedSkins.put(textureUrl, skin);
+                        requestedSkins.remove(textureUrl);
                     });
-            requestedSkins.put(playerId, future);
+            requestedSkins.put(textureUrl, future);
         } else {
             Skin skin = supplySkin(playerId, textureUrl);
             future = CompletableFuture.completedFuture(skin);
-            cachedSkins.put(playerId, skin);
+            cachedSkins.put(textureUrl, skin);
         }
         return future;
     }
@@ -168,11 +196,9 @@ public class SkinProvider {
         if (requestedCapes.containsKey(capeUrl)) return requestedCapes.get(capeUrl); // already requested
 
         boolean officialCape = provider == CapeProvider.MINECRAFT;
-        boolean validCache = (System.currentTimeMillis() - CACHE_INTERVAL) < cachedCapes.getOrDefault(capeUrl, EMPTY_CAPE).getRequestedOn();
-
-        if ((cachedCapes.containsKey(capeUrl) && officialCape) || validCache) {
-            // the cape is an official cape (static) or the cape doesn't need a update yet
-            return CompletableFuture.completedFuture(cachedCapes.get(capeUrl));
+        Cape cachedCape = cachedCapes.getIfPresent(capeUrl);
+        if (cachedCape != null) {
+            return CompletableFuture.completedFuture(cachedCape);
         }
 
         CompletableFuture<Cape> future;
@@ -245,7 +271,10 @@ public class SkinProvider {
     }
 
     public static CompletableFuture<Cape> requestBedrockCape(UUID playerID, boolean newThread) {
-        Cape bedrockCape = cachedCapes.getOrDefault(playerID.toString() + ".Bedrock", EMPTY_CAPE);
+        Cape bedrockCape = cachedCapes.getIfPresent(playerID.toString() + ".Bedrock");
+        if (bedrockCape == null) {
+            bedrockCape = EMPTY_CAPE;
+        }
         return CompletableFuture.completedFuture(bedrockCape);
     }
 
@@ -256,7 +285,7 @@ public class SkinProvider {
 
     public static void storeBedrockSkin(UUID playerID, String skinID, byte[] skinData) {
         Skin skin = new Skin(playerID, skinID, skinData, System.currentTimeMillis(), true, false);
-        cachedSkins.put(playerID, skin);
+        cachedSkins.put(skin.getTextureUrl(), skin);
     }
 
     public static void storeBedrockCape(UUID playerID, byte[] capeData) {
@@ -276,7 +305,7 @@ public class SkinProvider {
      * @param skin The skin to cache
      */
     public static void storeEarSkin(UUID playerID, Skin skin) {
-        cachedSkins.put(playerID, skin);
+        cachedSkins.put(skin.getTextureUrl(), skin);
     }
 
     /**
@@ -290,11 +319,12 @@ public class SkinProvider {
     }
 
     private static Skin supplySkin(UUID uuid, String textureUrl) {
-        byte[] skin = EMPTY_SKIN.getSkinData();
         try {
-            skin = requestImage(textureUrl, null);
+            byte[] skin = requestImage(textureUrl, null);
+            return new Skin(uuid, textureUrl, skin, System.currentTimeMillis(), false, false);
         } catch (Exception ignored) {} // just ignore I guess
-        return new Skin(uuid, textureUrl, skin, System.currentTimeMillis(), false, false);
+
+        return new Skin(uuid, "empty", EMPTY_SKIN.getSkinData(), System.currentTimeMillis(), false, false);
     }
 
     private static Cape supplyCape(String capeUrl, CapeProvider provider) {
@@ -356,11 +386,38 @@ public class SkinProvider {
         return existingSkin;
     }
 
+    @SuppressWarnings("ResultOfMethodCallIgnored")
     private static byte[] requestImage(String imageUrl, CapeProvider provider) throws Exception {
-        BufferedImage image = downloadImage(imageUrl, provider);
-        GeyserConnector.getInstance().getLogger().debug("Downloaded " + imageUrl);
+        BufferedImage image = null;
 
-        // if the requested image is an cape
+        // First see if we have a cached file. We also update the modification stamp so we know when the file was last used
+        File imageFile = Paths.get("cache", "images", UUID.nameUUIDFromBytes(imageUrl.getBytes()).toString() + ".png").toFile();
+        if (imageFile.exists()) {
+            try {
+                GeyserConnector.getInstance().getLogger().debug("Reading cached image from file " + imageFile.getPath() + " for " + imageUrl);
+                imageFile.setLastModified(System.currentTimeMillis());
+                image = ImageIO.read(imageFile);
+            } catch (IOException ignored) {}
+        }
+
+        // If no image we download it
+        if (image == null) {
+            image = downloadImage(imageUrl, provider);
+            GeyserConnector.getInstance().getLogger().debug("Downloaded " + imageUrl);
+
+            // Write to cache if we are allowed
+            if (GeyserConnector.getInstance().getConfig().getCacheImages() > 0) {
+                imageFile.getParentFile().mkdirs();
+                try {
+                    ImageIO.write(image, "png", imageFile);
+                    GeyserConnector.getInstance().getLogger().debug("Writing cached skin to file " + imageFile.getPath() + " for " + imageUrl);
+                } catch (IOException e) {
+                    GeyserConnector.getInstance().getLogger().error("Failed to write cached skin to file " + imageFile.getPath() + " for " + imageUrl);
+                }
+            }
+        }
+
+        // if the requested image is a cape
         if (provider != null) {
             while(image.getWidth() > 64) {
                 image = scale(image);

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

@@ -47,18 +47,21 @@ import java.util.function.Consumer;
 
 public class SkinUtils {
 
-    public static PlayerListPacket.Entry buildCachedEntry(GameProfile profile, long geyserId) {
+    public static PlayerListPacket.Entry buildCachedEntry(GeyserSession session, GameProfile profile, long geyserId) {
         GameProfileData data = GameProfileData.from(profile);
         SkinProvider.Cape cape = SkinProvider.getCachedCape(data.getCapeUrl());
 
         SkinProvider.SkinGeometry geometry = SkinProvider.SkinGeometry.getLegacy(data.isAlex());
 
+        SkinProvider.Skin skin = SkinProvider.getCachedSkin(data.getSkinUrl());
+
         return buildEntryManually(
+                session,
                 profile.getId(),
                 profile.getName(),
                 geyserId,
-                profile.getIdAsString(),
-                SkinProvider.getCachedSkin(profile.getId()).getSkinData(),
+                skin.getTextureUrl(),
+                skin.getSkinData(),
                 cape.getCapeId(),
                 cape.getCapeData(),
                 geometry.getGeometryName(),
@@ -66,12 +69,13 @@ public class SkinUtils {
         );
     }
 
-    public static PlayerListPacket.Entry buildDefaultEntry(GameProfile profile, long geyserId) {
+    public static PlayerListPacket.Entry buildDefaultEntry(GeyserSession session, GameProfile profile, long geyserId) {
         return buildEntryManually(
+                session,
                 profile.getId(),
                 profile.getName(),
                 geyserId,
-                profile.getIdAsString(),
+                "default",
                 SkinProvider.STEVE_SKIN,
                 SkinProvider.EMPTY_CAPE.getCapeId(),
                 SkinProvider.EMPTY_CAPE.getCapeData(),
@@ -80,16 +84,25 @@ public class SkinUtils {
         );
     }
 
-    public static PlayerListPacket.Entry buildEntryManually(UUID uuid, String username, long geyserId,
+    public static PlayerListPacket.Entry buildEntryManually(GeyserSession session, UUID uuid, String username, long geyserId,
                                                             String skinId, byte[] skinData,
                                                             String capeId, byte[] capeData,
                                                             String geometryName, String geometryData) {
         SerializedSkin serializedSkin = SerializedSkin.of(
                 skinId, geometryName, ImageData.of(skinData), Collections.emptyList(),
-                ImageData.of(capeData), geometryData, "", true, false, !capeId.equals(SkinProvider.EMPTY_CAPE.getCapeId()), capeId, uuid.toString()
+                ImageData.of(capeData), geometryData, "", true, false, !capeId.equals(SkinProvider.EMPTY_CAPE.getCapeId()), capeId, skinId
         );
 
-        PlayerListPacket.Entry entry = new PlayerListPacket.Entry(uuid);
+        PlayerListPacket.Entry entry;
+
+        // If we are building a PlayerListEntry for our own session we use our AuthData UUID instead of the Java UUID
+        // as bedrock expects to get back its own provided uuid
+        if (session.getPlayerEntity().getUuid().equals(uuid)) {
+            entry = new PlayerListPacket.Entry(session.getAuthData().getUUID());
+        } else {
+            entry = new PlayerListPacket.Entry(uuid);
+        }
+
         entry.setName(username);
         entry.setEntityId(geyserId);
         entry.setSkin(serializedSkin);
@@ -201,48 +214,34 @@ public class SkinUtils {
                                 }
                             }
 
-                            if (entity.getLastSkinUpdate() < skin.getRequestedOn()) {
-                                entity.setLastSkinUpdate(skin.getRequestedOn());
+                            entity.setLastSkinUpdate(skin.getRequestedOn());
 
-                                if (session.getUpstream().isInitialized()) {
-                                    PlayerListPacket.Entry updatedEntry = buildEntryManually(
-                                            entity.getUuid(),
-                                            entity.getUsername(),
-                                            entity.getGeyserId(),
-                                            entity.getUuid().toString(),
-                                            skin.getSkinData(),
-                                            cape.getCapeId(),
-                                            cape.getCapeData(),
-                                            geometry.getGeometryName(),
-                                            geometry.getGeometryData()
-                                    );
+                            if (session.getUpstream().isInitialized()) {
+                                PlayerListPacket.Entry updatedEntry = buildEntryManually(
+                                        session,
+                                        entity.getUuid(),
+                                        entity.getUsername(),
+                                        entity.getGeyserId(),
+                                        skin.getTextureUrl(),
+                                        skin.getSkinData(),
+                                        cape.getCapeId(),
+                                        cape.getCapeData(),
+                                        geometry.getGeometryName(),
+                                        geometry.getGeometryData()
+                                );
 
-                                    // If it is our skin we replace the UUID with the authdata UUID
-                                    if (session.getPlayerEntity() == entity) {
-                                        // Copy the entry with our identity instead.
-                                        PlayerListPacket.Entry copy = new PlayerListPacket.Entry(session.getAuthData().getUUID());
-                                        copy.setName(updatedEntry.getName());
-                                        copy.setEntityId(updatedEntry.getEntityId());
-                                        copy.setSkin(updatedEntry.getSkin());
-                                        copy.setXuid(updatedEntry.getXuid());
-                                        copy.setPlatformChatId(updatedEntry.getPlatformChatId());
-                                        copy.setTeacher(updatedEntry.isTeacher());
-                                        updatedEntry = copy;
-                                    }
 
+                                PlayerListPacket playerAddPacket = new PlayerListPacket();
+                                playerAddPacket.setAction(PlayerListPacket.Action.ADD);
+                                playerAddPacket.getEntries().add(updatedEntry);
+                                session.sendUpstreamPacket(playerAddPacket);
+
+                                if (!entity.isPlayerList()) {
                                     PlayerListPacket playerRemovePacket = new PlayerListPacket();
                                     playerRemovePacket.setAction(PlayerListPacket.Action.REMOVE);
                                     playerRemovePacket.getEntries().add(updatedEntry);
                                     session.sendUpstreamPacket(playerRemovePacket);
 
-                                    PlayerListPacket playerAddPacket = new PlayerListPacket();
-                                    playerAddPacket.setAction(PlayerListPacket.Action.ADD);
-                                    playerAddPacket.getEntries().add(updatedEntry);
-                                    session.sendUpstreamPacket(playerAddPacket);
-
-                                    if(entity.getUuid().equals(session.getPlayerEntity().getUuid())) {
-                                        session.fetchOurSkin(updatedEntry);
-                                    }
                                 }
                             }
                         } catch (Exception e) {

connector/src/main/resources/config.yml

@@ -94,6 +94,10 @@ show-cooldown: true
 # Geyser has direct access to the server itself.
 cache-chunks: false
 
+# Specify how many days images will be cached to disk to save downloading them from the internet.
+# A value of 0 is disabled. (Default: 0)
+cache-images: 0
+
 # Bedrock prevents building and displaying blocks above Y127 in the Nether -
 # enabling this config option works around that by changing the Nether dimension ID
 # to the End ID. The main downside to this is that the sky will resemble that of