Fix Registry#getKey implementation

13 files changed, 164 insertions, 277 deletions

diff --git a/paper-server/src/main/java/io/papermc/paper/util/Holderable.java b/paper-server/src/main/java/io/papermc/paper/util/Holderable.java
index 3401d0073f..c0115e0a31 100644
--- a/paper-server/src/main/java/io/papermc/paper/util/Holderable.java
+++ b/paper-server/src/main/java/io/papermc/paper/util/Holderable.java
@@ -7,7 +7,7 @@ import com.mojang.serialization.JsonOps;
 import net.kyori.adventure.key.Key;
 import net.minecraft.core.Holder;
 import net.minecraft.resources.RegistryOps;
-import org.bukkit.Keyed;
+import org.bukkit.NamespacedKey;
 import org.bukkit.Registry;
 import org.bukkit.craftbukkit.CraftRegistry;
 import org.bukkit.craftbukkit.util.Handleable;
@@ -25,7 +25,7 @@ public interface Holderable<M> extends Handleable<M> {
         return this.getHolder().value();
     }
 
-    static <T extends Keyed, M> @Nullable T fromBukkitSerializationObject(final Object deserialized, final Codec<? extends Holder<M>> codec, final Registry<T> registry) { // TODO remove Keyed
+    static <T extends org.bukkit.Keyed, M> @Nullable T fromBukkitSerializationObject(final Object deserialized, final Codec<? extends Holder<M>> codec, final Registry<T> registry) { // TODO remove Keyed
         return switch (deserialized) {
             case @Subst("key:value") final String string -> {
                 if (!(Key.parseable(string))) {
@@ -75,4 +75,12 @@ public interface Holderable<M> extends Handleable<M> {
     default String implToString() {
         return "%s{holder=%s}".formatted(this.getClass().getSimpleName(), this.getHolder().toString());
     }
+
+    default @Nullable NamespacedKey getKeyOrNull() {
+        return this.getHolder().unwrapKey().map(MCUtil::fromResourceKey).orElse(null);
+    }
+
+    default NamespacedKey getKey() {
+        return MCUtil.fromResourceKey(this.getHolder().unwrapKey().orElseThrow(() -> new IllegalStateException("Cannot get key for this registry item, because it is not registered.")));
+    }
 }
diff --git a/paper-server/src/main/java/io/papermc/paper/util/OldEnumHolderable.java b/paper-server/src/main/java/io/papermc/paper/util/OldEnumHolderable.java
new file mode 100644
index 0000000000..1e70146847
--- /dev/null
+++ b/paper-server/src/main/java/io/papermc/paper/util/OldEnumHolderable.java
@@ -0,0 +1,88 @@
+package io.papermc.paper.util;
+
+import com.google.common.base.Preconditions;
+import java.util.Locale;
+import net.minecraft.core.Holder;
+import org.bukkit.Keyed;
+import org.bukkit.NamespacedKey;
+import org.bukkit.util.OldEnum;
+import org.jspecify.annotations.NullMarked;
+import org.jspecify.annotations.Nullable;
+
+@SuppressWarnings("removal")
+@Deprecated
+@NullMarked
+public abstract class OldEnumHolderable<A extends OldEnum<A>, M> implements Holderable<M>, OldEnum<A>, Keyed {
+
+    private final Holder<M> holder;
+    private final int ordinal;
+    private final @Nullable String name;
+
+    protected OldEnumHolderable(final Holder<M> holder, final int ordinal) {
+        this.holder = holder;
+        this.ordinal = ordinal;
+        if (holder instanceof final Holder.Reference<M> reference) {
+            // For backwards compatibility, minecraft values will stile return the uppercase name without the namespace,
+            // in case plugins use for example the name as key in a config file to receive registry item specific values.
+            // Custom registry items will return the key with namespace. For a plugin this should look than like a new registry item
+            // (which can always be added in new minecraft versions and the plugin should therefore handle it accordingly).
+            if (NamespacedKey.MINECRAFT.equals(reference.key().location().getNamespace())) {
+                this.name = reference.key().location().getPath().toUpperCase(Locale.ROOT);
+            } else {
+                this.name = reference.key().location().toString();
+            }
+        } else {
+            this.name = null;
+        }
+    }
+
+    @Override
+    public Holder<M> getHolder() {
+        return this.holder;
+    }
+
+    @Override
+    @Deprecated
+    public int compareTo(A other) {
+        this.checkIsReference();
+        return this.ordinal - other.ordinal();
+    }
+
+    @Override
+    @Deprecated
+    public String name() {
+        this.checkIsReference();
+        return this.name;
+    }
+
+    @Override
+    @Deprecated
+    public int ordinal() {
+        this.checkIsReference();
+        return this.ordinal;
+    }
+
+    private void checkIsReference() {
+        Preconditions.checkState(this.holder.kind() == Holder.Kind.REFERENCE, "Cannot call method for this registry item, because it is not registered.");
+    }
+
+    @Override
+    public NamespacedKey getKey() {
+        return MCUtil.fromResourceKey(this.holder.unwrapKey().orElseThrow(() -> new IllegalStateException("Cannot get key for this registry item, because it is not registered.")));
+    }
+
+    @Override
+    public boolean equals(final Object obj) {
+        return this.implEquals(obj);
+    }
+
+    @Override
+    public int hashCode() {
+        return this.implHashCode();
+    }
+
+    @Override
+    public String toString() {
+        return this.implToString();
+    }
+}
diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/CraftArt.java b/paper-server/src/main/java/org/bukkit/craftbukkit/CraftArt.java
index 0207c7c507..d4edaea715 100644
--- a/paper-server/src/main/java/org/bukkit/craftbukkit/CraftArt.java
+++ b/paper-server/src/main/java/org/bukkit/craftbukkit/CraftArt.java
@@ -1,17 +1,15 @@
 package org.bukkit.craftbukkit;
 
-import com.google.common.base.Preconditions;
-import java.util.Locale;
+import io.papermc.paper.adventure.PaperAdventure;
+import io.papermc.paper.util.OldEnumHolderable;
+import net.kyori.adventure.text.Component;
 import net.minecraft.core.Holder;
 import net.minecraft.core.registries.Registries;
 import net.minecraft.world.entity.decoration.PaintingVariant;
 import org.bukkit.Art;
-import org.bukkit.NamespacedKey;
 import org.bukkit.Registry;
-import org.bukkit.craftbukkit.util.Handleable;
-import org.jetbrains.annotations.NotNull;
 
-public class CraftArt implements Art, Handleable<PaintingVariant> {
+public class CraftArt extends OldEnumHolderable<Art, PaintingVariant> implements Art {
 
     private static int count = 0;
 
@@ -20,7 +18,7 @@ public class CraftArt implements Art, Handleable<PaintingVariant> {
     }
 
     public static Art minecraftHolderToBukkit(Holder<PaintingVariant> minecraft) {
-        return CraftArt.minecraftToBukkit(minecraft.value());
+        return CraftRegistry.minecraftHolderToBukkit(minecraft, Registry.ART);
     }
 
     public static PaintingVariant bukkitToMinecraft(Art bukkit) {
@@ -28,118 +26,41 @@ public class CraftArt implements Art, Handleable<PaintingVariant> {
     }
 
     public static Holder<PaintingVariant> bukkitToMinecraftHolder(Art bukkit) {
-        Preconditions.checkArgument(bukkit != null);
-
-        net.minecraft.core.Registry<PaintingVariant> registry = CraftRegistry.getMinecraftRegistry(Registries.PAINTING_VARIANT);
-
-        if (registry.wrapAsHolder(CraftArt.bukkitToMinecraft(bukkit)) instanceof Holder.Reference<PaintingVariant> holder) {
-            return holder;
-        }
-
-        throw new IllegalArgumentException("No Reference holder found for " + bukkit
-                + ", this can happen if a plugin creates its own painting variant with out properly registering it.");
-    }
-
-    private final NamespacedKey key;
-    private final PaintingVariant paintingVariant;
-    private final String name;
-    private final int ordinal;
-
-    public CraftArt(NamespacedKey key, PaintingVariant paintingVariant) {
-        this.key = key;
-        this.paintingVariant = paintingVariant;
-        // For backwards compatibility, minecraft values will stile return the uppercase name without the namespace,
-        // in case plugins use for example the name as key in a config file to receive art specific values.
-        // Custom arts will return the key with namespace. For a plugin this should look than like a new art
-        // (which can always be added in new minecraft versions and the plugin should therefore handle it accordingly).
-        if (NamespacedKey.MINECRAFT.equals(key.getNamespace())) {
-            this.name = key.getKey().toUpperCase(Locale.ROOT);
-        } else {
-            this.name = key.toString();
-        }
-        this.ordinal = CraftArt.count++;
+        return CraftRegistry.bukkitToMinecraftHolder(bukkit, Registries.PAINTING_VARIANT);
     }
 
-    @Override
-    public PaintingVariant getHandle() {
-        return this.paintingVariant;
+    public CraftArt(Holder<PaintingVariant> paintingVariant) {
+        super(paintingVariant, count++);
     }
 
     @Override
     public int getBlockWidth() {
-        return this.paintingVariant.width();
+        return this.getHandle().width();
     }
 
     @Override
     public int getBlockHeight() {
-        return this.paintingVariant.height();
+        return this.getHandle().height();
     }
 
     // Paper start - Expand Art API
     @Override
-    public net.kyori.adventure.text.Component title() {
-        return this.paintingVariant.title().map(io.papermc.paper.adventure.PaperAdventure::asAdventure).orElse(null);
+    public Component title() {
+        return this.getHandle().title().map(PaperAdventure::asAdventure).orElse(null);
     }
 
     @Override
     public net.kyori.adventure.text.Component author() {
-        return this.paintingVariant.author().map(io.papermc.paper.adventure.PaperAdventure::asAdventure).orElse(null);
+        return this.getHandle().author().map(PaperAdventure::asAdventure).orElse(null);
     }
 
     public net.kyori.adventure.key.Key assetId() {
-        return io.papermc.paper.adventure.PaperAdventure.asAdventure(this.paintingVariant.assetId());
+        return PaperAdventure.asAdventure(this.getHandle().assetId());
     }
     // Paper end - Expand Art API
 
     @Override
     public int getId() {
-        return CraftRegistry.getMinecraftRegistry(Registries.PAINTING_VARIANT).getId(this.paintingVariant);
-    }
-
-    @NotNull
-    @Override
-    public NamespacedKey getKey() {
-        if (true) return java.util.Objects.requireNonNull(org.bukkit.Registry.ART.getKey(this), () -> this + " doesn't have a key"); // Paper
-        return this.key;
-    }
-
-    @Override
-    public int compareTo(@NotNull Art art) {
-        return this.ordinal - art.ordinal();
-    }
-
-    @NotNull
-    @Override
-    public String name() {
-        return this.name;
-    }
-
-    @Override
-    public int ordinal() {
-        return this.ordinal;
-    }
-
-    @Override
-    public String toString() {
-        // For backwards compatibility
-        return this.name();
-    }
-
-    @Override
-    public boolean equals(Object other) {
-        if (this == other) {
-            return true;
-        }
-
-        if (!(other instanceof CraftArt otherArt)) {
-            return false;
-        }
-
-        return this.getKey().equals(otherArt.getKey());
-    }
-
-    @Override
-    public int hashCode() {
-        return this.getKey().hashCode();
+        return CraftRegistry.getMinecraftRegistry(Registries.PAINTING_VARIANT).getId(this.getHandle());
     }
 }
diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/CraftMusicInstrument.java b/paper-server/src/main/java/org/bukkit/craftbukkit/CraftMusicInstrument.java
index aa19ac7513..520c3e2e1d 100644
--- a/paper-server/src/main/java/org/bukkit/craftbukkit/CraftMusicInstrument.java
+++ b/paper-server/src/main/java/org/bukkit/craftbukkit/CraftMusicInstrument.java
@@ -1,6 +1,7 @@
 package org.bukkit.craftbukkit;
 
 import com.google.common.base.Preconditions;
+import io.papermc.paper.util.Holderable;
 import net.minecraft.core.Holder;
 import net.minecraft.core.registries.Registries;
 import net.minecraft.world.item.Instrument;
@@ -75,8 +76,7 @@ public class CraftMusicInstrument extends MusicInstrument implements io.papermc.
     @NotNull
     @Override
     public NamespacedKey getKey() {
-        if (true) return java.util.Objects.requireNonNull(org.bukkit.Registry.INSTRUMENT.getKey(this), () -> this + " doesn't have a key"); // Paper
-        return this.key;
+        return Holderable.super.getKey();
     }
 
     // Paper start - add translationKey methods
diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/CraftRegistry.java b/paper-server/src/main/java/org/bukkit/craftbukkit/CraftRegistry.java
index a7e745bc13..a94cebfd26 100644
--- a/paper-server/src/main/java/org/bukkit/craftbukkit/CraftRegistry.java
+++ b/paper-server/src/main/java/org/bukkit/craftbukkit/CraftRegistry.java
@@ -1,6 +1,7 @@
 package org.bukkit.craftbukkit;
 
 import com.google.common.base.Preconditions;
+import io.papermc.paper.util.Holderable;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
@@ -197,7 +198,6 @@ public class CraftRegistry<B extends Keyed, M> implements Registry<B> {
 
     private final Class<?> bukkitClass; // Paper - relax preload class
     private final Map<NamespacedKey, B> cache = new HashMap<>();
-    private final Map<B, NamespacedKey> byValue = new java.util.IdentityHashMap<>(); // Paper - improve Registry
     private final net.minecraft.core.Registry<M> minecraftRegistry;
     private final io.papermc.paper.registry.entry.RegistryTypeMapper<M, B> minecraftToBukkit; // Paper - switch to Holder
     private final BiFunction<NamespacedKey, ApiVersion, NamespacedKey> serializationUpdater; // Paper - rename to make it *clear* what it is *only* for
@@ -251,7 +251,6 @@ public class CraftRegistry<B extends Keyed, M> implements Registry<B> {
         }
 
         this.cache.put(namespacedKey, bukkit);
-        this.byValue.put(bukkit, namespacedKey); // Paper - improve Registry
 
         return bukkit;
     }
@@ -298,7 +297,10 @@ public class CraftRegistry<B extends Keyed, M> implements Registry<B> {
     // Paper start - improve Registry
     @Override
     public NamespacedKey getKey(final B value) {
-        return this.byValue.get(value);
+        if (value instanceof Holderable<?> holderable) {
+            return holderable.getKeyOrNull();
+        }
+        return Registry.super.getKey(value);
     }
     // Paper end - improve Registry
 
diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/CraftSound.java b/paper-server/src/main/java/org/bukkit/craftbukkit/CraftSound.java
index e24d784674..451fe79b1e 100644
--- a/paper-server/src/main/java/org/bukkit/craftbukkit/CraftSound.java
+++ b/paper-server/src/main/java/org/bukkit/craftbukkit/CraftSound.java
@@ -1,17 +1,13 @@
 package org.bukkit.craftbukkit;
 
-import com.google.common.base.Preconditions;
-import java.util.Locale;
+import io.papermc.paper.util.OldEnumHolderable;
 import net.minecraft.core.Holder;
 import net.minecraft.core.registries.Registries;
 import net.minecraft.sounds.SoundEvent;
-import org.bukkit.NamespacedKey;
 import org.bukkit.Registry;
 import org.bukkit.Sound;
-import org.bukkit.craftbukkit.util.Handleable;
-import org.jetbrains.annotations.NotNull;
 
-public class CraftSound implements Sound, Handleable<SoundEvent> {
+public class CraftSound extends OldEnumHolderable<Sound, SoundEvent> implements Sound {
 
     private static int count = 0;
 
@@ -24,88 +20,11 @@ public class CraftSound implements Sound, Handleable<SoundEvent> {
     }
 
     public static Holder<SoundEvent> bukkitToMinecraftHolder(Sound bukkit) {
-        Preconditions.checkArgument(bukkit != null);
-
-        net.minecraft.core.Registry<SoundEvent> registry = CraftRegistry.getMinecraftRegistry(Registries.SOUND_EVENT);
-
-        if (registry.wrapAsHolder(CraftSound.bukkitToMinecraft(bukkit)) instanceof Holder.Reference<SoundEvent> holder) {
-            return holder;
-        }
-
-        throw new IllegalArgumentException("No Reference holder found for " + bukkit
-                + ", this can happen if a plugin creates its own sound effect with out properly registering it.");
-    }
-
-    private final NamespacedKey key;
-    private final SoundEvent soundEffect;
-    private final String name;
-    private final int ordinal;
-
-    public CraftSound(NamespacedKey key, SoundEvent soundEffect) {
-        this.key = key;
-        this.soundEffect = soundEffect;
-        // For backwards compatibility, minecraft values will stile return the uppercase name without the namespace,
-        // in case plugins use for example the name as key in a config file to receive sound specific values.
-        // Custom sounds will return the key with namespace. For a plugin this should look than like a new sound
-        // (which can always be added in new minecraft versions and the plugin should therefore handle it accordingly).
-        if (NamespacedKey.MINECRAFT.equals(key.getNamespace())) {
-            this.name = key.getKey().toUpperCase(Locale.ROOT).replace('.', '_');
-        } else {
-            this.name = key.toString();
-        }
-        this.ordinal = CraftSound.count++;
-    }
-
-    @Override
-    public SoundEvent getHandle() {
-        return this.soundEffect;
-    }
-
-    @NotNull
-    @Override
-    public NamespacedKey getKey() {
-        if (true) return java.util.Objects.requireNonNull(org.bukkit.Registry.SOUNDS.getKey(this), () -> this + " doesn't have a key"); // Paper
-        return this.key;
-    }
-
-    @Override
-    public int compareTo(@NotNull Sound sound) {
-        return this.ordinal - sound.ordinal();
-    }
-
-    @NotNull
-    @Override
-    public String name() {
-        return this.name;
-    }
-
-    @Override
-    public int ordinal() {
-        return this.ordinal;
-    }
-
-    @Override
-    public String toString() {
-        // For backwards compatibility
-        return this.name();
-    }
-
-    @Override
-    public boolean equals(Object other) {
-        if (this == other) {
-            return true;
-        }
-
-        if (!(other instanceof CraftSound otherSound)) {
-            return false;
-        }
-
-        return this.getKey().equals(otherSound.getKey());
+        return CraftRegistry.bukkitToMinecraftHolder(bukkit, Registries.SOUND_EVENT);
     }
 
-    @Override
-    public int hashCode() {
-        return this.getKey().hashCode();
+    public CraftSound(Holder<SoundEvent> soundEffect) {
+        super(soundEffect, count++);
     }
 
     // Paper start
diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/block/banner/CraftPatternType.java b/paper-server/src/main/java/org/bukkit/craftbukkit/block/banner/CraftPatternType.java
index 3addb382d0..749024b259 100644
--- a/paper-server/src/main/java/org/bukkit/craftbukkit/block/banner/CraftPatternType.java
+++ b/paper-server/src/main/java/org/bukkit/craftbukkit/block/banner/CraftPatternType.java
@@ -1,17 +1,14 @@
 package org.bukkit.craftbukkit.block.banner;
 
-import com.google.common.base.Preconditions;
-import java.util.Locale;
+import io.papermc.paper.util.OldEnumHolderable;
 import net.minecraft.core.Holder;
 import net.minecraft.core.registries.Registries;
 import net.minecraft.world.level.block.entity.BannerPattern;
-import org.bukkit.NamespacedKey;
 import org.bukkit.Registry;
 import org.bukkit.block.banner.PatternType;
 import org.bukkit.craftbukkit.CraftRegistry;
-import org.bukkit.craftbukkit.util.Handleable;
 
-public class CraftPatternType implements PatternType, Handleable<BannerPattern> {
+public class CraftPatternType extends OldEnumHolderable<PatternType, BannerPattern> implements PatternType {
 
     private static int count = 0;
 
@@ -20,7 +17,7 @@ public class CraftPatternType implements PatternType, Handleable<BannerPattern>
     }
 
     public static PatternType minecraftHolderToBukkit(Holder<BannerPattern> minecraft) {
-        return CraftPatternType.minecraftToBukkit(minecraft.value());
+        return CraftRegistry.minecraftHolderToBukkit(minecraft, Registry.BANNER_PATTERN);
     }
 
     public static BannerPattern bukkitToMinecraft(PatternType bukkit) {
@@ -28,86 +25,11 @@ public class CraftPatternType implements PatternType, Handleable<BannerPattern>
     }
 
     public static Holder<BannerPattern> bukkitToMinecraftHolder(PatternType bukkit) {
-        Preconditions.checkArgument(bukkit != null);
-
-        net.minecraft.core.Registry<BannerPattern> registry = CraftRegistry.getMinecraftRegistry(Registries.BANNER_PATTERN);
-
-        if (registry.wrapAsHolder(CraftPatternType.bukkitToMinecraft(bukkit)) instanceof Holder.Reference<BannerPattern> holder) {
-            return holder;
-        }
-
-        throw new IllegalArgumentException("No Reference holder found for " + bukkit
-                + ", this can happen if a plugin creates its own banner pattern without properly registering it.");
-    }
-
-    private final NamespacedKey key;
-    private final BannerPattern bannerPatternType;
-    private final String name;
-    private final int ordinal;
-
-    public CraftPatternType(NamespacedKey key, BannerPattern bannerPatternType) {
-        this.key = key;
-        this.bannerPatternType = bannerPatternType;
-        // For backwards compatibility, minecraft values will stile return the uppercase name without the namespace,
-        // in case plugins use for example the name as key in a config file to receive pattern type specific values.
-        // Custom pattern types will return the key with namespace. For a plugin this should look than like a new pattern type
-        // (which can always be added in new minecraft versions and the plugin should therefore handle it accordingly).
-        if (NamespacedKey.MINECRAFT.equals(key.getNamespace())) {
-            this.name = key.getKey().toUpperCase(Locale.ROOT);
-        } else {
-            this.name = key.toString();
-        }
-        this.ordinal = CraftPatternType.count++;
-    }
-
-    @Override
-    public BannerPattern getHandle() {
-        return this.bannerPatternType;
-    }
-
-    @Override
-    public NamespacedKey getKey() {
-        if (true) return java.util.Objects.requireNonNull(org.bukkit.Registry.BANNER_PATTERN.getKey(this), () -> this + " doesn't have a key"); // Paper
-        return this.key;
-    }
-
-    @Override
-    public int compareTo(PatternType patternType) {
-        return this.ordinal - patternType.ordinal();
+        return CraftRegistry.bukkitToMinecraftHolder(bukkit, Registries.BANNER_PATTERN);
     }
 
-    @Override
-    public String name() {
-        return this.name;
-    }
-
-    @Override
-    public int ordinal() {
-        return this.ordinal;
-    }
-
-    @Override
-    public String toString() {
-        // For backwards compatibility
-        return this.name();
-    }
-
-    @Override
-    public boolean equals(Object other) {
-        if (this == other) {
-            return true;
-        }
-
-        if (!(other instanceof CraftPatternType)) {
-            return false;
-        }
-
-        return this.getKey().equals(((PatternType) other).getKey());
-    }
-
-    @Override
-    public int hashCode() {
-        return this.getKey().hashCode();
+    public CraftPatternType(Holder<BannerPattern> bannerPatternType) {
+       super(bannerPatternType, count++);
     }
 
     @Override
diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/generator/structure/CraftStructure.java b/paper-server/src/main/java/org/bukkit/craftbukkit/generator/structure/CraftStructure.java
index 7ef679fdd2..09fd006244 100644
--- a/paper-server/src/main/java/org/bukkit/craftbukkit/generator/structure/CraftStructure.java
+++ b/paper-server/src/main/java/org/bukkit/craftbukkit/generator/structure/CraftStructure.java
@@ -42,7 +42,6 @@ public class CraftStructure extends Structure implements Handleable<net.minecraf
 
     @Override
     public NamespacedKey getKey() {
-        if (true) return java.util.Objects.requireNonNull(org.bukkit.Registry.STRUCTURE.getKey(this), () -> this + " doesn't have a key"); // Paper
         return this.key;
     }
 }
diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/inventory/trim/CraftTrimMaterial.java b/paper-server/src/main/java/org/bukkit/craftbukkit/inventory/trim/CraftTrimMaterial.java
index afee459c4d..9b7488cb45 100644
--- a/paper-server/src/main/java/org/bukkit/craftbukkit/inventory/trim/CraftTrimMaterial.java
+++ b/paper-server/src/main/java/org/bukkit/craftbukkit/inventory/trim/CraftTrimMaterial.java
@@ -1,6 +1,7 @@
 package org.bukkit.craftbukkit.inventory.trim;
 
 import com.google.common.base.Preconditions;
+import io.papermc.paper.util.Holderable;
 import net.minecraft.core.Holder;
 import net.minecraft.core.registries.Registries;
 import net.minecraft.network.chat.contents.TranslatableContents;
@@ -77,8 +78,7 @@ public class CraftTrimMaterial implements TrimMaterial, io.papermc.paper.util.Ho
     @Override
     @NotNull
     public NamespacedKey getKey() {
-        if (true) return java.util.Objects.requireNonNull(org.bukkit.Registry.TRIM_MATERIAL.getKey(this), () -> this + " doesn't have a key"); // Paper
-        return this.key;
+        return Holderable.super.getKey();
     }
 
     @NotNull
diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/inventory/trim/CraftTrimPattern.java b/paper-server/src/main/java/org/bukkit/craftbukkit/inventory/trim/CraftTrimPattern.java
index da951bd7e4..b5b209d16a 100644
--- a/paper-server/src/main/java/org/bukkit/craftbukkit/inventory/trim/CraftTrimPattern.java
+++ b/paper-server/src/main/java/org/bukkit/craftbukkit/inventory/trim/CraftTrimPattern.java
@@ -1,6 +1,7 @@
 package org.bukkit.craftbukkit.inventory.trim;
 
 import com.google.common.base.Preconditions;
+import io.papermc.paper.util.Holderable;
 import net.minecraft.core.Holder;
 import net.minecraft.core.registries.Registries;
 import net.minecraft.network.chat.contents.TranslatableContents;
@@ -77,8 +78,7 @@ public class CraftTrimPattern implements TrimPattern, io.papermc.paper.util.Hold
     @Override
     @NotNull
     public NamespacedKey getKey() {
-        if (true) return java.util.Objects.requireNonNull(org.bukkit.Registry.TRIM_PATTERN.getKey(this), () -> this + " doesn't have a key"); // Paper
-        return this.key;
+        return Holderable.super.getKey();
     }
 
     @NotNull
diff --git a/paper-server/src/test/java/org/bukkit/registry/RegistryConversionTest.java b/paper-server/src/test/java/org/bukkit/registry/RegistryConversionTest.java
index 092b88e0ac..293af3511c 100644
--- a/paper-server/src/test/java/org/bukkit/registry/RegistryConversionTest.java
+++ b/paper-server/src/test/java/org/bukkit/registry/RegistryConversionTest.java
@@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.*;
 import static org.junit.jupiter.api.Assumptions.*;
 import static org.mockito.Mockito.*;
 import com.google.common.base.Joiner;
+import io.papermc.paper.registry.RegistryKey;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
@@ -259,6 +260,8 @@ public class RegistryConversionTest {
                 Joiner.on('\n').withKeyValueSeparator(" got: ").join(notMatching)));
     }
 
+    static final Set<RegistryKey<?>> IGNORE_FOR_DIRECT_HOLDER = Set.of(RegistryKey.TRIM_MATERIAL, RegistryKey.TRIM_PATTERN, RegistryKey.INSTRUMENT, RegistryKey.PAINTING_VARIANT, RegistryKey.BANNER_PATTERN, RegistryKey.SOUND_EVENT); // Paper
+
     /**
      * Minecraft registry can return a default key / value
      * when the passed minecraft value is not registry in this case, we want it to throw an error.
@@ -269,7 +272,7 @@ public class RegistryConversionTest {
                                                       Class<? extends Keyed> craftClazz, Class<?> minecraftClazz) throws IllegalAccessException {
         this.checkValidMinecraftToBukkit(clazz);
 
-        if (type == io.papermc.paper.registry.RegistryKey.TRIM_MATERIAL || type == io.papermc.paper.registry.RegistryKey.TRIM_PATTERN || type == io.papermc.paper.registry.RegistryKey.INSTRUMENT) return; // Paper - manually skip for now
+        assumeFalse(IGNORE_FOR_DIRECT_HOLDER.contains(type), "skipped because these types support direct holders"); // Paper - manually skip for now
         try {
 
             Object minecraft = mock(minecraftClazz);
diff --git a/test-plugin/src/main/java/io/papermc/testplugin/TestPlugin.java b/test-plugin/src/main/java/io/papermc/testplugin/TestPlugin.java
index 671c37fa40..c640638154 100644
--- a/test-plugin/src/main/java/io/papermc/testplugin/TestPlugin.java
+++ b/test-plugin/src/main/java/io/papermc/testplugin/TestPlugin.java
@@ -1,5 +1,9 @@
 package io.papermc.testplugin;
 
+import io.papermc.paper.registry.RegistryAccess;
+import io.papermc.paper.registry.RegistryKey;
+import org.bukkit.Art;
+import org.bukkit.Registry;
 import org.bukkit.event.Listener;
 import org.bukkit.plugin.java.JavaPlugin;
 
@@ -10,6 +14,10 @@ public final class TestPlugin extends JavaPlugin implements Listener {
         this.getServer().getPluginManager().registerEvents(this, this);
 
         // io.papermc.testplugin.brigtests.Registration.registerViaOnEnable(this);
+
+        final Registry<Art> registry = RegistryAccess.registryAccess().getRegistry(RegistryKey.PAINTING_VARIANT);
+        final Art art = registry.get(TestPluginBootstrap.NEW);
+        System.out.println(art.getKey());
     }
 
 }
diff --git a/test-plugin/src/main/java/io/papermc/testplugin/TestPluginBootstrap.java b/test-plugin/src/main/java/io/papermc/testplugin/TestPluginBootstrap.java
index fe2b287b25..5d3ec9f604 100644
--- a/test-plugin/src/main/java/io/papermc/testplugin/TestPluginBootstrap.java
+++ b/test-plugin/src/main/java/io/papermc/testplugin/TestPluginBootstrap.java
@@ -2,13 +2,30 @@ package io.papermc.testplugin;
 
 import io.papermc.paper.plugin.bootstrap.BootstrapContext;
 import io.papermc.paper.plugin.bootstrap.PluginBootstrap;
+import io.papermc.paper.registry.TypedKey;
+import io.papermc.paper.registry.event.RegistryEvents;
+import io.papermc.paper.registry.keys.PaintingVariantKeys;
+import net.kyori.adventure.key.Key;
+import org.bukkit.Art;
 import org.jetbrains.annotations.NotNull;
 
+import static net.kyori.adventure.text.Component.text;
+
 public class TestPluginBootstrap implements PluginBootstrap {
 
+    static final TypedKey<Art> NEW = PaintingVariantKeys.create(Key.key("test:test"));
     @Override
     public void bootstrap(@NotNull BootstrapContext context) {
         // io.papermc.testplugin.brigtests.Registration.registerViaBootstrap(context);
+
+        context.getLifecycleManager().registerEventHandler(RegistryEvents.PAINTING_VARIANT.freeze(), event -> {
+            event.registry().register(NEW, builder -> {
+                builder.assetId(Key.key("wind"))
+                    .author(text("ME"))
+                    .width(2)
+                    .height(2);
+            });
+        });
     }
 
 }