Skip to content

Commit

Permalink
Refactor ModLoadingIssue Warning/Error Translations (#167)
Browse files Browse the repository at this point in the history 
* Move all implicit mod loading issue arguments to indices 100+ and assert that all loading issue translations have a special prefix.

* Update translation key usage in Java code

* Reformat caused by processing of files.

* Updated translations to new key scheme

* Make argument 101 the "affected file path" and use it.
  • Loading branch information
@shartte
shartte authored Jun 24, 2024
1 parent 89a32f9 commit 74cb237
Show file tree
Hide file tree
Showing 49 changed files with 1,238 additions and 1,232 deletions.
4 changes: 2 additions & 2 deletions loader/src/main/java/net/neoforged/fml/ModContainer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public final <T extends Event & IModBusEvent> void acceptEvent(T e) {
LOGGER.trace(LOADING, "Fired event for modid {} : {}", this.getModId(), e);
} catch (Throwable t) {
LOGGER.error(LOADING, "Caught exception during event {} dispatch for modid {}", e, this.getModId(), t);
throw new ModLoadingException(ModLoadingIssue.error("fml.modloading.errorduringevent", e.getClass().getName()).withAffectedMod(modInfo).withCause(t));
throw new ModLoadingException(ModLoadingIssue.error("fml.modloadingissue.errorduringevent", e.getClass().getName()).withAffectedMod(modInfo).withCause(t));
}
}

Expand All @@ -189,7 +189,7 @@ public final <T extends Event & IModBusEvent> void acceptEvent(EventPriority pha
LOGGER.trace(LOADING, "Fired event for phase {} for modid {} : {}", phase, this.getModId(), e);
} catch (Throwable t) {
LOGGER.error(LOADING, "Caught exception during event {} dispatch for modid {}", e, this.getModId(), t);
throw new ModLoadingException(ModLoadingIssue.error("fml.modloading.errorduringevent", e.getClass().getName()).withAffectedMod(modInfo).withCause(t));
throw new ModLoadingException(ModLoadingIssue.error("fml.modloadingissue.errorduringevent", e.getClass().getName()).withAffectedMod(modInfo).withCause(t));
}
}
}
4 changes: 2 additions & 2 deletions loader/src/main/java/net/neoforged/fml/ModLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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", fb, ForgeFeature.featureValue(fb)).withAffectedMod(fb.modInfo()));
loadingIssues.add(ModLoadingIssue.error("fml.modloadingissue.feature.missing", fb, ForgeFeature.featureValue(fb)).withAffectedMod(fb.modInfo()));
}
cancelLoading(modList);
throw new ModLoadingException(loadingIssues);
Expand Down Expand Up @@ -297,7 +297,7 @@ private static void addLoadingIssuesFromException(String context, Throwable erro
if (error instanceof ModLoadingException modLoadingException) {
loadingIssues.addAll(modLoadingException.getIssues());
} else {
loadingIssues.add(ModLoadingIssue.error("fml.modloading.uncaughterror", context).withCause(error));
loadingIssues.add(ModLoadingIssue.error("fml.modloadingissue.uncaughterror", context).withCause(error));
}
}

Expand Down
7 changes: 7 additions & 0 deletions loader/src/main/java/net/neoforged/fml/ModLoadingIssue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ public ModLoadingIssue(Severity severity, String translationKey, List<Object> tr
this(severity, translationKey, translationArgs, null, null, null, null);
}

public ModLoadingIssue {
// Make sure it's easy for us to know which translation keys get access to the implicit args at indices 100 and above
if (translationKey.startsWith("fml.") && !translationKey.startsWith("fml.modloadingissue.")) {
throw new IllegalArgumentException("When using FML translation keys, only use fml.modloadingissue. keys for mod loading issues: " + translationKey);
}
}

public static ModLoadingIssue error(String translationKey, Object... args) {
return new ModLoadingIssue(Severity.ERROR, translationKey, List.of(args));
}
Expand Down
26 changes: 13 additions & 13 deletions loader/src/main/java/net/neoforged/fml/common/asm/enumextension/EnumPrototype.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,23 +42,23 @@ static List<EnumPrototype> load(IModInfo mod, Path path) {

String enumName = entryObj.get("enum").getAsString();
if (!isValidClassDescriptor(enumName)) {
error("fml.modloading.enumextender.invalid_enum_name", mod, enumName);
error("fml.modloadingissue.enumextender.invalid_enum_name", mod, enumName);
continue;
}

String fieldName = entryObj.get("name").getAsString();
if (!fieldName.toLowerCase(Locale.ROOT).startsWith(mod.getModId())) {
error("fml.modloading.enumextender.field_name.missing_prefix", mod, fieldName, enumName);
error("fml.modloadingissue.enumextender.field_name.missing_prefix", mod, fieldName, enumName);
continue;
}
if (!SourceVersion.isIdentifier(fieldName)) {
error("fml.modloading.enumextender.field_name.invalid", mod, fieldName, enumName);
error("fml.modloadingissue.enumextender.field_name.invalid", mod, fieldName, enumName);
continue;
}

String ctorDesc = entryObj.get("constructor").getAsString();
if (!isValidConstructorDescriptor(ctorDesc)) {
error("fml.modloading.enumextender.invalid_constructor", mod, ctorDesc, enumName);
error("fml.modloadingissue.enumextender.invalid_constructor", mod, ctorDesc, enumName);
continue;
}

Expand All @@ -73,30 +73,30 @@ static List<EnumPrototype> load(IModInfo mod, Path path) {
JsonObject obj = paramElem.getAsJsonObject();
String className = obj.get("class").getAsString();
if (!isValidClassDescriptor(className)) {
error("fml.modloading.enumextender.argument.reference.invalid_src_class", mod, className, fieldName, enumName);
error("fml.modloadingissue.enumextender.argument.reference.invalid_src_class", mod, className, fieldName, enumName);
continue;
}

if (obj.has("method")) {
String srcMethodName = obj.get("method").getAsString();
if (!SourceVersion.isIdentifier(srcMethodName)) {
error("fml.modloading.enumextender.argument.reference.invalid_src_method", mod, srcMethodName, fieldName, enumName);
error("fml.modloadingissue.enumextender.argument.reference.invalid_src_method", mod, srcMethodName, fieldName, enumName);
continue;
}
ctorParams = new EnumParameters.MethodReference(Type.getObjectType(className), srcMethodName);
} else if (obj.has("field")) {
String srcFieldName = obj.get("field").getAsString();
if (!SourceVersion.isIdentifier(srcFieldName)) {
error("fml.modloading.enumextender.argument.reference.invalid_src_field", mod, srcFieldName, fieldName, enumName);
error("fml.modloadingissue.enumextender.argument.reference.invalid_src_field", mod, srcFieldName, fieldName, enumName);
continue;
}
ctorParams = new EnumParameters.FieldReference(Type.getObjectType(className), srcFieldName);
} else {
error("fml.modloading.enumextender.argument.reference.unexpected_decl", mod, paramElem, fieldName, enumName);
error("fml.modloadingissue.enumextender.argument.reference.unexpected_decl", mod, paramElem, fieldName, enumName);
continue;
}
} else {
error("fml.modloading.enumextender.argument.unexpected_decl", mod, paramElem, fieldName, enumName);
error("fml.modloadingissue.enumextender.argument.unexpected_decl", mod, paramElem, fieldName, enumName);
continue;
}

Expand All @@ -106,7 +106,7 @@ static List<EnumPrototype> load(IModInfo mod, Path path) {
}
return prototypes;
} catch (Throwable e) {
ModLoader.addLoadingIssue(ModLoadingIssue.error("fml.modloading.enumextender.loading_error", path)
ModLoader.addLoadingIssue(ModLoadingIssue.error("fml.modloadingissue.enumextender.loading_error", path)
.withAffectedMod(mod)
.withCause(e));
return List.of();
Expand All @@ -117,7 +117,7 @@ private static EnumParameters loadConstantParameters(IModInfo mod, String enumNa
List<Object> params = new ArrayList<>(obj.size());
Type[] argTypes = Type.getArgumentTypes(ctorDesc);
if (argTypes.length != obj.size()) {
error("fml.modloading.enumextender.argument.constant.count_mismatch", mod, obj.size(), argTypes.length, ctorDesc, fieldName, enumName);
error("fml.modloadingissue.enumextender.argument.constant.count_mismatch", mod, obj.size(), argTypes.length, ctorDesc, fieldName, enumName);
return null;
}

Expand All @@ -129,7 +129,7 @@ private static EnumParameters loadConstantParameters(IModInfo mod, String enumNa
case "C" -> {
String param = element.getAsString();
if (param.length() != 1) {
error("fml.modloading.enumextender.argument.constant.invalid_char", mod, param, idx, fieldName, enumName);
error("fml.modloadingissue.enumextender.argument.constant.invalid_char", mod, param, idx, fieldName, enumName);
return null;
}
params.add(param.charAt(0));
Expand All @@ -143,7 +143,7 @@ private static EnumParameters loadConstantParameters(IModInfo mod, String enumNa
case "Ljava/lang/String;" -> params.add(element.isJsonNull() ? null : element.getAsString());
default -> {
if (!element.isJsonNull()) {
error("fml.modloading.enumextender.argument.constant.unsupported_type", mod, argType, idx, fieldName, enumName);
error("fml.modloadingissue.enumextender.argument.constant.unsupported_type", mod, argType, idx, fieldName, enumName);
return null;
}
params.add(null);
Expand Down
2 changes: 1 addition & 1 deletion loader/src/main/java/net/neoforged/fml/common/asm/enumextension/RuntimeEnumExtender.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ public static void loadEnumPrototypes(Map<IModInfo, Path> paths) {
if (prevProto != null) {
foundDupe = true;
ModLoader.addLoadingIssue(ModLoadingIssue.error(
"fml.modloading.enumextender.duplicate",
"fml.modloadingissue.enumextender.duplicate",
proto.fieldName(),
proto.enumName(),
proto.owningMod(),
Expand Down
39 changes: 28 additions & 11 deletions loader/src/main/java/net/neoforged/fml/i18n/FMLTranslations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.apache.logging.log4j.Logger;
import org.apache.maven.artifact.versioning.VersionRange;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.Nullable;

@ApiStatus.Internal
@SuppressWarnings("deprecation")
Expand Down Expand Up @@ -118,8 +119,24 @@ public static String translateIssue(ModLoadingIssue issue) {
}

private static Object[] getTranslationArgs(ModLoadingIssue issue) {
var args = new ArrayList<>(3 + issue.translationArgs().size());
var args = new ArrayList<>(103);
args.addAll(issue.translationArgs());
// Pad up to 100
while (args.size() < 100) {
args.add(null);
}

// Implicit arguments start at index 100
args.add(getModInfo(issue)); // {100} = affected mod
args.add(getAffectedPath(issue)); // {101} = affected file-path
args.add(issue.cause()); // {102} = exception

args.replaceAll(FMLTranslations::formatArg);

return args.toArray(Object[]::new);
}

private static @Nullable IModInfo getModInfo(ModLoadingIssue issue) {
var modInfo = issue.affectedMod();
var file = issue.affectedModFile();
while (modInfo == null && file != null) {
Expand All @@ -128,17 +145,17 @@ private static Object[] getTranslationArgs(ModLoadingIssue issue) {
}
file = file.getDiscoveryAttributes().parent();
}
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 modInfo;
}

return args.toArray(Object[]::new);
private static @Nullable Path getAffectedPath(ModLoadingIssue issue) {
if (issue.affectedPath() != null) {
return issue.affectedPath();
} else if (issue.affectedModFile() != null) {
return issue.affectedModFile().getFilePath();
} else {
return null;
}
}

private static Object formatArg(Object arg) {
Expand Down
2 changes: 1 addition & 1 deletion loader/src/main/java/net/neoforged/fml/javafmlmod/FMLJavaModLanguageProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public void validate(IModFile file, Collection<ModContainer> loadedContainers, I
.forEach(data -> {
var modId = data.annotationData().get("value");
var entrypointClass = data.clazz().getClassName();
var issue = ModLoadingIssue.error("fml.modloading.javafml.dangling_entrypoint", modId, entrypointClass, file.getFilePath()).withAffectedModFile(file);
var issue = ModLoadingIssue.error("fml.modloadingissue.javafml.dangling_entrypoint", modId, entrypointClass, file.getFilePath()).withAffectedModFile(file);
reporter.addIssue(issue);
});
}
Expand Down
6 changes: 3 additions & 3 deletions loader/src/main/java/net/neoforged/fml/javafmlmod/FMLModContainer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public FMLModContainer(IModInfo info, List<String> entrypoints, ModFileScanData
LOGGER.trace(LOADING, "Loaded modclass {} with {}", cls.getName(), cls.getClassLoader());
} catch (Throwable e) {
LOGGER.error(LOADING, "Failed to load class {}", entrypoint, e);
throw new ModLoadingException(ModLoadingIssue.error("fml.modloading.failedtoloadmodclass").withCause(e).withAffectedMod(info));
throw new ModLoadingException(ModLoadingIssue.error("fml.modloadingissue.failedtoloadmodclass").withCause(e).withAffectedMod(info));
}
}
} finally {
Expand Down Expand Up @@ -118,7 +118,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").withCause(e).withAffectedMod(modInfo));
throw new ModLoadingException(ModLoadingIssue.error("fml.modloadingissue.failedtoloadmod").withCause(e).withAffectedMod(modInfo));
}
}
try {
Expand All @@ -127,7 +127,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").withCause(e).withAffectedMod(modInfo));
throw new ModLoadingException(ModLoadingIssue.error("fml.modloadingissue.failedtoloadmod").withCause(e).withAffectedMod(modInfo));
}
}

Expand Down
2 changes: 1 addition & 1 deletion loader/src/main/java/net/neoforged/fml/loading/FMLServiceProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public List<? extends ITransformer<?>> transformers() {
// Throwing here would cause the game to immediately crash without a proper error screen,
// since this method is called by ModLauncher directly.
ModLoader.addLoadingIssue(
ModLoadingIssue.error("fml.modloading.coremod_error", coreMod.getClass().getName(), sourceFile).withCause(e));
ModLoadingIssue.error("fml.modloadingissue.coremod_error", coreMod.getClass().getName(), sourceFile).withCause(e));
}
}

Expand Down
4 changes: 2 additions & 2 deletions loader/src/main/java/net/neoforged/fml/loading/LanguageProviderLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,11 @@ 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, "-").withAffectedModFile(mf));
throw new ModLoadingException(ModLoadingIssue.error("fml.modloadingissue.language.missingversion", modLoader, modLoaderVersion, "-").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());
throw new ModLoadingException(ModLoadingIssue.error("fml.language.missingversion", modLoader, modLoaderVersion, languageFileName, mlw.version()).withAffectedModFile(mf));
throw new ModLoadingException(ModLoadingIssue.error("fml.modloadingissue.language.missingversion", modLoader, modLoaderVersion, mlw.version()).withAffectedModFile(mf));
}

return mlw.modLanguageProvider();
Expand Down
2 changes: 1 addition & 1 deletion loader/src/main/java/net/neoforged/fml/loading/LoadingModList.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public void addEnumExtenders() {
.forEach(mod -> mod.getConfig().<String>getConfigElement("enumExtensions").ifPresent(file -> {
Path path = mod.getOwningFile().getFile().findResource(file);
if (!Files.isRegularFile(path)) {
ModLoader.addLoadingIssue(ModLoadingIssue.error("fml.modloading.enumextender.file_not_found", path).withAffectedMod(mod));
ModLoader.addLoadingIssue(ModLoadingIssue.error("fml.modloadingissue.enumextender.file_not_found", path).withAffectedMod(mod));
return;
}
pathPerMod.put(mod, path);
Expand Down
Loading

0 comments on commit 74cb237

Please sign in to comment.