SPIGOT-7882, #1467: Fix conversion of name in Profile Component to empty if it is missing

By: Doc <nachito94@msn.com>

paper-server/src/main/java/org/bukkit/craftbukkit/block/CraftSkull.java

@@ -27,7 +27,7 @@ import org.jetbrains.annotations.Nullable;
 public class CraftSkull extends CraftBlockEntityState<TileEntitySkull> implements Skull {
 
     private static final int MAX_OWNER_LENGTH = 16;
-    private GameProfile profile;
+    private ResolvableProfile profile;
 
     public CraftSkull(World world, TileEntitySkull tileEntity) {
         super(world, tileEntity);
@@ -43,7 +43,7 @@ public class CraftSkull extends CraftBlockEntityState<TileEntitySkull> implement
 
         ResolvableProfile owner = skull.getOwnerProfile();
         if (owner != null) {
-            profile = owner.gameProfile();
+            profile = owner;
         }
     }
 
@@ -54,7 +54,7 @@ public class CraftSkull extends CraftBlockEntityState<TileEntitySkull> implement
 
     @Override
     public String getOwner() {
-        return hasOwner() ? profile.getName() : null;
+        return hasOwner() ? profile.name().orElse(null) : null;
     }
 
     @Override
@@ -68,19 +68,19 @@ public class CraftSkull extends CraftBlockEntityState<TileEntitySkull> implement
             return false;
         }
 
-        this.profile = profile;
+        this.profile = new ResolvableProfile(profile);
         return true;
     }
 
     @Override
     public OfflinePlayer getOwningPlayer() {
-        if (profile != null) {
+        if (hasOwner()) {
-            if (!profile.getId().equals(SystemUtils.NIL_UUID)) {
+            if (profile.id().filter(u -> !u.equals(SystemUtils.NIL_UUID)).isPresent()) {
-                return Bukkit.getOfflinePlayer(profile.getId());
+                return Bukkit.getOfflinePlayer(profile.id().get());
             }
 
-            if (!profile.getName().isEmpty()) {
+            if (profile.name().filter(s -> !s.isEmpty()).isPresent()) {
-                return Bukkit.getOfflinePlayer(profile.getName());
+                return Bukkit.getOfflinePlayer(profile.name().get());
             }
         }
 
@@ -91,10 +91,10 @@ public class CraftSkull extends CraftBlockEntityState<TileEntitySkull> implement
     public void setOwningPlayer(OfflinePlayer player) {
         Preconditions.checkNotNull(player, "player");
 
-        if (player instanceof CraftPlayer) {
+        if (player instanceof CraftPlayer craftPlayer) {
-            this.profile = ((CraftPlayer) player).getProfile();
+            this.profile = new ResolvableProfile(craftPlayer.getProfile());
         } else {
-            this.profile = new GameProfile(player.getUniqueId(), player.getName());
+            this.profile = new ResolvableProfile(new GameProfile(player.getUniqueId(), (player.getName() == null) ? "" : player.getName()));
         }
     }
 
@@ -112,7 +112,7 @@ public class CraftSkull extends CraftBlockEntityState<TileEntitySkull> implement
         if (profile == null) {
             this.profile = null;
         } else {
-            this.profile = CraftPlayerProfile.validateSkullProfile(((CraftPlayerProfile) profile).buildGameProfile());
+            this.profile = new ResolvableProfile(CraftPlayerProfile.validateSkullProfile(((CraftPlayerProfile) profile).buildGameProfile()));
         }
     }
 
@@ -134,7 +134,7 @@ public class CraftSkull extends CraftBlockEntityState<TileEntitySkull> implement
     @Override
     public BlockFace getRotation() {
         BlockData blockData = getBlockData();
-        return (blockData instanceof Rotatable) ? ((Rotatable) blockData).getRotation() : ((Directional) blockData).getFacing();
+        return (blockData instanceof Rotatable rotatable) ? rotatable.getRotation() : ((Directional) blockData).getFacing();
     }
 
     @Override
@@ -187,7 +187,7 @@ public class CraftSkull extends CraftBlockEntityState<TileEntitySkull> implement
         super.applyTo(skull);
 
         if (getSkullType() == SkullType.PLAYER) {
-            skull.setOwner((profile != null) ? new ResolvableProfile(profile) : null);
+            skull.setOwner(hasOwner() ? profile : null);
         }
     }
 

paper-server/src/main/java/org/bukkit/craftbukkit/inventory/CraftMetaSkull.java

@@ -36,15 +36,14 @@ class CraftMetaSkull extends CraftMetaItem implements SkullMeta {
     static final ItemMetaKeyType<MinecraftKey> NOTE_BLOCK_SOUND = new ItemMetaKeyType<>(DataComponents.NOTE_BLOCK_SOUND, "note_block_sound");
     static final int MAX_OWNER_LENGTH = 16;
 
-    private GameProfile profile;
+    private ResolvableProfile profile;
     private MinecraftKey noteBlockSound;
 
     CraftMetaSkull(CraftMetaItem meta) {
         super(meta);
-        if (!(meta instanceof CraftMetaSkull)) {
+        if (!(meta instanceof CraftMetaSkull skullMeta)) {
             return;
         }
-        CraftMetaSkull skullMeta = (CraftMetaSkull) meta;
         this.setProfile(skullMeta.profile);
         this.noteBlockSound = skullMeta.noteBlockSound;
     }
@@ -52,21 +51,17 @@ class CraftMetaSkull extends CraftMetaItem implements SkullMeta {
     CraftMetaSkull(DataComponentPatch tag) {
         super(tag);
 
-        getOrEmpty(tag, SKULL_PROFILE).ifPresent((resolvableProfile) -> {
+        getOrEmpty(tag, SKULL_PROFILE).ifPresent(this::setProfile);
-            this.setProfile(resolvableProfile.gameProfile());
-        });
 
-        getOrEmpty(tag, NOTE_BLOCK_SOUND).ifPresent((minecraftKey) -> {
+        getOrEmpty(tag, NOTE_BLOCK_SOUND).ifPresent((minecraftKey) -> this.noteBlockSound = minecraftKey);
-            this.noteBlockSound = minecraftKey;
-        });
     }
 
     CraftMetaSkull(Map<String, Object> map) {
         super(map);
         if (profile == null) {
             Object object = map.get(SKULL_OWNER.BUKKIT);
-            if (object instanceof PlayerProfile) {
+            if (object instanceof PlayerProfile playerProfile) {
-                setOwnerProfile((PlayerProfile) object);
+                setOwnerProfile(playerProfile);
             } else {
                 setOwner(SerializableMeta.getString(map, SKULL_OWNER.BUKKIT, true));
             }
@@ -92,7 +87,7 @@ class CraftMetaSkull extends CraftMetaItem implements SkullMeta {
                 skullTag.putUUID("Id", uuid);
             }
 
-            this.setProfile(ResolvableProfile.CODEC.parse(DynamicOpsNBT.INSTANCE, skullTag).result().get().gameProfile());
+            ResolvableProfile.CODEC.parse(DynamicOpsNBT.INSTANCE, skullTag).result().ifPresent(this::setProfile);
         }
 
         if (tag.contains(BLOCK_ENTITY_TAG.NBT, CraftMagicNumbers.NBT.TAG_COMPOUND)) {
@@ -103,7 +98,7 @@ class CraftMetaSkull extends CraftMetaItem implements SkullMeta {
         }
     }
 
-    private void setProfile(GameProfile profile) {
+    private void setProfile(ResolvableProfile profile) {
         this.profile = profile;
     }
 
@@ -111,15 +106,15 @@ class CraftMetaSkull extends CraftMetaItem implements SkullMeta {
     void applyToItem(CraftMetaItem.Applicator tag) {
         super.applyToItem(tag);
 
-        if (profile != null) {
+        if (hasOwner()) {
             // SPIGOT-6558: Set initial textures
-            tag.put(SKULL_PROFILE, new ResolvableProfile(profile));
+            tag.put(SKULL_PROFILE, profile);
             // Fill in textures
             PlayerProfile ownerProfile = new CraftPlayerProfile(profile); // getOwnerProfile may return null
             if (ownerProfile.getTextures().isEmpty()) {
                 ownerProfile.update().thenAccept((filledProfile) -> {
                     setOwnerProfile(filledProfile);
-                    tag.put(SKULL_PROFILE, new ResolvableProfile(profile));
+                    tag.put(SKULL_PROFILE, profile);
                 });
             }
         }
@@ -145,23 +140,23 @@ class CraftMetaSkull extends CraftMetaItem implements SkullMeta {
 
     @Override
     public boolean hasOwner() {
-        return profile != null && !profile.getName().isEmpty();
+        return profile != null;
     }
 
     @Override
     public String getOwner() {
-        return hasOwner() ? profile.getName() : null;
+        return hasOwner() ? profile.name().orElse(null) : null;
     }
 
     @Override
     public OfflinePlayer getOwningPlayer() {
         if (hasOwner()) {
-            if (!profile.getId().equals(SystemUtils.NIL_UUID)) {
+            if (profile.id().filter(u -> !u.equals(SystemUtils.NIL_UUID)).isPresent()) {
-                return Bukkit.getOfflinePlayer(profile.getId());
+                return Bukkit.getOfflinePlayer(profile.id().get());
             }
 
-            if (!profile.getName().isEmpty()) {
+            if (profile.name().filter(s -> !s.isEmpty()).isPresent()) {
-                return Bukkit.getOfflinePlayer(profile.getName());
+                return Bukkit.getOfflinePlayer(profile.name().get());
             }
         }
 
@@ -177,7 +172,7 @@ class CraftMetaSkull extends CraftMetaItem implements SkullMeta {
         if (name == null) {
             setProfile(null);
         } else {
-            setProfile(new GameProfile(SystemUtils.NIL_UUID, name));
+            setProfile(new ResolvableProfile(new GameProfile(SystemUtils.NIL_UUID, name)));
         }
 
         return true;
@@ -187,10 +182,10 @@ class CraftMetaSkull extends CraftMetaItem implements SkullMeta {
     public boolean setOwningPlayer(OfflinePlayer owner) {
         if (owner == null) {
             setProfile(null);
-        } else if (owner instanceof CraftPlayer) {
+        } else if (owner instanceof CraftPlayer craftPlayer) {
-            setProfile(((CraftPlayer) owner).getProfile());
+            setProfile(new ResolvableProfile(craftPlayer.getProfile()));
         } else {
-            setProfile(new GameProfile(owner.getUniqueId(), owner.getName()));
+            setProfile(new ResolvableProfile(new GameProfile(owner.getUniqueId(), (owner.getName() == null) ? "" : owner.getName())));
         }
 
         return true;
@@ -207,10 +202,10 @@ class CraftMetaSkull extends CraftMetaItem implements SkullMeta {
 
     @Override
     public void setOwnerProfile(PlayerProfile profile) {
-        if (profile == null) {
+        if (profile instanceof CraftPlayerProfile craftPlayerProfile) {
-            setProfile(null);
+            setProfile(CraftPlayerProfile.validateSkullProfile(craftPlayerProfile.buildResolvableProfile()));
         } else {
-            setProfile(CraftPlayerProfile.validateSkullProfile(((CraftPlayerProfile) profile).buildGameProfile()));
+            setProfile(null);
         }
     }
 
@@ -246,11 +241,8 @@ class CraftMetaSkull extends CraftMetaItem implements SkullMeta {
         if (!super.equalsCommon(meta)) {
             return false;
         }
-        if (meta instanceof CraftMetaSkull) {
+        if (meta instanceof CraftMetaSkull that) {
-            CraftMetaSkull that = (CraftMetaSkull) meta;
+            return Objects.equals(this.profile, that.profile) && Objects.equals(this.noteBlockSound, that.noteBlockSound);
-
-            // SPIGOT-5403: equals does not check properties
-            return (this.profile != null ? that.profile != null && this.profile.equals(that.profile) && this.profile.getProperties().equals(that.profile.getProperties()) : that.profile == null) && Objects.equals(this.noteBlockSound, that.noteBlockSound);
         }
         return true;
     }
@@ -264,7 +256,7 @@ class CraftMetaSkull extends CraftMetaItem implements SkullMeta {
     Builder<String, Object> serialize(Builder<String, Object> builder) {
         super.serialize(builder);
 
-        if (this.profile != null) {
+        if (this.hasOwner()) {
             builder.put(SKULL_OWNER.BUKKIT, new CraftPlayerProfile(this.profile));
         }
 

paper-server/src/main/java/org/bukkit/craftbukkit/profile/CraftPlayerProfile.java

@@ -12,6 +12,7 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
 import java.util.stream.Collectors;
@@ -19,6 +20,7 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import net.minecraft.SystemUtils;
 import net.minecraft.server.dedicated.DedicatedServer;
+import net.minecraft.world.item.component.ResolvableProfile;
 import org.apache.commons.lang.StringUtils;
 import org.bukkit.Bukkit;
 import org.bukkit.configuration.serialization.SerializableAs;
@@ -26,6 +28,7 @@ import org.bukkit.craftbukkit.CraftServer;
 import org.bukkit.craftbukkit.configuration.ConfigSerializationUtil;
 import org.bukkit.profile.PlayerProfile;
 import org.bukkit.profile.PlayerTextures;
+import org.jetbrains.annotations.ApiStatus;
 
 @SerializableAs("PlayerProfile")
 public final class CraftPlayerProfile implements PlayerProfile {
@@ -40,6 +43,15 @@ public final class CraftPlayerProfile implements PlayerProfile {
         return gameProfile;
     }
 
+    @Nonnull
+    public static ResolvableProfile validateSkullProfile(@Nonnull ResolvableProfile resolvableProfile) {
+        // The ResolvableProfile needs to contain either both a uuid and textures, or a name.
+        boolean isValidSkullProfile = (resolvableProfile.name().isPresent())
+                || (resolvableProfile.id().isPresent() && resolvableProfile.properties().containsKey(CraftPlayerTextures.PROPERTY_NAME));
+        Preconditions.checkArgument(isValidSkullProfile, "The skull profile is missing a name or textures!");
+        return resolvableProfile;
+    }
+
     @Nullable
     public static Property getProperty(@Nonnull GameProfile profile, String propertyName) {
         return Iterables.getFirst(profile.getProperties().get(propertyName), null);
@@ -53,15 +65,21 @@ public final class CraftPlayerProfile implements PlayerProfile {
 
     public CraftPlayerProfile(UUID uniqueId, String name) {
         Preconditions.checkArgument((uniqueId != null) || !StringUtils.isBlank(name), "uniqueId is null or name is blank");
-        this.uniqueId = (uniqueId == null) ? SystemUtils.NIL_UUID : uniqueId;
+        this.uniqueId = uniqueId;
-        this.name = (name == null) ? "" : name;
+        this.name = name;
+    }
+
+    @ApiStatus.Internal
+    public CraftPlayerProfile(@Nonnull ResolvableProfile resolvableProfile) {
+        this(resolvableProfile.id().orElse(null), resolvableProfile.name().orElse(null));
+        this.properties.putAll(resolvableProfile.properties());
     }
 
     // The Map of properties of the given GameProfile is not immutable. This captures a snapshot of the properties of
     // the given GameProfile at the time this CraftPlayerProfile is created.
     public CraftPlayerProfile(@Nonnull GameProfile gameProfile) {
         this(gameProfile.getId(), gameProfile.getName());
-        properties.putAll(gameProfile.getProperties());
+        this.properties.putAll(gameProfile.getProperties());
     }
 
     private CraftPlayerProfile(@Nonnull CraftPlayerProfile other) {
@@ -72,12 +90,12 @@ public final class CraftPlayerProfile implements PlayerProfile {
 
     @Override
     public UUID getUniqueId() {
-        return (uniqueId.equals(SystemUtils.NIL_UUID)) ? null : uniqueId;
+        return (Objects.equals(uniqueId, SystemUtils.NIL_UUID)) ? null : uniqueId;
     }
 
     @Override
     public String getName() {
-        return (name.isEmpty()) ? null : name;
+        return (StringUtils.isBlank(name)) ? null : name;
     }
 
     @Nullable
@@ -145,6 +163,14 @@ public final class CraftPlayerProfile implements PlayerProfile {
         return new CraftPlayerProfile(profile);
     }
 
+    // This always returns a new GameProfile instance to ensure that property changes to the original or previously
+    // built ResolvableProfile don't affect the use of this profile in other contexts.
+    @Nonnull
+    public ResolvableProfile buildResolvableProfile() {
+        rebuildDirtyProperties();
+        return new ResolvableProfile(Optional.ofNullable(this.name), Optional.ofNullable(this.uniqueId), this.properties);
+    }
+
     // This always returns a new GameProfile instance to ensure that property changes to the original or previously
     // built GameProfiles don't affect the use of this profile in other contexts.
     @Nonnull
@@ -184,8 +210,7 @@ public final class CraftPlayerProfile implements PlayerProfile {
     @Override
     public boolean equals(Object obj) {
         if (this == obj) return true;
-        if (!(obj instanceof CraftPlayerProfile)) return false;
+        if (!(obj instanceof CraftPlayerProfile other)) return false;
-        CraftPlayerProfile other = (CraftPlayerProfile) obj;
         if (!Objects.equals(uniqueId, other.uniqueId)) return false;
         if (!Objects.equals(name, other.name)) return false;
 
@@ -238,18 +263,16 @@ public final class CraftPlayerProfile implements PlayerProfile {
     @Override
     public Map<String, Object> serialize() {
         Map<String, Object> map = new LinkedHashMap<>();
-        if (getUniqueId() != null) {
+        if (this.uniqueId != null) {
-            map.put("uniqueId", getUniqueId().toString());
+            map.put("uniqueId", this.uniqueId.toString());
         }
-        if (getName() != null) {
+        if (this.name != null) {
             map.put("name", getName());
         }
         rebuildDirtyProperties();
-        if (!properties.isEmpty()) {
+        if (!this.properties.isEmpty()) {
             List<Object> propertiesData = new ArrayList<>();
-            properties.forEach((propertyName, property) -> {
+            this.properties.forEach((propertyName, property) -> propertiesData.add(CraftProfileProperty.serialize(property)));
-                propertiesData.add(CraftProfileProperty.serialize(property));
-            });
             map.put("properties", propertiesData);
         }
         return map;
@@ -264,7 +287,7 @@ public final class CraftPlayerProfile implements PlayerProfile {
 
         if (map.containsKey("properties")) {
             for (Object propertyData : (List<?>) map.get("properties")) {
-                Preconditions.checkArgument(propertyData instanceof Map, "Propertu data (%s) is not a valid Map", propertyData);
+                Preconditions.checkArgument(propertyData instanceof Map, "Property data (%s) is not a valid Map", propertyData);
                 Property property = CraftProfileProperty.deserialize((Map<?, ?>) propertyData);
                 profile.properties.put(property.name(), property);
             }

paper-server/src/test/java/org/bukkit/craftbukkit/profile/PlayerProfileTest.java

@@ -7,6 +7,7 @@ import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.UUID;
 import net.minecraft.SystemUtils;
+import net.minecraft.world.item.component.ResolvableProfile;
 import org.bukkit.configuration.InvalidConfigurationException;
 import org.bukkit.configuration.file.YamlConfiguration;
 import org.bukkit.configuration.serialization.ConfigurationSerialization;
@@ -90,7 +91,10 @@ public class PlayerProfileTest {
     public void testGameProfileWrapping() {
         // Invalid profiles:
         assertThrows(NullPointerException.class, () -> {
-            new CraftPlayerProfile(null);
+            new CraftPlayerProfile((GameProfile) null);
+        });
+        assertThrows(NullPointerException.class, () -> {
+            new CraftPlayerProfile((ResolvableProfile) null);
         });
 
         // Valid profiles: