cabaletta / baritone

google maps for block game
GNU Lesser General Public License v3.0
7.16k stars 1.44k forks source link

MC 1.12.2 Forge Crash - BlockPos IllegalAccessError #3820

Closed TheWoolsinator closed 1 year ago

TheWoolsinator commented 1 year ago

Some information

Operating system: Windows 10 Java version: 1.8.0_271 Minecraft version: 1.12.2 Baritone version: v1.2.* (tried many releases, api and standalone) Other mods (if used): Tons. Using in a modpack. (I understand that this is unsupported).

I removed the RandomPatches mod which was causing a forge mixin crash and was able to load the instance successfully. I have a feeling something else is modifying/changing the interface to internal classes and causing the BlockPos->BetterBlockPos issues. Validated that -noverify is not being passed to java (as best as i can tell).

Also, Baritone does not show up in the mod list unless I manually add a mcmod.info file to it. Not sure whats up with that.. am I simply not loading the mod correctly or using an incompatible baritone version?

Crash occurs on server connection.

Any guidance would be much appreciated.

Exception, error or logs

See attached crash report. crash-2023-01-29_19.17.21-client.txt

How to reproduce

  1. Install Technic 1.12.2 Modpack via MultiMC
  2. Remove 'RandomPatches' mod from the mods folder
  3. Copy 1.12 compatible baritone forge artifact into mods folder
  4. launch (wait forever for everything to init lol)
  5. connect to a server
  6. observe crash report

Modified settings

To get the modified settings run #modified in game

Final checklist

ZacSharp commented 1 year ago

I can confirm this bug happens, however so far I only had it happen in really nonstandard setups (srg mapped unoptimized 1.12.2 build of Baritone in the dev env of another mod) so I didn't care too much.

If I remember correctly it's some remapper choking on the shadowing relation between fields in BlockPos and BetterBlockPos so there's bytecode trying to access a private field of BlockPos instead of the public equivalent in BetterBlockPos even though the object is a BetterBlockPos and the original code was targeting the public field.

TheWoolsinator commented 1 year ago

@ZacSharp that makes a lot of sense and would definitely explain the behavior I've noted. Thank you for sharing the information it gives me a better idea of where to look.

It's been a while since I've debugged a metaprogramming issue and I haven't been in Java land for nearly 7 years so this will be interesting to say the least.

If I find any fix(es) I will include the information here or in a PR if appropriate. Though I doubt that any fix would apply to baritone itself, and imagine it would require a monkeypatch on whatever code is remapping things.

TheWoolsinator commented 1 year ago

Isolated the mod package responsible for introducing this behavior: https://github.com/LoliKingdom/LoliASM

Conveniently there was a configuration boolean within 'loliasm.cfg' under the config folder which immediately fixed the issue, no code deep dive or patch necessary.

Set to false in the cfg file: `remapper {

Optimizing Forge's Remapper for not storing redundant entries -

B:optimizeFMLRemapper=true

}`

I still never resolved the forge loader mixin crash introduced by the RandomPatches package within this modpack but that is out of scope for this issue as named.

Thanks again @ZacSharp, hope this helps someone else in future!

TheWoolsinator commented 1 year ago

Resolved the mixin crash from RandomPatches.

Set the following to false in the 'randompatches.cfg' file within the config folder: ` # Set this to false to disable the Minecraft class patches (the Toggle Narrator keybind and custom window title/icon).

Default: true

B:patchMinecraftClass=true`
ZacSharp commented 1 year ago

I did some searching (because https://github.com/cabaletta/baritone/pull/3855#issuecomment-1446027424 got me worried about this again) and it seems like that LoliASM's optimization effectively disables the fix for https://github.com/MinecraftForge/MinecraftForge/issues/3043. Not sure whether it is intentionally blocking the required mappings as "redundant" (they always map a name to itself) or whether that's an accident.

The problem I experienced myself on the other hand is probably https://github.com/md-5/SpecialSource/issues/72 and actually not the same. EDIT: nope, that issue seems to be yet another problem

wagyourtail commented 1 year ago

ahh. so would actually using srg mappings for forge builds in 1.12 fix it?

ZacSharp commented 1 year ago

First off to be clear: the problem this issue is about was fixed a long time ago and only reappeared due to an overly aggressive optimization mod. EDIT: There's also a corner case with class loading order.

The other problem I'm talking about will not be fixed by using srg mappings. So far I only had it happen with srg mappings, though #3859 and #2865 suggest it can happen with notch mappings as well. (EDIT: Confirmed with standard notch mapped Baritone and a standard Forge install). If we want to discuss this in more detail we should do so either in one of those issues or in a new one.

Tom28281 commented 3 weeks ago

how do i fix the loliasm issue?