Disable upgrading wrap operations in inject transformer when not requ…

…ired

Improve EPD algo to better detect inserted and removed parameters

definition/src/main/java/org/sinytra/adapter/patch/analysis/params/EnhancedParamsDiff.java

@@ -68,6 +68,7 @@ private static void buildDiff(ParamsDiffSnapshotBuilder builder, List<Type> clea
 
     private static void buildDiffWithContext(ParamsDiffSnapshotBuilder builder, List<TypeWithContext> cleanQueue, List<TypeWithContext> dirtyQueue, boolean compareNames) {
         int dirtySize = dirtyQueue.size();
+        boolean sameSize = cleanQueue.size() == dirtyQueue.size();
 
         if (DEBUG) {
             LOGGER.info("Comparing types:\n{}", printTable(cleanQueue, dirtyQueue));
@@ -76,7 +77,7 @@ private static void buildDiffWithContext(ParamsDiffSnapshotBuilder builder, List
         while (!cleanQueue.isEmpty()) {
             // Look ahead for matching types at the beginning of the list
             // If the first two are equal, remove the first ones and repeat
-            if (sameParameter(cleanQueue, dirtyQueue, compareNames)) {
+            if (predictParameterMatch(builder, cleanQueue, dirtyQueue, compareNames, sameSize)) {
                 cleanQueue.remove(0);
                 dirtyQueue.remove(0);
                 continue;
@@ -127,11 +128,60 @@ else if (cleanQueue.size() == dirtyQueue.size() && (dirtyTypeIndex = findClosest
         }
     }
 
-    private static boolean sameParameter(List<TypeWithContext> cleanQueue, List<TypeWithContext> dirtyQueue, boolean compareNames) {
-        return cleanQueue.size() > 1 && dirtyQueue.size() > 1 && (
-            compareNames ? cleanQueue.get(0).matches(dirtyQueue.get(0)) && cleanQueue.get(1).matches(dirtyQueue.get(1))
-                : cleanQueue.get(0).sameType(dirtyQueue.get(0)) && cleanQueue.get(1).sameType(dirtyQueue.get(1))
-        );
+    private static boolean predictParameterMatch(ParamsDiffSnapshotBuilder builder, List<TypeWithContext> cleanQueue, List<TypeWithContext> dirtyQueue, boolean compareNames, boolean sameSize) {
+        // For same-sized comparisons, we can be certain there have been no insertions
+        if (sameSize && !cleanQueue.isEmpty() && !dirtyQueue.isEmpty()) {
+            if (!cleanQueue.get(0).sameType(dirtyQueue.get(0))) {
+                // If the first parameters differ, it's possible one of them has been removed
+                // == Clean ==
+                // 0 Foo
+                // 1 Bar
+                // 2 World
+                // == Dirty ==
+                // 0 Bar
+                // 1 World
+                // 2 Sheep
+                if (cleanQueue.size() > 2 && dirtyQueue.size() > 2 && cleanQueue.get(1).matches(dirtyQueue.get(0)) && cleanQueue.get(2).matches(dirtyQueue.get(1))) {
+                    builder.remove(cleanQueue.get(0).pos());
+                    cleanQueue.remove(0);
+                    return true;
+                }
+                return false;
+            }
+            return true;
+        }
+        else if (cleanQueue.size() > 1 && dirtyQueue.size() > 1) {
+            // Comparing names can be useful in cases where a parameter is inserted in between multiple ones of the same type
+            if (compareNames) {
+                if (cleanQueue.get(0).matches(dirtyQueue.get(0))) {
+                    if (cleanQueue.get(1).matches(dirtyQueue.get(1))) {
+                        // If the first two parameters match, we can be sure the first ones are identical without any insertions
+                        return true;
+                    }
+                    // Check for inserted parameters
+                    // == Clean ==
+                    // 0 F one
+                    // 1 F two
+                    // 2 F three
+                    // == Dirty ==
+                    // 0 F one
+                    // 1 F <inserted>
+                    // 2 F two
+                    else if (dirtyQueue.size() > 2 && cleanQueue.get(1).matches(dirtyQueue.get(2))) {
+                        builder.insert(dirtyQueue.get(1).pos(), dirtyQueue.get(1).type());
+                        cleanQueue.remove(0);
+                        dirtyQueue.remove(0);
+                        dirtyQueue.remove(0);
+                        return true;
+                    }
+                }
+            }
+            else {
+                // If the first two parameters match, we can be sure the first ones are identical without any insertions
+                return cleanQueue.get(0).sameType(dirtyQueue.get(0)) && cleanQueue.get(1).sameType(dirtyQueue.get(1));
+            }
+        }
+        return false;
     }
 
     private static int findClosestMatch(List<TypeWithContext> cleanQueue, List<TypeWithContext> dirtyQueue) {

definition/src/main/java/org/sinytra/adapter/patch/analysis/params/LayeredParamsDiffSnapshot.java

@@ -196,7 +196,7 @@ public LayeredParamsDiffSnapshot offset(int offset, int limit) {
     }
 
     @Override
-    public MethodTransform asParameterTransformer(ParamTransformTarget type, boolean withOffset) {
+    public MethodTransform asParameterTransformer(ParamTransformTarget type, boolean withOffset, boolean upgradeWrapOperation) {
         List<ParameterTransformer> transformers = this.modifications.stream().map(ParamModification::asParameterTransformer).toList();
         return new TransformParameters(transformers, withOffset, type);
     }

definition/src/main/java/org/sinytra/adapter/patch/analysis/params/ParamsDiffSnapshot.java

@@ -18,5 +18,9 @@ public interface ParamsDiffSnapshot {
 
     ParamsDiffSnapshot offset(int offset, int limit);
 
-    MethodTransform asParameterTransformer(ParamTransformTarget type, boolean withOffset);
+    default MethodTransform asParameterTransformer(ParamTransformTarget type, boolean withOffset) {
+        return asParameterTransformer(type, withOffset, true);
+    }
+
+    MethodTransform asParameterTransformer(ParamTransformTarget type, boolean withOffset, boolean upgradeWrapOperation);
 }

definition/src/main/java/org/sinytra/adapter/patch/analysis/params/SimpleParamsDiffSnapshot.java

@@ -88,16 +88,16 @@ public SimpleParamsDiffSnapshot offset(int offset, int limit) {
     }
 
     @Override
-    public MethodTransform asParameterTransformer(ParamTransformTarget type, boolean withOffset) {
+    public MethodTransform asParameterTransformer(ParamTransformTarget type, boolean withOffset, boolean upgradeWrapOperation) {
         List<MethodTransform> list = new ArrayList<>();
         SimpleParamsDiffSnapshot light = new SimpleParamsDiffSnapshot(List.of(), this.replacements, this.swaps, this.substitutes, this.removals, this.moves, this.inlines);
         if (!light.isEmpty()) {
             list.add(new ModifyMethodParams(light, type, false, null));
         }
         if (!this.insertions.isEmpty()) {
             list.add(new TransformParameters(this.insertions.stream()
-                .<ParameterTransformer>map(p -> new InjectParameterTransform(p.getFirst(), p.getSecond()))
-                .toList(), true, type));
+                .<ParameterTransformer>map(p -> new InjectParameterTransform(p.getFirst(), p.getSecond(), upgradeWrapOperation))
+                .toList(), withOffset, type));
         }
         return new BundledMethodTransform(list);
     }

definition/src/main/java/org/sinytra/adapter/patch/fixes/MethodUpgrader.java

@@ -51,7 +51,7 @@ private static void upgradeWrapOperation(ClassNode classNode, MethodNode methodN
         // Create diff
         SimpleParamsDiffSnapshot diff = EnhancedParamsDiff.create(originalDesc, modifiedDesc);
         if (!diff.isEmpty()) {
-            MethodTransform patch = diff.asParameterTransformer(ParamTransformTarget.ALL, true);
+            MethodTransform patch = diff.asParameterTransformer(ParamTransformTarget.ALL, false, false);
             patch.apply(classNode, methodNode, methodContext, methodContext.patchContext());
         }
     }

definition/src/main/java/org/sinytra/adapter/patch/transformer/param/InjectParameterTransform.java

@@ -23,12 +23,16 @@
 import static org.sinytra.adapter.patch.transformer.param.ParamTransformationUtil.calculateLVTIndex;
 import static org.sinytra.adapter.patch.transformer.param.ParamTransformationUtil.extractWrapOperation;
 
-public record InjectParameterTransform(int index, Type type) implements ParameterTransformer {
+public record InjectParameterTransform(int index, Type type, boolean upgradeWrapOperation) implements ParameterTransformer {
     static final Codec<InjectParameterTransform> CODEC = RecordCodecBuilder.create(in -> in.group(
         Codec.intRange(0, 255).fieldOf("index").forGetter(InjectParameterTransform::index),
         AdapterUtil.TYPE_CODEC.fieldOf("parameterType").forGetter(InjectParameterTransform::type)
     ).apply(in, InjectParameterTransform::new));
 
+    public InjectParameterTransform(int index, Type type) {
+        this(index, type, true);
+    }
+
     @Override
     public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodContext methodContext, PatchContext context, List<Type> parameters, int offset) {
         boolean isNonStatic = (methodNode.access & Opcodes.ACC_STATIC) == 0;
@@ -68,8 +72,10 @@ public Patch.Result apply(ClassNode classNode, MethodNode methodNode, MethodCont
             methodNode.localVariables.add(index + (isNonStatic ? 1 : 0), new LocalVariableNode(newParameter.name, type.getDescriptor(), null, self.start, self.end, lvtIndex));
         });
 
-        extractWrapOperation(methodContext, methodNode, parameters, wrapOpModification -> wrapOpModification
-            .insertParameter(index, nodes -> nodes.add(new VarInsnNode(Opcodes.ALOAD, lvtIndex))));
+        if (this.upgradeWrapOperation) {
+            extractWrapOperation(methodContext, methodNode, parameters, wrapOpModification -> wrapOpModification
+                .insertParameter(index, nodes -> nodes.add(new VarInsnNode(Opcodes.ALOAD, lvtIndex))));
+        }
 
         return Patch.Result.APPLY;
     }

definition/src/test/java/org/sinytra/adapter/patch/test/EnhancedParamsDiffTest.java

@@ -331,6 +331,27 @@ void testCompareReplacedParameters2() {
         assertTrue(diff.moves().isEmpty());
     }
 
+    @Test
+    void testCompareReplacedParameters3() {
+        List<Type> original = List.of(
+            Type.getObjectType("net/minecraft/world/item/ItemStack"),
+            Type.getObjectType("net/minecraft/item/Item"),
+            Type.getObjectType("com/llamalad7/mixinextras/injector/wrapoperation/Operation")
+        );
+        List<Type> modified = List.of(
+            Type.getObjectType("net/minecraft/world/item/ItemStack"),
+            Type.getObjectType("net/minecraft/world/entity/LivingEntity"),
+            Type.getObjectType("com/llamalad7/mixinextras/injector/wrapoperation/Operation")
+        );
+
+        SimpleParamsDiffSnapshot diff = EnhancedParamsDiff.create(original, modified);
+        assertTrue(diff.insertions().isEmpty());
+        assertEquals(1, diff.replacements().size());
+        assertTrue(diff.removals().isEmpty());
+        assertTrue(diff.swaps().isEmpty());
+        assertTrue(diff.moves().isEmpty());
+    }
+
     @Test
     void testCompareCombinedParameters() {
         List<Type> original = List.of(
@@ -398,7 +419,7 @@ void testCompareRemovedParameters() {
 
         SimpleParamsDiffSnapshot diff = EnhancedParamsDiff.create(original, modified);
         assertEquals(1, diff.insertions().size());
-        assertEquals(Pair.of(4, Type.DOUBLE_TYPE), diff.insertions().get(0));
+        assertEquals(Pair.of(3, Type.DOUBLE_TYPE), diff.insertions().get(0));
         assertTrue(diff.replacements().isEmpty());
         assertEquals(1, diff.removals().size());
         assertTrue(diff.swaps().isEmpty());