Fix Registry#getKey implementation

This commit is contained in:
Jake Potrebic 2024-12-21 14:56:01 -08:00
parent 54debf494f
commit 46c6f497c7
No known key found for this signature in database
GPG key ID: ECE0B3C133C016C5
13 changed files with 164 additions and 277 deletions

View file

@ -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.")));
}
}

View file

@ -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();
}
}

View file

@ -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;
return CraftRegistry.bukkitToMinecraftHolder(bukkit, Registries.PAINTING_VARIANT);
}
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++;
}
@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());
}
}

View file

@ -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

View file

@ -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

View file

@ -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;
return CraftRegistry.bukkitToMinecraftHolder(bukkit, Registries.SOUND_EVENT);
}
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());
}
@Override
public int hashCode() {
return this.getKey().hashCode();
public CraftSound(Holder<SoundEvent> soundEffect) {
super(soundEffect, count++);
}
// Paper start

View file

@ -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;
return CraftRegistry.bukkitToMinecraftHolder(bukkit, Registries.BANNER_PATTERN);
}
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();
}
@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

View file

@ -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;
}
}

View file

@ -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

View file

@ -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

View file

@ -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);

View file

@ -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());
}
}

View file

@ -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);
});
});
}
}