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

Convert Coremods from JS to Java #785

Merged
merged 8 commits into from
Sep 19, 2024
Merged

Conversation

shartte
Copy link
Contributor

@shartte shartte commented Apr 5, 2024

Based on neoforged/FancyModLoader#79

This removes the need to load the JS engine if only NF is present, by removing our JS coremods and replacing them with Transformer based Java Coremods instead.

This also bumps JCC since it chokes on the subproject being present on the classpath as folders of classes, rather than jar files.

Coremods:

  • MethodRedirector as the name implies, redirects method calls to other classes. This is based on a JSON file of target classes (as previously) to actually scan. I did not update the JSON file. Currently this is only used for redirecting finalizeSpawn all around the codebase to our hook class, which emits the corresponding event.
  • ReplaceFieldComparisonWithInstanceOf is meant to replace expressions like stack.getItem() == Items.XYZ by stack.getItem() instanceof XYZ allowing mod subclassed items to work with Vanilla logic.
  • ReplaceFieldWithGetterAccess replaces usage of this.field for private fields with use of the corresponding getter, allowing mods that subclass to change what the superclass uses.

@neoforged-pr-publishing
Copy link

neoforged-pr-publishing bot commented Apr 5, 2024

  • Publish PR to GitHub Packages

Last commit published: b41b9de4c07b88b3a0cb19c80787f71bf60d2803.

PR Publishing

The artifacts published by this PR:

Repository Declaration

In order to use the artifacts published by the PR, add the following repository to your buildscript:

repositories {
    maven {
        name 'Maven for PR #785' // https://github.com/neoforged/NeoForge/pull/785
        url 'https://prmaven.neoforged.net/NeoForge/pr785'
        content {
            includeModule('net.neoforged', 'testframework')
            includeModule('net.neoforged', 'neoforge')
        }
    }
}

MDK installation

In order to setup a MDK using the latest PR version, run the following commands in a terminal.
The script works on both *nix and Windows as long as you have the JDK bin folder on the path.
The script will clone the MDK in a folder named NeoForge-pr785.
On Powershell you will need to remove the -L flag from the curl invocation.

mkdir NeoForge-pr785
cd NeoForge-pr785
curl -L https://prmaven.neoforged.net/NeoForge/pr785/net/neoforged/neoforge/21.1.60-pr-785-coremods/mdk-pr785.zip -o mdk.zip
jar xf mdk.zip
rm mdk.zip || del mdk.zip

To test a production environment, you can download the installer from here.

@Minecraftschurli Minecraftschurli added the needs porting Patch conflicts; needs porting and updating. label Apr 25, 2024
@shartte shartte force-pushed the coremods branch 2 times, most recently from 2fa70ee to d889f90 Compare April 27, 2024 15:19
@shartte shartte added 1.20.5 Targeted at Minecraft 1.20.5 and removed needs porting Patch conflicts; needs porting and updating. labels Apr 27, 2024
@shartte shartte force-pushed the coremods branch 2 times, most recently from f8145d3 to 13fd7d5 Compare April 27, 2024 16:17
@neoforged-automation

This comment was marked as resolved.

@shartte shartte changed the base branch from 1.20.x to 1.21.x August 30, 2024 22:07
@shartte shartte added 1.21.1 Targeted at Minecraft 1.21.1 and removed needs rebase 1.20.5 Targeted at Minecraft 1.20.5 labels Aug 30, 2024
@shartte shartte marked this pull request as ready for review September 2, 2024 22:48
Copy link
Member

@Matyrobbrt Matyrobbrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, the field_to_instanceof transformer isn't used. If we decide to keep it, we should yeet its JSON file and make it similar to ReplaceFieldWithGetterAccess (i.e. having them declared in NeoForgeCoreMod)

Copy link
Member

@Matyrobbrt Matyrobbrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just as a note for future™, we should start generating the finalize spawn targets file.

@shartte shartte merged commit 46112b3 into neoforged:1.21.x Sep 19, 2024
6 checks passed
@shartte shartte deleted the coremods branch September 19, 2024 19:38
@neoforged-releases
Copy link

🚀 This PR has been released as NeoForge version 21.1.56.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.21.1 Targeted at Minecraft 1.21.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants