Enum extensions #190
Description
See SpongePowered#387 for prior discussion on this subject. This implementation idea is mostly from @LlamaLad7 on Discord, and it's probably the cleanest way to handle enum additions in Fabric, so I think we should go ahead with it.
Accessing enum constants from the outside
This part of the problem will be left to ClassTweaker, which will simply add the enum field to the enum for compile time (and irrelevantly at runtime too). This is enough for javac to fully recognize the enum constant, both for accessing the field and for switches. Every decompiler I tested accepted the code too, although obviously didn't generate the enum constructor call.
If the user for whatever reason can't/doesn't want to use ClassTweaker, it will also be possible for the user to make their own field pointing to their added enum constant, although this obviously can't be used in switches:
// recommended to use Class Tweaker instead of:
public static final VanillaEnum MYMOD_ADDITION_1 = VanillaEnum.valueOf("MYMOD_ADDITION_1");The Mixin syntax
Enum additions would work with a new enum Mixin syntax, whose constants are appended to the target enum:
@Mixin(VanillaEnum.class)
public enum VanillaEnumMixin {
MYMOD_ADDITION_1, MYMOD_ADDITION_2("constructor args");
// The @Shadow annotation may be placed on constructors for clarity, however it is optional in order to allow for an implicit constructor to also work. Annotation or not, implicit or not, each constructor *must* shadow a constructor in the target enum.
@Shadow
VanillaEnumMixin() {}
@Shadow
VanillaEnumMixin(String arg) {}
}Abstract enums can also be added to, and the abstractness of the enum Mixin must match the abstractness of the target class:
@Mixin(Animal.class)
public enum AnimalMixin {
MYMOD_CAT {
@Override protected void speak() { System.out.println("Meow!"); }
};
@Shadow protected abstract void speak();
}Notice that all enum constants are prefixed by MYMOD_. This is by convention and shall be considered good practice to avoid conflicting with enum constants added by other mods. IDE warnings can be created to encourage this practice.
Ordinals
The ordinals of the target class' original constants are guaranteed to be unchanged. To preserve mod compatibility, mod code should never rely on the ordinal of a specific added constant being a specific value. The ordinal represents the index into the enum's values() array, which is added to by each Mixin in the order that the Mixins are applied, which shall be consistent across runs across all operating systems.
After transformation, the values() array will contain enum constants in the following order:
- All constants from the target class (in the same order)
- For each Mixin in ascending order of priority, breaking ties in an undocumented but guaranteed consistent way, all constants from that Mixin in the same order they were declared in that Mixin.
Some vanilla enums have this "wannabe" ordinal parameter for some reason. Here's an example:
public enum GameType {
SURVIVAL(0, "survival"),
CREATIVE(1, "creative"),
ADVENTURE(2, "adventure"),
SPECTATOR(3, "spectator");
// ...
}These "wannabe" ordinals are used to convert the enum to and from an integer for the purpose of network serialization, which requires unique integers to be passed into this parameter for each enum constant. The implementation additionally requires these integers to be consecutive.
To emulate this, Mixin will provide a magic static method, calls to which will be replaced at transformation time with the ordinal of the enum value being constructed. Again, modders shall not rely on the substitued value being any particular value, only that it is equal to the ordinal of the enum constant, whatever that may be. Here's an example:
@Mixin(GameType.class)
public enum GameTypeMixin {
MYMOD_GAME_TYPE(EnumMixins.currentOrdinal(), "mymod_game_type");
}All of the above is designed to support added enum values being able to be shared over the network, as long as the server and client each have the same set of Mixins to the enum. Vanilla doesn't ever save an enum's ordinal value to disk, and mods should never do that either. Instead, enums should be serialized to disk by a name-based string value.
Implementation concerns
Switch statements inside the enum Mixin or a nestmate of the enum Mixin
When javac encounters a switch statement on an enum which is written inside that same enum, or if the enum is an inner or outer class (i.e. nestmate) to the class containing the switch statement, then enums will be switched directly on the ordinal of the enum rather than creating a switch map. This has been happening since JDK 21 to fix JDK-7176515, and isn't a problem from a binary compatibility perspective as the enum switch will be recompiled along with any changes to the enum anyway as you're in the same compilation unit. However it poses an extra challenge to us.
Firstly if the enum switch is in the target class or one of its nestmates, there is still nothing extra we need to do, as we're not changing any of the target class' original ordinals, and the default branch is still reached and behaves as expected.
However if the user writes an enum switch within their Mixin for the Mixin class, then we have a problem. Take the following example:
public enum FooMixin {
MYMOD_A, MYMOD_B, MYMOD_C;
void print() {
switch (this) {
case MYMOD_A -> System.out.println("A");
case MYMOD_B -> System.out.println("B");
case MYMOD_C -> System.out.println("C");
}
}
}This will be compiled to bytecode identical to compiling the following:
public enum FooMixin {
MYMOD_A, MYMOD_B, MYMOD_C;
void print() {
switch (this.ordinal()) {
case 0 -> System.out.println("A");
case 1 -> System.out.println("B");
case 2 -> System.out.println("C");
default -> {}
}
}
}Which assumes that the ordinals of the added enum values start at 0, which they firstly most likely don't, and secondly it's invalid to rely on the values of ordinals anyway.
To fix this, I propose we throw an error if this pattern is detected in Mixin classes or their nestmates. Thankfully the pattern isn't too hard to detect. If the user wants to switch on this, they should double-cast to the target class (which Class Tweaker will have added the constants to), and switch on that instead. This will cause javac to generate a switch map and therefore the ordinals are no longer hardcoded or relied upon.
Implementation of tie-breaking order for Mixin application
There are a number of ways to do this, but LlamaLad suggested that we should simply define the order for all Mixins (which is currently undefined). It is currently sorted by modid in production, this can be made the defined behavior. Same-priority Mixins to the same enum within the same mod can be sorted alphabetically, or something else arbitrary but consistent.
Changes to network serialized enum constants between mod versions
I would like to express that it is unfortunate that enums are serialized on the network in this way, and that some enums that may commonly want to be targetted by enum additions, such as game types, are network serializable. I would strongly encourage modders to implement some kind of protocol versioning and handshaking if they plan on modifying a network serializable enum. Perhaps the Fabric networking API could make such mod protocol version handshaking easy to opt into with a single line of code. It may potentially be possible to provide IDE warnings if a network serializable enum is added to without ever calling this handshaking functionality from Fabric networking API.
Fabric networking API could also cover any common usecases by detecting if a commonly modified network serializable enum has been modified (e.g. using values().length), and switching to using a custom packet based on the enum name instead in that case.
This unfortunate situation however I don't believe is a good reason to not support it at all, as there is generally not a good alternative, and the need to modify network-serializable enums is getting rarer.