#1401: Add a config option to accept old keys in registry get calls

PaperMC · May 28, 2024 · 94e44ec · 94e44ec
1 parent a437019
commit 94e44ec
Show file tree

Hide file tree

Showing 8 changed files with 60 additions and 7 deletions.
diff --git a/src/main/java/org/bukkit/craftbukkit/CraftServer.java b/src/main/java/org/bukkit/craftbukkit/CraftServer.java
@@ -2,6 +2,7 @@
 
 import com.google.common.base.Charsets;
 import com.google.common.base.Function;
+import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterators;
@@ -302,6 +303,7 @@ public final class CraftServer implements Server {
     public boolean ignoreVanillaPermissions = false;
     private final List<CraftPlayer> playerView;
     public int reloadCount;
+    public Set<String> activeCompatibilities = Collections.emptySet();
 
     static {
         ConfigurationSerialization.registerClass(CraftOfflinePlayer.class);
@@ -377,6 +379,7 @@ public CraftPlayer apply(EntityPlayer player) {
         TicketType.PLUGIN.timeout = configuration.getInt("chunk-gc.period-in-ticks");
         minimumAPI = ApiVersion.getOrCreateVersion(configuration.getString("settings.minimum-api"));
         loadIcon();
+        loadCompatibilities();
 
         // Set map color cache
         if (configuration.getBoolean("settings.use-map-color-cache")) {
@@ -420,6 +423,25 @@ private void saveCommandsConfig() {
         }
     }
 
+    private void loadCompatibilities() {
+        ConfigurationSection compatibilities = configuration.getConfigurationSection("settings.compatibility");
+        if (compatibilities == null) {
+            activeCompatibilities = Collections.emptySet();
+            return;
+        }
+
+        activeCompatibilities = compatibilities
+                .getKeys(false)
+                .stream()
+                .filter(compatibilities::getBoolean)
+                .collect(Collectors.toSet());
+
+        if (!activeCompatibilities.isEmpty()) {
+            logger.info("Using following compatibilities: `" + Joiner.on("`, `").join(activeCompatibilities) + "`, this will affect performance and other plugins behavior.");
+            logger.info("Only use when necessary and prefer updating plugins if possible.");
+        }
+    }
+
     public void loadPlugins() {
         pluginManager.registerInterface(JavaPluginLoader.class);
 
@@ -896,6 +918,7 @@ public void reload() {
         printSaveWarning = false;
         console.autosavePeriod = configuration.getInt("ticks-per.autosave");
         loadIcon();
+        loadCompatibilities();
 
         try {
             playerList.getIpBans().load();

diff --git a/src/main/java/org/bukkit/craftbukkit/legacy/FieldRename.java b/src/main/java/org/bukkit/craftbukkit/legacy/FieldRename.java
@@ -12,6 +12,7 @@
 import org.bukkit.craftbukkit.legacy.fieldrename.FieldRenameData;
 import org.bukkit.craftbukkit.legacy.reroute.DoNotReroute;
 import org.bukkit.craftbukkit.legacy.reroute.InjectPluginVersion;
+import org.bukkit.craftbukkit.legacy.reroute.RequireCompatibility;
 import org.bukkit.craftbukkit.legacy.reroute.RerouteMethodName;
 import org.bukkit.craftbukkit.legacy.reroute.RerouteStatic;
 import org.bukkit.craftbukkit.util.ApiVersion;
@@ -55,6 +56,7 @@ public static <T extends Enum<T>> T valueOf(Class<T> enumClass, String name, @In
         return Enum.valueOf(enumClass, rename(apiVersion, enumClass.getName().replace('.', '/'), name));
     }
 
+    @RequireCompatibility("allow-old-keys-in-registry")
     public static <T extends Keyed> T get(Registry<T> registry, NamespacedKey namespacedKey) {
         // We don't have version-specific changes, so just use current, and don't inject a version
         return CraftRegistry.get(registry, namespacedKey, ApiVersion.CURRENT);

diff --git a/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RequireCompatibility.java b/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RequireCompatibility.java
@@ -0,0 +1,13 @@
+package org.bukkit.craftbukkit.legacy.reroute;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.METHOD)
+public @interface RequireCompatibility {
+
+    String value();
+}
diff --git a/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RerouteBuilder.java b/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RerouteBuilder.java
@@ -112,6 +112,11 @@ public static RerouteMethodData buildFromMethod(Method method) {
 
         boolean inBukkit = !method.isAnnotationPresent(NotInBukkit.class);
 
-        return new RerouteMethodData(methodKey, sourceDesc, sourceOwner, methodName, rerouteStatic != null, targetType, Type.getInternalName(method.getDeclaringClass()), method.getName(), arguments, rerouteReturn, inBukkit);
+        String requiredCompatibility = null;
+        if (method.isAnnotationPresent(RequireCompatibility.class)) {
+            requiredCompatibility = method.getAnnotation(RequireCompatibility.class).value();
+        }
+
+        return new RerouteMethodData(methodKey, sourceDesc, sourceOwner, methodName, rerouteStatic != null, targetType, Type.getInternalName(method.getDeclaringClass()), method.getName(), arguments, rerouteReturn, inBukkit, requiredCompatibility);
     }
 }
diff --git a/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RerouteMethodData.java b/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RerouteMethodData.java
@@ -1,9 +1,11 @@
 package org.bukkit.craftbukkit.legacy.reroute;
 
 import java.util.List;
+import org.jetbrains.annotations.Nullable;
 import org.objectweb.asm.Type;
 
 public record RerouteMethodData(String source, Type sourceDesc, Type sourceOwner, String sourceName,
                                 boolean staticReroute, Type targetType, String targetOwner, String targetName,
-                                List<RerouteArgument> arguments, RerouteReturn rerouteReturn, boolean isInBukkit) {
+                                List<RerouteArgument> arguments, RerouteReturn rerouteReturn, boolean isInBukkit,
+                                @Nullable String requiredCompatibility) {
 }
diff --git a/src/main/java/org/bukkit/craftbukkit/util/Commodore.java b/src/main/java/org/bukkit/craftbukkit/util/Commodore.java
@@ -6,6 +6,7 @@
 import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Enumeration;
 import java.util.HashSet;
 import java.util.List;
@@ -117,7 +118,7 @@ private static void convert(File in, File out) {
                             byte[] b = ByteStreams.toByteArray(is);
 
                             if (entry.getName().endsWith(".class")) {
-                                b = convert(b, "dummy", ApiVersion.NONE);
+                                b = convert(b, "dummy", ApiVersion.NONE, Collections.emptySet());
                                 entry = new JarEntry(entry.getName());
                             }
 
@@ -135,7 +136,7 @@ private static void convert(File in, File out) {
         }
     }
 
-    public static byte[] convert(byte[] b, final String pluginName, final ApiVersion pluginVersion) {
+    public static byte[] convert(byte[] b, final String pluginName, final ApiVersion pluginVersion, final Set<String> activeCompatibilities) {
         final boolean modern = pluginVersion.isNewerThanOrSameAs(ApiVersion.FLATTENING);
         ClassReader cr = new ClassReader(b);
         ClassWriter cw = new ClassWriter(cr, 0);
@@ -385,7 +386,7 @@ private void handleMethod(MethodPrinter visitor, int opcode, String owner, Strin
                     }
 
                     private boolean checkReroute(MethodPrinter visitor, Map<String, RerouteMethodData> rerouteMethodDataMap, int opcode, String owner, String name, String desc, Type samMethodType, Type instantiatedMethodType) {
-                        return rerouteMethods(rerouteMethodDataMap, opcode == Opcodes.INVOKESTATIC || opcode == Opcodes.H_INVOKESTATIC, owner, name, desc, data -> {
+                        return rerouteMethods(activeCompatibilities, rerouteMethodDataMap, opcode == Opcodes.INVOKESTATIC || opcode == Opcodes.H_INVOKESTATIC, owner, name, desc, data -> {
                             visitor.visit(Opcodes.INVOKESTATIC, className, buildMethodName(data), buildMethodDesc(data), isInterface, samMethodType, instantiatedMethodType);
                             rerouteMethodData.add(data);
                         });
@@ -555,7 +556,7 @@ The question then becomes one about performance (since this is not the most perf
     But since it is only applied for each class and method call once when they get first loaded, it should not be that bad.
     (Although some load time testing could be done)
      */
-    public static boolean rerouteMethods(Map<String, RerouteMethodData> rerouteMethodDataMap, boolean staticCall, String owner, String name, String desc, Consumer<RerouteMethodData> consumer) {
+    public static boolean rerouteMethods(Set<String> activeCompatibilities, Map<String, RerouteMethodData> rerouteMethodDataMap, boolean staticCall, String owner, String name, String desc, Consumer<RerouteMethodData> consumer) {
         Type ownerType = Type.getObjectType(owner);
         Class<?> ownerClass;
         try {
@@ -578,6 +579,10 @@ public static boolean rerouteMethods(Map<String, RerouteMethodData> rerouteMetho
                 continue;
             }
 
+            if (data.requiredCompatibility() != null && !activeCompatibilities.contains(data.requiredCompatibility())) {
+                return false;
+            }
+
             consumer.accept(data);
             return true;
         }

diff --git a/src/main/java/org/bukkit/craftbukkit/util/CraftMagicNumbers.java b/src/main/java/org/bukkit/craftbukkit/util/CraftMagicNumbers.java
@@ -53,6 +53,7 @@
 import org.bukkit.craftbukkit.CraftEquipmentSlot;
 import org.bukkit.craftbukkit.CraftFeatureFlag;
 import org.bukkit.craftbukkit.CraftRegistry;
+import org.bukkit.craftbukkit.CraftServer;
 import org.bukkit.craftbukkit.attribute.CraftAttribute;
 import org.bukkit.craftbukkit.attribute.CraftAttributeInstance;
 import org.bukkit.craftbukkit.block.data.CraftBlockData;
@@ -323,7 +324,7 @@ public static boolean isLegacy(PluginDescriptionFile pdf) {
     @Override
     public byte[] processClass(PluginDescriptionFile pdf, String path, byte[] clazz) {
         try {
-            clazz = Commodore.convert(clazz, pdf.getName(), ApiVersion.getOrCreateVersion(pdf.getAPIVersion()));
+            clazz = Commodore.convert(clazz, pdf.getName(), ApiVersion.getOrCreateVersion(pdf.getAPIVersion()), ((CraftServer) Bukkit.getServer()).activeCompatibilities);
         } catch (Exception ex) {
             Bukkit.getLogger().log(Level.SEVERE, "Fatal error trying to convert " + pdf.getFullName() + ":" + path, ex);
         }

diff --git a/src/main/resources/configurations/bukkit.yml b/src/main/resources/configurations/bukkit.yml
@@ -23,6 +23,8 @@ settings:
     shutdown-message: Server closed
     minimum-api: none
     use-map-color-cache: true
+    compatibility:
+        allow-old-keys-in-registry: false
 spawn-limits:
     monsters: 70
     animals: 10