Fix translation issues and introduce translation formatting for issues (

#139)

Split translation keys if they're used for both warnings and errors since the implicitly added translation arguments differ.

Added a lot of tests for translated error messages.

loader/src/main/java/net/neoforged/fml/ModLoader.java

@@ -117,7 +117,7 @@ public static void gatherAndInitializeMods(final Executor syncExecutor, final Ex
         if (!failedBounds.isEmpty()) {
             LOGGER.fatal(CORE, "Failed to validate feature bounds for mods: {}", failedBounds);
             for (var fb : failedBounds) {
-                loadingIssues.add(ModLoadingIssue.error("fml.modloading.feature.missing", null, fb, ForgeFeature.featureValue(fb)).withAffectedMod(fb.modInfo()));
+                loadingIssues.add(ModLoadingIssue.error("fml.modloading.feature.missing", fb, ForgeFeature.featureValue(fb)).withAffectedMod(fb.modInfo()));
             }
             cancelLoading(modList);
             throw new ModLoadingException(loadingIssues);

loader/src/main/java/net/neoforged/fml/ModLoadingIssue.java

@@ -5,11 +5,8 @@
 
 package net.neoforged.fml;
 
-import com.google.common.collect.Streams;
 import java.nio.file.Path;
 import java.util.List;
-import java.util.stream.Stream;
-import net.neoforged.fml.i18n.FMLTranslations;
 import net.neoforged.neoforgespi.language.IModInfo;
 import net.neoforged.neoforgespi.locating.IModFile;
 import org.jetbrains.annotations.Nullable;
@@ -54,22 +51,23 @@ public ModLoadingIssue withCause(Throwable cause) {
         return new ModLoadingIssue(severity, translationKey, translationArgs, cause, affectedPath, affectedModFile, affectedMod);
     }
 
-    public String getTranslatedMessage() {
-        Object[] formattingArgs;
-        // TODO: cleanup null here - this requires moving all indices in the translations
-        if (severity == Severity.ERROR) {
-            // Error translations included a "cause" in position 2
-            formattingArgs = Streams.concat(Stream.of(affectedMod, null, cause), translationArgs.stream()).toArray();
-        } else {
-            formattingArgs = Streams.concat(Stream.of(affectedMod, null), translationArgs.stream()).toArray();
-        }
-
-        return FMLTranslations.parseEnglishMessage(translationKey, formattingArgs);
-    }
-
     @Override
     public String toString() {
-        return severity + ": " + getTranslatedMessage();
+        var result = new StringBuilder(severity + ": " + translationKey);
+        if (!translationArgs.isEmpty()) {
+            result.append(" [");
+            for (int i = 0; i < translationArgs.size(); i++) {
+                if (i > 0) {
+                    result.append("; ");
+                }
+                result.append(translationArgs.get(i));
+            }
+            result.append("]");
+        }
+        if (cause != null) {
+            result.append(" caused by ").append(cause);
+        }
+        return result.toString();
     }
     public enum Severity {
         WARNING,

loader/src/main/java/net/neoforged/fml/i18n/FMLTranslations.java

@@ -6,9 +6,11 @@
 package net.neoforged.fml.i18n;
 
 import com.google.common.base.CharMatcher;
+import java.nio.file.Path;
 import java.text.FieldPosition;
 import java.text.Format;
 import java.text.ParsePosition;
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
@@ -18,6 +20,8 @@
 import java.util.function.Supplier;
 import java.util.regex.Pattern;
 import net.neoforged.fml.Logging;
+import net.neoforged.fml.ModLoadingIssue;
+import net.neoforged.fml.loading.FMLLoader;
 import net.neoforged.fml.loading.StringUtils;
 import net.neoforged.neoforgespi.language.IModInfo;
 import org.apache.commons.lang3.text.ExtendedMessageFormat;
@@ -108,6 +112,7 @@ public static String parseEnglishMessage(final String i18n, Object... args) {
 
     public static String parseFormat(String format, final Object... args) {
         final AtomicInteger i = new AtomicInteger();
+        // Converts Mojang translation format (%s) to the one used by Apache Commons ({0})
         format = FORMAT_PATTERN.matcher(format).replaceAll(matchResult -> {
             if (matchResult.group(0).equals("%%")) {
                 return "%";
@@ -120,6 +125,41 @@ public static String parseFormat(String format, final Object... args) {
         return extendedMessageFormat.format(args);
     }
 
+    public static String translateIssue(ModLoadingIssue issue) {
+        var args = new ArrayList<>(3 + issue.translationArgs().size());
+
+        var modInfo = issue.affectedMod();
+        if (modInfo == null && issue.affectedModFile() != null) {
+            if (!issue.affectedModFile().getModInfos().isEmpty()) {
+                modInfo = issue.affectedModFile().getModInfos().getFirst();
+            }
+        }
+        args.add(modInfo);
+        args.add(null); // Previously mod-loading phase
+        // For errors, we expose the cause
+        if (issue.severity() == ModLoadingIssue.Severity.ERROR) {
+            args.add(issue.cause());
+        }
+        args.addAll(issue.translationArgs());
+
+        args.replaceAll(FMLTranslations::formatArg);
+
+        return parseMessage(issue.translationKey(), args.toArray(Object[]::new));
+    }
+
+    private static Object formatArg(Object arg) {
+        if (arg instanceof Path path) {
+            var gameDir = FMLLoader.getGamePath();
+            if (path.startsWith(gameDir)) {
+                return gameDir.relativize(path).toString();
+            } else {
+                return path.toString();
+            }
+        } else {
+            return arg;
+        }
+    }
+
     public static String stripControlCodes(String text) {
         return PATTERN_CONTROL_CODE.matcher(text).replaceAll("");
     }

loader/src/main/java/net/neoforged/fml/i18n/I18nManager.java

@@ -9,6 +9,7 @@
 import com.google.gson.reflect.TypeToken;
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
 import java.util.Collections;
 import java.util.Map;
 import org.slf4j.Logger;
@@ -30,7 +31,7 @@ public static void injectTranslations(Map<String, String> translations) {
     public static Map<String, String> loadTranslations(String language) {
         var stream = FMLTranslations.class.getResourceAsStream("/lang/" + language + ".json");
         if (stream != null) {
-            try (var reader = new InputStreamReader(stream)) {
+            try (var reader = new InputStreamReader(stream, StandardCharsets.UTF_8)) {
                 return GSON.fromJson(reader, new TypeToken<>() {});
             } catch (IOException e) {
                 LOGGER.error("Failed to load translations for locale {}", language, e);

loader/src/main/java/net/neoforged/fml/javafmlmod/FMLModContainer.java

@@ -112,7 +112,7 @@ protected void constructMod() {
             } catch (Throwable e) {
                 if (e instanceof InvocationTargetException) e = e.getCause(); // exceptions thrown when a reflected method call throws are wrapped in an InvocationTargetException. However, this isn't useful for the end user who has to dig through the logs to find the actual cause.
                 LOGGER.error(LOADING, "Failed to create mod instance. ModID: {}, class {}", getModId(), modClass.getName(), e);
-                throw new ModLoadingException(ModLoadingIssue.error("fml.modloading.failedtoloadmod", e, modClass).withCause(e).withAffectedMod(modInfo));
+                throw new ModLoadingException(ModLoadingIssue.error("fml.modloading.failedtoloadmod").withCause(e).withAffectedMod(modInfo));
             }
         }
         try {
@@ -121,7 +121,7 @@ protected void constructMod() {
             LOGGER.trace(LOADING, "Completed Automatic event subscribers for {}", getModId());
         } catch (Throwable e) {
             LOGGER.error(LOADING, "Failed to register automatic subscribers. ModID: {}", getModId(), e);
-            throw new ModLoadingException(ModLoadingIssue.error("fml.modloading.failedtoloadmod", e).withCause(e).withAffectedMod(modInfo));
+            throw new ModLoadingException(ModLoadingIssue.error("fml.modloading.failedtoloadmod").withCause(e).withAffectedMod(modInfo));
         }
     }
 

loader/src/main/java/net/neoforged/fml/loading/LanguageProviderLoader.java

@@ -58,7 +58,7 @@ public IModLanguageLoader findLanguage(ModFile mf, String modLoader, VersionRang
         final ModLanguageWrapper mlw = languageProviderMap.get(modLoader);
         if (mlw == null) {
             LOGGER.error(LogMarkers.LOADING, "Missing language {} version {} wanted by {}", modLoader, modLoaderVersion, languageFileName);
-            throw new ModLoadingException(ModLoadingIssue.error("fml.language.missingversion", modLoader, modLoaderVersion, languageFileName, "null").withAffectedModFile(mf));
+            throw new ModLoadingException(ModLoadingIssue.error("fml.language.missingversion", modLoader, modLoaderVersion, languageFileName, "-").withAffectedModFile(mf));
         }
         if (!VersionSupportMatrix.testVersionSupportMatrix(modLoaderVersion, modLoader, "languageloader", (llid, range) -> range.containsVersion(mlw.version()))) {
             LOGGER.error(LogMarkers.LOADING, "Missing language {} version {} wanted by {}, found {}", modLoader, modLoaderVersion, languageFileName, mlw.version());

loader/src/main/java/net/neoforged/fml/loading/UniqueModListBuilder.java

@@ -72,30 +72,30 @@ public UniqueModListData buildUniqueList() {
 
         // Its theoretically possible that some mod has somehow moved an id to a secondary place, thus causing a dupe.
         // We can't handle this
-        final List<String> dupedModErrors = modIds.values().stream()
+        final List<ModLoadingIssue> dupedModErrors = modIds.values().stream()
                 .filter(modInfos -> modInfos.size() > 1)
-                .map(mods -> String.format("\tMod ID: '%s' from mod files: %s",
-                        mods.get(0).getModId(),
-                        mods.stream()
-                                .map(modInfo -> modInfo.getOwningFile().getFile().getFileName()).collect(joining(", "))))
+                .map(mods -> ModLoadingIssue.error(
+                        "fml.modloading.duplicate_mod",
+                        mods.getFirst().getModId(),
+                        mods.stream().map(modInfo -> modInfo.getOwningFile().getFile().getFileName()).collect(joining(", "))))
                 .toList();
 
         if (!dupedModErrors.isEmpty()) {
-            LOGGER.error(LOADING, "Found duplicate mods:\n{}", String.join("\n", dupedModErrors));
-            throw new ModLoadingException(dupedModErrors.stream().map(ModLoadingIssue::error).toList());
+            LOGGER.error(LOADING, "Found duplicate mods:\n{}", dupedModErrors.stream().map(Object::toString).collect(joining()));
+            throw new ModLoadingException(dupedModErrors);
         }
 
-        final List<String> dupedLibErrors = versionedLibIds.values().stream()
+        final List<ModLoadingIssue> dupedLibErrors = versionedLibIds.values().stream()
                 .filter(modFiles -> modFiles.size() > 1)
-                .map(mods -> String.format("\tLibrary: '%s' from files: %s",
-                        getModId(mods.get(0)),
-                        mods.stream()
-                                .map(modFile -> modFile.getFileName()).collect(joining(", "))))
+                .map(mods -> ModLoadingIssue.error(
+                        "fml.modloading.duplicate_mod",
+                        getModId(mods.getFirst()),
+                        mods.stream().map(ModFile::getFileName).collect(joining(", "))))
                 .toList();
 
         if (!dupedLibErrors.isEmpty()) {
-            LOGGER.error(LOADING, "Found duplicate plugins or libraries:\n{}", String.join("\n", dupedLibErrors));
-            throw new ModLoadingException(dupedLibErrors.stream().map(ModLoadingIssue::error).toList());
+            LOGGER.error(LOADING, "Found duplicate plugins or libraries:\n{}", dupedLibErrors.stream().map(Object::toString).collect(joining()));
+            throw new ModLoadingException(dupedLibErrors);
         }
 
         // Collect unique mod files by module name. This will be used for deduping purposes

loader/src/main/java/net/neoforged/fml/loading/moddiscovery/ModDiscoverer.java

@@ -221,7 +221,7 @@ public Optional<IModFile> addJarContent(JarContents jarContents, ModFileDiscover
                     addIssue(ModLoadingIssue.warning(reason.get().getReason(), jarContents.getPrimaryPath()).withAffectedPath(jarContents.getPrimaryPath()));
                 } else if (reporting == IncompatibleFileReporting.WARN_ALWAYS) {
                     LOGGER.warn(LogMarkers.SCAN, "Ignoring incompatible jar {} for an unknown reason.", jarContents.getPrimaryPath());
-                    addIssue(ModLoadingIssue.warning("fml.modloading.brokenfile", jarContents.getPrimaryPath()).withAffectedPath(jarContents.getPrimaryPath()));
+                    addIssue(ModLoadingIssue.warning("fml.modloading.brokenfile.unknown", jarContents.getPrimaryPath()).withAffectedPath(jarContents.getPrimaryPath()));
                 }
             }
             return Optional.empty();

loader/src/main/java/net/neoforged/fml/loading/moddiscovery/ModFileInfo.java

@@ -69,7 +69,7 @@ public ModFileInfo(final ModFile modFile, final IConfigurable config, Consumer<I
                 .orElse("");
         // Validate the license is set. Only apply this validation to mods.
         if (this.license.isBlank()) {
-            throw new InvalidModFileException("fml.modloading.missinglicense", this);
+            throw new InvalidModFileException("Missing license", this);
         }
         this.showAsResourcePack = config.<Boolean>getConfigElement("showAsResourcePack")
                 .orElse(false);

loader/src/main/java/net/neoforged/fml/loading/moddiscovery/ModValidator.java

@@ -84,7 +84,7 @@ private void validateLanguages() {
                 issues.addAll(e.getIssues());
                 iterator.remove();
             } catch (Exception e) {
-                issues.add(ModLoadingIssue.error("").withAffectedModFile(modFile).withCause(e));
+                issues.add(ModLoadingIssue.error("fml.modloading.technical_error").withAffectedModFile(modFile).withCause(e));
                 iterator.remove();
             }
         }

loader/src/main/java/net/neoforged/fml/loading/moddiscovery/locators/JarInJarDependencyLocator.java

@@ -83,14 +83,13 @@ protected ModLoadingException exception(Collection<JarSelector.ResolutionFailure
     }
 
     private ModLoadingIssue buildExceptionData(final JarSelector.ResolutionFailureInformation<IModFile> entry) {
-        return ModLoadingIssue.error(
-                getErrorTranslationKey(entry),
-                entry.identifier().group() + ":" + entry.identifier().artifact(),
-                entry.sources()
-                        .stream()
-                        .flatMap(this::getModWithVersionRangeStream)
-                        .map(this::formatError)
-                        .collect(Collectors.joining(", ")));
+        var artifact = entry.identifier().group() + ":" + entry.identifier().artifact();
+        var requestedBy = entry.sources()
+                .stream()
+                .flatMap(this::getModWithVersionRangeStream)
+                .map(this::formatError)
+                .collect(Collectors.joining(", "));
+        return ModLoadingIssue.error(getErrorTranslationKey(entry), artifact, requestedBy);
     }
 
     private String getErrorTranslationKey(final JarSelector.ResolutionFailureInformation<IModFile> entry) {

loader/src/main/java/net/neoforged/fml/loading/moddiscovery/locators/ModsFolderLocator.java

@@ -58,12 +58,12 @@ public void findCandidates(ILaunchContext context, IDiscoveryPipeline pipeline)
                     .sorted(Comparator.comparing(path -> StringUtils.toLowerCase(path.getFileName().toString())))
                     .toList();
         } catch (UncheckedIOException | IOException e) {
-            throw new ModLoadingException(ModLoadingIssue.error("fml.modloading.failed_to_list_folder_content").withAffectedPath(this.modFolder).withCause(e));
+            throw new ModLoadingException(ModLoadingIssue.error("fml.modloading.failed_to_list_folder_content", this.modFolder).withAffectedPath(this.modFolder).withCause(e));
         }
 
         for (var file : directoryContent) {
             if (!Files.isRegularFile(file)) {
-                pipeline.addIssue(ModLoadingIssue.warning("fml.modloading.brokenfile", file).withAffectedPath(file));
+                pipeline.addIssue(ModLoadingIssue.warning("fml.modloading.brokenfile.unknown", file).withAffectedPath(file));
                 continue;
             }
 

loader/src/main/java/net/neoforged/neoforgespi/language/IConfigurable.java

@@ -14,5 +14,5 @@
 public interface IConfigurable {
     <T> Optional<T> getConfigElement(final String... key);
 
-    public List<? extends IConfigurable> getConfigList(final String... key);
+    List<? extends IConfigurable> getConfigList(final String... key);
 }

loader/src/main/java/net/neoforged/neoforgespi/locating/ForgeFeature.java

@@ -32,9 +32,7 @@ private ForgeFeature() {}
     private static final Map<String, IFeatureTest<?>> features = new HashMap<>();
 
     public static <T> void registerFeature(final String featureName, final IFeatureTest<T> featureTest) {
-        if (features.putIfAbsent(featureName, featureTest) != null) {
-            throw new IllegalArgumentException("ForgeFeature with name " + featureName + " exists");
-        }
+        features.put(featureName, featureTest);
     }
 
     private static final MissingFeatureTest MISSING = new MissingFeatureTest();

loader/src/main/resources/lang/da_dk.json

@@ -20,16 +20,16 @@
   "fml.dependencyloading.conflictingdependencies": "Some mods have requested conflicting versions of: §6{3}§r. Requested by: §e{4}§r.",
   "fml.dependencyloading.mismatchedcontaineddependencies": "Some mods have agreed upon an acceptable version range for : §6{3}§r, but no jar was provided which matched the range. Requested by: §e{4}§r.",
   "fml.modloading.corrupted_installation": "Your NeoForge installation is corrupted, please try to reinstall",
-  "fml.modloading.cycle": "Detected a mod dependency cycle: {0}",
+  "fml.modloading.cycle": "Detected a mod dependency cycle: {3}",
   "fml.modloading.failedtoprocesswork": "{0,modinfo,name} ({0,modinfo,id}) encountered an error processing deferred work\n§7{2,exc,msg}",
-  "fml.modloading.brokenfile": "File {2} is not a valid mod file",
+  "fml.modloading.brokenfile": "File {3} is not a valid mod file",
   "fml.modloading.brokenfile.oldforge": "File {2} is for an old version of Minecraft Forge and cannot be loaded",
   "fml.modloading.brokenfile.minecraft_forge": "File {2} is for Minecraft Forge or an older version of NeoForge, and cannot be loaded",
   "fml.modloading.brokenfile.liteloader": "File {2} is a LiteLoader mod and cannot be loaded",
   "fml.modloading.brokenfile.fabric": "File {2} is a Fabric mod and cannot be loaded",
   "fml.modloading.brokenfile.optifine": "File {2} is an incompatible version of OptiFine",
   "fml.modloading.brokenfile.bukkit": "File {2} is a Bukkit or Bukkit-implementor (Spigot, Paper, etc.) plugin and cannot be loaded",
-  "fml.modloading.brokenfile.invalidzip": "File {2} is not a jar file",
+  "fml.modloading.brokenfile.invalidzip": "File {3} is not a jar file",
   "fml.modloading.brokenresources": "File {2} failed to load a valid ResourcePackInfo",
   "fml.modloading.missinglicense": "Missing License Information in file {3}",
   "fml.modloading.technical_error": "A technical error occurred during mod loading: {3}",

loader/src/main/resources/lang/de_de.json

@@ -20,16 +20,16 @@
   "fml.dependencyloading.conflictingdependencies": "Some mods have requested conflicting versions of: §6{3}§r. Requested by: §e{4}§r.",
   "fml.dependencyloading.mismatchedcontaineddependencies": "Some mods have agreed upon an acceptable version range for : §6{3}§r, but no jar was provided which matched the range. Requested by: §e{4}§r.",
   "fml.modloading.corrupted_installation": "Your NeoForge installation is corrupted, please try to reinstall",
-  "fml.modloading.cycle": "Detected a mod dependency cycle: {0}",
+  "fml.modloading.cycle": "Detected a mod dependency cycle: {3}",
   "fml.modloading.failedtoprocesswork": "{0,modinfo,name} ({0,modinfo,id}) encountered an error processing deferred work\n§7{2,exc,msg}",
-  "fml.modloading.brokenfile": "File {2} is not a valid mod file",
+  "fml.modloading.brokenfile": "File {3} is not a valid mod file",
   "fml.modloading.brokenfile.oldforge": "File {2} is for an old version of Minecraft Forge and cannot be loaded",
   "fml.modloading.brokenfile.minecraft_forge": "File {2} is for Minecraft Forge or an older version of NeoForge, and cannot be loaded",
   "fml.modloading.brokenfile.liteloader": "File {2} is a LiteLoader mod and cannot be loaded",
   "fml.modloading.brokenfile.fabric": "File {2} is a Fabric mod and cannot be loaded",
   "fml.modloading.brokenfile.optifine": "File {2} is an incompatible version of OptiFine",
   "fml.modloading.brokenfile.bukkit": "File {2} is a Bukkit or Bukkit-implementor (Spigot, Paper, etc.) plugin and cannot be loaded",
-  "fml.modloading.brokenfile.invalidzip": "File {2} is not a jar file",
+  "fml.modloading.brokenfile.invalidzip": "File {3} is not a jar file",
   "fml.modloading.brokenresources": "File {2} failed to load a valid ResourcePackInfo",
   "fml.modloading.missinglicense": "Missing License Information in file {3}",
   "fml.modloading.technical_error": "A technical error occurred during mod loading: {3}",