Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The Modular FML #22

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

The Modular FML #22

wants to merge 2 commits into from

Conversation

Su5eD
Copy link
Contributor

@Su5eD Su5eD commented Sep 25, 2023

This PR introduces support for using module descriptors in mods, as well as adding built-in module declarations for FML projects. It aims to push support for modules in the neo ecosystem forward and provide better IDE integration for modular projects.

Motivation

Starting with minecraft 1.17, Forge started using the java module system for handling game launch and modloading. This brough many positive changes to the toolchain backend. A good example is the simplified classloader hierarchy, which effectively eliminates "classloader hell", as each module is now assigned a classloader and its classes won't ever be loaded by other ones.

However, for a long time now, mods were not given the option to take advantage of modules. By adding a module declaration to their mods, modders could benefit from great features offered by the module system, including:

  • Explicit dependency declaration, making dependencies easier to keep track of
  • Strong encapsulation, allowing for separation of public and internal code
  • Simpler declaration of used / provided services
  • Improved IDE support

Goals

  • Allowing modders to use module declarations in their mods
  • Adding first-party module declarations to FML subprojects
  • Providing better IDE integration for modular projects

Non-goals

  • Breaking existing consumers of FML code, both of reflective and public access
  • Encapsulating FML internals

Modular Mods

If a module descriptor file exists in a mod, FML will prefer using it over generating a descriptor at runtime. However, a few adjustments must be made to the provided descriptor before it can be used.

If a mod does not include a descriptor, we fall back to the old module generation logic. In addition, modders can now provide their own module name via the Automatic-Module-Name manifest attribute.

Version

We set the module's version to use the one provided by mod metadata, in order to keep it the same in all places and avoid possible confusion. This is needed in cases where the java compiler does not bake a version into the module during compilation.

Benefits for modders

Module goodness!

Modders gain access to great features of the java module system, including:

  • Explicit dependency declaration, making dependencies easier to keep track of
  • Strong encapsulation, allowing for separation of public and internal code (more on that below in Questions, alternatives)
  • Simpler declaration of used / provided services
  • IDE support for module access information

IDE Syntax Highlighting

Given that neo already uses modules at runtime, module encapsulation is enforced. Mods might accidentally use classes from libraries that are not exported without getting warned by the IDE. If a mod / project doesn't contain a module declaration, the IDE will treat it as a classpath library, ignoring any module information of dependencies. This can lead to unexpected crashes at runtime and needless confusion for modders.

void fancyMethod() {
    // The IDE issues no error, and neither does the java compiler
    // Throws an exception at runtime:
    // java.lang.IllegalAccessError: class com.example.examplemod.ExampleMod (in module examplemod) cannot access class cpw.mods.util.LambdaExceptionUtils$Supplier_WithExceptions (in module cpw.mods.securejarhandler) because module cpw.mods.securejarhandler does not export cpw.mods.util to module examplemod
    cpw.mods.util.LambdaExceptionUtils.rethrowSupplier(() -> null);
}

By adding a module descriptor to our project, we enable all access-related checks (both for the IDE and compiler), eliminating possible IllegalAccessErrors.

void fancyMethod() {
    // Does not compile, the IDE issues an error as well
    cpw.mods.util.LambdaExceptionUtils.rethrowSupplier(() -> null);
    //      ^
    // (package cpw.mods.util is declared in module cpw.mods.securejarhandler, but module examplemod does not read it)
}

Module declarations for FML projects

Adds compile-time module declarations for all FML projects. For now, all modules are declared open and export all their packages. This allows for an easier transition period without breaking dependents, should we want to encapsulate these modules in the future.

The module naming conventions follows the scheme of net.neoforged.fancymodloader.<project name>. In total, 7 modules are being added:

  • net.neoforged.fancymodloader.core
  • net.neoforged.fancymodloader.earlydisplay
  • net.neoforged.fancymodloader.events
  • net.neoforged.fancymodloader.language.java
  • net.neoforged.fancymodloader.language.lowcode
  • net.neoforged.fancymodloader.language.minecraft
  • net.neoforged.fancymodloader.loader

Questions and alternatives

Should we keep all FML modules open?

Currently, people are allowed reflective access into FML. I think it's a good idea to keep FML modules open even after adding declaration files during a short transition period window.

Should provided mod descriptors be forced open?

FML requires reflective access into mods, and other neo libraries (such as eventbus) might need it as well. We can open mod modules to only our own libraries, keeping them "protected" from reflective access from other mods. Alternatively, we can flag entire mod modules as open.

@Technici4n
Copy link
Member

Our solution to this is making all mod modules read minecraft when they're loaded. In dev, this doesn't affect anything, as the minecraft module exists but doesn't contain any code. In production, it gives mods access to minecraft code.

Couldn't we make forge transitive-read minecraft in prod?

@Technici4n Technici4n marked this pull request as draft February 4, 2024 07:15
@neoforged-automation neoforged-automation bot added the needs rebase This Pull Request needs to be rebased before being merged label Aug 8, 2024
@neoforged-automation
Copy link

@Su5eD, this pull request has conflicts, please resolve them for this PR to move forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs rebase This Pull Request needs to be rebased before being merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants