Refactor player add code for 1.19.3

Players are now not always added into the tab list.

core/src/main/java/org/geysermc/geyser/entity/type/player/PlayerEntity.java

@@ -77,7 +77,6 @@ public class PlayerEntity extends LivingEntity {
     }
 
     private String username;
-    private boolean playerList = true; // Player is in the player list
 
     /**
      * The textures property from the GameProfile.
@@ -417,4 +416,11 @@ public class PlayerEntity extends LivingEntity {
             session.sendUpstreamPacket(packet);
         }
     }
+
+    /**
+     * @return the UUID that should be used when dealing with Bedrock's tab list.
+     */
+    public UUID getTabListUuid() {
+        return getUuid();
+    }
 }

core/src/main/java/org/geysermc/geyser/entity/type/player/SessionPlayerEntity.java

@@ -250,4 +250,9 @@ public class SessionPlayerEntity extends PlayerEntity {
             dirtyMetadata.put(EntityData.PLAYER_HAS_DIED, (byte) 0);
         }
     }
+
+    @Override
+    public UUID getTabListUuid() {
+        return session.getAuthData().uuid();
+    }
 }

core/src/main/java/org/geysermc/geyser/entity/type/player/SkullPlayerEntity.java

@@ -48,7 +48,6 @@ public class SkullPlayerEntity extends PlayerEntity {
 
     public SkullPlayerEntity(GeyserSession session, long geyserId) {
         super(session, 0, geyserId, UUID.randomUUID(), Vector3f.ZERO, Vector3f.ZERO, 0, 0, 0, "", null);
-        setPlayerList(false);
     }
 
     @Override

core/src/main/java/org/geysermc/geyser/session/cache/EntityCache.java

@@ -123,7 +123,8 @@ public class EntityCache {
     }
 
     public void addPlayerEntity(PlayerEntity entity) {
-        playerEntities.put(entity.getUuid(), entity);
+        // putIfAbsent matches the behavior of playerInfoMap in Java as of 1.19.3
+        playerEntities.putIfAbsent(entity.getUuid(), entity);
     }
 
     public PlayerEntity getPlayerEntity(UUID uuid) {

core/src/main/java/org/geysermc/geyser/skin/FakeHeadProvider.java

@@ -29,10 +29,6 @@ import com.github.steveice10.opennbt.tag.builtin.CompoundTag;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
-import com.nukkitx.protocol.bedrock.data.skin.ImageData;
-import com.nukkitx.protocol.bedrock.data.skin.SerializedSkin;
-import com.nukkitx.protocol.bedrock.packet.PlayerListPacket;
-import com.nukkitx.protocol.bedrock.packet.PlayerSkinPacket;
 import lombok.AllArgsConstructor;
 import lombok.Getter;
 import lombok.Setter;
@@ -45,7 +41,6 @@ import org.geysermc.geyser.text.GeyserLocale;
 import javax.annotation.Nonnull;
 import java.awt.*;
 import java.awt.image.BufferedImage;
-import java.util.Collections;
 import java.util.Objects;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
@@ -111,7 +106,7 @@ public class FakeHeadProvider {
             try {
                 SkinProvider.SkinData mergedSkinData = MERGED_SKINS_LOADING_CACHE.get(new FakeHeadEntry(texturesProperty, fakeHeadSkinUrl, entity));
 
-                sendSkinPacket(session, entity, mergedSkinData);
+                SkinManager.sendSkinPacket(session, entity, mergedSkinData);
             } catch (ExecutionException e) {
                 GeyserImpl.getInstance().getLogger().error("Couldn't merge skin of " + entity.getUsername() + " with head skin url " + fakeHeadSkinUrl, e);
             }
@@ -133,50 +128,10 @@ public class FakeHeadProvider {
                 return;
             }
 
-            sendSkinPacket(session, entity, skinData);
+            SkinManager.sendSkinPacket(session, entity, skinData);
         });
     }
 
-    private static void sendSkinPacket(GeyserSession session, PlayerEntity entity, SkinProvider.SkinData skinData) {
-        SkinProvider.Skin skin = skinData.skin();
-        SkinProvider.Cape cape = skinData.cape();
-        SkinProvider.SkinGeometry geometry = skinData.geometry();
-
-        if (entity.getUuid().equals(session.getPlayerEntity().getUuid())) {
-            PlayerListPacket.Entry updatedEntry = SkinManager.buildEntryManually(
-                    session,
-                    entity.getUuid(),
-                    entity.getUsername(),
-                    entity.getGeyserId(),
-                    skin.getTextureUrl(),
-                    skin.getSkinData(),
-                    cape.getCapeId(),
-                    cape.getCapeData(),
-                    geometry
-            );
-
-            PlayerListPacket playerAddPacket = new PlayerListPacket();
-            playerAddPacket.setAction(PlayerListPacket.Action.ADD);
-            playerAddPacket.getEntries().add(updatedEntry);
-            session.sendUpstreamPacket(playerAddPacket);
-        } else {
-            PlayerSkinPacket packet = new PlayerSkinPacket();
-            packet.setUuid(entity.getUuid());
-            packet.setOldSkinName("");
-            packet.setNewSkinName(skin.getTextureUrl());
-            packet.setSkin(getSkin(skin.getTextureUrl(), skin, cape, geometry));
-            packet.setTrustedSkin(true);
-            session.sendUpstreamPacket(packet);
-        }
-    }
-
-    private static SerializedSkin getSkin(String skinId, SkinProvider.Skin skin, SkinProvider.Cape cape, SkinProvider.SkinGeometry geometry) {
-        return SerializedSkin.of(skinId, "", geometry.getGeometryName(),
-                ImageData.of(skin.getSkinData()), Collections.emptyList(),
-                ImageData.of(cape.getCapeData()), geometry.getGeometryData(),
-                "", true, false, false, cape.getCapeId(), skinId);
-    }
-
     @AllArgsConstructor
     @Getter
     @Setter

core/src/main/java/org/geysermc/geyser/skin/SkinManager.java

@@ -32,6 +32,7 @@ import com.github.steveice10.opennbt.tag.builtin.StringTag;
 import com.nukkitx.protocol.bedrock.data.skin.ImageData;
 import com.nukkitx.protocol.bedrock.data.skin.SerializedSkin;
 import com.nukkitx.protocol.bedrock.packet.PlayerListPacket;
+import com.nukkitx.protocol.bedrock.packet.PlayerSkinPacket;
 import org.geysermc.geyser.GeyserImpl;
 import org.geysermc.geyser.api.network.AuthType;
 import org.geysermc.geyser.entity.type.player.PlayerEntity;
@@ -67,10 +68,8 @@ public class SkinManager {
                 playerEntity.getUuid(),
                 playerEntity.getUsername(),
                 playerEntity.getGeyserId(),
-                skin.getTextureUrl(),
+                skin,
-                skin.getSkinData(),
+                cape,
-                cape.getCapeId(),
-                cape.getCapeData(),
                 geometry
         );
     }
@@ -79,14 +78,10 @@ public class SkinManager {
      * With all the information needed, build a Bedrock player entry with translated skin information.
      */
     public static PlayerListPacket.Entry buildEntryManually(GeyserSession session, UUID uuid, String username, long geyserId,
-                                                            String skinId, byte[] skinData,
+                                                            SkinProvider.Skin skin,
-                                                            String capeId, byte[] capeData,
+                                                            SkinProvider.Cape cape,
                                                             SkinProvider.SkinGeometry geometry) {
-        SerializedSkin serializedSkin = SerializedSkin.of(
+        SerializedSkin serializedSkin = getSkin(skin.getTextureUrl(), skin, cape, geometry);
-                skinId, "", geometry.getGeometryName(), ImageData.of(skinData), Collections.emptyList(),
-                ImageData.of(capeData), geometry.getGeometryData(), "", true, false,
-                !capeId.equals(SkinProvider.EMPTY_CAPE.getCapeId()), capeId, skinId
-        );
 
         // This attempts to find the XUID of the player so profile images show up for Xbox accounts
         String xuid = "";
@@ -116,6 +111,45 @@ public class SkinManager {
         return entry;
     }
 
+    public static void sendSkinPacket(GeyserSession session, PlayerEntity entity, SkinProvider.SkinData skinData) {
+        SkinProvider.Skin skin = skinData.skin();
+        SkinProvider.Cape cape = skinData.cape();
+        SkinProvider.SkinGeometry geometry = skinData.geometry();
+
+        if (entity.getUuid().equals(session.getPlayerEntity().getUuid())) {
+            // TODO is this special behavior needed?
+            PlayerListPacket.Entry updatedEntry = buildEntryManually(
+                    session,
+                    entity.getUuid(),
+                    entity.getUsername(),
+                    entity.getGeyserId(),
+                    skin,
+                    cape,
+                    geometry
+            );
+
+            PlayerListPacket playerAddPacket = new PlayerListPacket();
+            playerAddPacket.setAction(PlayerListPacket.Action.ADD);
+            playerAddPacket.getEntries().add(updatedEntry);
+            session.sendUpstreamPacket(playerAddPacket);
+        } else {
+            PlayerSkinPacket packet = new PlayerSkinPacket();
+            packet.setUuid(entity.getUuid());
+            packet.setOldSkinName("");
+            packet.setNewSkinName(skin.getTextureUrl());
+            packet.setSkin(getSkin(skin.getTextureUrl(), skin, cape, geometry));
+            packet.setTrustedSkin(true);
+            session.sendUpstreamPacket(packet);
+        }
+    }
+
+    private static SerializedSkin getSkin(String skinId, SkinProvider.Skin skin, SkinProvider.Cape cape, SkinProvider.SkinGeometry geometry) {
+        return SerializedSkin.of(skinId, "", geometry.getGeometryName(),
+                ImageData.of(skin.getSkinData()), Collections.emptyList(),
+                ImageData.of(cape.getCapeData()), geometry.getGeometryData(),
+                "", true, false, false, cape.getCapeId(), skinId);
+    }
+
     public static void requestAndHandleSkinAndCape(PlayerEntity entity, GeyserSession session,
                                                    Consumer<SkinProvider.SkinAndCape> skinAndCapeConsumer) {
         SkinProvider.requestSkinData(entity).whenCompleteAsync((skinData, throwable) -> {
@@ -128,34 +162,7 @@ public class SkinManager {
             }
 
             if (skinData.geometry() != null) {
-                SkinProvider.Skin skin = skinData.skin();
+                sendSkinPacket(session, entity, skinData);
-                SkinProvider.Cape cape = skinData.cape();
-                SkinProvider.SkinGeometry geometry = skinData.geometry();
-
-                PlayerListPacket.Entry updatedEntry = buildEntryManually(
-                        session,
-                        entity.getUuid(),
-                        entity.getUsername(),
-                        entity.getGeyserId(),
-                        skin.getTextureUrl(),
-                        skin.getSkinData(),
-                        cape.getCapeId(),
-                        cape.getCapeData(),
-                        geometry
-                );
-
-
-                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);
-                }
             }
 
             if (skinAndCapeConsumer != null) {

core/src/main/java/org/geysermc/geyser/translator/protocol/java/entity/player/JavaPlayerInfoRemoveTranslator.java

@@ -45,17 +45,15 @@ public class JavaPlayerInfoRemoveTranslator extends PacketTranslator<Clientbound
         for (UUID id : packet.getProfileIds()) {
             // As the player entity is no longer present, we can remove the entry
             PlayerEntity entity = session.getEntityCache().removePlayerEntity(id);
+            UUID removeId;
             if (entity != null) {
                 // Just remove the entity's player list status
                 // Don't despawn the entity - the Java server will also take care of that.
-                entity.setPlayerList(false);
+                removeId = entity.getTabListUuid();
-            }
-            if (entity == session.getPlayerEntity()) {
-                // If removing ourself we use our AuthData UUID
-                translate.getEntries().add(new PlayerListPacket.Entry(session.getAuthData().uuid()));
             } else {
-                translate.getEntries().add(new PlayerListPacket.Entry(id));
+                removeId = id;
             }
+            translate.getEntries().add(new PlayerListPacket.Entry(removeId));
         }
 
         session.sendUpstreamPacket(translate);

core/src/main/java/org/geysermc/geyser/translator/protocol/java/entity/player/JavaPlayerInfoUpdateTranslator.java

@@ -38,70 +38,91 @@ import org.geysermc.geyser.skin.SkinManager;
 import org.geysermc.geyser.translator.protocol.PacketTranslator;
 import org.geysermc.geyser.translator.protocol.Translator;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+
 @Translator(packet = ClientboundPlayerInfoUpdatePacket.class)
 public class JavaPlayerInfoUpdateTranslator extends PacketTranslator<ClientboundPlayerInfoUpdatePacket> {
     @Override
     public void translate(GeyserSession session, ClientboundPlayerInfoUpdatePacket packet) {
-        if (!packet.getActions().contains(PlayerListEntryAction.ADD_PLAYER)) {
+        Set<PlayerListEntryAction> actions = packet.getActions();
-            return;
-        }
 
-        PlayerListPacket translate = new PlayerListPacket();
+        if (actions.contains(PlayerListEntryAction.ADD_PLAYER)) {
-        translate.setAction(PlayerListPacket.Action.ADD);
+            for (PlayerListEntry entry : packet.getEntries()) {
+                GameProfile profile = entry.getProfile();
+                PlayerEntity playerEntity;
+                boolean self = profile.getId().equals(session.getPlayerEntity().getUuid());
 
-        for (PlayerListEntry entry : packet.getEntries()) {
+                GameProfile.Property textures = profile.getProperty("textures");
-            GameProfile profile = entry.getProfile();
+                String texturesProperty = textures == null ? null : textures.getValue();
-            PlayerEntity playerEntity;
-            boolean self = profile.getId().equals(session.getPlayerEntity().getUuid());
 
-            if (self) {
+                if (self) {
-                // Entity is ourself
+                    // Entity is ourself
-                playerEntity = session.getPlayerEntity();
+                    playerEntity = session.getPlayerEntity();
-            } else {
+                } else {
-                playerEntity = session.getEntityCache().getPlayerEntity(profile.getId());
+                    // It's a new player
-            }
+                    playerEntity = new PlayerEntity(
+                            session,
+                            -1,
+                            session.getEntityCache().getNextEntityId().incrementAndGet(),
+                            profile.getId(),
+                            Vector3f.ZERO,
+                            Vector3f.ZERO,
+                            0, 0, 0,
+                            profile.getName(),
+                            texturesProperty
+                    );
 
-            GameProfile.Property textures = profile.getProperty("textures");
+                    session.getEntityCache().addPlayerEntity(playerEntity);
-            String texturesProperty = textures == null ? null : textures.getValue();
+                }
-
-            if (playerEntity == null) {
-                // It's a new player
-                playerEntity = new PlayerEntity(
-                        session,
-                        -1,
-                        session.getEntityCache().getNextEntityId().incrementAndGet(),
-                        profile.getId(),
-                        Vector3f.ZERO,
-                        Vector3f.ZERO,
-                        0, 0, 0,
-                        profile.getName(),
-                        texturesProperty
-                );
-
-                session.getEntityCache().addPlayerEntity(playerEntity);
-            } else {
                 playerEntity.setUsername(profile.getName());
                 playerEntity.setTexturesProperty(texturesProperty);
-            }
 
-            playerEntity.setPlayerList(true);
 
-            // We'll send our own PlayerListEntry in requestAndHandleSkinAndCape
+                // We'll send our own PlayerListEntry in requestAndHandleSkinAndCape
-            // But we need to send other player's entries so they show up in the player list
+                // But we need to send other player's entries so they show up in the player list
-            // without processing their skin information - that'll be processed when they spawn in
+                // without processing their skin information - that'll be processed when they spawn in
-            if (self) {
+                if (self) {
-                SkinManager.requestAndHandleSkinAndCape(playerEntity, session, skinAndCape ->
+                    SkinManager.requestAndHandleSkinAndCape(playerEntity, session, skinAndCape ->
-                        GeyserImpl.getInstance().getLogger().debug("Loaded Local Bedrock Java Skin Data for " + session.getClientData().getUsername()));
+                            GeyserImpl.getInstance().getLogger().debug("Loaded Local Bedrock Java Skin Data for " + session.getClientData().getUsername()));
-            } else {
+                } else {
-                playerEntity.setValid(true);
+                    playerEntity.setValid(true);
-                PlayerListPacket.Entry playerListEntry = SkinManager.buildCachedEntry(session, playerEntity);
+                }
-
-                translate.getEntries().add(playerListEntry);
             }
         }
 
-        if (!translate.getEntries().isEmpty()) {
+        if (actions.contains(PlayerListEntryAction.UPDATE_LISTED)) {
-            session.sendUpstreamPacket(translate);
+            List<PlayerListPacket.Entry> toAdd = new ArrayList<>();
+            List<PlayerListPacket.Entry> toRemove = new ArrayList<>();
+
+            for (PlayerListEntry entry : packet.getEntries()) {
+                PlayerEntity entity = session.getEntityCache().getPlayerEntity(entry.getProfileId());
+                if (entity == null) {
+                    session.getGeyser().getLogger().debug("Ignoring player info update for " + entry.getProfileId());
+                    continue;
+                }
+
+                if (entry.isListed()) {
+                    PlayerListPacket.Entry playerListEntry = SkinManager.buildCachedEntry(session, entity);
+                    toAdd.add(playerListEntry);
+                } else {
+                    toRemove.add(new PlayerListPacket.Entry(entity.getTabListUuid()));
+                }
+            }
+
+            if (!toAdd.isEmpty()) {
+                PlayerListPacket tabListPacket = new PlayerListPacket();
+                tabListPacket.setAction(PlayerListPacket.Action.ADD);
+                tabListPacket.getEntries().addAll(toAdd);
+                session.sendUpstreamPacket(tabListPacket);
+            }
+            if (!toRemove.isEmpty()) {
+                PlayerListPacket tabListPacket = new PlayerListPacket();
+                tabListPacket.setAction(PlayerListPacket.Action.REMOVE);
+                tabListPacket.getEntries().addAll(toRemove);
+                session.sendUpstreamPacket(tabListPacket);
+            }
         }
     }
 }