From bd809e5af7f2c295aefd5f9c72acbbbd297a3d31 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Sat, 27 Jul 2024 22:56:20 +0100 Subject: [PATCH] Prevent community mods from warning on unused translations in the common assets Currently when linting translations, we check for any unused translation keys. This works fine for the default mods, which own the entire sets of translation files. For community mods, they often import the translation files from the common mod assets, but they may only use some of the translations provided. Currently, they would get warnings about not using translations from the common files they have imported. Since the community mods don't own those translations, getting warnings about it is annoying. To solve this issue, introduce a AllowUnusedTranslationsInExternalPackages in the mod.yaml which defaults to true. This will prevent reporting of unused translation keys from external assets. Keys that are used for external assets will still be validated, and keys from the mod assets will be both validated and unused keys will be reported. We default the new flag to true and don't provide an update rule. This means community mods will get the new behaviour. For the default mods, we do want to check the "external" assets, since we control those assets. So the default mods have their mod.yaml updated to disable the flag and retain the existing behaviour of checking everything. --- OpenRA.Game/FileSystem/FileSystem.cs | 18 ++++----------- OpenRA.Game/Manifest.cs | 7 +++++- OpenRA.Game/Map/Map.cs | 4 ++-- OpenRA.Game/Map/MapPreview.cs | 4 ++-- .../Lint/CheckTranslationReference.cs | 22 +++++++++++++++---- OpenRA.Mods.Common/UpdateRules/UpdateUtils.cs | 18 +++++++-------- .../UtilityCommands/ExtractYamlStrings.cs | 2 +- mods/cnc/mod.yaml | 2 ++ mods/d2k/mod.yaml | 2 ++ mods/ra/mod.yaml | 2 ++ mods/ts/mod.yaml | 2 ++ 11 files changed, 50 insertions(+), 33 deletions(-) diff --git a/OpenRA.Game/FileSystem/FileSystem.cs b/OpenRA.Game/FileSystem/FileSystem.cs index f1e8a7a6f0..c6b7f9260c 100644 --- a/OpenRA.Game/FileSystem/FileSystem.cs +++ b/OpenRA.Game/FileSystem/FileSystem.cs @@ -23,7 +23,7 @@ namespace OpenRA.FileSystem bool TryGetPackageContaining(string path, out IReadOnlyPackage package, out string filename); bool TryOpen(string filename, out Stream s); bool Exists(string filename); - bool IsExternalModFile(string filename); + bool IsExternalFile(string filename); } public class FileSystem : IReadOnlyFileSystem @@ -270,21 +270,11 @@ namespace OpenRA.FileSystem } /// - /// Returns true if the given filename references an external mod via an explicit mount. + /// Returns true if the given filename references any file outside the mod mount. /// - public bool IsExternalModFile(string filename) + public bool IsExternalFile(string filename) { - var explicitSplit = filename.IndexOf('|'); - if (explicitSplit < 0) - return false; - - if (!explicitMounts.TryGetValue(filename[..explicitSplit], out var explicitPackage)) - return false; - - if (installedMods[modID].Package == explicitPackage) - return false; - - return modPackages.Contains(explicitPackage); + return !filename.StartsWith($"{modID}|", StringComparison.Ordinal); } /// diff --git a/OpenRA.Game/Manifest.cs b/OpenRA.Game/Manifest.cs index 9bf719e7d2..632d4f5ee7 100644 --- a/OpenRA.Game/Manifest.cs +++ b/OpenRA.Game/Manifest.cs @@ -74,6 +74,7 @@ namespace OpenRA public readonly string[] SpriteFormats = Array.Empty(); public readonly string[] PackageFormats = Array.Empty(); public readonly string[] VideoFormats = Array.Empty(); + public bool AllowUnusedTranslationsInExternalPackages = true; readonly string[] reservedModuleNames = { @@ -81,7 +82,7 @@ namespace OpenRA "Sequences", "ModelSequences", "Cursors", "Chrome", "Assemblies", "ChromeLayout", "Weapons", "Voices", "Notifications", "Music", "Translations", "TileSets", "ChromeMetrics", "Missions", "Hotkeys", "ServerTraits", "LoadScreen", "DefaultOrderGenerator", "SupportsMapsFrom", "SoundFormats", "SpriteFormats", "VideoFormats", - "RequiresMods", "PackageFormats" + "RequiresMods", "PackageFormats", "AllowUnusedTranslationsInExternalPackages" }; readonly TypeDictionary modules = new(); @@ -166,6 +167,10 @@ namespace OpenRA if (yaml.TryGetValue("VideoFormats", out entry)) VideoFormats = FieldLoader.GetValue("VideoFormats", entry.Value); + + if (yaml.TryGetValue("AllowUnusedTranslationsInExternalPackages", out entry)) + AllowUnusedTranslationsInExternalPackages = + FieldLoader.GetValue("AllowUnusedTranslationsInExternalPackages", entry.Value); } public void LoadCustomData(ObjectCreator oc) diff --git a/OpenRA.Game/Map/Map.cs b/OpenRA.Game/Map/Map.cs index 06418f98c5..6e3ea206b2 100644 --- a/OpenRA.Game/Map/Map.cs +++ b/OpenRA.Game/Map/Map.cs @@ -1427,11 +1427,11 @@ namespace OpenRA return modData.DefaultFileSystem.Exists(filename); } - public bool IsExternalModFile(string filename) + public bool IsExternalFile(string filename) { // Explicit package paths never refer to a map if (filename.Contains('|')) - return modData.DefaultFileSystem.IsExternalModFile(filename); + return modData.DefaultFileSystem.IsExternalFile(filename); return false; } diff --git a/OpenRA.Game/Map/MapPreview.cs b/OpenRA.Game/Map/MapPreview.cs index f4c7a53f45..37c7e7cb84 100644 --- a/OpenRA.Game/Map/MapPreview.cs +++ b/OpenRA.Game/Map/MapPreview.cs @@ -654,11 +654,11 @@ namespace OpenRA return modData.DefaultFileSystem.Exists(filename); } - bool IReadOnlyFileSystem.IsExternalModFile(string filename) + bool IReadOnlyFileSystem.IsExternalFile(string filename) { // Explicit package paths never refer to a map if (filename.Contains('|')) - return modData.DefaultFileSystem.IsExternalModFile(filename); + return modData.DefaultFileSystem.IsExternalFile(filename); return false; } diff --git a/OpenRA.Mods.Common/Lint/CheckTranslationReference.cs b/OpenRA.Mods.Common/Lint/CheckTranslationReference.cs index 14196bc260..67cea55ecb 100644 --- a/OpenRA.Mods.Common/Lint/CheckTranslationReference.cs +++ b/OpenRA.Mods.Common/Lint/CheckTranslationReference.cs @@ -44,7 +44,12 @@ namespace OpenRA.Mods.Common.Lint foreach (var language in GetTranslationLanguages(modData)) { - CheckKeys(modData.Manifest.Translations.Concat(mapTranslations), map.Open, usedKeys, language, false, emitError, emitWarning); + // Check keys and variables are not missing across all language files. + // But for maps we don't warn on unused keys. They might be unused on *this* map, + // but the mod or another map may use them and we don't have sight of that. + CheckKeys( + modData.Manifest.Translations.Concat(mapTranslations), map.Open, usedKeys, + language, _ => false, emitError, emitWarning); var modTranslation = new Translation(language, modData.Manifest.Translations, modData.DefaultFileSystem, _ => { }); var mapTranslation = new Translation(language, mapTranslations, map, error => emitError(error.Message)); @@ -79,7 +84,13 @@ namespace OpenRA.Mods.Common.Lint CheckModWidgets(modData, usedKeys, testedFields); // With the fully populated keys, check keys and variables are not missing and not unused across all language files. - var keyWithAttrs = CheckKeys(modData.Manifest.Translations, modData.DefaultFileSystem.Open, usedKeys, language, true, emitError, emitWarning); + var keyWithAttrs = CheckKeys( + modData.Manifest.Translations, modData.DefaultFileSystem.Open, usedKeys, + language, + file => + !modData.Manifest.AllowUnusedTranslationsInExternalPackages || + !modData.DefaultFileSystem.IsExternalFile(file), + emitError, emitWarning); foreach (var group in usedKeys.KeysWithContext) { @@ -322,7 +333,10 @@ namespace OpenRA.Mods.Common.Lint CheckChrome(n, translationReferencesByWidgetField, usedKeys); } - static HashSet CheckKeys(IEnumerable translationFiles, Func openFile, TranslationKeys usedKeys, string language, bool checkUnusedKeys, Action emitError, Action emitWarning) + static HashSet CheckKeys( + IEnumerable translationFiles, Func openFile, TranslationKeys usedKeys, + string language, Func checkUnusedKeysForFile, + Action emitError, Action emitWarning) { var keyWithAttrs = new HashSet(); foreach (var file in translationFiles) @@ -351,7 +365,7 @@ namespace OpenRA.Mods.Common.Lint foreach (var (node, attributeName) in nodeAndAttributeNames) { keyWithAttrs.Add(attributeName == null ? key : $"{key}.{attributeName}"); - if (checkUnusedKeys) + if (checkUnusedKeysForFile(file)) CheckUnusedKey(key, attributeName, file, usedKeys, emitWarning); CheckVariables(node, key, attributeName, file, usedKeys, emitError, emitWarning); } diff --git a/OpenRA.Mods.Common/UpdateRules/UpdateUtils.cs b/OpenRA.Mods.Common/UpdateRules/UpdateUtils.cs index b54d77edd3..6c817d067f 100644 --- a/OpenRA.Mods.Common/UpdateRules/UpdateUtils.cs +++ b/OpenRA.Mods.Common/UpdateRules/UpdateUtils.cs @@ -48,7 +48,7 @@ namespace OpenRA.Mods.Common.UpdateRules { return FieldLoader.GetValue("value", yaml.Value) .Where(f => f.Contains('|')) - .SelectMany(f => LoadModYaml(modData, FilterExternalModFiles(modData, new[] { f }, externalFilenames))) + .SelectMany(f => LoadModYaml(modData, FilterExternalFiles(modData, new[] { f }, externalFilenames))) .ToList(); } @@ -182,11 +182,11 @@ namespace OpenRA.Mods.Common.UpdateRules return MiniYaml.Merge(yaml).ConvertAll(n => new MiniYamlNodeBuilder(n)); } - public static IEnumerable FilterExternalModFiles(ModData modData, IEnumerable files, HashSet externalFilenames) + public static IEnumerable FilterExternalFiles(ModData modData, IEnumerable files, HashSet externalFilenames) { foreach (var f in files) { - if (f.Contains('|') && modData.DefaultFileSystem.IsExternalModFile(f)) + if (f.Contains('|') && modData.DefaultFileSystem.IsExternalFile(f)) { externalFilenames.Add(f); continue; @@ -200,12 +200,12 @@ namespace OpenRA.Mods.Common.UpdateRules { var manualSteps = new List(); - var modRules = LoadModYaml(modData, FilterExternalModFiles(modData, modData.Manifest.Rules, externalFilenames)); - var modWeapons = LoadModYaml(modData, FilterExternalModFiles(modData, modData.Manifest.Weapons, externalFilenames)); - var modSequences = LoadModYaml(modData, FilterExternalModFiles(modData, modData.Manifest.Sequences, externalFilenames)); - var modTilesets = LoadModYaml(modData, FilterExternalModFiles(modData, modData.Manifest.TileSets, externalFilenames)); - var modChromeLayout = LoadModYaml(modData, FilterExternalModFiles(modData, modData.Manifest.ChromeLayout, externalFilenames)); - var modChromeProvider = LoadModYaml(modData, FilterExternalModFiles(modData, modData.Manifest.Chrome, externalFilenames)); + var modRules = LoadModYaml(modData, FilterExternalFiles(modData, modData.Manifest.Rules, externalFilenames)); + var modWeapons = LoadModYaml(modData, FilterExternalFiles(modData, modData.Manifest.Weapons, externalFilenames)); + var modSequences = LoadModYaml(modData, FilterExternalFiles(modData, modData.Manifest.Sequences, externalFilenames)); + var modTilesets = LoadModYaml(modData, FilterExternalFiles(modData, modData.Manifest.TileSets, externalFilenames)); + var modChromeLayout = LoadModYaml(modData, FilterExternalFiles(modData, modData.Manifest.ChromeLayout, externalFilenames)); + var modChromeProvider = LoadModYaml(modData, FilterExternalFiles(modData, modData.Manifest.Chrome, externalFilenames)); // Find and add shared map includes foreach (var package in modData.MapCache.EnumerateMapPackagesWithoutCaching()) diff --git a/OpenRA.Mods.Common/UtilityCommands/ExtractYamlStrings.cs b/OpenRA.Mods.Common/UtilityCommands/ExtractYamlStrings.cs index 4780c579b2..e0362ee2d4 100644 --- a/OpenRA.Mods.Common/UtilityCommands/ExtractYamlStrings.cs +++ b/OpenRA.Mods.Common/UtilityCommands/ExtractYamlStrings.cs @@ -47,7 +47,7 @@ namespace OpenRA.Mods.Common.UtilityCommands .Where(t => t.Value.Length > 0) .ToDictionary(t => t.Key, t => t.Value); - var modRules = UpdateUtils.LoadModYaml(modData, UpdateUtils.FilterExternalModFiles(modData, modData.Manifest.Rules, new HashSet())); + var modRules = UpdateUtils.LoadModYaml(modData, UpdateUtils.FilterExternalFiles(modData, modData.Manifest.Rules, new HashSet())); // Include files referenced in maps. foreach (var package in modData.MapCache.EnumerateMapPackagesWithoutCaching()) diff --git a/mods/cnc/mod.yaml b/mods/cnc/mod.yaml index 2756051b80..0b14cba55e 100644 --- a/mods/cnc/mod.yaml +++ b/mods/cnc/mod.yaml @@ -149,6 +149,8 @@ Translations: cnc|languages/chrome/en.ftl cnc|languages/rules/en.ftl +AllowUnusedTranslationsInExternalPackages: false + Voices: cnc|audio/voices.yaml diff --git a/mods/d2k/mod.yaml b/mods/d2k/mod.yaml index 96489bd691..5256b7d1c2 100644 --- a/mods/d2k/mod.yaml +++ b/mods/d2k/mod.yaml @@ -130,6 +130,8 @@ Translations: d2k|languages/chrome/en.ftl d2k|languages/rules/en.ftl +AllowUnusedTranslationsInExternalPackages: false + Weapons: d2k|weapons/debris.yaml d2k|weapons/smallguns.yaml diff --git a/mods/ra/mod.yaml b/mods/ra/mod.yaml index 7fd1fb3a9a..b188ed1a83 100644 --- a/mods/ra/mod.yaml +++ b/mods/ra/mod.yaml @@ -146,6 +146,8 @@ Translations: ra|languages/chrome/en.ftl ra|languages/rules/en.ftl +AllowUnusedTranslationsInExternalPackages: false + Weapons: ra|weapons/explosions.yaml ra|weapons/ballistics.yaml diff --git a/mods/ts/mod.yaml b/mods/ts/mod.yaml index b06d986f60..cdf0521df0 100644 --- a/mods/ts/mod.yaml +++ b/mods/ts/mod.yaml @@ -191,6 +191,8 @@ Translations: ts|languages/chrome/en.ftl ts|languages/rules/en.ftl +AllowUnusedTranslationsInExternalPackages: false + Voices: ts|audio/voices.yaml