split direct holder support up from ctor accepting Holder

Enchantment shouldn't support direct holders despite the ctor
accepting a Holder type. We want to limit the types
to ones that are actually used as direct holders in the game

paper-server/src/main/java/io/papermc/paper/registry/PaperRegistries.java

@@ -97,21 +97,21 @@ public final class PaperRegistries {
             start(Registries.MENU, RegistryKey.MENU).craft(MenuType.class, CraftMenuType::new).build(),
             start(Registries.ATTRIBUTE, RegistryKey.ATTRIBUTE).craft(Attribute.class, CraftAttribute::new).build(),
             start(Registries.FLUID, RegistryKey.FLUID).craft(Fluid.class, CraftFluid::new).build(),
-            start(Registries.SOUND_EVENT, RegistryKey.SOUND_EVENT).craft(Sound.class, CraftSound::new).build(),
+            start(Registries.SOUND_EVENT, RegistryKey.SOUND_EVENT).craft(Sound.class, CraftSound::new, true).build(),
             start(Registries.DATA_COMPONENT_TYPE, RegistryKey.DATA_COMPONENT_TYPE).craft(DataComponentTypes.class, PaperDataComponentType::of).build(),
 
             // data-drivens
             start(Registries.BIOME, RegistryKey.BIOME).craft(Biome.class, CraftBiome::new).build().delayed(),
             start(Registries.STRUCTURE, RegistryKey.STRUCTURE).craft(Structure.class, CraftStructure::new).build().delayed(),
-            start(Registries.TRIM_MATERIAL, RegistryKey.TRIM_MATERIAL).craft(TrimMaterial.class, CraftTrimMaterial::new).build().delayed(),
+            start(Registries.TRIM_MATERIAL, RegistryKey.TRIM_MATERIAL).craft(TrimMaterial.class, CraftTrimMaterial::new, true).build().delayed(),
-            start(Registries.TRIM_PATTERN, RegistryKey.TRIM_PATTERN).craft(TrimPattern.class, CraftTrimPattern::new).build().delayed(),
+            start(Registries.TRIM_PATTERN, RegistryKey.TRIM_PATTERN).craft(TrimPattern.class, CraftTrimPattern::new, true).build().delayed(),
             start(Registries.DAMAGE_TYPE, RegistryKey.DAMAGE_TYPE).craft(DamageType.class, CraftDamageType::new).writable(PaperDamageTypeRegistryEntry.PaperBuilder::new).delayed(),
             start(Registries.WOLF_VARIANT, RegistryKey.WOLF_VARIANT).craft(Wolf.Variant.class, CraftWolf.CraftVariant::new).build().delayed(),
             start(Registries.ENCHANTMENT, RegistryKey.ENCHANTMENT).craft(Enchantment.class, CraftEnchantment::new).serializationUpdater(FieldRename.ENCHANTMENT_RENAME).writable(PaperEnchantmentRegistryEntry.PaperBuilder::new).delayed(),
             start(Registries.JUKEBOX_SONG, RegistryKey.JUKEBOX_SONG).craft(JukeboxSong.class, CraftJukeboxSong::new).build().delayed(),
-            start(Registries.BANNER_PATTERN, RegistryKey.BANNER_PATTERN).craft(PatternType.class, CraftPatternType::new).writable(PaperBannerPatternRegistryEntry.PaperBuilder::new).delayed(),
+            start(Registries.BANNER_PATTERN, RegistryKey.BANNER_PATTERN).craft(PatternType.class, CraftPatternType::new, true).writable(PaperBannerPatternRegistryEntry.PaperBuilder::new).delayed(),
-            start(Registries.PAINTING_VARIANT, RegistryKey.PAINTING_VARIANT).craft(Art.class, CraftArt::new).writable(PaperPaintingVariantRegistryEntry.PaperBuilder::new).delayed(),
+            start(Registries.PAINTING_VARIANT, RegistryKey.PAINTING_VARIANT).craft(Art.class, CraftArt::new, true).writable(PaperPaintingVariantRegistryEntry.PaperBuilder::new).delayed(),
-            start(Registries.INSTRUMENT, RegistryKey.INSTRUMENT).craft(MusicInstrument.class, CraftMusicInstrument::new).build().delayed(),
+            start(Registries.INSTRUMENT, RegistryKey.INSTRUMENT).craft(MusicInstrument.class, CraftMusicInstrument::new, true).build().delayed(),
 
             // api-only
             start(Registries.ENTITY_TYPE, RegistryKey.ENTITY_TYPE).apiOnly(PaperSimpleRegistry::entityType),

paper-server/src/main/java/io/papermc/paper/registry/PaperRegistryAccess.java

@@ -107,7 +107,7 @@ public class PaperRegistryAccess implements RegistryAccess {
 
     public <M> void lockReferenceHolders(final ResourceKey<? extends net.minecraft.core.Registry<M>> resourceKey) {
         final RegistryEntry<M, Keyed> entry = PaperRegistries.getEntry(resourceKey);
-        if (entry == null || !(entry.meta() instanceof final RegistryEntryMeta.ServerSide<M, Keyed> serverSide) || !serverSide.registryTypeMapper().supportsDirectHolders()) {
+        if (entry == null || !(entry.meta() instanceof final RegistryEntryMeta.ServerSide<M, Keyed> serverSide) || !serverSide.registryTypeMapper().constructorUsesHolder()) {
             return;
         }
         final CraftRegistry<?, M> registry = (CraftRegistry<?, M>) this.getRegistry(entry.apiKey());

paper-server/src/main/java/io/papermc/paper/registry/data/InlinedRegistryBuilderProviderImpl.java

@@ -24,7 +24,7 @@ public final class InlinedRegistryBuilderProviderImpl implements InlinedRegistry
         Preconditions.checkArgument(buildableMeta.registryTypeMapper().supportsDirectHolders(), "Registry type mapper must support direct holders");
         final PaperRegistryBuilderFactory<M, A, B> builderFactory = new PaperRegistryBuilderFactory<>(Conversions.global(), buildableMeta.builderFiller(), CraftRegistry.getMinecraftRegistry(buildableMeta.mcKey())::getValue);
         value.accept(builderFactory);
-        return buildableMeta.registryTypeMapper().convertDirectHolder(Holder.direct(builderFactory.requireBuilder().build()));
+        return buildableMeta.registryTypeMapper().createBukkit(Holder.direct(builderFactory.requireBuilder().build()));
     }
 
     @Override

paper-server/src/main/java/io/papermc/paper/registry/entry/RegistryEntryBuilder.java

@@ -42,7 +42,11 @@ public class RegistryEntryBuilder<M, A extends Keyed> { // TODO remove Keyed
     }
 
     public CraftStage<M, A> craft(final Class<?> classToPreload, final Function<Holder<M>, ? extends A> minecraftToBukkit) {
-        return new CraftStage<>(this.mcKey, this.apiKey, classToPreload, new RegistryTypeMapper<>(minecraftToBukkit));
+        return this.craft(classToPreload, minecraftToBukkit, false);
+    }
+
+    public CraftStage<M, A> craft(final Class<?> classToPreload, final Function<Holder<M>, ? extends A> minecraftToBukkit, final boolean allowDirect) {
+        return new CraftStage<>(this.mcKey, this.apiKey, classToPreload, new RegistryTypeMapper<>(minecraftToBukkit, allowDirect));
     }
 
     public static final class CraftStage<M, A extends Keyed> extends RegistryEntryBuilder<M, A> { // TODO remove Keyed

paper-server/src/main/java/io/papermc/paper/registry/entry/RegistryTypeMapper.java

@@ -1,7 +1,7 @@
 package io.papermc.paper.registry.entry;
 
-import com.google.common.base.Preconditions;
 import com.mojang.datafixers.util.Either;
+import io.papermc.paper.util.MCUtil;
 import java.util.function.BiFunction;
 import java.util.function.Function;
 import net.minecraft.core.Holder;
@@ -9,29 +9,40 @@ import org.bukkit.NamespacedKey;
 
 public final class RegistryTypeMapper<M, A> {
 
-    final Either<BiFunction<? super NamespacedKey, M, ? extends A>, Function<Holder<M>, ? extends A>> minecraftToBukkit;
+    private static <M, A> Function<Holder<M>, ? extends A> wrap(final BiFunction<? super NamespacedKey, M, ? extends A> byValueCreator) {
+        return holder -> {
+            if (!(holder instanceof final Holder.Reference<M> reference)) {
+                throw new IllegalArgumentException("This type does not support direct holders: " + holder);
+            }
+            return byValueCreator.apply(MCUtil.fromResourceKey(reference.key()), reference.value());
+        };
+    }
+
+    final Either<Function<Holder<M>, ? extends A>, BiFunction<? super NamespacedKey, M, ? extends A>> minecraftToBukkit;
+    private final boolean supportsDirectHolders;
 
     public RegistryTypeMapper(final BiFunction<? super NamespacedKey, M, ? extends A> byValueCreator) {
-        this.minecraftToBukkit = Either.left(byValueCreator);
+        this.minecraftToBukkit = Either.right(byValueCreator);
+        this.supportsDirectHolders = false;
     }
 
-    public RegistryTypeMapper(final Function<Holder<M>, ? extends A> byHolderCreator) {
+    public RegistryTypeMapper(final Function<Holder<M>, ? extends A> byHolderCreator, final boolean supportsDirectHolders) {
-        this.minecraftToBukkit = Either.right(byHolderCreator);
+        this.minecraftToBukkit = Either.left(byHolderCreator);
+        this.supportsDirectHolders = supportsDirectHolders;
     }
 
-    public A createBukkit(final NamespacedKey key, final Holder<M> minecraft) {
+    public A createBukkit(final Holder<M> minecraft) {
-        return this.minecraftToBukkit.map(
+        return this.minecraftToBukkit.<Function<Holder<M>, ? extends A>>map(
-            minecraftToBukkit -> minecraftToBukkit.apply(key, minecraft.value()),
+            Function.identity(),
-            minecraftToBukkit -> minecraftToBukkit.apply(minecraft)
+            RegistryTypeMapper::wrap
-        );
+        ).apply(minecraft);
     }
 
     public boolean supportsDirectHolders() {
-        return this.minecraftToBukkit.right().isPresent();
+        return this.supportsDirectHolders;
     }
 
-    public A convertDirectHolder(final Holder<M> directHolder) {
+    public boolean constructorUsesHolder() {
-        Preconditions.checkArgument(this.supportsDirectHolders() && directHolder.kind() == Holder.Kind.DIRECT);
+        return this.minecraftToBukkit.left().isPresent();
-        return this.minecraftToBukkit.right().orElseThrow().apply(directHolder);
     }
 }

paper-server/src/main/java/io/papermc/paper/util/Holderable.java

@@ -27,7 +27,7 @@ public interface Holderable<M> extends Handleable<M> {
         return this.getHolder().value();
     }
 
-    static <T extends org.bukkit.Keyed, M> @Nullable T fromBukkitSerializationObject(final Object deserialized, final Codec<? extends Holder<M>> codec, final RegistryKey<T> registryKey) { // TODO remove Keyed
+    static <T extends org.bukkit.Keyed, M> @Nullable T fromBukkitSerializationObject(final Object deserialized, final Codec<M> directCodec, final RegistryKey<T> registryKey) { // TODO remove Keyed
         final Registry<T> registry = RegistryAccess.registryAccess().getRegistry(registryKey);
         return switch (deserialized) {
             case @Subst("key:value") final String string -> {
@@ -41,18 +41,18 @@ public interface Holderable<M> extends Handleable<M> {
                     throw new IllegalArgumentException("Cannot deserialize direct holders for " + registry);
                 }
                 final RegistryOps<JsonElement> ops = CraftRegistry.getMinecraftRegistry().createSerializationContext(JsonOps.INSTANCE);
-                final Holder<M> holder = codec.decode(ops, element).getOrThrow().getFirst();
+                final M holder = directCodec.decode(ops, element).getOrThrow().getFirst();
-                yield ((CraftRegistry<T, M>) registry).convertDirectHolder(holder);
+                yield ((CraftRegistry<T, M>) registry).createBukkit(Holder.direct(holder));
             }
             default -> throw new IllegalArgumentException("Cannot deserialize " + deserialized);
         };
     }
 
-    default Object toBukkitSerializationObject(final Codec<? super Holder<M>> codec) {
+    default Object toBukkitSerializationObject(final Codec<? super M> directCodec) {
         return switch (this.getHolder()) {
             case final Holder.Direct<M> direct -> {
                 final RegistryOps<JsonElement> ops = CraftRegistry.getMinecraftRegistry().createSerializationContext(JsonOps.INSTANCE);
-                yield new JsonObjectWrapper(codec.encodeStart(ops, direct).getOrThrow().getAsJsonObject());
+                yield new JsonObjectWrapper(directCodec.encodeStart(ops, direct.value()).getOrThrow().getAsJsonObject());
             }
             case final Holder.Reference<M> reference -> reference.key().location().toString();
             default -> throw new IllegalArgumentException("Cannot serialize " + this.getHolder());

paper-server/src/main/java/org/bukkit/craftbukkit/CraftMusicInstrument.java

@@ -32,7 +32,7 @@ public class CraftMusicInstrument extends MusicInstrument implements io.papermc.
     public static Object bukkitToString(MusicInstrument bukkit) { // Paper - switch to Holder
         Preconditions.checkArgument(bukkit != null);
 
-        return ((CraftMusicInstrument) bukkit).toBukkitSerializationObject(Instrument.CODEC); // Paper - switch to Holder
+        return ((CraftMusicInstrument) bukkit).toBukkitSerializationObject(Instrument.DIRECT_CODEC); // Paper - switch to Holder
     }
 
     public static MusicInstrument stringToBukkit(Object string) { // Paper - switch to Holder

paper-server/src/main/java/org/bukkit/craftbukkit/CraftRegistry.java

@@ -62,7 +62,7 @@ public class CraftRegistry<B extends Keyed, M> implements Registry<B> {
         final Registry<B> bukkitRegistry = RegistryAccess.registryAccess().getRegistry(PaperRegistries.registryFromNms(registryKey));
         final java.util.Optional<ResourceKey<M>> resourceKey = registry.getResourceKey(minecraft);
         if (resourceKey.isEmpty() && bukkitRegistry instanceof final CraftRegistry<?, ?> craftRegistry && craftRegistry.supportsDirectHolders()) {
-            return ((CraftRegistry<B, M>) bukkitRegistry).convertDirectHolder(Holder.direct(minecraft));
+            return ((CraftRegistry<B, M>) bukkitRegistry).createBukkit(Holder.direct(minecraft));
         } else if (resourceKey.isEmpty()) {
             throw new IllegalStateException(String.format("Cannot convert '%s' to bukkit representation, since it is not registered.", minecraft));
         }
@@ -82,7 +82,7 @@ public class CraftRegistry<B extends Keyed, M> implements Registry<B> {
                 if (!(bukkitRegistry instanceof final CraftRegistry<?, ?> craftRegistry) || !craftRegistry.supportsDirectHolders()) {
                     throw new IllegalArgumentException("Cannot convert direct holder to bukkit representation");
                 }
-                yield ((CraftRegistry<B, M>) bukkitRegistry).convertDirectHolder(direct);
+                yield ((CraftRegistry<B, M>) bukkitRegistry).createBukkit(direct);
             }
             case final Holder.Reference<M> reference -> bukkitRegistry.get(MCUtil.fromResourceKey(reference.key()));
             default -> throw new IllegalArgumentException("Unknown holder: " + minecraft);
@@ -126,8 +126,8 @@ public class CraftRegistry<B extends Keyed, M> implements Registry<B> {
     // Paper start - fixup upstream being dum
     public static <T extends Keyed, M> Optional<T> unwrapAndConvertHolder(final RegistryKey<T> registryKey, final Holder<M> value) {
         final Registry<T> registry = RegistryAccess.registryAccess().getRegistry(registryKey);
-        if (registry instanceof CraftRegistry<?,?> craftRegistry && craftRegistry.supportsDirectHolders() && value.kind() == Holder.Kind.DIRECT) {
+        if (registry instanceof final CraftRegistry<?,?> craftRegistry && craftRegistry.supportsDirectHolders() && value.kind() == Holder.Kind.DIRECT) {
-            return Optional.of(((CraftRegistry<T, M>) registry).convertDirectHolder(value));
+            return Optional.of(((CraftRegistry<T, M>) registry).createBukkit(value));
         }
         return value.unwrapKey().map(key -> registry.get(CraftNamespacedKey.fromMinecraft(key.location())));
     }
@@ -178,7 +178,7 @@ public class CraftRegistry<B extends Keyed, M> implements Registry<B> {
         this.minecraftRegistry = minecraftRegistry;
         this.minecraftToBukkit = minecraftToBukkit;
         this.serializationUpdater = serializationUpdater;
-        this.lockReferenceHolders = !this.minecraftToBukkit.supportsDirectHolders();
+        this.lockReferenceHolders = !this.minecraftToBukkit.constructorUsesHolder();
     }
 
     public void lockReferenceHolders() {
@@ -189,7 +189,7 @@ public class CraftRegistry<B extends Keyed, M> implements Registry<B> {
         } catch (final ClassNotFoundException e) {
             throw new IllegalStateException("Failed to load class " + this.bukkitClass.getSimpleName(), e);
         }
-        if (!this.minecraftToBukkit.supportsDirectHolders()) {
+        if (!this.minecraftToBukkit.constructorUsesHolder()) {
             return;
         }
         Preconditions.checkState(!this.lockReferenceHolders, "Reference holders are already locked");
@@ -209,7 +209,7 @@ public class CraftRegistry<B extends Keyed, M> implements Registry<B> {
         final Holder.Reference<M> holder;
         if (holderOptional.isPresent()) {
             holder = holderOptional.get();
-        } else if (!this.lockReferenceHolders && this.minecraftToBukkit.supportsDirectHolders()) { // only works if its Holderable
+        } else if (!this.lockReferenceHolders && this.minecraftToBukkit.constructorUsesHolder()) { // only works if its Holderable
             // we lock the reference holders after the preload class has been initialized
             // this is to support the vanilla mechanic of preventing vanilla registry entries being loaded. We need
             // to create something to fill the API constant fields, so we create a dummy reference holder.
@@ -217,7 +217,7 @@ public class CraftRegistry<B extends Keyed, M> implements Registry<B> {
         } else {
             holder = null;
         }
-        final B bukkit = this.createBukkit(namespacedKey, holder);
+        final B bukkit = this.createBukkit(holder);
         if (bukkit == null) {
             return null;
         }
@@ -227,16 +227,6 @@ public class CraftRegistry<B extends Keyed, M> implements Registry<B> {
         return bukkit;
     }
 
-    @NotNull
-    @Override
-    public B getOrThrow(@NotNull NamespacedKey namespacedKey) {
-        B object = this.get(namespacedKey);
-
-        Preconditions.checkArgument(object != null, "No %s registry entry found for key %s.", this.minecraftRegistry.key(), namespacedKey);
-
-        return object;
-    }
-
     @NotNull
     @Override
     public Stream<B> stream() {
@@ -248,24 +238,18 @@ public class CraftRegistry<B extends Keyed, M> implements Registry<B> {
         return this.stream().iterator();
     }
 
-    public B createBukkit(NamespacedKey namespacedKey, Holder<M> minecraft) { // Paper - switch to Holder
+    public B createBukkit(Holder<M> minecraft) {
         if (minecraft == null) {
             return null;
         }
 
-        return this.minecraftToBukkit.createBukkit(namespacedKey, minecraft); // Paper - switch to Holder
+        return this.minecraftToBukkit.createBukkit(minecraft);
     }
 
-    // Paper start - support Direct Holders
     public boolean supportsDirectHolders() {
         return this.minecraftToBukkit.supportsDirectHolders();
     }
 
-    public B convertDirectHolder(Holder<M> holder) {
-        return this.minecraftToBukkit.convertDirectHolder(holder);
-    }
-    // Paper end - support Direct Holders
-
     // Paper start - improve Registry
     @Override
     public NamespacedKey getKey(final B value) {

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

@@ -36,7 +36,7 @@ public class CraftTrimMaterial implements TrimMaterial, io.papermc.paper.util.Ho
     public static Object bukkitToObject(TrimMaterial bukkit) {
         Preconditions.checkArgument(bukkit != null);
 
-        return ((CraftTrimMaterial) bukkit).toBukkitSerializationObject(net.minecraft.world.item.equipment.trim.TrimMaterial.CODEC); // Paper - switch to Holder
+        return ((CraftTrimMaterial) bukkit).toBukkitSerializationObject(net.minecraft.world.item.equipment.trim.TrimMaterial.DIRECT_CODEC); // Paper - switch to Holder
     }
 
     public static TrimMaterial objectToBukkit(Object object) {

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

@@ -36,7 +36,7 @@ public class CraftTrimPattern implements TrimPattern, io.papermc.paper.util.Hold
     public static Object bukkitToObject(TrimPattern bukkit) {
         Preconditions.checkArgument(bukkit != null);
 
-        return ((CraftTrimPattern) bukkit).toBukkitSerializationObject(net.minecraft.world.item.equipment.trim.TrimPattern.CODEC); // Paper - switch to Holder
+        return ((CraftTrimPattern) bukkit).toBukkitSerializationObject(net.minecraft.world.item.equipment.trim.TrimPattern.DIRECT_CODEC); // Paper - switch to Holder
     }
 
     public static TrimPattern objectToBukkit(Object object) {

paper-server/src/test/java/org/bukkit/registry/RegistryConversionTest.java

@@ -263,7 +263,7 @@ 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, RegistryKey.ENCHANTMENT); // Paper
+    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

test-plugin/src/main/java/io/papermc/testplugin/TestPlugin.java

@@ -1,5 +1,11 @@
 package io.papermc.testplugin;
 
+import io.papermc.paper.event.player.ChatEvent;
+import io.papermc.paper.registry.keys.PaintingVariantKeys;
+import org.bukkit.Art;
+import org.bukkit.entity.Entity;
+import org.bukkit.entity.Painting;
+import org.bukkit.event.EventHandler;
 import org.bukkit.event.Listener;
 import org.bukkit.plugin.java.JavaPlugin;
 
@@ -12,4 +18,11 @@ public final class TestPlugin extends JavaPlugin implements Listener {
         // io.papermc.testplugin.brigtests.Registration.registerViaOnEnable(this);
     }
 
+    @EventHandler
+    public void onEvent(ChatEvent event) {
+        final Entity targetEntity = event.getPlayer().getTargetEntity(10);
+        if (targetEntity instanceof Painting painting) {
+            painting.setArt(Art.create(f -> f.copyFrom(PaintingVariantKeys.BAROQUE).height(10)));
+        }
+    }
 }