can't run baritone with `:fabric:runClient` with litematica in `run/client/mods` due to conflict between mojmap and malilib method names

tolland commented 3 months ago

Some information

Operating system: Java version: 17.0.11 Minecraft version: 1.20.4 Baritone version: 1.10.2 Other mods (if used): litematica and malilib

Exception, error or logs

Please find your latest.log or debug.log in this folder and attach it to the issue

minecraft.log

How to reproduce

Build baritone fabric from a branch, i.e. this one https://github.com/tolland/baritone/tree/1.20.4-bump1
add litematica (and it's dependency malilib) to the fabric/run/client/mods directory
- https://mediafilez.forgecdn.net/files/5170/70/litematica-fabric-1.20.4-0.17.2.jar
- https://mediafilez.forgecdn.net/files/5096/807/malilib-fabric-1.20.4-0.18.1.jar
execute gradle :fabric:runClient

Minecraft will run, and baritone is working. however it will crash when litematica menus are interacted with, due to conflict between mojmap methods.

Caused by: java.lang.StackOverflowError
        at com.mojang.blaze3d.platform.InputConstants.isKeyDown(InputConstants.java:473) ~[minecraft-1.20.4-fabric-merged+fixed-mojmap+intermediary.jar:?]
        at net.minecraft.client.gui.screens.Screen.hasShiftDown(Screen.java:377) ~[minecraft-1.20.4-fabric-merged+fixed-mojmap+intermediary.jar:?]
        at fi.dy.masa.malilib.gui.GuiBase.isShiftDown(GuiBase.java:675) ~[malilib-0.18.0-64b5720b4b825f21.jar:?]
        at fi.dy.masa.malilib.gui.GuiTextFieldGeneric.setCursorPosition(GuiTextFieldGeneric.java:93) ~[malilib-0.18.0-64b5720b4b825f21.jar:?]
        at net.minecraft.client.gui.components.EditBox.moveCursorTo(EditBox.java:256) ~[minecraft-1.20.4-fabric-merged+fixed-mojmap+intermediary.jar:?]
        at fi.dy.masa.malilib.gui.GuiTextFieldGeneric.setCursorPosition(GuiTextFieldGeneric.java:93) ~[malilib-0.18.0-64b5720b4b825f21.jar:?]
        at net.minecraft.client.gui.components.EditBox.moveCursorTo(EditBox.java:256) ~[minecraft-1.20.4-fabric-merged+fixed-mojmap+intermediary.jar:?]

Final checklist

[x] I know how to properly use check boxes
[x] I have included the version of Minecraft I'm running, baritone's version and forge mods (if used).
[x] I have included logs, exceptions and / or steps to reproduce the issue.
[x] I have not used any OwO's or UwU's in this issue.

tolland commented 3 months ago

So if I build the baritone mod and run them in a vanilla minecraft deploy, I don't see this error. This seems to occur purely in a dev environment.

this issue appears to be the same one: https://github.com/maruohon/malilib/issues/149

There is a similar issue here : https://github.com/cabaletta/baritone/issues/4325 with some suggestions, but it's not clear how to implement them if you are not familiar with unimined gradle plugin

It seems like this might be some way to work around the problem: https://github.com/unimined/unimined/blob/main/testing/1.20.4-NeoForged-Forge-Fabric/build.gradle#L45-L49

ZacSharp commented 3 months ago

@tolland it seems like @wagyourtail answered your question in https://github.com/unimined/unimined/issues/74 and fixing name clashes of litematica/malilib with mojmap isn't really in scope for Baritone so I think this issue can be closed?

tolland commented 3 months ago

I'm not sure its completely out of scope, as baritone integrates with litematica (which depends on malilib) via schematic building... and there are a bunch of bugs in the litematica code which are very hard to fix unless you can build baritone together with litematica in intellij

tolland commented 3 months ago

@tolland it seems like @wagyourtail answered your question in unimined/unimined#74 and fixing name clashes of litematica/malilib with mojmap isn't really in scope for Baritone so I think this issue can be closed?

It seems a like a good place to start, but I am unfamiliar with the mappings bit and also with the unimined plugin, so I've not managed to get that working despite some attempts based on the docs provided

ZacSharp commented 3 months ago

Putting this at the bottom of the mappings block in the root build.gradle seems to work:

            stub.withMappings("official", ["mojmap"]) {
                c("exp", []) {
                    m("e", "(I)V", ["setCursorPositionVanilla"])
                }
            }

I wouldn't expect to have to use notch names for this, but trying stub.withMappings("intermediary", ["mojmap"]) resulted in unimined complaining about conflicting target names (both exp and net/minecraft/class_342 mapping to net/minecraft/class_342) and some other combination made fabric runtime remapping crash.

tolland commented 3 months ago

That's total magic to me. But it does seem to work. Thanks!

tolland commented 3 months ago

Ah ok, it seems that getCursorPosition needs to be fixed as well

    at fi.dy.masa.malilib.gui.GuiTextFieldGeneric.getCursorPosition(GuiTextFieldGeneric.java:88) ~[malilib-fabric-1.20.4-0.18.1-mapped-intermediary-1.20.4-mojmap-1.20.4-parchment-2024.04.14-stub-a0f9afd53-mojmap-intermediary.jar:?]
java.lang.StackOverflowError: Unexpected error
    at fi.dy.masa.malilib.gui.GuiTextFieldGeneric.getCursorPosition(GuiTextFieldGeneric.java:88)

ZacSharp commented 3 months ago

Maybe try this one:

            stub.withMappings("official", ["mojmap"]) {
                c("exp", []) {
                    m("e", "(I)V", ["setCursorPositionVanilla"])
                    m("e", "()I", ["getCursorPositionVanilla"])
                }
            }

I made sure it compiles, but didn't test whether it fixes the problem.

tolland commented 3 months ago

I tried that one. It crashes out with an exception...

masa has released a version of malilib and litematica that don't need those methods, so it avoids the crash when running baritone+litematica+malilib, so I think there is a usable work-around.

ZacSharp commented 3 months ago

I tried actually running the game now and after fixing MixinCommandSuggestionHelper to use the new name (seems like I didn't built everything last time?) the game started successfully and randomly clicking some buttons in the Litematica GUI did not cause a crash. Anyway, given how common mojmap is I think malilib / litematica fixing the collision is the only proper fix.

cabaletta / baritone