LlamaLad7 / MixinExtras

Companion library to SpongePowered Mixin with many custom injectors for a more expressive experience.
MIT License
319 stars 17 forks source link

WrapOperation - weird crash on Forge 1.20.2 user env #49

Closed kristofbolyai closed 1 year ago

kristofbolyai commented 1 year ago
@Mixin(GuiGraphics.class)
public abstract class ForgeGuiGraphicsMixin {
    @WrapOperation(
            method =
                    "renderTooltip(Lnet/minecraft/client/gui/Font;Ljava/util/List;Ljava/util/Optional;Lnet/minecraft/world/item/ItemStack;II)V",
            at =
                    @At(
                            value = "INVOKE",
                            target =
                                    "Lnet/minecraft/client/gui/GuiGraphics;renderTooltip(Lnet/minecraft/client/gui/Font;Ljava/util/List;Ljava/util/Optional;II)V",
                    remap = false),
            remap = false)
    private void renderTooltipPre(
            GuiGraphics instance,
            Font font,
            List<Component> tooltipLines,
            Optional<TooltipComponent> visualTooltipComponent,
            int mouseX,
            int mouseY,
            Operation<Void> operation,
            @Local ItemStack itemStack) {}
}

This mixin fails to apply in user env, but works fine in dev env.. I am not quite sure what I am doing wrong here, I tried experimenting with remap = false/trues..

Using Mixinextra-0.2.0-rc.5.

Crash log: https://gist.github.com/kristofbolyai/60455d83d353fcc72ab4495a63d8eb89

kristofbolyai commented 1 year ago

I also have a "post" inject at return, which works, so I am guessing this is either the wrapoperation or the @At value being wrong.

LlamaLad7 commented 1 year ago

Fairly sure the At does need remapping. Check the dump with -Dmixin.dumpTargetOnFailure=true I very much doubt that this is an issue with MixinExtras itself.

kristofbolyai commented 1 year ago

I am happy to move off this issue if you wish me to. However, I'll share my findings too, in case you do know why this is..

The project is using Architectury, so we have multiple mixins.

    // Common GuiGraphicsMixin, works fine
    public void m_280153_(Font p_282308_, ItemStack p_282781_, int p_282687_, int p_282292_) {
        this.tooltipStack = p_282781_;
        List var10002 = Screen.m_280152_(this.f_279544_, p_282781_);
        Optional injectorAllocatedLocal9 = p_282781_.m_150921_();
        List injectorAllocatedLocal8 = var10002;
        Operation var10007 = (var0) -> {
            WrapOperationRuntime.checkArgumentCount(var0, 6, "[net.minecraft.client.gui.GuiGraphics, net.minecraft.client.gui.Font, java.util.List, java.util.Optional, int, int]");
            ((GuiGraphics)var0[0]).m_280677_((Font)var0[1], (List)var0[2], (Optional)var0[3], (Integer)var0[4], (Integer)var0[5]);
            return (Void)null;
        };
        LocalRefImpl ref12 = new LocalRefImpl();
        this.wrapOperation$zbe001$renderTooltipPre$mixinextras$bridge$88(this, p_282308_, injectorAllocatedLocal8, injectorAllocatedLocal9, p_282687_, p_282292_, var10007, ref12);
        this.tooltipStack = ItemStack.f_41583_;
        this.handler$zbe000$renderTooltipPost(p_282308_, p_282781_, p_282687_, p_282292_, new CallbackInfo("m_280153_", false));
    }

     // Forge only method, ForgeGuiMixin, the WrapOperation doesn't inject, post inject does..
    public void renderTooltip(Font font, List<Component> textComponents, Optional<TooltipComponent> tooltipComponent, ItemStack stack, int mouseX, int mouseY) {
        this.tooltipStack = stack;
        this.m_280677_(font, textComponents, tooltipComponent, mouseX, mouseY);
        this.tooltipStack = ItemStack.f_41583_;
        this.handler$zcl000$renderTooltipPost(font, textComponents, tooltipComponent, stack, mouseX, mouseY, new CallbackInfo("renderTooltip", false));
    }

This dump is created with @At(remap = true).

kristofbolyai commented 1 year ago
@MixinMerged(
        mixin = "com.wynntils.forge.mixins.ForgeGuiGraphicsMixin",
        priority = 1000,
        sessionId = "adacc952-f694-422d-878f-7ba6937512f9"
    )
    private void wrapOperation$zcl002$renderTooltipPre(GuiGraphics instance, Font font, List tooltipLines, Optional visualTooltipComponent, int mouseX, int mouseY, Operation operation, ItemStack itemStack) {
        ItemTooltipRenderEvent.Pre event = new ItemTooltipRenderEvent.Pre(this, itemStack, Screen.m_280152_(McUtils.mc(), itemStack), mouseX, mouseY);
        MixinHelper.post(event);
        if (!event.isCanceled()) {
            operation.call(new Object[]{instance, font, event.getTooltips(), event.getItemStack().m_150921_(), event.getMouseX(), event.getMouseY()});
        }
    }

@MixinMerged(
        mixin = "com.wynntils.forge.mixins.ForgeGuiGraphicsMixin",
        priority = 1000,
        sessionId = "adacc952-f694-422d-878f-7ba6937512f9"
    )
    @SugarBridge
    private void wrapOperation$zcl002$renderTooltipPre$mixinextras$bridge$89(GuiGraphics instance, Font font, List tooltipLines, Optional visualTooltipComponent, int mouseX, int mouseY, Operation operation, LocalRef itemStack) {
        this.wrapOperation$zcl002$renderTooltipPre(instance, font, tooltipLines, visualTooltipComponent, mouseX, mouseY, operation, (ItemStack)itemStack.get());
    }

Also exists but is not called.

LlamaLad7 commented 1 year ago

And what's in the refmap? Have you registered the AP properly?

kristofbolyai commented 1 year ago

And what's in the refmap?

Weird, it doesn't seem included.

wynntils-forge-refmap.json:

{
  "mappings": {
    "com/wynntils/forge/mixins/ForgeGuiMixin": {
      "render(Lnet/minecraft/client/gui/GuiGraphics;F)V": "Lnet/minecraftforge/client/gui/overlay/ForgeGui;m_280421_(Lnet/minecraft/client/gui/GuiGraphics;F)V"
    }
  },
  "data": {
    "named:intermediary": {
      "com/wynntils/forge/mixins/ForgeGuiMixin": {
        "render(Lnet/minecraft/client/gui/GuiGraphics;F)V": "Lnet/minecraftforge/client/gui/overlay/ForgeGui;m_280421_(Lnet/minecraft/client/gui/GuiGraphics;F)V"
      }
    }
  }
}

wynntils.mixins.forge.json:

{
  "client": [
    "ForgeGuiMixin",
    "ForgeGuiGraphicsMixin"
  ],
  "compatibilityLevel": "JAVA_17",
  "injectors": {
    "defaultRequire": 1
  },
  "minVersion": "0.8.5",
  "mixins": [],
  "package": "com.wynntils.forge.mixins",
  "required": true
}
LlamaLad7 commented 1 year ago

I checked out the problematic commit, added remap = true in the @At, and it works just fine. Note that this is not the same as leaving the remap value as unspecified, since in that case it inherits the injector's false. If this doesn't fix the issue for you then feel free to reopen.

kristofbolyai commented 1 year ago

I didn't know about the inheritance.. Thank you.