Mod TranslationFileWriter re-work (#5219)

* Mod TranslationFileWriter re-work

* Mod TranslationFileWriter re-work - style and existing in base treatment

* Mod TranslationFileWriter re-work - style and existing in base treatment

core/src/com/unciv/models/translations/TranslationFileWriter.kt

@@ -24,31 +24,49 @@ object TranslationFileWriter {
     const val templateFileLocation = "jsons/translations/template.properties"
     private const val languageFileLocation = "jsons/translations/%s.properties"
 
-    fun writeNewTranslationFiles(translations: Translations) {
+    fun writeNewTranslationFiles(): String {
+        try {
+            val translations = Translations()
+            translations.readAllLanguagesTranslation()
 
-        val percentages = generateTranslationFiles(translations)
+            val percentages = generateTranslationFiles(translations)
-        writeLanguagePercentages(percentages)
+            writeLanguagePercentages(percentages)
 
-        // try to do the same for the mods
+            // See #5168 for some background on this
-        for (modFolder in Gdx.files.local("mods").list().filter { it.isDirectory })
+            for ((modName, modTranslations) in translations.modsWithTranslations) {
-            generateTranslationFiles(translations, modFolder)
+                val modFolder = Gdx.files.local("mods").child(modName)
-        // write percentages is not needed: for an individual mod it makes no sense
+                val modPercentages = generateTranslationFiles(modTranslations, modFolder, translations)
+                writeLanguagePercentages(modPercentages, modFolder)  // unused by the game but maybe helpful for the mod developer
+            }
 
+            return "Translation files are generated successfully."
+        } catch (ex: Throwable) {
+            return ex.localizedMessage
+        }
     }
 
     private fun getFileHandle(modFolder: FileHandle?, fileLocation: String) =
             if (modFolder != null) modFolder.child(fileLocation)
             else Gdx.files.local(fileLocation)
 
-    private fun generateTranslationFiles(translations: Translations, modFolder: FileHandle? = null): HashMap<String, Int> {
+    /**
+     * Writes new language files per Mod or for BaseRuleset - only each language that exists in [translations].
+     * @param baseTranslations For a mod, pass the base translations here so strings already existing there can be seen
+     * @return a map with the percentages of translated lines per language
+     */
+    private fun generateTranslationFiles(
+        translations: Translations,
+        modFolder: FileHandle? = null,
+        baseTranslations: Translations? = null
+    ): HashMap<String, Int> {
 
         val fileNameToGeneratedStrings = LinkedHashMap<String, MutableSet<String>>()
-        val linesFromTemplates = mutableListOf<String>()
+        val linesToTranslate = mutableListOf<String>()
 
         if (modFolder == null) { // base game
             val templateFile = getFileHandle(modFolder, templateFileLocation) // read the template
             if (templateFile.exists())
-                linesFromTemplates.addAll(templateFile.reader(TranslationFileReader.charset).readLines())
+                linesToTranslate.addAll(templateFile.reader(TranslationFileReader.charset).readLines())
 
             for (baseRuleset in BaseRuleset.values()) {
                 val generatedStringsFromBaseRuleset =
@@ -58,28 +76,27 @@ object TranslationFileWriter {
             }
 
             fileNameToGeneratedStrings["Tutorials"] = generateTutorialsStrings()
-        } else fileNameToGeneratedStrings.putAll(generateStringsFromJSONs(modFolder))
+        } else {
+            fileNameToGeneratedStrings.putAll(generateStringsFromJSONs(modFolder.child("jsons")))
+        }
 
-        // Tutorials are a bit special
+        for (key in fileNameToGeneratedStrings.keys) {
-        if (modFolder == null)          // this is for base only, not mods
+            linesToTranslate.add("\n#################### Lines from $key ####################\n")
-
+            linesToTranslate.addAll(fileNameToGeneratedStrings.getValue(key))
-            for (key in fileNameToGeneratedStrings.keys) {
+        }
-                linesFromTemplates.add("\n#################### Lines from $key ####################\n")
-                linesFromTemplates.addAll(fileNameToGeneratedStrings.getValue(key))
-            }
 
         var countOfTranslatableLines = 0
         val countOfTranslatedLines = HashMap<String, Int>()
 
         // iterate through all available languages
-        for (language in translations.getLanguages()) {
+        for ((languageIndex, language) in translations.getLanguages().withIndex()) {
             var translationsOfThisLanguage = 0
             val stringBuilder = StringBuilder()
 
             // This is so we don't add the same keys twice if we have the same value in both Vanilla and G&K
             val existingTranslationKeys = HashSet<String>()
 
-            for (line in linesFromTemplates) {
+            for (line in linesToTranslate) {
                 if (!line.contains(" = ")) {
                     // small hack to insert empty lines
                     if (line.startsWith(specialNewLineCode)) {
@@ -99,16 +116,21 @@ object TranslationFileWriter {
                 if (existingTranslationKeys.contains(hashMapKey)) continue // don't add it twice
                 existingTranslationKeys.add(hashMapKey)
 
-                // count translatable lines only once (e.g. for English)
+                // count translatable lines only once
-                if (language == "English") countOfTranslatableLines++
+                if (languageIndex == 0) countOfTranslatableLines++
 
-                var translationValue = ""
+                val existingTranslation = translations[hashMapKey]
-
+                var translationValue = if (existingTranslation != null && language in existingTranslation){
-                val translationEntry = translations[hashMapKey]
-                if (translationEntry != null && translationEntry.containsKey(language)) {
-                    translationValue = translationEntry[language]!!
                     translationsOfThisLanguage++
-                } else stringBuilder.appendLine(" # Requires translation!")
+                    existingTranslation[language]!!
+                } else if (baseTranslations?.get(hashMapKey)?.containsKey(language) == true) {
+                    // String is used in the mod but also exists in base - ignore
+                    continue
+                } else {
+                    // String is not translated either here or in base
+                    stringBuilder.appendLine(" # Requires translation!")
+                    ""
+                }
 
                 // THE PROBLEM
                 // When we come to change params written in the TranslationFileWriter,
@@ -141,22 +163,19 @@ object TranslationFileWriter {
 
         // Calculate the percentages of translations
         // It should be done after the loop of languages, since the countOfTranslatableLines is not known in the 1st iteration
-        for (key in countOfTranslatedLines.keys)
+        for (entry in countOfTranslatedLines)
-            countOfTranslatedLines[key] = if (countOfTranslatableLines > 0) countOfTranslatedLines.getValue(key) * 100 / countOfTranslatableLines
+            entry.setValue(if (countOfTranslatableLines <= 0) 100 else entry.value * 100 / countOfTranslatableLines)
-            else 100
 
         return countOfTranslatedLines
     }
 
-    private fun writeLanguagePercentages(percentages: HashMap<String, Int>) {
+    private fun writeLanguagePercentages(percentages: HashMap<String, Int>, modFolder: FileHandle? = null) {
-        val stringBuilder = StringBuilder()
+        val output = percentages.asSequence()
-        for (entry in percentages) {
+            .joinToString("\n", postfix = "\n") { "${it.key} = ${it.value}" }
-            stringBuilder.appendLine(entry.key + " = " + entry.value)
+        getFileHandle(modFolder, TranslationFileReader.percentagesFileLocation)
-        }
+            .writeString(output, false)
-        Gdx.files.local(TranslationFileReader.percentagesFileLocation).writeString(stringBuilder.toString(), false)
     }
 
-
     private fun generateTutorialsStrings(): MutableSet<String> {
 
         val tutorialsStrings = mutableSetOf<String>()

core/src/com/unciv/models/translations/Translations.kt

@@ -5,6 +5,7 @@ import com.unciv.UncivGame
 import com.unciv.models.stats.Stats
 import java.util.*
 import kotlin.collections.HashMap
+import kotlin.collections.LinkedHashSet
 
 /**
  *  This collection holds all translations for the game.
@@ -30,7 +31,7 @@ class Translations : LinkedHashMap<String, TranslationEntry>(){
     var percentCompleteOfLanguages = HashMap<String,Int>()
             .apply { put("English",100) } // So even if we don't manage to load the percentages, we can still pass the language screen
 
-    private var modsWithTranslations: HashMap<String, Translations> = hashMapOf() // key == mod name
+    internal var modsWithTranslations: HashMap<String, Translations> = hashMapOf() // key == mod name
 
     // used by tr() whenever GameInfo not initialized (allowing new game screen to use mod translations)
     var translationActiveMods = LinkedHashSet<String>()
@@ -64,17 +65,16 @@ class Translations : LinkedHashMap<String, TranslationEntry>(){
         return get(text, language, activeMods)?.get(language) ?: text
     }
 
-    fun getLanguages(): List<String> {
+    /** Get all languages present in `this`, used for [TranslationFileWriter] and `TranslationTests` */
-        val toReturn =  mutableListOf<String>()
+    fun getLanguages() = linkedSetOf<String>().apply {
-
+            for (entry in values)
-        for(entry in values)
+                for (languageName in entry.keys)
-            for(languageName in entry.keys)
+                    add(languageName)
-                if(!toReturn.contains(languageName)) toReturn.add(languageName)
+        }
-
-        return toReturn
-    }
-
 
+    /** This reads all translations for a specific language, including _all_ installed mods.
+     *  Vanilla translations go into `this` instance, mod translations into [modsWithTranslations].
+     */
     private fun tryReadTranslationForLanguage(language: String, printOutput: Boolean) {
         val translationStart = System.currentTimeMillis()
 
@@ -86,6 +86,7 @@ class Translations : LinkedHashMap<String, TranslationEntry>(){
             // which is super odd because everyone should support UTF-8
             languageTranslations = TranslationFileReader.read(Gdx.files.internal(translationFileName))
         } catch (ex: Exception) {
+            println("Exception reading translations for $language: ${ex.message}")
             return
         }
 
@@ -93,33 +94,36 @@ class Translations : LinkedHashMap<String, TranslationEntry>(){
         for (modFolder in Gdx.files.local("mods").list()) {
             val modTranslationFile = modFolder.child(translationFileName)
             if (modTranslationFile.exists()) {
-                val translationsForMod = Translations()
+                var translationsForMod = modsWithTranslations[modFolder.name()]
-                createTranslations(language, TranslationFileReader.read(modTranslationFile), translationsForMod)
+                if (translationsForMod == null) {
-
+                    translationsForMod = Translations()
-                modsWithTranslations[modFolder.name()] = translationsForMod
+                    modsWithTranslations[modFolder.name()] = translationsForMod
+                }
+                try {
+                    translationsForMod.createTranslations(language, TranslationFileReader.read(modTranslationFile))
+                } catch (ex: Exception) {
+                    println("Exception reading translations for ${modFolder.name()} $language: ${ex.message}")
+                }
             }
         }
 
         createTranslations(language, languageTranslations)
 
         val translationFilesTime = System.currentTimeMillis() - translationStart
-        if(printOutput) println("Loading translation file for $language - " + translationFilesTime + "ms")
+        if (printOutput) println("Loading translation file for $language - " + translationFilesTime + "ms")
     }
 
-    private fun createTranslations(language: String,
+    private fun createTranslations(language: String, languageTranslations: HashMap<String,String>) {
-                                  languageTranslations: HashMap<String,String>,
-                                  targetTranslations: Translations = this) {
         for (translation in languageTranslations) {
             val hashKey = if (translation.key.contains('['))
                 translation.key.getPlaceholderText()
             else translation.key
-            if (!containsKey(hashKey))
+            var entry = this[hashKey]
-                targetTranslations[hashKey] = TranslationEntry(translation.key)
+            if (entry == null) {
-
+                entry = TranslationEntry(translation.key)
-            // why not in one line, Because there were actual crashes.
+                this[hashKey] = entry
-            // I'm pretty sure I solved this already, but hey double-checking doesn't cost anything.
+            }
-            val entry = targetTranslations[hashKey]
+            entry[language] = translation.value
-            if (entry != null) entry[language] = translation.value
         }
     }
 
@@ -127,6 +131,7 @@ class Translations : LinkedHashMap<String, TranslationEntry>(){
         tryReadTranslationForLanguage(UncivGame.Current.settings.language, false)
     }
 
+    /** Get a list of supported languages for [readAllLanguagesTranslation] */
     // This function is too strange for me, however, let's keep it "as is" for now. - JackRainy
     private fun getLanguagesWithTranslationFile(): List<String> {
 
@@ -134,10 +139,10 @@ class Translations : LinkedHashMap<String, TranslationEntry>(){
         // So apparently the Locales don't work for everyone, which is horrendous
         // So for those players, which seem to be Android-y, we try to see what files exist directly...yeah =/
         try{
-            for(file in Gdx.files.internal("jsons/translations").list())
+            for (file in Gdx.files.internal("jsons/translations").list())
                 languages.add(file.nameWithoutExtension())
         }
-        catch (ex:Exception){} // Iterating on internal files will not work when running from a .jar
+        catch (ex:Exception) {} // Iterating on internal files will not work when running from a .jar
 
         languages.addAll(Locale.getAvailableLocales() // And this should work for Desktop, meaning from a .jar
                 .map { it.getDisplayName(Locale.ENGLISH) }) // Maybe THIS is the problem, that the DISPLAY locale wasn't english
@@ -156,6 +161,7 @@ class Translations : LinkedHashMap<String, TranslationEntry>(){
                 .filter { Gdx.files.internal("jsons/translations/$it.properties").exists() }
     }
 
+    /** Ensure _all_ languages are loaded, used by [TranslationFileWriter] and `TranslationTests` */
     fun readAllLanguagesTranslation(printOutput:Boolean=false) {
         // Apparently you can't iterate over the files in a directory when running out of a .jar...
         // https://www.badlogicgames.com/forum/viewtopic.php?f=11&t=27250
@@ -168,7 +174,7 @@ class Translations : LinkedHashMap<String, TranslationEntry>(){
         }
 
         val translationFilesTime = System.currentTimeMillis() - translationStart
-        if(printOutput) println("Loading translation files - "+translationFilesTime+"ms")
+        if(printOutput) println("Loading translation files - ${translationFilesTime}ms")
     }
 
     fun loadPercentageCompleteOfLanguages(){
@@ -177,7 +183,7 @@ class Translations : LinkedHashMap<String, TranslationEntry>(){
         percentCompleteOfLanguages = TranslationFileReader.readLanguagePercentages()
 
         val translationFilesTime = System.currentTimeMillis() - startTime
-        println("Loading percent complete of languages - "+translationFilesTime+"ms")
+        println("Loading percent complete of languages - ${translationFilesTime}ms")
     }
 }
 

core/src/com/unciv/ui/worldscreen/mainmenu/OptionsPopup.kt

@@ -512,11 +512,9 @@ class OptionsPopup(val previousScreen: CameraStageBaseScreen) : Popup(previousSc
         if (Gdx.app.type == Application.ApplicationType.Desktop) {
             val generateTranslationsButton = "Generate translation files".toTextButton()
             val generateAction = {
-                val translations = Translations()
+                val result = TranslationFileWriter.writeNewTranslationFiles()
-                translations.readAllLanguagesTranslation()
-                TranslationFileWriter.writeNewTranslationFiles(translations)
                 // notify about completion
-                generateTranslationsButton.setText("Translation files are generated successfully.".tr())
+                generateTranslationsButton.setText(result.tr())
                 generateTranslationsButton.disable()
             }
             generateTranslationsButton.onClick(generateAction)