CadixDev / Mercury

A source transformation and -remapping framework for Java.
Eclipse Public License 2.0
54 stars 23 forks source link

Cannot remap method overriding with generic parameter #8

Open qouteall opened 4 years ago

qouteall commented 4 years ago

Example:

//mc code
public abstract class Feature<FC extends FeatureConfig>{
    public abstract boolean generate(IWorld world, ChunkGenerator<? extends ChunkGeneratorConfig> generator, Random random, BlockPos pos, FC config);
}

//my code
public class SimpleSpawnerFeature extends Feature<DefaultFeatureConfig>{
    @Override
    public boolean generate(
        IWorld world,
        ChunkGenerator<? extends ChunkGeneratorConfig> generator,
        Random random,
        BlockPos pos,
        DefaultFeatureConfig config
    ){...}
}

In my code this "generate" cannot be remapped. In AbstractClassMappingImpl#getMethodMapping, the input signature is MethodSignature{name=generate, descriptor=(Lnet/minecraft/world/IWorld;Lnet/minecraft/world/gen/chunk/ChunkGenerator;Ljava/util/Random;Lnet/minecraft/util/math/BlockPos;Lnet/minecraft/world/gen/feature/DefaultFeatureConfig;)Z}

And in this.methods it has MethodSignature{name=generate, descriptor=(Lnet/minecraft/world/IWorld;Lnet/minecraft/world/gen/chunk/ChunkGenerator;Ljava/util/Random;Lnet/minecraft/util/math/BlockPos;Lnet/minecraft/world/gen/feature/FeatureConfig;)Z} So getMethodMapping will return null in this case.

Possible solution: In AbstractClassMappingImpl#getMethodMapping, if it cannot find the method mapping that exactly match the signature, then find the method mapping that has the same name.

jamierocks commented 4 years ago

Perhaps we could add a computeMethodMapping to Lorenz, that in the absence of a mapping checks for other mappings of the same name that can assign to the exact signature?

Thoughts @Minecrell ?

dimitriye98 commented 3 years ago

I implemented a work around of this issue in my project, see https://github.com/dimitriye98/filigree/blob/f7530776bff4eaedc259d317a06ef5684a8f4a56/src/main/java/com/dimitriye/filigree/remapper/SimpleRemapperVisitor.java#L105

If you're interested, I could compose a pull request to add it to the upstream. I haven't done so yet as my workaround is kinda hacky in that it bypasses Lorenz and Bombe and just crawls the AST.

As best as I can tell, an actual solution would require incorporating type parameter support into Lorenz, Bombe, and Mercury. I briefly considered forking and trying at it, but this hot fix is good enough for my purposes.