Merged
Conversation
kylev
approved these changes
Apr 15, 2025
Member
Plugin version checkWould handle it as low prio in the future. If MDG is not applied it throws a Test mod run directoryWould 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. neoForge {
runs {
// a
testMod { }
// b
create("testMod") { }
// c
createTestMod { }
}
}This would let us easily change the default |
LLytho
approved these changes
Apr 16, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Proposed Changes
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
@GameTestHolderwith 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
AlmostGradleExtensionalready 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.