AppleCore

AppleCore

56M Downloads

[Crash] MFR

artdude543 opened this issue ยท 10 comments

commented

Updated to 1.2.1 as suggested from #15 but I am crashing during the load up in regards to MFR.

Crash Log: https://gist.github.com/artdude543/33b9bc30395118171c32#file-crash-report-L5726

If you need more information just ask :)

commented

Looks like I need some more time to test/refine the stuff added in 1.2.0. I think I'll just remove that version for now and re-release once I know everything's working again.

Use v1.1.0 instead for now.

Thanks for reporting the crash, leaving this open until it's fixed.

commented

No worries happy to help out, I'll downgrade until you get a stable ish build out. I'm also happy to help test builds if needed :)

commented

I rushed this version out for no real reason; it is mostly just an API upgrade, but no mods use the new API stuff yet, so for end users there's really no difference between v1.2.0 and v1.1.0.

commented

I tried to find out what's breaking, but I have no idea :(
The MFR method isn't all that complicated and the ASM code seems to be all fine.

commented

I think I know what's going on:
In #15 a crash was posted with this exception:
java.lang.RuntimeException: mods/natura/blocks/crops/BerryBush: updateTick method not found

The fix for this was a83e895, but I think this is not a correct fix for the issue. The exception was thrown because the transformer was looking for a deobfuscated method in an obfuscated environment (it was looking for 'updateTick'). a83e895 was a fix to check if classnames are obfuscated and only use the obfuscated references in that case. This will only work when the exception was to find a obfuscated method in an deobfuscated environment (or reference frame), which was not the case in the exception!

a83e895 should be reverted and the real fix should be to find why the 'updateTick' reference was not obfuscated.

But I could also be totally wrong :P

commented

The fix in a83e895 is (or should be) correct. Here's what's going on:

  • In a development environment, method/field/class names use MCP names (like updateTick) in both Minecraft classes and (deobf compiled and/or CCC deobfed) mod classes (if CCC is not installed, then you'd have to check for both SRG and MCP names because mods compiled normally will have SRG names)
  • In obfuscated environments, Minecraft classes will have obfuscated method/field/class names in class transformers that use a sorting index of < 1000, at which point, FML's deobfuscating class remapper will run and convert all obfuscated names to their SRG equivalents.
  • In obfuscated environments, mod classes will always have SRG method/field/class names.

So, this means:

  • For obfuscated classes (Minecraft classes), I need to use the obfuscated names
  • For non-obfuscated classes in an obfuscated environment, I need to use SRG names
  • For all classes in a deobfuscated (dev) environment, I need to use MCP names.

As for this MFR crash, I've not looked into it too much because I got frustrated, but I've run into array index errors when calculating frames before that were due to incorrect method parameters in an invoke instruction. I have no idea how that could be the case here, though. I'm also not sure if I can get away with not calculating frames in ModulePlantFertilization.

commented

Ah, so even when mods reference obfuscated classes they will do that in SRG names, as those obfuscated classes get deobfuscated. I didn't know that!

But this doesn't explain is why in #15 the Transformer was looking for a MCP name instead of SRG:
java.lang.RuntimeException: mods/natura/blocks/crops/BerryBush: updateTick method not found.

commented

That's because, before the fix, I was using only environment obfuscation to check which method name to use instead of class obfuscation (which is only true for Minecraft classes).

commented

The "good" news is that this error also occurs in a dev environment, so it'll be at least somewhat less painful to debug. Working on other stuff at the moment, though.

commented

From skyboy on IRC:

[23:12] <+skyboy> squeek: if squeek_ doesn't exist for the growable method, rename the original method to that, then replace the original (otherwise do nothing - assume they have implemented compat)
[23:12] <+skyboy> your replacement does all the logic you need prior, calls the other method, stows the value, does everything it needs to after, then returns it
[23:13] not sure what you mean
[23:14] <+skyboy> you can create new methods
[23:14] yes
[23:14] <+skyboy> my suggestion is that you create a new method containing the original code of the method you are altering
[23:14] <+skyboy> then replace entirely the method you're looking at, and call out to your created method for the default implementation
[23:14] ah i see
[23:15] what would be the benefit of that?
[23:15] seems like it'd have the same result
[23:15] <+skyboy> it avoids fucking with the scope, LVT, and local variables of the method
[23:15] <+skyboy> which is what's causing your problem