From 45105b7e432c41ddeb4ae5249b222f50aa3aac41 Mon Sep 17 00:00:00 2001 From: Matyrobbrt <65940752+Matyrobbrt@users.noreply.github.com> Date: Tue, 26 Nov 2024 00:46:57 +0200 Subject: [PATCH] Add dependency overrides (#214) (#221) --- .../net/neoforged/fml/loading/FMLConfig.java | 79 +++++++++++++++++++ .../net/neoforged/fml/loading/ModSorter.java | 38 ++++++++- loader/src/main/resources/lang/en_us.json | 2 + .../neoforged/fml/loading/FMLLoaderTest.java | 43 ++++++++++ .../neoforged/fml/loading/ModFileBuilder.java | 4 + .../fml/loading/SimulatedInstallation.java | 7 ++ 6 files changed, 170 insertions(+), 3 deletions(-) diff --git a/loader/src/main/java/net/neoforged/fml/loading/FMLConfig.java b/loader/src/main/java/net/neoforged/fml/loading/FMLConfig.java index a6e587d5f..6f40eb354 100644 --- a/loader/src/main/java/net/neoforged/fml/loading/FMLConfig.java +++ b/loader/src/main/java/net/neoforged/fml/loading/FMLConfig.java @@ -6,6 +6,7 @@ package net.neoforged.fml.loading; import com.electronwill.nightconfig.core.CommentedConfig; +import com.electronwill.nightconfig.core.Config; import com.electronwill.nightconfig.core.ConfigSpec; import com.electronwill.nightconfig.core.InMemoryFormat; import com.electronwill.nightconfig.core.file.CommentedFileConfig; @@ -19,9 +20,16 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.function.Function; +import java.util.stream.Collectors; +import org.jetbrains.annotations.Unmodifiable; +import org.jetbrains.annotations.UnmodifiableView; import org.slf4j.Logger; public class FMLConfig { @@ -95,6 +103,7 @@ private static Object maxThreads(final Object value) { private static final Logger LOGGER = LogUtils.getLogger(); private static final FMLConfig INSTANCE = new FMLConfig(); + private static Map> dependencyOverrides = Map.of(); private static final ConfigSpec configSpec = new ConfigSpec( // Make sure the values are written in the same order as the enum. InMemoryFormat.withUniversalSupport().createConfig(LinkedHashMap::new)); @@ -103,6 +112,17 @@ private static Object maxThreads(final Object value) { for (ConfigValue cv : ConfigValue.values()) { cv.buildConfigEntry(configSpec, configComments); } + + // Make sure that we don't end up "correcting" the config and removing dependency overrides + // We accept any objects (parsing and validation is done when the config is loaded) + configSpec.define("dependencyOverrides", () -> null, object -> true); + configComments.set("dependencyOverrides", configComments.createSubConfig()); + configComments.setComment("dependencyOverrides", """ + Define dependency overrides below + Dependency overrides can be used to forcibly remove a dependency constraint from a mod or to force a mod to load AFTER another mod + Using dependency overrides can cause issues. Use at your own risk. + Example dependency override for the mod with the id 'targetMod': dependency constraints (incompatibility clauses or restrictive version ranges) against mod 'dep1' are removed, and the mod will now load after the mod 'dep2' + dependencyOverrides.targetMod = ["-dep1", "+dep2"]"""); } private CommentedConfig configData; @@ -124,6 +144,10 @@ private void loadFrom(Path configFile) { } else { // This populates the config with the default values. configSpec.correct(this.configData); + + // Since dependency overrides have an empty validator, they need to be added manually. + // (Correct doesn't correct an absent value since it's valid). + this.configData.set("dependencyOverrides", this.configData.createSubConfig()); } this.configData.putAllComments(configComments); @@ -144,6 +168,43 @@ public static void load() { } } FMLPaths.getOrCreateGameRelativePath(Paths.get(FMLConfig.getConfigValue(ConfigValue.DEFAULT_CONFIG_PATH))); + + // load dependency overrides + Map> dependencyOverrides = new HashMap<>(); + var overridesObject = INSTANCE.configData.get("dependencyOverrides"); + if (overridesObject != null) { + if (!(overridesObject instanceof Config cfg)) { + LOGGER.error("Invalid dependency overrides declaration in config. Expected object but found {}", overridesObject); + return; + } + + cfg.valueMap().forEach((modId, object) -> { + // We accept both dependencyOverrides.target = "-dep" and dependencyOverrides.target = ["-dep"] + var asList = object instanceof List ls ? ls : List.of(object); + var overrides = dependencyOverrides.computeIfAbsent(modId, k -> new ArrayList<>()); + for (Object o : asList) { + var str = (String) o; + var start = str.charAt(0); + if (start != '+' && start != '-') { + LOGGER.error("Found invalid dependency override for mod '{}'. Expected +/- in override '{}'. Did you forget to specify the override type?", modId, str); + } else { + var removal = start == '-'; + var depMod = str.substring(1); + overrides.add(new DependencyOverride(depMod, removal)); + } + } + }); + } + + if (!dependencyOverrides.isEmpty()) { + LOGGER.warn("*".repeat(30) + " Found dependency overrides " + "*".repeat(30)); + dependencyOverrides.forEach((modId, ov) -> LOGGER.warn("Dependency overrides for mod '{}': {}", modId, ov.stream().map(DependencyOverride::getMessage).collect(Collectors.joining(", ")))); + LOGGER.warn("*".repeat(88)); + } + + // Make the overrides immutable + dependencyOverrides.replaceAll((id, list) -> List.copyOf(list)); + FMLConfig.dependencyOverrides = Collections.unmodifiableMap(dependencyOverrides); } public static String getConfigValue(ConfigValue v) { @@ -172,4 +233,22 @@ public static void updateConfig(ConfigValue v, T value) { public static String defaultConfigPath() { return getConfigValue(ConfigValue.DEFAULT_CONFIG_PATH); } + + @Unmodifiable + public static List getOverrides(String modId) { + var ov = dependencyOverrides.get(modId); + if (ov == null) return List.of(); + return ov; + } + + @UnmodifiableView + public static Map> getDependencyOverrides() { + return Collections.unmodifiableMap(dependencyOverrides); + } + + public record DependencyOverride(String modId, boolean remove) { + public String getMessage() { + return (remove ? "softening dependency constraints against" : "adding explicit AFTER ordering against") + " '" + modId + "'"; + } + } } diff --git a/loader/src/main/java/net/neoforged/fml/loading/ModSorter.java b/loader/src/main/java/net/neoforged/fml/loading/ModSorter.java index 142cb47c1..b9b8fdeaa 100644 --- a/loader/src/main/java/net/neoforged/fml/loading/ModSorter.java +++ b/loader/src/main/java/net/neoforged/fml/loading/ModSorter.java @@ -75,7 +75,7 @@ public static LoadingModList sort(List plugins, List mods, fin // Otherwise, lets try and sort the modlist and proceed ModLoadingException modLoadingException = null; try { - ms.sort(); + ms.sort(issues); } catch (ModLoadingException e) { modLoadingException = e; } @@ -105,7 +105,7 @@ private static List concat(List... lists) { } @SuppressWarnings("UnstableApiUsage") - private void sort() { + private void sort(List issues) { // lambdas are identity based, so sorting them is impossible unless you hold reference to them final MutableGraph graph = GraphBuilder.directed().build(); AtomicInteger counter = new AtomicInteger(); @@ -120,6 +120,26 @@ private void sort() { .map(IModInfo::getDependencies).mapMulti(Iterable::forEach) .forEach(dep -> addDependency(graph, dep)); + // now consider dependency overrides + // we also check their validity here, and report unknown mods as warnings + FMLConfig.getDependencyOverrides().forEach((id, overrides) -> { + var target = (ModInfo) modIdNameLookup.get(id); + if (target == null) { + issues.add(ModLoadingIssue.warning("fml.modloadingissue.depoverride.unknown_target", id)); + } else { + for (FMLConfig.DependencyOverride override : overrides) { + var dep = (ModInfo) modIdNameLookup.get(override.modId()); + if (dep == null) { + issues.add(ModLoadingIssue.warning("fml.modloadingissue.depoverride.unknown_dependency", override.modId(), id)); + } else if (!override.remove()) { + // Add ordering dependency overrides (random order -> target AFTER dependency) + // We do not need to check for overrides that attempt to change the declared order as the sorter will detect the cycle itself and error + graph.putEdge(dep, target); + } + } + } + }); + final List sorted; try { sorted = TopologicalSort.topologicalSort(graph, Comparator.comparing(infos::get)); @@ -237,7 +257,19 @@ private DependencyResolutionResult verifyDependencyVersions() { final var modVersionDependencies = modFiles.stream() .map(ModFile::getModInfos) .mapMulti(Iterable::forEach) - .collect(groupingBy(Function.identity(), flatMapping(e -> e.getDependencies().stream(), toList()))); + .collect(groupingBy(Function.identity(), flatMapping(e -> { + var overrides = FMLConfig.getOverrides(e.getModId()); + // consider overrides and invalidate dependencies that are removed + if (!overrides.isEmpty()) { + var ids = overrides.stream() + .filter(FMLConfig.DependencyOverride::remove) + .map(FMLConfig.DependencyOverride::modId) + .collect(toSet()); + return e.getDependencies().stream() + .filter(v -> !ids.contains(v.getModId())); + } + return e.getDependencies().stream(); + }, toList()))); final var modRequirements = modVersionDependencies.values().stream().mapMulti(Iterable::forEach) .filter(mv -> mv.getSide().isCorrectSide()) diff --git a/loader/src/main/resources/lang/en_us.json b/loader/src/main/resources/lang/en_us.json index 707177132..096452c2e 100644 --- a/loader/src/main/resources/lang/en_us.json +++ b/loader/src/main/resources/lang/en_us.json @@ -17,6 +17,8 @@ "fml.modloadingissue.discouragedmod": "Mod §e{1}§r §ddiscourages§r the use of §3{0}§r §o{2,vr}§r\n§7Currently, §3{0}§r§7 is §o{3}§r\n§7The reason is:§r §o{4,i18ntranslate}§r", "fml.modloadingissue.discouragedmod.proceed": "Proceed at your own risk", "fml.modloadingissue.duplicate_mod": "Mod §e{0}§r is present in multiple files: {1}", + "fml.modloadingissue.depoverride.unknown_target": "Unknown dependency override target with id §e{0}§r", + "fml.modloadingissue.depoverride.unknown_dependency": "Unknown mod §e{0}§r referenced in dependency overrides for mod §e{1}§r", "fml.modloading.duplicate_library": "Library §e{3}§r is present in multiple files: {4}", "fml.modloading.incompatiblemod.noreason": "§eNo reason provided§r", "fml.modloading.discouragedmod.noreason": "§eNo reason provided§r", diff --git a/loader/src/test/java/net/neoforged/fml/loading/FMLLoaderTest.java b/loader/src/test/java/net/neoforged/fml/loading/FMLLoaderTest.java index 43a44f0af..5a6e2289c 100644 --- a/loader/src/test/java/net/neoforged/fml/loading/FMLLoaderTest.java +++ b/loader/src/test/java/net/neoforged/fml/loading/FMLLoaderTest.java @@ -10,7 +10,10 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import com.electronwill.nightconfig.core.Config; import java.nio.file.Files; +import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Map; import net.neoforged.fml.ModLoader; @@ -392,6 +395,46 @@ void testUnsatisfiedNeoForgeRange() throws Exception { assertThat(getTranslatedIssues(e.getIssues())).containsOnly("ERROR: Mod testproject requires neoforge 999.6 or above\nCurrently, neoforge is 1\n"); } + @Test + void testDependencyOverride() throws Exception { + installation.setupProductionClient(); + installation.writeConfig("[dependencyOverrides]", "targetmod = [\"-depmod\", \"-incompatiblemod\"]"); + installation.buildModJar("depmod.jar").withMod("depmod", "1.0").build(); + installation.buildModJar("incompatiblemod.jar").withMod("incompatiblemod", "1.0").build(); + installation.buildModJar("targetmod.jar") + .withModsToml(builder -> { + builder.unlicensedJavaMod(); + builder.addMod("targetmod", "1.0", c -> { + var sub = Config.inMemory(); + sub.set("modId", "depmod"); + sub.set("versionRange", "[2,)"); + sub.set("type", "required"); + + var sub2 = Config.inMemory(); + sub2.set("modId", "incompatiblemod"); + sub2.set("versionRange", "[1,"); + sub2.set("type", "incompatible"); + c.set("dependencies.targetmod", new ArrayList<>(Arrays.asList(sub, sub2))); + }); + }) + .build(); + assertThat(launchAndLoad("forgeclient").issues()).isEmpty(); + } + + @Test + void testInvalidDependencyOverride() throws Exception { + installation.setupProductionClient(); + + // Test that invalid targets and dependencies warn + installation.writeConfig("[dependencyOverrides]", "unknownmod = [\"-testmod\"]", "testmod = [\"+depdoesntexist\"]"); + installation.buildModJar("testmod.jar").withMod("testmod", "1.0").build(); + + var r = launchAndLoad("forgeclient"); + assertThat(getTranslatedIssues(r.issues())).containsOnly( + "WARNING: Unknown dependency override target with id unknownmod", + "WARNING: Unknown mod depdoesntexist referenced in dependency overrides for mod testmod"); + } + @Test void testDuplicateMods() throws Exception { installation.setupProductionClient(); diff --git a/loader/src/test/java/net/neoforged/fml/loading/ModFileBuilder.java b/loader/src/test/java/net/neoforged/fml/loading/ModFileBuilder.java index 3564e30e5..17342f932 100644 --- a/loader/src/test/java/net/neoforged/fml/loading/ModFileBuilder.java +++ b/loader/src/test/java/net/neoforged/fml/loading/ModFileBuilder.java @@ -64,6 +64,10 @@ public ModFileBuilder withTestmodModsToml(Consumer customizer) }); } + public ModFileBuilder withMod(String id, String version) { + return withModsToml(builder -> builder.unlicensedJavaMod().addMod(id, version)); + } + public ModFileBuilder withModTypeManifest(IModFile.Type type) { return withManifest(Map.of( "FMLModType", type.name())); diff --git a/loader/src/test/java/net/neoforged/fml/loading/SimulatedInstallation.java b/loader/src/test/java/net/neoforged/fml/loading/SimulatedInstallation.java index 1d3933a2c..02f75f691 100644 --- a/loader/src/test/java/net/neoforged/fml/loading/SimulatedInstallation.java +++ b/loader/src/test/java/net/neoforged/fml/loading/SimulatedInstallation.java @@ -316,6 +316,13 @@ public ModFileBuilder buildModJar(String filename) throws IOException { return new ModFileBuilder(path); } + public void writeConfig(String... lines) throws IOException { + var file = getGameDir().resolve("config/fml.toml"); + + Files.createDirectories(file.getParent()); + Files.writeString(file, String.join("\n", lines)); + } + public static void writeJarFile(Path file, IdentifiableContent... content) throws IOException { try (var fout = Files.newOutputStream(file)) { writeJarFile(fout, content);