LoliKingdom / LoliASM

The lolis are now preparing to bytecode manipulate your game.
GNU Lesser General Public License v2.1
105 stars 23 forks source link

Fix HWYLA's crash when looking at villagers #94

Closed xemnes closed 2 years ago

xemnes commented 2 years ago

getting mass crash spam (because the game doesnt close on crash lol) when looking at villagers, crash report seems to point to waila https://pastes.dev/bm2I0kHsNY

double checked to see if it happens without loliasm and it doesnt.

edit: wait nope. i think im wrong https://pastebin.com/sJjKCDt4 log is being spammed, just no crash report popping up.

edit2: looked at the code in an ide, i dont get it... the field exists

embeddedt commented 2 years ago

i dont get it... the field exists

The deobfuscated field only exists in your development environment. At runtime, it should be using field_175563_bv (the SRG name) instead. One of your mods must think the game is running in dev for some reason.

xemnes commented 2 years ago

hmmm that would be rather odd, though im fairly sure this is hwyla/waila thats at fault here but the reflection used does state both the obfuscated and deobfuscated field, 'careerId' is apparently the field that doesnt exist. i wondered if perhaps it was changed in later mappings, but nope. image

edit: so i ran the game in dev to double check... and i can confirm its got something to do with the mod itself because its now giving me this error in the console Caused by: net.minecraftforge.fml.relauncher.ReflectionHelper$UnableToAccessFieldException: net.minecraftforge.fml.relauncher.ReflectionHelper$UnableToFindFieldException: java.lang.NoSuchFieldException: field_175563_bv so now its literally the opposite, why would that happen... i have a hunch...

edit2: my hunch is correct, the obfuscated and deobfuscated field args are the wrong way round lmao image swapping them fixed it.

since hwyla is no longer being developed for 1.12, can i suggest this be implemented into lolisam as a mod fix?

Rongmario commented 2 years ago

I think this bug only surfaced because of LoliASM's deobfuscation mapper optimization.

xemnes commented 2 years ago

im not sure thats the case, if its happening without loliasm then surely its a problem with hwyla? the problem happened in the dev environment of hwylas source, i didnt add loliasm when i was testing it. seems the reflection code in the mod was truly incorrect.

Rongmario commented 2 years ago

That's because deobfuscation doesn't taken place in the dev env, hence why it'd happen in dev too.

xemnes commented 2 years ago

hmmm, ok. perhaps i just dont fully understand it.

embeddedt commented 2 years ago

Despite the ReflectionHelper code being seemingly incorrect, I can't seem to reproduce it in a regular instance with only LoliASM (5.2) and Hwyla installed. I ran /summon villager then looked at the villager. No errors were logged for me.

xemnes commented 2 years ago

thats really odd https://github.com/TehNut-Mods/HWYLA/blob/1.12/src/main/java/mcp/mobius/waila/addons/minecraft/HUDHandlerVillager.java the code does seem to be incorrect in the source, the fields being in the wrong places. ive tested with and without loliasm, just hwyla on its own and in dev and the outcome is always the same. console will be spammed with the field not existing. swapping the fields then compiling and testing fixes the problem for me.

embeddedt commented 2 years ago

This is a long shot but what Java version are you using?

xemnes commented 2 years ago

1.8.0_331

embeddedt commented 2 years ago

That's close enough to mine (I have 332). You have no other mods installed besides LoliASM when this happens?

xemnes commented 2 years ago

happens with or without loliasm

Rongmario commented 2 years ago

The plot thickens... What's your mod list? Would HWYLA on its own replicate it?

xemnes commented 2 years ago

hmmm, i do wonder if the source is the problem, could it be that it was changed in the source and the release version on curseforge is different? i do run a custom version of hwyla, but ive not touched the reflection code. my version just changes the hud to show energy as a bar (well it does to a certain degree, im stuggling to replicate the one probes code, the bar doesnt show on every tile entity that has energy capability and i dont know why) image

embeddedt commented 2 years ago

@xemnes Where did you get your custom Hwyla from? I can decompile it and check.

xemnes commented 2 years ago

i got it from here https://github.com/TehNut-Mods/HWYLA/tree/1.12 as far as im aware, its the original source

do you want me to add mt fork to github?

embeddedt commented 2 years ago

Okay, I'm using the release version so I'll decompile that one to compare.

embeddedt commented 2 years ago

No, looks like the code is exactly the same:

int careerId = ReflectionHelper.getPrivateValue(EntityVillager.class, (EntityVillager)ent, new String[]{"field_175563_bv", "careerId"});
xemnes commented 2 years ago

can you try running in dev to see if the problem occurs there?

xemnes commented 2 years ago

just pushed my changes to my fork if you want to test that https://github.com/xemnes/HWYLA/tree/1.12

xemnes commented 2 years ago

The plot thickens... What's your mod list? Would HWYLA on its own replicate it?

it does indeed https://gfycat.com/mistyplainblackrussianterrier i reverted the change i made that fixes it to show i can however confirm it doesnt happen with the version from curseforge, it only happens with a version compiled from the source. im using intellij idea with gradle to compile the source, also tested it with a completely unmodified version of the source and the same thing happens. in which case, i suppose if it doesnt happen in the release version on curseforge then no fix needs to be applied. logic would state if im using a custom version then using my own fix is perfectly good enough.

Rongmario commented 2 years ago

Closing bc the custom HWYLA build can resolve this issue.

xemnes commented 2 years ago

huh, i thought i had closed it in my last comment due to my conclusion. sorry about that.

Rongmario commented 2 years ago

No problem!