Modded Hybrid Bukkit/Forge Servers - Enum vs Static Classes

[CryptoMorin]

The Problem

So as most of you know, there are server software out there that basically combine Bukkit and Forge together to make both plugins and mods function properly together. These include Magma, Mohist, Ketting, etc.

They work kind of well for the most part, but of course they fail in some rares cases when plugins use advanced features. One thing that they do is that they inject enums into the org.bukkit.Material class in order to register materials used by other mods.
This fails if someone uses XMaterial, since this class is mostly shadowed for each plugin and not a part of the official Bukkit source code. So when a new material is encountered, XMaterial cannot match it and throws an error (which can be just ignored and return null, but that wouldn't work practically.)

Related Issues:

• [1.16.5] Modded material don't add in XMaterial enum MohistMC/Mohist#1446
• [1.12.2] No data value error MohistMC/Mohist#927
• [Support modded block] - Replace XMaterial enum with a normal class #126

Related Classes:

• https://github.com/MohistMC/Mohist/blob/1.20.1/forge/fmlloader/src/main/java/com/mohistmc/asm/SwitchTableFixer.java
• https://github.com/MohistMC/Mohist/blob/1.20.1/forge/fmlloader/src/main/java/com/mohistmc/asm/EnumDefinalizer.java
• https://github.com/MohistMC/Mohist/blob/241864b867410ba5d0950cd686ca17c02cc359d4/src/main/java/org/bukkit/Material.java#L4408-L4414

Throughout this entire post, I'll be focusing on XMaterial specifically, but this issue is not limited to this class and can include XPotion, XEnchantment, XSound and XBiome. XSound has recently gained the ability to play namespaced sounds, but the enum injection issue will still remain.

The Solution

Now the only way to fix this issue to make it actually support forge servers, is to turn the enum classes into static members like so:

public enum XMaterial {
  GRASS(1, "GRASS_BLOCK"), PAPER, ...
}

// ->

public class XMaterial {
  public static final XMaterial GRASS = new XMaterial(1, "GRASS_BLOCK");
  public static final XMaterial PAPER = new XMaterial();

  public String name() { /* Some cached reflection stuff */ }
  public static XMaterial[] values() { /* Some cached reflection stuff */ }

  public XMaterial matchXMaterial(Material mat) {
    if (unrecognized material in cache) {
      addToCache(new XMaterial(mat));
    }
  }
}

You might ask is it not possible to just a single enum and change its properties?

public enum XMaterial {
  GRASS, PAPER, CUSTOM;
}

public static XMaterial matchXMaterial(Material material) {
  CUSTOM.setName("Blah");
  CUSTOM.setMaterial(material);
  return CUSTOM;
}

No, this is not thread-safe. If multiple threads use this same method, it will fail. But most of the Bukkit's API is already thread-unsafe, so why not? We could probably synchronize it, but it's still not going to be viable when using the object for prolonged periods.

public synchronized static XMaterial matchXMaterial(Material material) {}

// Or this which is more reliable:
public static XMaterial matchXMaterial(Material material) {
  synchronized (CUSTOM) {}
}

Now, back to our previous solution. There are advantages and disadvantages to the static member approach.

Disadvantages

Conversion

Converting these enum classes might sound extremely hard to rewrite all these data, but a simple Java script code can generate the code for us since all the data is there.

Plugins that have Proguard configured to not remove enums (which you should definitely do for XSeries if you're using them for configs/databases too) might have to readjust their settings to not shrink these classes (if they obfuscate with shaded libs).

Java Bytecode

Doing this would mean that the underlying JVM bytecode is going to be changed. The solution is to simply recompile the plugin with the new XSeries version and redistribute it, but for people who somehow remotely implement XSeries, either by a utility plugin or downloading them, they'll encounter weird issues like the NoSuchFieldException. So they'd have to recompile too.

Source Code Format

As you can see, this is very messy and bloated compared to enums.

public class XMaterial {
  public static final XMaterial 
      GRASS = new XMaterial(1, "GRASS_BLOCK"),
      PAPER = new XMaterial()
      ;
}

This is nice and all, but not as concise as enums. Surprisingly, using Kotlin won't make this shorter. In fact, it will be longer!

class XMaterial {
  companion object {
    @JvmField val GRASS = XMaterial(1, "GRASS_BLOCK")
    @JvmField val PAPER = XMaterial()
  }
}

Scala and Java interoperability is just out of the way.

class XMaterial(val name: String, val data: Byte, ...):
  def name(): String =
    /* Some cached reflection stuff */
end XMaterial

object XMaterial {
    val GRASS = XMaterial(1, "GRASS_BLOCK")
    val PAPER = XMaterial()
}

from Java:

XMaterial mat = XMaterial$.MODULE$.GRASS();

Also, let's not forget that this will probably need Kotlin/Scala Runtime libs and that's a big no for most projects.

Switch Statements

// With enums you can do:
switch (material) {
  case GRASS:
    return something;
  case PAPER:
    return somethingElse;
}

// With class members switch statements are useless,
// unless you use Java 17 pattern matching which is unlikely:
import static XMaterial;
return switch (material) {
  case GRASS -> something;
  case PAPER -> somethingElse;
}

Object Comparison & Hash

• The == operator will continue to work correctly unless someone initializes another instance.
• ordinal() cannot be used anymore.
• With enums you can use wonderful optimized classes like EnumMap & EnumSet

One way to solve the singleton issue is to simply make equals() and hashcode() methods depend on the underlying Material object, but this will not work for unsupported materials in older versions correctly.

Advantages

Removal of valueOf()

This might sound like a disadvantage, but it's actually one of the common sources of errors for XSeries newcomers. This method should not be used in any circumstances.

Memory Space

Enums often require more than twice as much memory as static constants and will consume 10x more file size compared to static constants. (StackOverFlow)
Considering that we will be caching our own name() this won't be entirely true.

Abstraction

If we, at some point in time wanted to extend another class, we're free to do so.

Modded Support

For people who look to make their plugin compatible as mods and not just inside hybrid servers, this can be a huge step and will gain more attraction when your plugin/mod supports more platforms.

Conclusion

It would seem that the disadvantages outweigh the advantages, but the biggest advantage is obviously that we can support modded servers that way. Please let me know if I missed something or I provided wrong information.

Personally, I was completely against this at first, but now I'm unsure.

If you want this whole change to happen, react with 👍 if you agree and 👎 if you disagree or react with 😕 if you're unsure. Feel free to comment additional information if you like. I'd appreciate it if you only vote once you've actually read the entire post.