Skip to content

Commit

Permalink
Handle disabled vanilla registry values correctly (#11781)
Browse files Browse the repository at this point in the history
  • Loading branch information
Machine-Maker authored Dec 27, 2024
1 parent e10dcbf commit b4051db
Show file tree
Hide file tree
Showing 10 changed files with 95 additions and 70 deletions.
5 changes: 5 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ ij_java_generate_final_locals = true
ij_java_generate_final_parameters = true
ij_java_method_parameters_new_line_after_left_paren = true
ij_java_method_parameters_right_paren_on_new_line = true
ij_java_use_fq_class_names = false
ij_java_class_names_in_javadoc = 1

[paper-server/src/minecraft/java/**/*.java]
ij_java_use_fq_class_names = true

[paper-server/src/minecraft/resources/data/**/*.json]
indent_size = 2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
ResourceLocation resourceLocation = key.location();
LOADERS.put(resourceLocation, () -> bootstrap.run(registry));
WRITABLE_REGISTRY.register((ResourceKey)key, registry, RegistrationInfo.BUILT_IN);
@@ -328,7 +_,14 @@
@@ -328,16 +_,34 @@
}

public static void bootStrap() {
Expand All @@ -41,6 +41,26 @@
freeze();
validate(REGISTRY);
}

private static void createContents() {
+ // Paper start - class-load org.bukkit.Registry
+ // we have to class-load Registry here to create all the CraftRegistry instances
+ // that will be created when Registry is class-loaded before RegistryAccess#getRegistry
+ // would try to create them in lockReferenceHolder
+ try {
+ Class.forName(org.bukkit.Registry.class.getName());
+ } catch (final ClassNotFoundException ex) {
+ throw new RuntimeException(ex);
+ }
+ // Paper end - class-load org.bukkit.Registry
LOADERS.forEach((resourceLocation, supplier) -> {
if (supplier.get() == null) {
LOGGER.error("Unable to bootstrap registry '{}'", resourceLocation);
}
+ io.papermc.paper.registry.PaperRegistryAccess.instance().lockReferenceHolders(ResourceKey.createRegistryKey(resourceLocation)); // Paper - lock reference holder creation
});
}

@@ -346,6 +_,7 @@

for (Registry<?> registry : REGISTRY) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,12 @@
} catch (Exception var14) {
loadingErrors.put(
resourceKey,
@@ -283,7 +_,8 @@
@@ -283,7 +_,9 @@
}
}

- TagLoader.loadTagsForRegistry(resourceManager, registry);
+ io.papermc.paper.registry.PaperRegistryAccess.instance().lockReferenceHolders(registry.key()); // Paper - lock reference holders
+ io.papermc.paper.registry.PaperRegistryListenerManager.INSTANCE.runFreezeListeners(registry.key(), conversions); // Paper - run pre-freeze listeners
+ TagLoader.loadTagsForRegistry(resourceManager, registry, io.papermc.paper.plugin.lifecycle.event.registrar.ReloadableRegistrarEvent.Cause.INITIAL); // Paper - tag lifecycle - add cause
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import net.minecraft.resources.ResourceKey;
import org.bukkit.Keyed;
import org.bukkit.Registry;
import org.bukkit.craftbukkit.CraftRegistry;
import org.jetbrains.annotations.VisibleForTesting;
import org.jspecify.annotations.Nullable;

Expand Down Expand Up @@ -72,7 +73,7 @@ public <T extends Keyed> Registry<T> getRegistry(final RegistryKey<T> key) {
if (PaperRegistries.getEntry(key) == null) {
throw new NoSuchElementException(key + " is not a valid registry key");
}
final @Nullable RegistryHolder<T> registryHolder = (RegistryHolder<T>) this.registries.get(key);
final RegistryHolder<T> registryHolder = (RegistryHolder<T>) this.registries.get(key);
if (registryHolder == null) {
throw new IllegalArgumentException(key + " points to a registry that is not available yet");
}
Expand Down Expand Up @@ -104,13 +105,22 @@ public <M> void registerRegistry(final ResourceKey<? extends net.minecraft.core.
this.registerRegistry(resourceKey, registry, false);
}

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()) {
return;
}
final CraftRegistry<?, M> registry = (CraftRegistry<?, M>) this.getRegistry(entry.apiKey());
registry.lockReferenceHolders();
}

@SuppressWarnings("unchecked") // this method should be called right after any new MappedRegistry instances are created to later be used by the server.
private <M, B extends Keyed, R extends Registry<B>> void registerRegistry(final ResourceKey<? extends net.minecraft.core.Registry<M>> resourceKey, final net.minecraft.core.Registry<M> registry, final boolean replace) {
final @Nullable RegistryEntry<M, B> entry = PaperRegistries.getEntry(resourceKey);
final RegistryEntry<M, B> entry = PaperRegistries.getEntry(resourceKey);
if (entry == null) { // skip registries that don't have API entries
return;
}
final @Nullable RegistryHolder<B> registryHolder = (RegistryHolder<B>) this.registries.get(entry.apiKey());
final RegistryHolder<B> registryHolder = (RegistryHolder<B>) this.registries.get(entry.apiKey());
if (registryHolder == null || replace) {
// if the holder doesn't exist yet, or is marked as "replaceable", put it in the map.
this.registries.put(entry.apiKey(), entry.createRegistryHolder(registry));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@ public static MusicInstrument stringToBukkit(Object string) { // Paper - switch
return io.papermc.paper.util.Holderable.fromBukkitSerializationObject(string, Instrument.CODEC, Registry.INSTRUMENT); // Paper - switch to Holder
}

private final NamespacedKey key;
private final Instrument handle;

// Paper start - switch to Holder
@Override
public boolean equals(final Object o) {
Expand All @@ -63,8 +60,6 @@ public String toString() {
private final Holder<Instrument> holder;
public CraftMusicInstrument(Holder<Instrument> holder) {
this.holder = holder;
this.key = holder.unwrapKey().map(io.papermc.paper.util.MCUtil::fromResourceKey).orElse(null);
this.handle = holder.value();
// Paper end - switch to Holder
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@
import io.papermc.paper.registry.tag.Tag;
import io.papermc.paper.util.Holderable;
import java.util.Collection;
import io.papermc.paper.util.MCUtil;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.Optional;
import java.util.function.BiFunction;
import java.util.stream.Stream;
import net.minecraft.core.Holder;
import net.minecraft.core.HolderOwner;
import net.minecraft.core.RegistryAccess;
import net.minecraft.resources.ResourceKey;
import org.bukkit.Keyed;
Expand Down Expand Up @@ -162,7 +165,8 @@ public static <B extends Keyed> B get(Registry<B> bukkit, NamespacedKey namespac
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
private boolean init;
private final InvalidHolderOwner invalidHolderOwner = new InvalidHolderOwner();
private boolean lockReferenceHolders;

public CraftRegistry(Class<?> bukkitClass, net.minecraft.core.Registry<M> minecraftRegistry, BiFunction<? super NamespacedKey, M, B> minecraftToBukkit, BiFunction<NamespacedKey, ApiVersion, NamespacedKey> serializationUpdater) { // Paper - relax preload class
// Paper start - switch to Holder
Expand All @@ -177,6 +181,22 @@ public CraftRegistry(Class<?> bukkitClass, net.minecraft.core.Registry<M> minecr
this.minecraftRegistry = minecraftRegistry;
this.minecraftToBukkit = minecraftToBukkit;
this.serializationUpdater = serializationUpdater;
this.lockReferenceHolders = !this.minecraftToBukkit.supportsDirectHolders();
}

public void lockReferenceHolders() {
Preconditions.checkState(this.cache.isEmpty(), "Registry %s is already loaded", this.minecraftRegistry.key());

try {
Class.forName(this.bukkitClass.getName()); // this should always trigger the initialization of the class
} catch (final ClassNotFoundException e) {
throw new IllegalStateException("Failed to load class " + this.bukkitClass.getSimpleName(), e);
}
if (!this.minecraftToBukkit.supportsDirectHolders()) {
return;
}
Preconditions.checkState(!this.lockReferenceHolders, "Reference holders are already locked");
this.lockReferenceHolders = true;
}

// Paper - inline into CraftRegistry#get(Registry, NamespacedKey, ApiVersion) above
Expand All @@ -188,28 +208,19 @@ public B get(NamespacedKey namespacedKey) {
return cached;
}

// Make sure that the bukkit class is loaded before creating an instance.
// This ensures that only one instance with a given key is created.
//
// Without this code (when bukkit class is not loaded):
// Registry#get -> #createBukkit -> (load class -> create default) -> put in cache
// Result: Registry#get != <bukkitClass>.<field> for possible one registry item
//
// With this code (when bukkit class is not loaded):
// Registry#get -> (load class -> create default) -> Registry#get -> get from cache
// Result: Registry#get == <bukkitClass>.<field>
if (!this.init) {
this.init = true;
try {
Class.forName(this.bukkitClass.getName());
} catch (ClassNotFoundException e) {
throw new RuntimeException("Could not load registry class " + this.bukkitClass, e);
}

return this.get(namespacedKey);
final Optional<Holder.Reference<M>> holderOptional = this.minecraftRegistry.get(CraftNamespacedKey.toMinecraft(namespacedKey));
final Holder.Reference<M> holder;
if (holderOptional.isPresent()) {
holder = holderOptional.get();
} else if (!this.lockReferenceHolders && this.minecraftToBukkit.supportsDirectHolders()) { // 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.
holder = Holder.Reference.createStandAlone(this.invalidHolderOwner, MCUtil.toResourceKey(this.minecraftRegistry.key(), namespacedKey));
} else {
holder = null;
}

B bukkit = this.createBukkit(namespacedKey, this.minecraftRegistry.get(CraftNamespacedKey.toMinecraft(namespacedKey)).orElse(null)); // Paper - switch to Holder
final B bukkit = this.createBukkit(namespacedKey, holder);
if (bukkit == null) {
return null;
}
Expand Down Expand Up @@ -285,4 +296,7 @@ public Collection<Tag<B>> getTags() {
return this.minecraftRegistry.getTags().<Tag<B>>map(NamedRegistryKeySetImpl::new).toList();
}
// Paper end - RegistrySet API

final class InvalidHolderOwner implements HolderOwner<M> {
}
}
Original file line number Diff line number Diff line change
@@ -1,25 +1,24 @@
package org.bukkit.craftbukkit.enchantments;

import com.google.common.base.Preconditions;
import io.papermc.paper.util.Holderable;
import java.util.Locale;
import net.minecraft.Util;
import net.minecraft.core.Holder;
import net.minecraft.core.registries.Registries;
import net.minecraft.network.chat.contents.TranslatableContents;
import net.minecraft.tags.EnchantmentTags;
import org.bukkit.NamespacedKey;
import org.bukkit.Registry;
import org.bukkit.craftbukkit.CraftRegistry;
import org.bukkit.craftbukkit.inventory.CraftItemStack;
import org.bukkit.craftbukkit.legacy.FieldRename;
import org.bukkit.craftbukkit.util.ApiVersion;
import org.bukkit.craftbukkit.util.Handleable;
import org.bukkit.enchantments.Enchantment;
import org.bukkit.enchantments.EnchantmentTarget;
import org.bukkit.enchantments.EnchantmentWrapper;
import org.bukkit.inventory.ItemStack;

public class CraftEnchantment extends Enchantment implements Handleable<net.minecraft.world.item.enchantment.Enchantment> {
public class CraftEnchantment extends Enchantment implements Holderable<net.minecraft.world.item.enchantment.Enchantment> {

public static Enchantment minecraftToBukkit(net.minecraft.world.item.enchantment.Enchantment minecraft) {
return CraftRegistry.minecraftToBukkit(minecraft, Registries.ENCHANTMENT, Registry.ENCHANTMENT);
Expand Down Expand Up @@ -56,22 +55,20 @@ public static Enchantment stringToBukkit(String string) {
return CraftRegistry.get(Registry.ENCHANTMENT, key, ApiVersion.CURRENT);
}

private final NamespacedKey key;
private final Holder<net.minecraft.world.item.enchantment.Enchantment> handle;

public CraftEnchantment(NamespacedKey key, net.minecraft.world.item.enchantment.Enchantment handle) {
this.key = key;
this.handle = CraftRegistry.getMinecraftRegistry(Registries.ENCHANTMENT).wrapAsHolder(handle);
public CraftEnchantment(Holder<net.minecraft.world.item.enchantment.Enchantment> holder) {
this.handle = holder;
}

@Override
public net.minecraft.world.item.enchantment.Enchantment getHandle() {
return this.handle.value();
public Holder<net.minecraft.world.item.enchantment.Enchantment> getHolder() {
return this.handle;
}

@Override
public NamespacedKey getKey() {
return this.key;
return Holderable.super.getKey();
}

@Override
Expand Down Expand Up @@ -251,24 +248,16 @@ public String getTranslationKey() {

@Override
public boolean equals(Object other) {
if (this == other) {
return true;
}

if (!(other instanceof CraftEnchantment)) {
return false;
}

return this.getKey().equals(((Enchantment) other).getKey());
return Holderable.super.implEquals(other);
}

@Override
public int hashCode() {
return this.getKey().hashCode();
return Holderable.super.implHashCode();
}

@Override
public String toString() {
return "CraftEnchantment[" + this.getKey() + "]";
return Holderable.super.implToString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ public static Holder<net.minecraft.world.item.equipment.trim.TrimMaterial> bukki
return CraftRegistry.bukkitToMinecraftHolder(bukkit, Registries.TRIM_MATERIAL); // Paper - switch to Holder
}

private final NamespacedKey key;
private final net.minecraft.world.item.equipment.trim.TrimMaterial handle;

// Paper start - switch to Holder
private final Holder<net.minecraft.world.item.equipment.trim.TrimMaterial> holder;

Expand Down Expand Up @@ -64,8 +61,6 @@ public String toString() {
}

public CraftTrimMaterial(final Holder<net.minecraft.world.item.equipment.trim.TrimMaterial> holder) {
this.key = holder.unwrapKey().map(io.papermc.paper.util.MCUtil::fromResourceKey).orElse(null);
this.handle = holder.value();
this.holder = holder;
// Paper end - switch to Holder
}
Expand All @@ -84,14 +79,14 @@ public NamespacedKey getKey() {
@NotNull
@Override
public String getTranslationKey() {
if (!(this.handle.description().getContents() instanceof TranslatableContents)) throw new UnsupportedOperationException("Description isn't translatable!"); // Paper
return ((TranslatableContents) this.handle.description().getContents()).getKey();
if (!(this.getHandle().description().getContents() instanceof TranslatableContents)) throw new UnsupportedOperationException("Description isn't translatable!"); // Paper
return ((TranslatableContents) this.getHandle().description().getContents()).getKey();
}

// Paper start - adventure
@Override
public net.kyori.adventure.text.Component description() {
return io.papermc.paper.adventure.PaperAdventure.asAdventure(this.handle.description());
return io.papermc.paper.adventure.PaperAdventure.asAdventure(this.getHandle().description());
}
// Paper end - adventure
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ public static Holder<net.minecraft.world.item.equipment.trim.TrimPattern> bukkit
return CraftRegistry.bukkitToMinecraftHolder(bukkit, Registries.TRIM_PATTERN); // Paper - switch to Holder
}

private final NamespacedKey key;
private final net.minecraft.world.item.equipment.trim.TrimPattern handle;

// Paper start - switch to Holder
private final Holder<net.minecraft.world.item.equipment.trim.TrimPattern> holder; // Paper - switch to Holder

Expand Down Expand Up @@ -64,8 +61,6 @@ public String toString() {
}

public CraftTrimPattern(Holder<net.minecraft.world.item.equipment.trim.TrimPattern> handle) {
this.key = handle.unwrapKey().map(io.papermc.paper.util.MCUtil::fromResourceKey).orElse(null);
this.handle = handle.value();
this.holder = handle;
// Paper end - switch to Holder
}
Expand All @@ -84,14 +79,14 @@ public NamespacedKey getKey() {
@NotNull
@Override
public String getTranslationKey() {
if (!(this.handle.description().getContents() instanceof TranslatableContents)) throw new UnsupportedOperationException("Description isn't translatable!"); // Paper
return ((TranslatableContents) this.handle.description().getContents()).getKey();
if (!(this.getHandle().description().getContents() instanceof TranslatableContents)) throw new UnsupportedOperationException("Description isn't translatable!"); // Paper
return ((TranslatableContents) this.getHandle().description().getContents()).getKey();
}

// Paper start - adventure
@Override
public net.kyori.adventure.text.Component description() {
return io.papermc.paper.adventure.PaperAdventure.asAdventure(this.handle.description());
return io.papermc.paper.adventure.PaperAdventure.asAdventure(this.getHandle().description());
}
// Paper end - adventure
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public class RegistryLoadOrderTest {

private static boolean initInterface = false;
private static boolean initAbstract = false;
private static org.bukkit.Registry<Keyed> registry; // Paper - remap fix
private static Registry<Keyed> registry;

public static Stream<Arguments> data() {
return Stream.of(
Expand Down Expand Up @@ -60,6 +60,7 @@ public void testRegistryLoadOrder(Supplier<Boolean> init, Class<Keyed> keyedClas

RegistryLoadOrderTest.registry = new CraftRegistry<>(keyedClass, minecraftRegistry, minecraftToBukkit, (namespacedKey, apiVersion) -> namespacedKey);
this.testClassNotLoaded(init.get());
((CraftRegistry<?, ?>) RegistryLoadOrderTest.registry).lockReferenceHolders();

Object testOne = RegistryLoadOrderTest.registry.get(new NamespacedKey("bukkit", "test-one"));
Object otherTestOne = RegistryLoadOrderTest.registry.get(new NamespacedKey("bukkit", "test-one"));
Expand Down

0 comments on commit b4051db

Please sign in to comment.