Skip to content

Conversation

@rlnt
Copy link
Member

@rlnt rlnt commented Apr 15, 2025

Proposed Changes

  • added check for minimal Gradle version
  • added check for minimal ModDevGradle version
  • updated internal ModDevGradle version to 2.0.80
  • updated minimum ModDevGradle version to 2.0.64-beta
  • updated Gradle wrapper to 8.12.1
  • updated minimum Gradle version to 8.12.1
  • changed datagen run directory to temporary directory to avoid crashes with file-based runtime mods
  • changed game test namespace to the test mod instead of the main mod
  • changed game test run directory to temporary directory to improve load time

Explanation

Datagen run directory change

Plenty of mods cause issues in the datagen process. Since the datagen environment is not a proper client and only minimally bootstraps the Minecraft instance, many mods throw errors.

Since the introduction of NeoForge, it's been possible to load runtime mods by placing the JAR files into the run directory directly. To avoid loading these mods in the datagen run config, the run directory was changed to a temporary folder inside build/tmp/datagenRuns.

Game test namespace change

Previously, the mod ID was used as the registered namespace for game tests. This led to the requirement of registering the game tests within a @GameTestHolder with the mod ID as its target. This made little sense since the game tests are located inside the test mod to avoid shipping them to the end user and so they can make use of test mod objects. Additionally, a structure file for the game test plots needed to be placed inside the main mod namespace.

With this change, the test mod namespace now owns all game tests and the structure file.

Game test run directory change

This is similar to the run directory change. Game tests themselves should not make use of file-based runtime mods. To speed up the test process, the run directory was separated.

Questions

Plugin version check

The new logic to determine the version of a plugin has a fallback in case the plugin wasn't found. Right now, this check is only used for MDG itself. The fallback in that case is quite useless since the AlmostGradleExtension already throws an error if MDG is not present because of an import. Maybe we should extract all references to that import to another class or guard the access within a static class to have proper handling for the missing plugin. Right now, the error message could be confusing.

Test mod run directory

With this pull request, the run directory of game tests was changed. The question is whether the run directory for the test mod run config itself should also be changed. In my opinion, this should be a config option. It can be useful to share a run directory with the client run config because you can also load the file-based runtime mods or join the same world. However, it can also be annoying if the automatic world join is enabled.

Additional Context

This pull request supersedes #1.

@rlnt rlnt requested a review from LLytho April 15, 2025 18:33
@rlnt rlnt self-assigned this Apr 15, 2025
Copy link
Contributor

@kylev kylev left a comment

Choose a reason for hiding this comment

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

My change is correctly included and the gradle upgrade looks complete and correct, SHA256 of the wrapper verified on my machine.

image

@LLytho
Copy link
Member

LLytho commented Apr 16, 2025

Plugin version check

Would handle it as low prio in the future. If MDG is not applied it throws a NoClassDefFoundError which is enough in my opinion. Not a huge fan of having a guard class just for this.

Test mod run directory

Would be nice as a config option yes. But we probably need to find a better way in the future to handle additional features for run configs instead of adding a new config flag and handle it.
Typical in the runs block of MDG we can use create("name") { client() } and maybe here we can create something on our own.

neoForge {
    runs {
    	// a
        testMod { }
        
        // b
        create("testMod") { }
        
        // c
        createTestMod { }
    }
}

This would let us easily change the default run options together with our own. The setup code would be nearly the same then.

@rlnt rlnt merged commit c168c16 into main Apr 17, 2025
1 check passed
@rlnt rlnt deleted the version-1.2.0 branch April 17, 2025 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants