minecraft-dev / MinecraftDev

Plugin for IntelliJ IDEA that gives special support for Minecraft modding projects.
https://minecraftdev.org/
GNU Lesser General Public License v3.0
1.48k stars 183 forks source link

`RightClick->Get Mixin target reference` doesn't work on abstract classes #659

Open LeafHacker opened 4 years ago

LeafHacker commented 4 years ago

Please include the following information in all bug reports:

When targeting a method (possibly only methods with a default implementation) on an abstract class, the abstract class is used in the method, rather than the implementing class.

This may only be fixable if you assume the class that is currently open in intellij is the implementing/target class. Alternatively it may make sense to display a warning if the implementing class cannot be determined.


Example: targeting net.minecraft.client.gui.screen.MainMenuScreen#render's use of net.minecraft.client.gui.AbstractGui#blit

If I write the following mixin and then auto complete the target, I get Lnet/minecraft/client/gui/screen/MainMenuScreen;blit(IIIIFFIIII)V, as expected.

@Mixin(MainMenuScreen.class)
public abstract class MixinMainMenuScreen extends Screen {
    @Inject(
            method = "render",
            at = @At(
                    value = "INVOKE",
                    target = "",
                    shift = AFTER,
                    ordinal = 0
            )
    )
    private void afterDrawOverlay(int mouseX, int mouseY, float partialTicks, CallbackInfo ci) {

    }
}

However, if I navigate to net.minecraft.client.gui.screen.MainMenuScreen#render, right click on blit and select Copy Mixin Target Reference, it gives me Lnet/minecraft/client/gui/AbstractGui;blit(IIIIFFIIII)V.

Since net.minecraft.client.gui.AbstractGui is abstract, that doesn't exist.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

magneticflux- commented 4 years ago

I've encountered this as well. Copy Mixin Target Reference on a call to this.remove() inside FallingBlockEntity::tick generates Lnet/minecraft/entity/Entity;remove()V instead of the correct Lnet/minecraft/entity/FallingBlockEntity;remove()V for my @Redirect target.

This also results in a cryptic mixin error that only says it "failed injection check" which requires enabling mixin debugging to see what targets are considered.

This plugin could help prevent this from occurring, but there also could be a better error message output if you try to target a method implemented in a superclass.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Earthcomputer commented 3 years ago

Since #1221 was merged, completion of mixin target references is now correct for abstract classes, as well as the inspection for it. However, Copy Mixin target reference is still broken.