Account for logical breaking change in ASMAPI.findFirstInstructionBefore() (#56)

Add new methods that include a boolean parameter for fixing the logic.
This overrides the behavior imposed by CoreMods to fix the logic based
on MC version.

Author: Jonathing

src/main/java/net/minecraftforge/coremod/CoreModEngine.java

@@ -4,6 +4,7 @@
  */
 package net.minecraftforge.coremod;
 
+import cpw.mods.modlauncher.Launcher;
 import cpw.mods.modlauncher.api.*;
 import net.minecraftforge.forgespi.coremod.*;
 import org.apache.logging.log4j.LogManager;
@@ -51,6 +52,18 @@ public class CoreModEngine {
             "org.objectweb.asm.Label","org.objectweb.asm.Type",
             "org.objectweb.asm.TypePath","org.objectweb.asm.TypeReference"
     ));
+
+    // this is enabled by FML in Minecraft 1.21.1 and earlier, but disabled in 1.21.3 and later
+    // see ASMAPI.findFirstInstructionBefore for more details
+    public static final boolean DO_NOT_FIX_INSNBEFORE;
+
+    static {
+        var blackboardVar = Launcher.INSTANCE.blackboard().get(TypesafeMap.Key.getOrCreate(Launcher.INSTANCE.blackboard(), "coremods.use_old_findFirstInstructionBefore", Boolean.class));
+        if (DO_NOT_FIX_INSNBEFORE = blackboardVar.isPresent() && blackboardVar.get()) {
+            LOGGER.debug("CoreMods will preserve legacy behavior of ASMAPI.findFirstInstructionBefore for backwards-compatibility");
+        }
+    }
+
     void loadCoreMod(ICoreModFile coremod) {
         // We have a factory per coremod, to provide namespace and functional isolation between coremods
         final ScriptEngine scriptEngine = NashornFactory.createEngine();

src/main/java/net/minecraftforge/coremod/api/ASMAPI.java

@@ -6,6 +6,7 @@
 
 import cpw.mods.modlauncher.Launcher;
 import cpw.mods.modlauncher.api.INameMappingService;
+import net.minecraftforge.coremod.CoreModEngine;
 import net.minecraftforge.coremod.CoreModTracker;
 import org.jetbrains.annotations.Nullable;
 import org.objectweb.asm.Opcodes;
@@ -305,22 +306,60 @@ public static AbstractInsnNode findFirstInstructionAfter(MethodNode method, int
      * @param opCode     the opcode to search for
      * @param startIndex the index at which to start searching (inclusive)
      * @return the found instruction node or null if none matched before the given startIndex
+     *
+     * @apiNote In Minecraft 1.21.1 and earlier, this method contains broken logic that ignores the {@code startIndex}
+     *     parameter and searches for the requested instruction at the end of the method. This behavior is preserved to
+     *     not disrupt older coremods. If you are on one of these older versions and need to use the fixed logic, please
+     *     use {@link #findFirstInstructionBefore(MethodNode, int, int, boolean)}.
      */
     public static AbstractInsnNode findFirstInstructionBefore(MethodNode method, int opCode, int startIndex) {
         return findFirstInstructionBefore(method, opCode, null, startIndex);
     }
 
+    /**
+     * Finds the first instruction with matching opcode before the given index in reverse search.
+     *
+     * @param method     the method to search in
+     * @param opCode     the opcode to search for
+     * @param startIndex the index at which to start searching (inclusive)
+     * @param fixLogic   whether to use the fixed logic for finding instructions before the given startIndex (true by
+     *                   default on versions since 1.21.3, false otherwise)
+     * @return the found instruction node or null if none matched before the given startIndex
+     */
+    public static AbstractInsnNode findFirstInstructionBefore(MethodNode method, int opCode, int startIndex, boolean fixLogic) {
+        return findFirstInstructionBefore(method, opCode, null, startIndex, fixLogic);
+    }
+
     /**
      * Finds the first instruction with matching opcode before the given index in reverse search
      *
-     * @param method the method to search in
-     * @param opCode the opcode to search for
+     * @param method     the method to search in
+     * @param opCode     the opcode to search for
      * @param startIndex the index at which to start searching (inclusive)
      * @return the found instruction node or null if none matched before the given startIndex
+     *
+     * @apiNote In Minecraft 1.21.1 and earlier, this method contains broken logic that ignores the {@code startIndex}
+     *     parameter and searches for the requested instruction at the end of the method. This behavior is preserved to
+     *     not disrupt older coremods. If you are on one of these older versions and need to use the fixed logic, please
+     *     use {@link #findFirstInstructionBefore(MethodNode, int, InsnType, int, boolean)}.
      */
     public static AbstractInsnNode findFirstInstructionBefore(MethodNode method, int opCode, @Nullable InsnType type, int startIndex) {
+        return findFirstInstructionBefore(method, opCode, type, startIndex, !CoreModEngine.DO_NOT_FIX_INSNBEFORE);
+    }
+
+    /**
+     * Finds the first instruction with matching opcode before the given index in reverse search
+     *
+     * @param method     the method to search in
+     * @param opCode     the opcode to search for
+     * @param startIndex the index at which to start searching (inclusive)
+     * @param fixLogic   whether to use the fixed logic for finding instructions before the given startIndex (true by
+     *                   default on versions since 1.21.3, false otherwise)
+     * @return the found instruction node or null if none matched before the given startIndex
+     */
+    public static AbstractInsnNode findFirstInstructionBefore(MethodNode method, int opCode, @Nullable InsnType type, int startIndex, boolean fixLogic) {
         boolean checkType = type != null;
-        for (int i = Math.min(method.instructions.size() - 1, startIndex); i >= 0; i--) {
+        for (int i = fixLogic ? Math.min(method.instructions.size() - 1, startIndex) : startIndex; i >= 0; i--) {
             AbstractInsnNode ain = method.instructions.get(i);
             if (ain.getOpcode() == opCode) {
                 if (!checkType || type.get() == ain.getType()) {