The state of Mixins on (Neo)Forge #383
Closed
Su5eD
announced in
Announcements
Replies: 1 comment 1 reply
-
|
I've been hit by a few of these issues - though I do have one question. As an aside, why did you only mention JS coremods, and not those in java? From my experience they've always crashed upon failed transformations which does mitigate that one issue - though of course, that doesn't mitigate any of the other issues here and I do believe a mixin fork is a good idea. |
Beta Was this translation helpful? Give feedback.
1 reply
-
|
Didn't Forge decommission Java coremods starting with Minecraft 1.13? |
Beta Was this translation helpful? Give feedback.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
The state of Mixins on (Neo)Forge
In this post, I'd like to dive into the current situation regarding Mixins on Forge/NeoForge, which has gone unchanged for quite some time now. I'll go over what Mixin is, why it's important to us, what the current problems are, and how we can fix them.
For clarity, NeoForge and MinecraftForge are both referred to as simply "Forge" here, though I believe it doesn't make a huge difference, as the two handle mixin the same way.
Background
With the quick pace and magnitude Minecraft's codebase is growing, it is becoming harder to keep up with changes and to provide centralized modding API's for every feature used by developers, which creates the need for mods to modify minecraft's code by themselves. Modders often need to make small, quick and safe changes to the game's code without having to rely on raw bytecode transformers, which require advanced knowledge about java bytecode and are hard to get right.
This gave rise to Mixin, which has been the go-to framework for modders when it comes to modifying vanilla code. Mixin provides modders with an easy-to-use, high-level API with a variety of injectors and transformations to pick from. However, this all comes with a catch: the Mixin you might be familiar with from Fabric is not quite the same as what modders are used to on Forge. Let's see why, and what can be done about it.
Present state
Forge ships the official, upstream version of Mixin, whereas Fabric uses their own fork. As of the time of writing, the last update to Mixin's official github repository was on December 1, 2021, nearly 2 years ago. The upstream version is missing many features which are considered critical by modern standards and the demanding needs of modders. There's multiple reasons for how we got here, mainly coming down to features getting rejected over safety concerns or bugs that were discovered since mixin's last update.
Let's go over a few examples of ongoing problems below.
Interface mixins (#318 and #421)
Perhaps the most important feature missing from upstream mixin is interface injection. And I can give you a perfect example of why this matters with Fabric's content registries api's OxidizableMixin, which allows Fabric mods to register oxidization stages for their blocks. Forge, unfortunately, doesn't have any API that would allow this.
So let's say you're making a forge mod, and you'd like to add oxidization stages for one of your blocks. Unfortunately for you, minecraft's map "registry" containing this data is immutable by default and must first be converted into a
HashMap. Because it's wrapped in aSupplier, you can't easily modify it via reflection. Therefore, the only way to change it is by using coremods. Predictably, the first thing to come to your mind is Mixin, but you can't use that, as the version shipped by forge does not support interface injection. Your only remaining option is using js coremods, which I'll talk about later on.Arbitrary constructor injection points (#267)
This functionality was denied by upstream, stating that it would be "unsafe", and that mod authors should instead register their own injection point if they really need it.
However, this leads to unnecessary and pointless duplicate code across mods. Everyone single mod would have to include their own mixin plugin and register an injection point, just to overcome a small safeguard. In the end, it is likely people will resort to using libraries providing extended mixin functionality such as MixinExtras rather then implementing it themselves.
Modifying anonymous inner classes (#560)
This bug in Mixin's annotation processor prevents you from compiling anonymous class injectors, crashing the java compiler. As a result, modders were forced to downgrade their version of Mixin to avoid the issue. It was later fixed in Fabric's fork.
Fabric's Mixin fork
Fact is, none of these issues exist over in Fabric's Mixin fork, which is regularly maintained. They've successfully managed to eliminate all of the issues listed above with pretty much minimal maintanence costs. With upstream mixin's slow nature of development, you don't need to worry about syncing frequently or dealing with breaking changes. Actually, most of the ongoing issues can be fixed with minimal changes to the codebase.
Another benefit of Fabric's fork is a feature that prefixes all injector methods with the id of the mod they originate from. This alone makes debugging substantially easier for both developers and users, allowing them to track down mods causing issues in no time. With a few adjustments - namely fixing
@ModifyArgsto work on Modlauncher's modular classloader structure - it would take no effort to swap out the version used by Forge for another variant.The practice of forking libraries is nothing new, and is fairly common in the open source world - namely, many linux distros and the like do exactly this! They maintain their own forks of mainstream libraries, often fine-tuned for the exact use cases they need, where they can quickly implement critical patches and necessary features that would otherwise be deemed unfit for all consumers of the library, contributing important features back to upstream where possible. What I'm proposing here for mixin is essentially the same thing - let's maintain our fork capable of precisely what we need, and allow upstream to take their time implementing the features for everyone. Once we're ready to make the switch, the transition will be completely smooth without having to worry about accidentally breaking mods.
To sum it up, the arguments in favor of using a fork are:
A call for change
Forge mods rely on these features, and must mitigate issues on their own
Just because Forge has not taken action towards updating mixin for a long time doesn't mean modders are satisfied with its state. Over the past couple weeks, I've come across multiple mods that had resorted to doing their best in mitigating mixin restrictions and bugs on their own, just so that they're able to continue functioning. IMO, this is not something a major modloader should allow to happen, ever.
Even though they can't replace the runtime, several Forge mods are already using Fabric Mixin's annotation processor in their builds to mitigate issues. We can easily see this by searching GitHub repositories for Mixin AP declarations in Gradle buildscripts. Unlike the Mixin runtime, the AP is only need for compilation, and can therefore be easily replaced by devs. However, this still leaves them with an outdated runtime.
Next up is Mixin's
@ModifyArgsinjector, which has been broken on Forge since it started using Java modules in Minecraft 1.17. It relies on generating variants of theArgsclass in a synthetic package, which doesn't exist at compile time. Java loads its classes in modules based on packages that belong to them. Because this particular package is synthetic, it doesn't exist in themixinmodule, and trying to load its classes will load them in the unnamed module instead, preventing mods from accessing them.In an effort to mitigate this issue, some mods have started including this package via a dummy class. This allows java to pick up the package and add it to a named module, which is accessible from other mods. However, it doesn't come without dangerous side effects. Java's module system forbids split packages, which means a particular package can only ever exist in a single module at a time. If two mods were to apply the same mitigation and ship the
org.spongepowered.asm.synthetic.argspackage, it would quickly lead to a crash. The issue must be fixed exlusively in Mixin, which is responsible for modlauncher compatibility.Connector's approach to dealing with this issue is using MixinTransmogrifier, a library that replaces the mixin version used by Forge with Fabric's fork at runtime. This is done by changing the Mixin module's source URL to our own jar. However, while this might work for all classes that are loaded after the patch has been applied, existing classes already defined in the JVM are left unaffected.
The only way to update those is by using instrumentation, which comes with its own caveats:
Is instrumentation really required, though? Under normal circumstances, there only appear to be 5 loaded mixin classes before we are able to inject. Fortunately for us, none of these happen to be modified in the fork's code, so we can leave them untouched without having to redefine them with instrumentation. Sure, it's a risk, but given the side effects, I think it's worth it.
There are no safer alternatives
In cases where Mixin is not an option, modders are left with only Forge's official coremodding library, JS Coremods. Apart from using a terribly outdated JS standard (
let? never heard of that!), being riddled with bugs and incomplete features (#31, #38, #25), it is just generally a pain to use. By default, when a mixin fails, it immediately crashes your game, whereas JS coremods' errors are silently dumped into the log where you'll never notice them. I remember writing a JS coremod that used afor ... inloop, yet I couldn't get it to work. I kept going through the code over and over trying to find a potential oversight, but I could find nothing. In the end, it turned out the loop I was using was incorrect, and that I had to usefor ... ofinstead. There were no syntax or evaluation errors, everything appeared to be loaded perfectly, and yet it still didn't work.What I'm trying to prove here is that (Neo)Forge's approach to mixin safety is counterproductive, and only leads to the exact opposite. By keeping seemingly "unsafe" features out of Mixin, Forge leaves modders with JS Coremods as their only option for transforming game bytecode. As I just demonstrated above, this library does nothing but drastically increase the chances of modders shooting themselves - or someone else - in the foot, and it should be used solely as a last resort option. Mixin is by far the safest viable coremodding framework out there, and preventing modders from using its full potential will only force them to resort to worse and gradually more dangerous options.
Switching between forks is easy
The good news is that all ongoing issues are tied to the implementation, not the API. This allows us to easily switch beween various forks and derivates of mixin without having to make any adjustments to our code. Maintaining a Mixin fork is not difficult, and it doesn't need to exist forever, either, but rather only for the time necessary until upstream catches up.
Future
The duck test
Despite the lack of updates to the GitHub repository, according to its developers, Mixin is still being developed privately, and is set to receive a new update soon. I don't exactly understand why these are being kept private, especially given Mixin is open source.
NeoForge's stance
After a long discussion on the matter, NeoForge has made the decision to wait for Mixin's next release, which should come "soon" according to its maintainers. The real question is whether that means a few weeks, months, or even years. It should be our duty as modloader developers to ensure modders have the best possible experience using it. Mixin is a vastly popular tool, and there's certainly many community members willing to maintain it when needed. For now though, it seems like the best we can do is to wait and see how things develop over the following months. If you're a modder and the features currently missing from upstream Mixin are important to you, please let your feedback be heard.
Beta Was this translation helpful? Give feedback.
All reactions