diff --git a/definition/src/main/java/org/sinytra/adapter/patch/analysis/params/EnhancedParamsDiff.java b/definition/src/main/java/org/sinytra/adapter/patch/analysis/params/EnhancedParamsDiff.java index 2ebab09..38035c2 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/analysis/params/EnhancedParamsDiff.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/analysis/params/EnhancedParamsDiff.java @@ -68,6 +68,7 @@ private static void buildDiff(ParamsDiffSnapshotBuilder builder, List clea private static void buildDiffWithContext(ParamsDiffSnapshotBuilder builder, List cleanQueue, List 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 cleanQueue, List 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 cleanQueue, List 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 + // 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 cleanQueue, List dirtyQueue) { diff --git a/definition/src/main/java/org/sinytra/adapter/patch/analysis/params/LayeredParamsDiffSnapshot.java b/definition/src/main/java/org/sinytra/adapter/patch/analysis/params/LayeredParamsDiffSnapshot.java index 3c74d0e..5e41c98 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/analysis/params/LayeredParamsDiffSnapshot.java +++ b/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 transformers = this.modifications.stream().map(ParamModification::asParameterTransformer).toList(); return new TransformParameters(transformers, withOffset, type); } diff --git a/definition/src/main/java/org/sinytra/adapter/patch/analysis/params/ParamsDiffSnapshot.java b/definition/src/main/java/org/sinytra/adapter/patch/analysis/params/ParamsDiffSnapshot.java index 32406af..5efd9df 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/analysis/params/ParamsDiffSnapshot.java +++ b/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); } diff --git a/definition/src/main/java/org/sinytra/adapter/patch/analysis/params/SimpleParamsDiffSnapshot.java b/definition/src/main/java/org/sinytra/adapter/patch/analysis/params/SimpleParamsDiffSnapshot.java index b454f92..b4e8d04 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/analysis/params/SimpleParamsDiffSnapshot.java +++ b/definition/src/main/java/org/sinytra/adapter/patch/analysis/params/SimpleParamsDiffSnapshot.java @@ -88,7 +88,7 @@ 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 list = new ArrayList<>(); SimpleParamsDiffSnapshot light = new SimpleParamsDiffSnapshot(List.of(), this.replacements, this.swaps, this.substitutes, this.removals, this.moves, this.inlines); if (!light.isEmpty()) { @@ -96,8 +96,8 @@ public MethodTransform asParameterTransformer(ParamTransformTarget type, boolean } if (!this.insertions.isEmpty()) { list.add(new TransformParameters(this.insertions.stream() - .map(p -> new InjectParameterTransform(p.getFirst(), p.getSecond())) - .toList(), true, type)); + .map(p -> new InjectParameterTransform(p.getFirst(), p.getSecond(), upgradeWrapOperation)) + .toList(), withOffset, type)); } return new BundledMethodTransform(list); } diff --git a/definition/src/main/java/org/sinytra/adapter/patch/fixes/MethodUpgrader.java b/definition/src/main/java/org/sinytra/adapter/patch/fixes/MethodUpgrader.java index 47a348d..925e9bd 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/fixes/MethodUpgrader.java +++ b/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()); } } diff --git a/definition/src/main/java/org/sinytra/adapter/patch/transformer/param/InjectParameterTransform.java b/definition/src/main/java/org/sinytra/adapter/patch/transformer/param/InjectParameterTransform.java index 31445c6..ca98874 100644 --- a/definition/src/main/java/org/sinytra/adapter/patch/transformer/param/InjectParameterTransform.java +++ b/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 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 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; } diff --git a/definition/src/test/java/org/sinytra/adapter/patch/test/EnhancedParamsDiffTest.java b/definition/src/test/java/org/sinytra/adapter/patch/test/EnhancedParamsDiffTest.java index da4ee52..dd0ddff 100644 --- a/definition/src/test/java/org/sinytra/adapter/patch/test/EnhancedParamsDiffTest.java +++ b/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 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 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 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());