Thaumic Augmentation

Thaumic Augmentation

7M Downloads

Failed class transformer on launch crashing with Witchery: Companion

xJon opened this issue ยท 14 comments

commented

Might be a race condition as this does not occur consistently on launch.
See log: https://mclo.gs/XXfgHCO#L1560

commented

There seem to be some really weird things going on - in that second log for example, there are other coremods throwing exceptions / not finding things, not just TA. I haven't seen transformers randomly fail before - normally when things are not found some mod is rewriting frames, or deleting methods, or something like that.

I see from the stack trace that the coremod is being called from MixinBooter - I don't know if that would be the issue here. I know TA currently works fine in CleanroomMC which I thought had it built in, so I'm not sure.

commented

I do also want to add this: https://mclo.gs/hs6dGl9#L383
Another log of a horrible launch where there are many failed class transformers and possibly no link to Witchery: Companion but other mods.
Perhaps class transformers can fail at random which causes issues with various mods?

commented

@TheCodex6824 Indeed it is working fine, and these are only edge-case issues (also I wasn't running CleanroomMC but rather Forge). I've reproduced them on my machine randomly, and usually simply relaunching works fine. I also heard from another player who had the same experience (with Witchery: Companion), which lead me to believe it's not related to my computer.
That being said, I understand what you mean and this could be caused by a different mod altogether, however I'm not sure how to go about figuring that out.

commented

Yeah, figuring these out is tricky. If you upload a log where everything is working, we can compare the output to see if the order of when things run is changing. If the issue was an instruction in a method being missing, I'd say try installing thaumcraft fix since that has much better error reporting than TA does, but I don't think I got around to fleshing out the output for not finding the method itself yet. I think I'll be adding that later...

commented

Interesting... here's a log with a successful launch: https://mclo.gs/cqS1kqM

commented

This crash occurs because my Mixin TileEntityCauldronMixin fails to being applied to TileEntityCauldron class of Witchery: Resurrected, and I think the main reason is stated in those 4 lines of log:


[Client thread/WARN] [mixin]: Error loading class: net/minecraft/block/BlockFire (net.minecraftforge.fml.common.asm.ASMTransformerWrapper$TransformerException: Exception in class transformer thecodex6824.thaumicaugmentation.core.TATransformer@66eb81d8 from coremod Thaumic Augmentation Core Plugin)

[Client thread/FATAL] [mixin]: Mixin apply for mod unknown-owner failed mixins.witcherycompanion.json:block.entity.TileEntityCauldronMixin from mod unknown-owner -> net.msrandom.witchery.block.entity.TileEntityCauldron: org.spongepowered.asm.mixin.transformer.throwables.InvalidMixinException Unexpecteded ClassMetadataNotFoundException whilst transforming the mixin class: [MAIN Applicator Phase -> mixins.witcherycompanion.json:block.entity.TileEntityCauldronMixin from mod unknown-owner -> Apply Methods -> (Lnet/minecraft/block/state/IBlockState;Lcom/llamalad7/mixinextras/injector/wrapoperation/Operation;)Lnet/minecraft/block/Block;:wrapOperation$bij000$witcherycompanion_0_21_3_beta$WPallowOtherHeatSources -> Transform Instructions -> GETSTATIC net/minecraft/init/Blocks::field_150480_ab:Lnet/minecraft/block/BlockFire; -> desc=Lnet/minecraft/block/BlockFire;]

org.spongepowered.asm.mixin.transformer.throwables.InvalidMixinException: Unexpecteded ClassMetadataNotFoundException whilst transforming the mixin class: [MAIN Applicator Phase -> mixins.witcherycompanion.json:block.entity.TileEntityCauldronMixin from mod unknown-owner -> Apply Methods -> (Lnet/minecraft/block/state/IBlockState;Lcom/llamalad7/mixinextras/injector/wrapoperation/Operation;)Lnet/minecraft/block/Block;:wrapOperation$bij000$witcherycompanion_0_21_3_beta$WPallowOtherHeatSources -> Transform Instructions -> GETSTATIC net/minecraft/init/Blocks::field_150480_ab:Lnet/minecraft/block/BlockFire; -> desc=Lnet/minecraft/block/BlockFire;]

[...]

Caused by: org.spongepowered.asm.mixin.throwables.ClassMetadataNotFoundException: net.minecraft.block.BlockFire

I certainly do not have expertise on the topic, but I think this happens when your transformer is applied inbetween the phases that Mixin goes through, in particular I think between this phase (when metadata is generated) https://github.com/SpongePowered/Mixin/wiki/Flippin'-Mixins,-how-do-they-work%3F#3221-parsing-mixins and this one (where metada is used) https://github.com/SpongePowered/Mixin/wiki/Flippin'-Mixins,-how-do-they-work%3F#333-applying-mixins.

Probably your transform invalidates the metadata generated by my Mixin during the first mentioned phase, making it mixin invalid. This is because the mixin in question, (code available here https://github.com/michelegargiulo/Witchery-Companion/blob/master/src/main/java/com/smokeythebandicoot/witcherycompanion/mixins/block/entity/TileEntityCauldronMixin.java) references Blocks.FIRE, without modifying such class in any way, as the Mixin targets something entirely different.

I can see a few solutions, the first being the one I personally think would be the best:

  1. Port your ASM code to Mixins (your mod will require a dependency on MixinBooter) - also recommended by Cleanroom authors, as Mixins have some safeguards, can be made deterministic with priorities and in general work better when applied together
  2. Refactor my mixin to not reference Blocks.FIRE. While this in theory should solve the issue, it is just a workaround for this specific conflict. Your mod might still be incompatible with other mods out there which reference classes that you transformed in their mixins
  3. If your functionality (I think in this case, the Ward Focus) does not strictly require the transform, make it configurable and let modpack makers handle it (worst solution imho)
commented

Update:
I implemented a fix on my end that is basically option 2
I refactored my mixin to not reference directly Blocks.FIRE and instead rely on a call to an helper class to avoid BlockFire class loading. This seems to have fixed this particular issue.

commented

Update: I implemented a fix on my end that is basically option 2 I refactored my mixin to not reference directly Blocks.FIRE and instead rely on a call to an helper class to avoid BlockFire class loading. This seems to have fixed this particular issue.

Thanks for fixing the issue. I'm still going to see if I can fix this on my end, because as you mentioned this will probably just happen again with some other mod.

Since it seemed like mixin classloading BlockFire was the issue, I wonder if TA failing was due to the deobfuscation transformer not being ran yet. I use SRG names for all names*, so if the deobfuscator didn't remap the names from "Notch" / obfuscated names to SRG, that would explain why it couldn't find the method.

*with the exception of development environments, but that isn't happening here

commented

@TheCodex6824 I believe @michelegargiulo's fix addresses the original issue.
Regarding the TA errors in the second log I posted, I believe it's a separate issue that has nothing to do with Witchery: Companion, so feel free to close this.

Asking around CleanroomMC's Discord, the consensus seems to suggest that

DeobfuscationTransformer loads before mixin launch wrapper handling legacy class transformers

Which I am not sure has anything to do with Thaumic Augmentation in particular, although it definitely affects it, but there have been similar infrequent crashes on launch that mentioned other mods (such as Universal Tweaks - https://mclo.gs/QRJSOXf#L861 + ACGaming/UniversalTweaks#480) that I posted over CleanroomMC's Discord (also, here's a debug log that mentions TA: https://mclo.gs/MrJii0v).
What they all have in common (which normal launch logs don't) is this:
[org.spongepowered.asm.service.mojang.MixinServiceLaunchWrapper]: A re-entrant transformer 'net.minecraftforge.fml.common.asm.transformers.DeobfuscationTransformer' was detected and will no longer process meta class data

commented

I need to read more about how the "legacy transformers" work and when they get ran. I have to think about whether I want to move everything over to mixins or not, but if it would happen it would have to be on a major release so I don't randomly break modpacks in some random patch.

One interesting thing from the debug log you linked that mentions TA: even though TA currently has horrible coremod diagnostics, JEID printed out something helpful:

[19:38:50] [main/TRACE] [mixin]: Catching
org.dimdev.jeid.core.ASMException: JustEnoughIDs - Class transformation error
addEnchantment func_77966_a: (Lnet/minecraft/enchantment/Enchantment;I)V cannot be found in aip
<init>: (Laow;)V
<init>: (Laow;I)V
<init>: (Laow;II)V
<init>: (Lain;)V
<init>: (Lain;I)V
<init>: (Lain;II)V
<init>: (Lain;IILfy;)V
F: ()V
<init>: (Lfy;)V
b: ()Z
a: (Lry;)V
a: (I)Laip;
c: ()Lain;
a: (Laed;Lamu;Let;Lub;Lfa;FFF)Lud;
onItemUseFirst: (Laed;Lamu;Let;Lub;Lfa;FFF)Lud;
a: (Lawt;)F
<more methods>

It looks like JEID was looking for a SRG name (it's named func_ instead of being horribly obfuscated, and there are class names), but everything in the class seems to be completely obfuscated (class names are obfuscated, method names are letters). I don't know why that's happening though...

commented

Indeed someone mentioned how "notch mapped fields, methods and classes don't get turned into searge mappings". Again I assume this is not Thaumic Augmentation's fault. Also Universal Tweaks uses Mixin and has also come up in these random failed launches, but then again mixins probably have better mod intercompatibility which would avoid issues like this problem with Witchery: Companion.

commented

From some extensive testing, I believe this is caused by LoliASM's optimizeFMLRemapper feature - I can't reproduce the issues when setting B:optimizeFMLRemapper=false.

commented

From some extensive testing, I believe this is caused by LoliASM's optimizeFMLRemapper feature - I can't reproduce the issues when setting B:optimizeFMLRemapper=false.

Nice work. I'm going to close this then since it is their issue, although from the discussion here I am evaluating whether using mixin would be an option in the future. A lot of the current coremod could probably be moved to mixin without any issues, but the parts I'm worried about are those that need to find complex (aka multiple) instruction sequences, or those that are creating what are effectively new if statements, or other things like that.

commented

A bit unrelated to the issue, but if in the future you will decide to port your asm to mixin, I'll be happy to lend an hand however I can. I'm by no means an expert on the topic, but I gained a fair bit of experience from my latest project (that is 95% mixins).

Mixin is an extremely flexible and powerful tool, and despite the fact that it can't do 100% (I think) of what can be done with pure asm, it is extremely easier to code, maintain and way more compatible.

Feel free to take a look to the mixin package of my mod https://github.com/michelegargiulo/Witchery-Companion/tree/master/src/main/java/com/smokeythebandicoot/witcherycompanion/mixins for a variety of examples that I'm trying to keep it as organized as possible, with comments, documentation and explanations.