Locate Mod Errors choose base ruleset to do complex check against (#6263)

* Some Ruleset linting

* Mod-check display remove unnecessary FormattedLine

* Mod-check can run on selectable base ruleset instead of Vanilla only

* Mod-check show final severity on Expander

* Mod-check base selectbox translatable

android/assets/jsons/translations/French.properties

@@ -548,7 +548,8 @@ Show unit movement arrows = Afficher les flèches de déplacement des unités
 Continuous rendering = Rendu en continu
 When disabled, saves battery life but certain animations will be suspended = Lorsque désactivé, permet d'économiser la batterie mais certaines animations seront suspendues
 Order trade offers by amount = Classer les offres d'échange par valeur
-Check extension mods based on vanilla = Vérifier les mods d'extension basés sur la version vanilla
+Check extension mods based on vanilla = Vérifier les mods d'extension basés sur:
+-none- = -rien-
 Reload mods = Recharger les mods
 Checking mods for errors... = Vérification des erreurs des mods...
 No problems found. = Aucun problème trouvé.

android/assets/jsons/translations/German.properties

@@ -548,7 +548,8 @@ Show unit movement arrows = Bewegungspfeile für Einheiten anzeigen
 Continuous rendering = Kontinuierliches Rendern
 When disabled, saves battery life but certain animations will be suspended = Es spart Akku, wenn es deaktiviert ist, aber bestimmte Animationen werden nicht angezeigt.
 Order trade offers by amount = Handelsangebote nach Menge sortieren
-Check extension mods based on vanilla = Erweiterungs-Mods mit Vanilla-Regelsatz prüfen
+Check extension mods based on: = Erweiterungs-Mods prüfen auf Basis von:
+-none- = -nichts-
 Reload mods = Mods erneut laden
 Checking mods for errors... = Mods werden geprüft...
 No problems found. = Keine Probleme gefunden.

android/assets/jsons/translations/Spanish.properties

@@ -548,7 +548,8 @@ Show unit movement arrows = Mostrar flechas de movimiento de únidad
 Continuous rendering = Renderizado Continuo
 When disabled, saves battery life but certain animations will be suspended = Cuando está deshabilitado, ahorra batería pero ciertas animaciones serán suspendidas
 Order trade offers by amount = Organizar ofertas por cantidad
-Check extension mods based on vanilla = Buscar mods de extensión basados en vainilla
+Check extension mods based on vanilla = Examinar mods de extensión basados en:
+-none- = -nada-
 Reload mods = recargar mods
 Checking mods for errors... = Buscando errores en mods...
 No problems found. = No se encontraron problemas.

android/assets/jsons/translations/template.properties

@@ -553,7 +553,8 @@ Show unit movement arrows =
 Continuous rendering = 
 When disabled, saves battery life but certain animations will be suspended = 
 Order trade offers by amount = 
-Check extension mods based on vanilla = 
+Check extension mods based on: = 
+-none- = 
 Reload mods = 
 Checking mods for errors... = 
 No problems found. = 

core/src/com/unciv/models/ruleset/Ruleset.kt

@@ -2,6 +2,7 @@ package com.unciv.models.ruleset
 
 import com.badlogic.gdx.Gdx
 import com.badlogic.gdx.files.FileHandle
+import com.badlogic.gdx.graphics.Color
 import com.unciv.Constants
 import com.unciv.JsonParser
 import com.unciv.logic.UncivShowableException
@@ -54,13 +55,15 @@ class ModOptions : IHasUniques {
     val maxXPfromBarbarians = 30
 
     override var uniques = ArrayList<String>()
-    // If this is delegated with "by lazy", the mod download process crashes and burns
+
+    // If these two are delegated with "by lazy", the mod download process crashes and burns 
+    // Instead, Ruleset.load sets them, which is preferable in this case anyway
     override var uniqueObjects: List<Unique> = listOf()
     override var uniqueMap: Map<String, List<Unique>> = mapOf()
+
     override fun getUniqueTarget() = UniqueTarget.ModOptions
 
     val constants = ModConstants()
-    
 }
 
 class Ruleset {
@@ -206,16 +209,16 @@ class Ruleset {
         // therefore does not guarantee keeping the order of elements like a LinkedHashMap does.
         // Using map{} sidesteps this problem
         eras.map { it.value }.withIndex().forEach { it.value.eraNumber = it.index }
-        
+
         val unitTypesFile = folderHandle.child("UnitTypes.json")
         if (unitTypesFile.exists()) unitTypes += createHashmap(jsonParser.getFromJson(Array<UnitType>::class.java, unitTypesFile))
-        
+
         val unitsFile = folderHandle.child("Units.json")
         if (unitsFile.exists()) units += createHashmap(jsonParser.getFromJson(Array<BaseUnit>::class.java, unitsFile))
 
         val promotionsFile = folderHandle.child("UnitPromotions.json")
         if (promotionsFile.exists()) unitPromotions += createHashmap(jsonParser.getFromJson(Array<Promotion>::class.java, promotionsFile))
-        
+
         val questsFile = folderHandle.child("Quests.json")
         if (questsFile.exists()) quests += createHashmap(jsonParser.getFromJson(Array<Quest>::class.java, questsFile))
 
@@ -249,7 +252,7 @@ class Ruleset {
         val ruinRewardsFile = folderHandle.child("Ruins.json")
         if (ruinRewardsFile.exists())
             ruinRewards += createHashmap(jsonParser.getFromJson(Array<RuinReward>::class.java, ruinRewardsFile))
-        
+
         val nationsFile = folderHandle.child("Nations.json")
         if (nationsFile.exists()) {
             nations += createHashmap(jsonParser.getFromJson(Array<Nation>::class.java, nationsFile))
@@ -264,7 +267,7 @@ class Ruleset {
         if (globalUniquesFile.exists()) {
             globalUniques = jsonParser.getFromJson(GlobalUniques::class.java, globalUniquesFile)
         }
-        
+
         val gameBasicsLoadTime = System.currentTimeMillis() - gameBasicsStartTime
         if (printOutput) println("Loading ruleset - " + gameBasicsLoadTime + "ms")
     }
@@ -283,7 +286,7 @@ class Ruleset {
             }
         }
     }
-    
+
     /** Used for displaying a RuleSet's name */
     override fun toString() = when {
         name.isNotEmpty() -> name
@@ -431,14 +434,14 @@ class Ruleset {
 
 
     class RulesetError(val text:String, val errorSeverityToReport: RulesetErrorSeverity)
-    enum class RulesetErrorSeverity {
+    enum class RulesetErrorSeverity(val color: Color) {
-        OK,
+        OK(Color.GREEN),
-        WarningOptionsOnly,
+        WarningOptionsOnly(Color.YELLOW),
-        Warning,
+        Warning(Color.YELLOW),
-        Error,
+        Error(Color.RED),
     }
 
-    class RulesetErrorList:ArrayList<RulesetError>() {
+    class RulesetErrorList : ArrayList<RulesetError>() {
         operator fun plusAssign(text: String) {
             add(text, RulesetErrorSeverity.Error)
         }
@@ -447,7 +450,7 @@ class Ruleset {
             add(RulesetError(text, errorSeverityToReport))
         }
 
-        private fun getFinalSeverity(): RulesetErrorSeverity {
+        fun getFinalSeverity(): RulesetErrorSeverity {
             if (isEmpty()) return RulesetErrorSeverity.OK
             return this.maxOf { it.errorSeverityToReport }
         }
@@ -465,7 +468,7 @@ class Ruleset {
                 .joinToString("\n") { it.errorSeverityToReport.name + ": " + it.text }
     }
 
-    fun checkModLinks(forOptionsPopup:Boolean = false): RulesetErrorList {
+    fun checkModLinks(forOptionsPopup: Boolean = false): RulesetErrorList {
         val lines = RulesetErrorList()
 
         // Checks for all mods - only those that can succeed without loading a base ruleset
@@ -806,18 +809,18 @@ object RulesetCache : HashMap<String,Ruleset>() {
      */
     fun getComplexRuleset(mods: LinkedHashSet<String>, optionalBaseRuleset: String? = null): Ruleset {
         val newRuleset = Ruleset()
-        
+
         val baseRuleset =
             if (containsKey(optionalBaseRuleset) && this[optionalBaseRuleset]!!.modOptions.isBaseRuleset) this[optionalBaseRuleset]!!
             else getVanillaRuleset()
-        
+
-        
+
         val loadedMods = mods
             .filter { containsKey(it) }
             .map { this[it]!! }
             .filter { !it.modOptions.isBaseRuleset } + 
             baseRuleset
-        
+
         for (mod in loadedMods.sortedByDescending { it.modOptions.isBaseRuleset }) {
             newRuleset.add(mod)
             newRuleset.mods += mod.name

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

@@ -13,6 +13,8 @@ import com.unciv.logic.MapSaver
 import com.unciv.logic.civilization.PlayerType
 import com.unciv.models.UncivSound
 import com.unciv.models.ruleset.Ruleset
+import com.unciv.models.ruleset.Ruleset.RulesetError
+import com.unciv.models.ruleset.Ruleset.RulesetErrorSeverity
 import com.unciv.models.ruleset.RulesetCache
 import com.unciv.models.ruleset.tile.ResourceType
 import com.unciv.models.ruleset.unique.Unique
@@ -23,6 +25,7 @@ import com.unciv.models.translations.tr
 import com.unciv.ui.audio.MusicTrackChooserFlags
 import com.unciv.ui.civilopedia.FormattedLine
 import com.unciv.ui.civilopedia.MarkupRenderer
+import com.unciv.ui.newgamescreen.TranslatedSelectBox
 import com.unciv.ui.utils.*
 import com.unciv.ui.utils.LanguageTable.Companion.addLanguageTables
 import com.unciv.ui.utils.UncivTooltip.Companion.addTooltip
@@ -43,12 +46,16 @@ class OptionsPopup(val previousScreen: BaseScreen) : Popup(previousScreen) {
     private val tabs: TabbedPager
     private val resolutionArray = com.badlogic.gdx.utils.Array(arrayOf("750x500", "900x600", "1050x700", "1200x800", "1500x1000"))
     private var modCheckFirstRun = true   // marker for automatic first run on selecting the page
-    private var modCheckCheckBox: CheckBox? = null
+    private var modCheckBaseSelect: TranslatedSelectBox? = null
-    private var modCheckResultTable = Table()
+    private val modCheckResultTable = Table()
     private val selectBoxMinWidth: Float
 
     //endregion
 
+    companion object {
+        private const val modCheckWithoutBase = "-none-"
+    }
+
     init {
         settings.addCompletedTutorialTask("Open the options table")
 
@@ -264,21 +271,30 @@ class OptionsPopup(val previousScreen: BaseScreen) : Popup(previousScreen) {
     private fun getModCheckTab() = Table(BaseScreen.skin).apply {
         defaults().pad(10f).align(Align.top)
         val reloadModsButton = "Reload mods".toTextButton().onClick {
-            runModChecker(modCheckCheckBox!!.isChecked)
+            runModChecker(modCheckBaseSelect!!.selected.value)
         }
         add(reloadModsButton).row()
-        modCheckCheckBox = "Check extension mods based on vanilla".toCheckBox {
+
-            runModChecker(it)
+        val labeledBaseSelect = Table(BaseScreen.skin).apply {
+            add("Check extension mods based on:".toLabel()).padRight(10f)
+            val baseMods = listOf(modCheckWithoutBase) + RulesetCache.getSortedBaseRulesets()
+            modCheckBaseSelect = TranslatedSelectBox(baseMods, modCheckWithoutBase, BaseScreen.skin).apply {
+                selectedIndex = 0
+                onChange {
+                    runModChecker(modCheckBaseSelect!!.selected.value)
+                }
+            }
+            add(modCheckBaseSelect)
         }
-        add(modCheckCheckBox).row()
+        add(labeledBaseSelect).row()
 
         add(modCheckResultTable)
     }
 
-    private fun runModChecker(complex: Boolean = false) {
+    private fun runModChecker(base: String = modCheckWithoutBase) {
-        
+
         modCheckFirstRun = false
-        if (modCheckCheckBox == null) return
+        if (modCheckBaseSelect == null) return
 
         modCheckResultTable.clear()
 
@@ -289,30 +305,21 @@ class OptionsPopup(val previousScreen: BaseScreen) : Popup(previousScreen) {
                 errorTable.add(rulesetError.toLabel()).width(stage.width / 2).row()
             modCheckResultTable.add(errorTable)
         }
-        
+
         modCheckResultTable.add("Checking mods for errors...".toLabel()).row()
-        modCheckCheckBox!!.disable()
+        modCheckBaseSelect!!.isDisabled = true
 
         crashHandlingThread(name="ModChecker") {
             for (mod in RulesetCache.values.sortedBy { it.name }) {
-                var noProblem = true
+                if (base != modCheckWithoutBase && mod.modOptions.isBaseRuleset) continue
-                val lines = ArrayList<FormattedLine>()
 
                 val modLinks =
-                    if (complex) RulesetCache.checkCombinedModLinks(linkedSetOf(mod.name))
+                        if (base == modCheckWithoutBase) mod.checkModLinks(forOptionsPopup = true)
-                    else mod.checkModLinks(forOptionsPopup = true)
+                        else RulesetCache.checkCombinedModLinks(linkedSetOf(mod.name), base)
-                for (error in modLinks.sortedByDescending { it.errorSeverityToReport }) {
+                modLinks.sortByDescending { it.errorSeverityToReport }
-                    val color = when (error.errorSeverityToReport) {
+                val noProblem = !modLinks.isNotOK()
-                        Ruleset.RulesetErrorSeverity.OK -> "#00FF00"
+                if (modLinks.isNotEmpty()) modLinks += RulesetError("", RulesetErrorSeverity.OK)
-                        Ruleset.RulesetErrorSeverity.Warning,
+                if (noProblem) modLinks += RulesetError("No problems found.".tr(), RulesetErrorSeverity.OK)
-                        Ruleset.RulesetErrorSeverity.WarningOptionsOnly -> "#FFFF00"
-                        Ruleset.RulesetErrorSeverity.Error -> "#FF0000"
-                    }
-                    lines += FormattedLine(error.text, color = color)
-                }
-                if (modLinks.isNotOK()) noProblem = false
-                lines += FormattedLine()
-                if (noProblem) lines += FormattedLine("No problems found.".tr())
 
                 postCrashHandlingRunnable {
                     // When the options popup is already closed before this postRunnable is run,
@@ -322,7 +329,17 @@ class OptionsPopup(val previousScreen: BaseScreen) : Popup(previousScreen) {
                     // Don't use .toLabel() either, since that activates translations as well, which is what we're trying to avoid,
                     // Instead, some manual work needs to be put in.
 
-                    val expanderTab = ExpanderTab(mod.name, startsOutOpened = false){
+                    val iconColor = modLinks.getFinalSeverity().color
+                    val iconName = when(iconColor) {
+                        Color.RED -> "OtherIcons/Stop"
+                        Color.YELLOW -> "OtherIcons/ExclamationMark"
+                        else -> "OtherIcons/Checkmark"
+                    }
+                    val icon = ImageGetter.getImage(iconName)
+                        .apply { color = Color.BLACK }
+                        .surroundWithCircle(30f, color = iconColor)
+
+                    val expanderTab = ExpanderTab(mod.name, icon = icon, startsOutOpened = false) {
                         it.defaults().align(Align.left)
                         if (!noProblem && mod.folderLocation != null) {
                             val replaceableUniques = getDeprecatedReplaceableUniques(mod)
@@ -330,18 +347,16 @@ class OptionsPopup(val previousScreen: BaseScreen) : Popup(previousScreen) {
                                 it.add("Autoupdate mod uniques".toTextButton()
                                     .onClick { autoUpdateUniques(mod, replaceableUniques) }).pad(10f).row()
                         }
-                        for (line in lines) {
+                        for (line in modLinks) {
-                            val label = if (line.starred) Label(line.text + "\n", BaseScreen.skin)
+                            val label = Label(line.text, BaseScreen.skin)
-                                .apply { setFontScale(22 / Fonts.ORIGINAL_FONT_SIZE) }
+                                .apply { color = line.errorSeverityToReport.color }
-                            else Label(line.text + "\n", BaseScreen.skin)
-                                .apply { if (line.color != "") color = Color.valueOf(line.color) }
                             label.wrap = true
                             it.add(label).width(stage.width / 2).row()
                         }
-                        if(!noProblem)
+                        if (!noProblem)
                             it.add("Copy to clipboard".toTextButton().onClick {
-                                Gdx.app.clipboard.contents = lines.map { it.text }.filterNot { it=="" }
+                                Gdx.app.clipboard.contents = modLinks
-                                    .joinToString("\n")  
+                                    .joinToString("\n") { line -> line.text }
                             }).row()
                     }
 
@@ -355,7 +370,7 @@ class OptionsPopup(val previousScreen: BaseScreen) : Popup(previousScreen) {
             // done with all mods!
             postCrashHandlingRunnable {
                 modCheckResultTable.removeActor(modCheckResultTable.children.last())
-                modCheckCheckBox!!.enable()
+                modCheckBaseSelect!!.isDisabled = false
             }
         }
     }