Noaaan / MythicMetals

Fabric based Minecraft mod that adds new materials into the game. Includes new tools, ores, anvils, and sets of armor.
Other
73 stars 26 forks source link

Replace AbstractMinecartEntity$Type ASM with enum mixin #222

Closed unilock closed 5 months ago

unilock commented 5 months ago

This PR replaces the Fabric-ASM "early riser" with a mixin targetting AbstractMinecartEntity$Type, using some interesting hacks to accomplish the same thing. This allows the mod to run on Forge via Sinytra Connector (previously the game would crash during startup in such a case).

Noaaan commented 5 months ago

I am yet to test this code, I will assume it works fine and functionally works. I do find the PR a bit unsatisfying though. If I merge this hack would ASM not be redundant? Since the type field is created in the Mixin class, the rest of the ASM code should probably just be removed/replaced using it.

unilock commented 5 months ago

The only other use of Fabric-ASM in the mod is the following line in MythicMetals.java: https://github.com/Noaaan/MythicMetals/blob/237d4c29ee820470308a38017f0c937c42b8b4c9/src/main/java/nourl/mythicmetals/MythicMetals.java#L60

...which can probably be replaced with something like:

public static final AbstractMinecartEntity.Type BANGLUM_TNT = Arrays.stream(AbstractMinecartEntity.Type.class.getEnumConstants()).filter(constant -> constant.name().equals("BANGLUM_TNT")).toList().get(0);

...since that's just about what ClassTinkerers#getEnum does anyway.

EDIT: I've tested it, and it works, so... hurray!