search

Chocohead / Fabric-ASM

Just what Asie never wanted
Mozilla Public License 2.0
158 stars 19 forks source link

Make EnumExtender Java 16 ready #9

Closed PlanetTeamSpeakk closed 3 years ago

PlanetTeamSpeakk commented 3 years ago

With Java 12 enums got a bit of an overhaul which means that all enums compiled on that version of Java or newer don't make the values array in their static constructor anymore, but instead in a synthetic static method which breaks this mod's functionality. This change restores it however, while still maintaining compatibility for classes compiled on a Java version older than Java 12.

Also converted a couple large for-loops to use the amazing Stream API instead.

Fixes #8

Note that this only fixes the two examples I mentioned in #8, I have not tested making enums with subclasses.

UpcraftLP commented 3 years ago

Can confirm this works with subclass enums, tested with EnchantmentTarget (yarn name).

Chocohead commented 3 years ago

I think this is going to be a yes but no. For Minecraft it certainly works, and the whitespace/import changes are fixed easily enough, but the way it works is a bit of a problem.

The change at hand which necessitates this comes from a J14 feature request asking to mirror what the ECJ had done (more on that later) to allow increasing the number of constants an enum can have. From there it was added to OpenJDK as part of J15, so clearly testing for J12 and above is a little out. That's no bother to change of course, what is a bother is how the ECJ implemented the feature first. It doesn't create the values array in a separate method, but sets the constant fields themselves in overflow methods named along the lines of enum constant initialization$1 (with a leading space for extra excitement). The way this PR is written it will trip up over these overflow methods expecting one to be the array creation instead.

I've done some restructuring as part of my own fix of #8 to save needing the successive if (modern) ... else ..., this PR did highlight just how oddly repetitive the logic is right now especially for subclasses, so it's certainly been helpful despite not being merged.