#1493: Improve reroute performance and add some tests

By: DerFrZocker <derrieple@gmail.com>

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

@@ -383,6 +383,7 @@ public final class CraftServer implements Server {
         minimumAPI = ApiVersion.getOrCreateVersion(configuration.getString("settings.minimum-api"));
         loadIcon();
         loadCompatibilities();
+        CraftMagicNumbers.INSTANCE.getCommodore().updateReroute(activeCompatibilities::contains);
 
         // Set map color cache
         if (configuration.getBoolean("settings.use-map-color-cache")) {
@@ -930,6 +931,7 @@ public final class CraftServer implements Server {
         console.autosavePeriod = configuration.getInt("ticks-per.autosave");
         loadIcon();
         loadCompatibilities();
+        CraftMagicNumbers.INSTANCE.getCommodore().updateReroute(activeCompatibilities::contains);
 
         try {
             playerList.getIpBans().load();

paper-server/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RequirePluginVersionData.java

@@ -7,7 +7,7 @@ public record RequirePluginVersionData(ApiVersion minInclusive, ApiVersion maxIn
     public static RequirePluginVersionData create(RequirePluginVersion requirePluginVersion) {
         if (!requirePluginVersion.value().isBlank()) {
             if (!requirePluginVersion.minInclusive().isBlank() || !requirePluginVersion.maxInclusive().isBlank()) {
-                throw new RuntimeException("When setting value, min inclusive and max inclusive data is not allowed.");
+                throw new IllegalArgumentException("When setting value, min inclusive and max inclusive data is not allowed.");
             }
 
             return new RequirePluginVersionData(ApiVersion.getOrCreateVersion(requirePluginVersion.value()), ApiVersion.getOrCreateVersion(requirePluginVersion.value()));
@@ -23,6 +23,12 @@ public record RequirePluginVersionData(ApiVersion minInclusive, ApiVersion maxIn
             maxInclusive = ApiVersion.getOrCreateVersion(requirePluginVersion.maxInclusive());
         }
 
+        if (minInclusive != null && maxInclusive != null) {
+            if (minInclusive.isNewerThan(maxInclusive)) {
+                throw new IllegalArgumentException("Min inclusive cannot be newer than max inclusive.");
+            }
+        }
+
         return new RequirePluginVersionData(minInclusive, maxInclusive);
     }
 

paper-server/src/main/java/org/bukkit/craftbukkit/legacy/reroute/Reroute.java

@@ -0,0 +1,120 @@
+package org.bukkit.craftbukkit.legacy.reroute;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Consumer;
+import org.bukkit.craftbukkit.util.ApiVersion;
+import org.bukkit.craftbukkit.util.ClassTraverser;
+import org.jetbrains.annotations.VisibleForTesting;
+import org.objectweb.asm.Type;
+
+public class Reroute {
+
+    @VisibleForTesting
+    final Map<String, RerouteDataHolder> rerouteDataMap;
+
+    Reroute(Map<String, RerouteDataHolder> rerouteDataMap) {
+        this.rerouteDataMap = rerouteDataMap;
+    }
+
+    /*
+    This method looks (and probably is) overengineered, but it gives the most flexible when it comes to remapping normal methods to static one.
+    The problem with normal owner and desc replacement is that child classes have them as an owner, instead there parents for there parents methods
+
+    For example, if we have following two interfaces org.bukkit.BlockData and org.bukkit.Orientable extents BlockData
+    and BlockData has the method org.bukkit.Material getType which we want to reroute to the static method
+    org.bukkit.Material org.bukkit.craftbukkit.legacy.EnumEvil#getType(org.bukkit.BlockData)
+
+    If we now call BlockData#getType we get as the owner org/bukkit/BlockData and as desc ()Lorg/bukkit/Material;
+    Which we can nicely reroute by checking if the owner is BlockData and the name getType
+    The problem, starts if we use Orientable#getType no we get as owner org/bukkit/Orientable and as desc ()Lorg/bukkit/Material;
+    Now we can now longer safely say to which getType method we need to reroute (assume there are multiple getType methods from different classes,
+    which are not related to BlockData), simple using the owner class will not work, since would reroute to
+    EnumEvil#getType(org.bukkit.Orientable) which is not EnumEvil#getType(org.bukkit.BlockData) and will throw a method not found error
+    at runtime.
+
+    Meaning we would need to add checks for each subclass, which would be pur insanity.
+
+    To solve this, we go through each super class and interfaces (and their super class and interfaces etc.) and try to get an owner
+    which matches with one of our replacement methods. Based on how inheritance works in java, this method should be safe to use.
+
+    As a site note: This method could also be used for the other method reroute, e.g. legacy method rerouting, where only the replacement
+    method needs to be written, and this method figures out the rest, which could reduce the size and complexity of the Commodore class.
+    The question then becomes one about performance (since this is not the most performance way) and convenience.
+    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 boolean apply(ApiVersion pluginVersion, String owner, String name, String desc, boolean staticCall, Consumer<RerouteMethodData> consumer) {
+        RerouteDataHolder rerouteData = rerouteDataMap.get(desc + name);
+        if (rerouteData == null) {
+            return false;
+        }
+
+        Type ownerType = Type.getObjectType(owner);
+        RerouteMethodData data = rerouteData.get(ownerType);
+
+        if (staticCall && data == null) {
+            return false;
+        }
+
+        if (data != null) {
+            if (data.requiredPluginVersion() != null && !data.requiredPluginVersion().test(pluginVersion)) {
+                return false;
+            }
+
+            consumer.accept(data);
+            return true;
+        }
+
+        Class<?> ownerClass;
+        try {
+            ownerClass = Class.forName(ownerType.getClassName(), false, Reroute.class.getClassLoader());
+        } catch (ClassNotFoundException e) {
+            return false;
+        }
+
+        ClassTraverser it = new ClassTraverser(ownerClass);
+        while (it.hasNext()) {
+            Class<?> clazz = it.next();
+
+            data = rerouteData.get(Type.getType(clazz));
+
+            if (data == null) {
+                continue;
+            }
+
+            if (data.requiredPluginVersion() != null && !data.requiredPluginVersion().test(pluginVersion)) {
+                return false;
+            }
+
+            consumer.accept(data);
+            return true;
+        }
+
+        return false;
+    }
+
+    static class RerouteDataHolder {
+
+        @VisibleForTesting
+        final Map<String, RerouteMethodData> rerouteMethodDataMap = new HashMap<>();
+
+        public RerouteMethodData get(Class<?> clazz) {
+            return rerouteMethodDataMap.get(Type.getInternalName(clazz));
+        }
+
+        private RerouteMethodData get(Type owner) {
+            return rerouteMethodDataMap.get(owner.getInternalName());
+        }
+
+        void add(RerouteMethodData value) {
+            RerouteMethodData rerouteMethodData = get(value.sourceOwner());
+
+            if (rerouteMethodData != null) {
+                throw new IllegalStateException("Reroute method data already exists: " + rerouteMethodData);
+            }
+
+            rerouteMethodDataMap.put(value.sourceOwner().getInternalName(), value);
+        }
+    }
+}

paper-server/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RerouteBuilder.java

@@ -1,53 +1,69 @@
 package org.bukkit.craftbukkit.legacy.reroute;
 
 import com.google.common.base.Preconditions;
+import java.lang.reflect.AnnotatedElement;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.lang.reflect.Parameter;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Predicate;
 import org.bukkit.craftbukkit.util.ApiVersion;
 import org.objectweb.asm.Type;
 
 public class RerouteBuilder {
 
-    public static Map<String, RerouteMethodData> buildFromClass(Class<?> clazz) {
-        Preconditions.checkArgument(!clazz.isInterface(), "Interface Classes are currently not supported");
+    private final List<Class<?>> classes = new ArrayList<>();
+    private final Predicate<String> compatibilityPresent;
 
-        Map<String, RerouteMethodData> result = new HashMap<>();
-
-        for (Method method : clazz.getDeclaredMethods()) {
-            if (method.isBridge()) {
-                continue;
-            }
-
-            if (method.isSynthetic()) {
-                continue;
-            }
-
-            if (!Modifier.isPublic(method.getModifiers())) {
-                continue;
-            }
-
-            if (!Modifier.isStatic(method.getModifiers())) {
-                continue;
-            }
-
-            if (method.isAnnotationPresent(DoNotReroute.class)) {
-                continue;
-            }
-
-            RerouteMethodData rerouteMethodData = buildFromMethod(method);
-            result.put(rerouteMethodData.source(), rerouteMethodData);
-        }
-
-        return Collections.unmodifiableMap(result);
+    private RerouteBuilder(Predicate<String> compatibilityPresent) {
+        this.compatibilityPresent = compatibilityPresent;
     }
 
-    public static RerouteMethodData buildFromMethod(Method method) {
+    public static RerouteBuilder create(Predicate<String> compatibilityPresent) {
+        return new RerouteBuilder(compatibilityPresent);
+    }
+
+    public RerouteBuilder forClass(Class<?> clazz) {
+        classes.add(clazz);
+        return this;
+    }
+
+    public Reroute build() {
+        Map<String, Reroute.RerouteDataHolder> rerouteDataHolderMap = new HashMap<>();
+
+        for (Class<?> clazz : classes) {
+            List<RerouteMethodData> data = buildFromClass(clazz, compatibilityPresent);
+            data.forEach(value -> rerouteDataHolderMap.computeIfAbsent(value.methodKey(), v -> new Reroute.RerouteDataHolder()).add(value));
+        }
+
+        return new Reroute(rerouteDataHolderMap);
+    }
+
+    private static List<RerouteMethodData> buildFromClass(Class<?> clazz, Predicate<String> compatibilityPresent) {
+        Preconditions.checkArgument(!clazz.isInterface(), "Interface Classes are currently not supported");
+
+        List<RerouteMethodData> result = new ArrayList<>();
+        boolean shouldInclude = shouldInclude(getRequireCompatibility(clazz), true, compatibilityPresent);
+
+        for (Method method : clazz.getDeclaredMethods()) {
+            if (!isMethodValid(method)) {
+                continue;
+            }
+
+            if (!shouldInclude(getRequireCompatibility(method), shouldInclude, compatibilityPresent)) {
+                continue;
+            }
+
+            result.add(buildFromMethod(method));
+        }
+
+        return result;
+    }
+
+    private static RerouteMethodData buildFromMethod(Method method) {
         RerouteReturn rerouteReturn = new RerouteReturn(Type.getReturnType(method));
         List<RerouteArgument> arguments = new ArrayList<>();
         List<RerouteArgument> sourceArguments = new ArrayList<>();
@@ -113,7 +129,10 @@ public class RerouteBuilder {
         if (rerouteStatic != null) {
             sourceOwner = Type.getObjectType(rerouteStatic.value());
         } else {
-            RerouteArgument argument = sourceArguments.get(0);
+            if (sourceArguments.isEmpty()) {
+                throw new RuntimeException("Source argument list is empty, no owner class found");
+            }
+            RerouteArgument argument = sourceArguments.getFirst();
             sourceOwner = argument.sourceType();
             sourceArguments.remove(argument);
         }
@@ -135,23 +154,12 @@ public class RerouteBuilder {
             methodName = method.getName();
         }
 
-        String methodKey = sourceOwner.getInternalName()
-                + " "
-                + sourceDesc.getDescriptor()
-                + " "
-                + methodName;
+        String methodKey = sourceDesc.getDescriptor() + methodName;
 
         Type targetType = Type.getType(method);
 
         boolean inBukkit = !method.isAnnotationPresent(NotInBukkit.class) && !method.getDeclaringClass().isAnnotationPresent(NotInBukkit.class);
 
-        String requiredCompatibility = null;
-        if (method.isAnnotationPresent(RequireCompatibility.class)) {
-            requiredCompatibility = method.getAnnotation(RequireCompatibility.class).value();
-        } else if (method.getDeclaringClass().isAnnotationPresent(RequireCompatibility.class)) {
-            requiredCompatibility = method.getDeclaringClass().getAnnotation(RequireCompatibility.class).value();
-        }
-
         RequirePluginVersionData requiredPluginVersion = null;
         if (method.isAnnotationPresent(RequirePluginVersion.class)) {
             requiredPluginVersion = RequirePluginVersionData.create(method.getAnnotation(RequirePluginVersion.class));
@@ -159,6 +167,47 @@ public class RerouteBuilder {
             requiredPluginVersion = RequirePluginVersionData.create(method.getDeclaringClass().getAnnotation(RequirePluginVersion.class));
         }
 
-        return new RerouteMethodData(methodKey, sourceDesc, sourceOwner, methodName, rerouteStatic != null, targetType, Type.getInternalName(method.getDeclaringClass()), method.getName(), arguments, rerouteReturn, inBukkit, requiredCompatibility, requiredPluginVersion);
+        return new RerouteMethodData(methodKey, sourceDesc, sourceOwner, methodName, rerouteStatic != null, targetType, Type.getInternalName(method.getDeclaringClass()), method.getName(), arguments, rerouteReturn, inBukkit, requiredPluginVersion);
+    }
+
+    private static boolean isMethodValid(Method method) {
+        if (method.isBridge()) {
+            return false;
+        }
+
+        if (method.isSynthetic()) {
+            return false;
+        }
+
+        if (!Modifier.isPublic(method.getModifiers())) {
+            return false;
+        }
+
+        if (!Modifier.isStatic(method.getModifiers())) {
+            return false;
+        }
+
+        if (method.isAnnotationPresent(DoNotReroute.class)) {
+            return false;
+        }
+
+        return true;
+    }
+
+    private static String getRequireCompatibility(AnnotatedElement element) {
+        RequireCompatibility annotation = element.getAnnotation(RequireCompatibility.class);
+        if (annotation == null) {
+            return null;
+        }
+
+        return annotation.value();
+    }
+
+    private static boolean shouldInclude(String string, boolean parent, Predicate<String> compatibilityPresent) {
+        if (string == null) {
+            return parent;
+        }
+
+        return compatibilityPresent.test(string);
     }
 }

paper-server/src/main/java/org/bukkit/craftbukkit/legacy/reroute/RerouteMethodData.java

@@ -4,8 +4,8 @@ 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,
+public record RerouteMethodData(String methodKey, Type sourceDesc, Type sourceOwner, String sourceName,
                                 boolean staticReroute, Type targetType, String targetOwner, String targetName,
                                 List<RerouteArgument> arguments, RerouteReturn rerouteReturn, boolean isInBukkit,
-                                @Nullable String requiredCompatibility, @Nullable RequirePluginVersionData requiredPluginVersion) {
+                                @Nullable RequirePluginVersionData requiredPluginVersion) {
 }

paper-server/src/main/java/org/bukkit/craftbukkit/util/Commodore.java

@@ -1,5 +1,6 @@
 package org.bukkit.craftbukkit.util;
 
+import com.google.common.base.Predicates;
 import com.google.common.collect.HashMultimap;
 import com.google.common.collect.Multimap;
 import com.google.common.io.ByteStreams;
@@ -16,6 +17,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Consumer;
+import java.util.function.Predicate;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 import java.util.jar.JarOutputStream;
@@ -28,6 +30,7 @@ import org.bukkit.craftbukkit.legacy.FieldRename;
 import org.bukkit.craftbukkit.legacy.MaterialRerouting;
 import org.bukkit.craftbukkit.legacy.MethodRerouting;
 import org.bukkit.craftbukkit.legacy.enums.EnumEvil;
+import org.bukkit.craftbukkit.legacy.reroute.Reroute;
 import org.bukkit.craftbukkit.legacy.reroute.RerouteArgument;
 import org.bukkit.craftbukkit.legacy.reroute.RerouteBuilder;
 import org.bukkit.craftbukkit.legacy.reroute.RerouteMethodData;
@@ -87,18 +90,38 @@ public class Commodore {
             "org/bukkit/block/banner/PatternType", "NOP"
     );
 
-    private static Map<String, RerouteMethodData> createReroutes(Class<?> clazz) {
-        Map<String, RerouteMethodData> reroutes = RerouteBuilder.buildFromClass(clazz);
-        REROUTES.add(reroutes);
-        return reroutes;
+    private final List<Reroute> reroutes = new ArrayList<>(); // only for testing
+    private Reroute materialReroute;
+    private Reroute reroute;
+
+    public Commodore() {
+    }
+
+    public Commodore(Predicate<String> compatibilityPresent) {
+        updateReroute(compatibilityPresent);
+    }
+
+    public void updateReroute(Predicate<String> compatibilityPresent) {
+        materialReroute = RerouteBuilder
+                .create(compatibilityPresent)
+                .forClass(MaterialRerouting.class)
+                .build();
+        reroute = RerouteBuilder
+                .create(compatibilityPresent)
+                .forClass(FieldRename.class)
+                .forClass(MethodRerouting.class)
+                .forClass(EnumEvil.class)
+                .build();
+
+        reroutes.clear();
+        reroutes.add(materialReroute);
+        reroutes.add(reroute);
     }
 
     @VisibleForTesting
-    public static final List<Map<String, RerouteMethodData>> REROUTES = new ArrayList<>(); // Only used for testing
-    private static final Map<String, RerouteMethodData> FIELD_RENAME_METHOD_REROUTE = createReroutes(FieldRename.class);
-    private static final Map<String, RerouteMethodData> MATERIAL_METHOD_REROUTE = createReroutes(MaterialRerouting.class);
-    private static final Map<String, RerouteMethodData> METHOD_REROUTE = createReroutes(MethodRerouting.class);
-    private static final Map<String, RerouteMethodData> ENUM_METHOD_REROUTE = createReroutes(EnumEvil.class);
+    public List<Reroute> getReroutes() {
+        return reroutes;
+    }
 
     public static void main(String[] args) {
         OptionParser parser = new OptionParser();
@@ -110,6 +133,8 @@ public class Commodore {
         File input = options.valueOf(inputFlag);
         File output = options.valueOf(outputFlag);
 
+        Commodore commodore = new Commodore(Predicates.alwaysTrue());
+
         if (input.isDirectory()) {
             if (!output.isDirectory()) {
                 System.err.println("If input directory specified, output directory required too");
@@ -118,15 +143,15 @@ public class Commodore {
 
             for (File in : input.listFiles()) {
                 if (in.getName().endsWith(".jar")) {
-                    convert(in, new File(output, in.getName()));
+                    convert(in, new File(output, in.getName()), commodore);
                 }
             }
         } else {
-            convert(input, output);
+            convert(input, output, commodore);
         }
     }
 
-    private static void convert(File in, File out) {
+    private static void convert(File in, File out, Commodore commodore) {
         System.out.println("Attempting to convert " + in + " to " + out);
 
         try {
@@ -144,7 +169,7 @@ public class Commodore {
                             byte[] b = ByteStreams.toByteArray(is);
 
                             if (entry.getName().endsWith(".class")) {
-                                b = convert(b, "dummy", ApiVersion.NONE, Collections.emptySet());
+                                b = commodore.convert(b, "dummy", ApiVersion.NONE, Collections.emptySet());
                                 entry = new JarEntry(entry.getName());
                             }
 
@@ -162,7 +187,7 @@ public class Commodore {
         }
     }
 
-    public static byte[] convert(byte[] b, final String pluginName, final ApiVersion pluginVersion, final Set<String> activeCompatibilities) {
+    public byte[] convert(byte[] b, final String pluginName, final ApiVersion pluginVersion, final Set<String> activeCompatibilities) {
         final boolean modern = pluginVersion.isNewerThanOrSameAs(ApiVersion.FLATTENING);
         final boolean enumCompatibility = pluginVersion.isOlderThanOrSameAs(ApiVersion.getOrCreateVersion("1.20.6")) && activeCompatibilities.contains("enum-compatibility-mode");
         ClassReader cr = new ClassReader(b);
@@ -329,10 +354,7 @@ public class Commodore {
                     }
 
                     private void handleMethod(MethodPrinter visitor, int opcode, String owner, String name, String desc, boolean itf, Type samMethodType, Type instantiatedMethodType) {
-                        if (checkReroute(visitor, FIELD_RENAME_METHOD_REROUTE, opcode, owner, name, desc, samMethodType, instantiatedMethodType)) {
-                            return;
-                        }
-                        if (checkReroute(visitor, METHOD_REROUTE, opcode, owner, name, desc, samMethodType, instantiatedMethodType)) {
+                        if (checkReroute(visitor, reroute, opcode, owner, name, desc, samMethodType, instantiatedMethodType)) {
                             return;
                         }
 
@@ -353,10 +375,6 @@ public class Commodore {
                             }
                         }
 
-                        if (checkReroute(visitor, ENUM_METHOD_REROUTE, opcode, owner, name, desc, samMethodType, instantiatedMethodType)) {
-                            return;
-                        }
-
                         // SPIGOT-4496
                         if (owner.equals("org/bukkit/map/MapView") && name.equals("getId") && desc.equals("()S")) {
                             // Should be same size on stack so just call other method
@@ -448,15 +466,15 @@ public class Commodore {
                         }
 
                         // TODO 2024-05-21: Move this up, when material gets fully replaced with ItemType and BlockType
-                        if (owner.startsWith("org/bukkit") && checkReroute(visitor, MATERIAL_METHOD_REROUTE, opcode, owner, name, desc, samMethodType, instantiatedMethodType)) {
+                        if (owner.startsWith("org/bukkit") && checkReroute(visitor, materialReroute, opcode, owner, name, desc, samMethodType, instantiatedMethodType)) {
                             return;
                         }
 
                         visitor.visit(opcode, owner, name, desc, itf, samMethodType, instantiatedMethodType);
                     }
 
-                    private boolean checkReroute(MethodPrinter visitor, Map<String, RerouteMethodData> rerouteMethodDataMap, int opcode, String owner, String name, String desc, Type samMethodType, Type instantiatedMethodType) {
-                        return rerouteMethods(activeCompatibilities, pluginVersion, rerouteMethodDataMap, opcode == Opcodes.INVOKESTATIC || opcode == Opcodes.H_INVOKESTATIC, owner, name, desc, data -> {
+                    private boolean checkReroute(MethodPrinter visitor, Reroute reroute, int opcode, String owner, String name, String desc, Type samMethodType, Type instantiatedMethodType) {
+                        return rerouteMethods(pluginVersion, reroute, 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);
                         });
@@ -599,69 +617,8 @@ public class Commodore {
         };
     }
 
-    /*
-    This method looks (and probably is) overengineered, but it gives the most flexible when it comes to remapping normal methods to static one.
-    The problem with normal owner and desc replacement is that child classes have them as an owner, instead there parents for there parents methods
-
-    For example, if we have following two interfaces org.bukkit.BlockData and org.bukkit.Orientable extents BlockData
-    and BlockData has the method org.bukkit.Material getType which we want to reroute to the static method
-    org.bukkit.Material org.bukkit.craftbukkit.legacy.EnumEvil#getType(org.bukkit.BlockData)
-
-    If we now call BlockData#getType we get as the owner org/bukkit/BlockData and as desc ()Lorg/bukkit/Material;
-    Which we can nicely reroute by checking if the owner is BlockData and the name getType
-    The problem, starts if we use Orientable#getType no we get as owner org/bukkit/Orientable and as desc ()Lorg/bukkit/Material;
-    Now we can now longer safely say to which getType method we need to reroute (assume there are multiple getType methods from different classes,
-    which are not related to BlockData), simple using the owner class will not work, since would reroute to
-    EnumEvil#getType(org.bukkit.Orientable) which is not EnumEvil#getType(org.bukkit.BlockData) and will throw a method not found error
-    at runtime.
-
-    Meaning we would need to add checks for each subclass, which would be pur insanity.
-
-    To solve this, we go through each super class and interfaces (and their super class and interfaces etc.) and try to get an owner
-    which matches with one of our replacement methods. Based on how inheritance works in java, this method should be safe to use.
-
-    As a site note: This method could also be used for the other method reroute, e.g. legacy method rerouting, where only the replacement
-    method needs to be written, and this method figures out the rest, which could reduce the size and complexity of the Commodore class.
-    The question then becomes one about performance (since this is not the most performance way) and convenience.
-    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(Set<String> activeCompatibilities, ApiVersion pluginVersion, Map<String, RerouteMethodData> rerouteMethodDataMap, boolean staticCall, String owner, String name, String desc, Consumer<RerouteMethodData> consumer) {
-        Type ownerType = Type.getObjectType(owner);
-        Class<?> ownerClass;
-        try {
-            ownerClass = Class.forName(ownerType.getClassName());
-        } catch (ClassNotFoundException e) {
-            return false;
-        }
-
-        ClassTraverser it = new ClassTraverser(ownerClass);
-        while (it.hasNext()) {
-            Class<?> clazz = it.next();
-
-            String methodKey = Type.getInternalName(clazz) + " " + desc + " " + name;
-
-            RerouteMethodData data = rerouteMethodDataMap.get(methodKey);
-            if (data == null) {
-                if (staticCall) {
-                    return false;
-                }
-                continue;
-            }
-
-            if (data.requiredCompatibility() != null && !activeCompatibilities.contains(data.requiredCompatibility())) {
-                return false;
-            }
-
-            if (data.requiredPluginVersion() != null && !data.requiredPluginVersion().test(pluginVersion)) {
-                return false;
-            }
-
-            consumer.accept(data);
-            return true;
-        }
-
-        return false;
+    public static boolean rerouteMethods(ApiVersion pluginVersion, Reroute reroute, boolean staticCall, String owner, String name, String desc, Consumer<RerouteMethodData> consumer) {
+        return reroute.apply(pluginVersion, owner, name, desc, staticCall, consumer);
     }
 
     private static List<String> getMethodSignatures(byte[] clazz) {

paper-server/src/main/java/org/bukkit/craftbukkit/util/CraftMagicNumbers.java

@@ -74,7 +74,9 @@ import org.bukkit.potion.PotionType;
 
 @SuppressWarnings("deprecation")
 public final class CraftMagicNumbers implements UnsafeValues {
-    public static final UnsafeValues INSTANCE = new CraftMagicNumbers();
+    public static final CraftMagicNumbers INSTANCE = new CraftMagicNumbers();
+
+    private final Commodore commodore = new Commodore();
 
     private CraftMagicNumbers() {}
 
@@ -165,6 +167,10 @@ public final class CraftMagicNumbers implements UnsafeValues {
         return CraftLegacy.toLegacyData(data);
     }
 
+    public Commodore getCommodore() {
+        return this.commodore;
+    }
+
     @Override
     public Material toLegacy(Material material) {
         return CraftLegacy.toLegacy(material);
@@ -320,7 +326,7 @@ public final class CraftMagicNumbers implements UnsafeValues {
     @Override
     public byte[] processClass(PluginDescriptionFile pdf, String path, byte[] clazz) {
         try {
-            clazz = Commodore.convert(clazz, pdf.getName(), ApiVersion.getOrCreateVersion(pdf.getAPIVersion()), ((CraftServer) Bukkit.getServer()).activeCompatibilities);
+            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);
         }