Fix mod check not offering to auto-update Uniques for extension mods (#13535)

* Reenable Unique auto-update by fixing the validation on the replacement

* Make mod name search in Mod Checker find those disabled placeholders too

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

@@ -174,15 +174,15 @@ object RulesetCache : HashMap<String, Ruleset>() {
         mods: LinkedHashSet<String>,
         baseRuleset: String? = null,
         tryFixUnknownUniques: Boolean = false
-    ): RulesetErrorList {
+    ): Pair<Ruleset?, RulesetErrorList> {
         return try {
             val newRuleset = getComplexRuleset(mods, baseRuleset)
             newRuleset.modOptions.isBaseRuleset = true // This is so the checkModLinks finds all connections
-            newRuleset.getErrorList(tryFixUnknownUniques)
+            newRuleset to newRuleset.getErrorList(tryFixUnknownUniques)
         } catch (ex: UncivShowableException) {
             // This happens if a building is dependent on a tech not in the base ruleset
             //  because newRuleset.updateBuildingCosts() in getComplexRuleset() throws an error
-            RulesetErrorList.of(ex.message, RulesetErrorSeverity.Error)
+            null to RulesetErrorList.of(ex.message, RulesetErrorSeverity.Error)
         }
     }
 }

core/src/com/unciv/models/ruleset/validation/UniqueAutoUpdater.kt

@@ -4,13 +4,17 @@ import com.unciv.models.ruleset.Ruleset
 import com.unciv.models.ruleset.RulesetFile
 import com.unciv.models.ruleset.unique.Unique
 import com.unciv.utils.Log
-import com.unciv.utils.debug
 
 object UniqueAutoUpdater {
 
+    /** Apply Unique replacements.
+     *  @param mod The Ruleset to process. It must have a `folderLocation`.
+     *  @param replaceableUniques A map old to new. The default is only for use with the 'mod-ci' command line argument from `DeskTopLauncher`.
+     *                            The ModCheck UI will have produced this map using a combined Ruleset for replacement validation when appropriate.
+     */
     fun autoupdateUniques(
         mod: Ruleset,
-        replaceableUniques: HashMap<String, String> = getDeprecatedReplaceableUniques(mod)
+        replaceableUniques: HashMap<String, String> = getDeprecatedReplaceableUniques(mod, mod)
     ) {
         val filesToReplace = RulesetFile.entries.map { it.filename }
         val jsonFolder = mod.folderLocation!!.child("jsons")
@@ -26,12 +30,18 @@ object UniqueAutoUpdater {
         }
     }
 
-
+    /** Determine possible auto-corrections from Deprecation annotations.
-
+     *  @param mod The Ruleset to scan
-    fun getDeprecatedReplaceableUniques(mod: Ruleset): HashMap<String, String> {
+     *  @param rulesetForValidation The Ruleset to test potential fixed Uniques against. Can be same as [mod],
+     *                              otherwise it should be a complex Ruleset combining [mod] with an appropriate base ruleset.
+     *  @return A map of old to new. Will not include 'new' values that don't pass Unique validation against [rulesetForValidation].
+     */
+    fun getDeprecatedReplaceableUniques(mod: Ruleset, rulesetForValidation: Ruleset): HashMap<String, String> {
         val allUniques = mod.allUniques()
         val allDeprecatedUniques = HashSet<String>()
         val deprecatedUniquesToReplacementText = HashMap<String, String>()
+        val validator = UniqueValidator(rulesetForValidation)
+        val reportRulesetSpecificErrors = rulesetForValidation.modOptions.isBaseRuleset
 
         val deprecatedUniques = allUniques
             .filter { it.getDeprecationAnnotation() != null }
@@ -52,30 +62,13 @@ object UniqueAutoUpdater {
                 uniqueReplacementText += " <${conditional.text}>"
             val replacementUnique = Unique(uniqueReplacementText)
 
-            val modInvariantErrors = UniqueValidator(mod).checkUnique(
+            val modErrors = validator.checkUnique(replacementUnique, false, null, reportRulesetSpecificErrors)
-                replacementUnique,
+            for (error in modErrors)
-                false,
+                Log.error("ModError: %s - %s", error.text, error.errorSeverityToReport)
-                null,
+            if (modErrors.isNotEmpty()) continue // errors means no autoreplace
-                true
-            )
-            for (error in modInvariantErrors)
-                Log.error("ModInvariantError: %s - %s", error.text, error.errorSeverityToReport)
-            if (modInvariantErrors.isNotEmpty()) continue // errors means no autoreplace
-
-            if (mod.modOptions.isBaseRuleset) {
-                val modSpecificErrors = UniqueValidator(mod).checkUnique(
-                    replacementUnique,
-                    false,
-                    null,
-                    true
-                )
-                for (error in modSpecificErrors)
-                    Log.error("ModSpecificError: %s - %s", error.text, error.errorSeverityToReport)
-                if (modSpecificErrors.isNotEmpty()) continue
-            }
 
             deprecatedUniquesToReplacementText[deprecatedUnique.text] = uniqueReplacementText
-            debug("Replace \"%s\" with \"%s\"", deprecatedUnique.text, uniqueReplacementText)
+            Log.debug("Replace \"%s\" with \"%s\"", deprecatedUnique.text, uniqueReplacementText)
         }
 
         return deprecatedUniquesToReplacementText

core/src/com/unciv/ui/popups/options/ModCheckTab.kt

@@ -50,10 +50,12 @@ class ModCheckTab(
 
     private var modCheckBaseSelect: TranslatedSelectBox? = null
     private val searchModsTextField: UncivTextField
-    private val modCheckResultTable = Table()
+    private val modCheckResultTable = Table().apply {
+        defaults().uniformX().fillX().top()
+    }
     private val loadingImage = LoadingImage(48f, LoadingImage.Style(loadingColor = Color.SCARLET))
     /** Used to repopulate modCheckResultTable when searching */
-    private val modResultExpanderTabs = ArrayList<ExpanderTab>()
+    private val modResultTables = ArrayList<Table>()
 
     private var runningCheck: Job? = null
 
@@ -65,7 +67,8 @@ class ModCheckTab(
     private val emptyRuleset = Ruleset()
 
     init {
-        defaults().pad(10f).align(Align.top)
+        top()
+        defaults().pad(10f).top()
 
         fixedContent.defaults().pad(10f).align(Align.top)
         val reloadModsButton = "Reload mods".toTextButton().onClick(::runAction)
@@ -127,18 +130,19 @@ class ModCheckTab(
 
         loadingImage.show()
 
-        val openedExpanderTitles = modResultExpanderTabs
+        val openedExpanderTitles = modResultTables
+            .filterIsInstance<ExpanderTab>()
             .filter { it.isOpen }.map { it.title }.toSet()
 
         modCheckResultTable.clear()
-        modResultExpanderTabs.clear()
+        modResultTables.clear()
 
         val loadingErrors = RulesetCache.loadRulesets()
         if (loadingErrors.isNotEmpty()) {
             val errorTable = Table().apply { defaults().pad(2f) }
             for (loadingError in loadingErrors)
                 errorTable.add(loadingError.toLabel()).width(stage.width / 2).row()
-            modCheckResultTable.add(errorTable)
+            modCheckResultTable.add(errorTable).row()
         }
 
         runningCheck = Concurrency.run("ModChecker") {
@@ -162,9 +166,14 @@ class ModCheckTab(
                 if (!shouldCheckMod(mod, baseForThisMod)) continue
                 if (!isActive) break
 
-                val modLinks =
+                val (combinedRuleset, modLinks) =
-                    if (baseForThisMod == MOD_CHECK_WITHOUT_BASE) mod.getErrorList(tryFixUnknownUniques = true)
+                    if (baseForThisMod == MOD_CHECK_WITHOUT_BASE) {
-                    else RulesetCache.checkCombinedModLinks(linkedSetOf(mod.name), baseForThisMod, tryFixUnknownUniques = true)
+                        mod to mod.getErrorList(tryFixUnknownUniques = true)
+                    } else {
+                        val (ruleset, errors) = RulesetCache.checkCombinedModLinks(linkedSetOf(mod.name), baseForThisMod)
+                        (ruleset ?: mod) to errors
+                    }
+
                 if (!isActive) break
 
                 modLinks.sortByDescending { it.errorSeverityToReport }
@@ -172,7 +181,7 @@ class ModCheckTab(
                 if (!modLinks.isNotOK()) modLinks.add("No problems found.".tr(), RulesetErrorSeverity.OK, sourceObject = null)
 
                 launchOnGLThread {
-                    addModResult(mod, baseForThisMod, modLinks, mod.name in openedExpanderTitles)
+                    addModResult(mod, combinedRuleset, modLinks, mod.name in openedExpanderTitles)
                 }
             }
 
@@ -189,8 +198,8 @@ class ModCheckTab(
             // The last check, whether finished or not, included all mods we want to filter
             synchronized(modCheckResultTable) {
                 modCheckResultTable.clear()
-                for (expanderTab in modResultExpanderTabs) {
+                for (expanderTab in modResultTables) {
-                    if (expanderTab.title.filterApplies())
+                    if (expanderTab.name.filterApplies())
                         modCheckResultTable.add(expanderTab).row()
                 }
             }
@@ -220,7 +229,17 @@ class ModCheckTab(
             ?.name
     }
 
-    private fun addModResult(mod: Ruleset, base: String, modLinks: RulesetErrorList, startsOutOpened: Boolean) {
+    private fun addResultCommon(mod: Ruleset, table: Table) {
+        table.name = mod.name // used when searching
+
+        synchronized(modCheckResultTable) {
+            modResultTables.add(table)
+            if (mod.name.filterApplies())
+                modCheckResultTable.add(table).row()
+        }
+    }
+
+    private fun addModResult(mod: Ruleset, combinedRuleset: Ruleset, modLinks: RulesetErrorList, startsOutOpened: Boolean) {
         // When the options popup is already closed before this postRunnable is run,
         // Don't add the labels, as otherwise the game will crash
         if (stage == null) return
@@ -238,11 +257,11 @@ class ModCheckTab(
             it.defaults().pad(10f)
 
             val openUniqueBuilderButton = "Open unique builder".toTextButton()
-            openUniqueBuilderButton.onClick { openUniqueBuilder(mod, base) }
+            openUniqueBuilderButton.onClick { openUniqueBuilder(combinedRuleset) }
             it.add(openUniqueBuilderButton).row()
 
             if (severity != RulesetErrorSeverity.OK && mod.folderLocation != null) {
-                val replaceableUniques = UniqueAutoUpdater.getDeprecatedReplaceableUniques(mod)
+                val replaceableUniques = UniqueAutoUpdater.getDeprecatedReplaceableUniques(mod, combinedRuleset)
                 if (replaceableUniques.isNotEmpty())
                     it.add("Autoupdate mod uniques".toTextButton()
                         .onClick { autoUpdateUniques(screen, mod, replaceableUniques) }).row()
@@ -260,12 +279,7 @@ class ModCheckTab(
                 }).row()
         }
         expanderTab.header.left()
-
+        addResultCommon(mod, expanderTab)
-        synchronized(modCheckResultTable) {
-            modResultExpanderTabs.add(expanderTab)
-            if (mod.name.filterApplies())
-                modCheckResultTable.add(expanderTab).row()
-        }
     }
 
     private fun addDisabledPlaceholder(mod: Ruleset) {
@@ -277,15 +291,11 @@ class ModCheckTab(
             add(mod.name.toLabel(Color.LIGHT_GRAY, Constants.headingFontSize, alignment = Align.left)).left().grow()
             addTooltip("Requirements could not be determined.\nChoose a base to check this Mod.", 16f, targetAlign = Align.top)
         }
-        synchronized(modCheckResultTable) {
+        addResultCommon(mod, table)
-            modCheckResultTable.add(table).growX().row()
-        }
     }
 
-    private fun openUniqueBuilder(mod: Ruleset, base: String) {
+    private fun openUniqueBuilder(combinedRuleset: Ruleset) {
-        val ruleset = if (base == MOD_CHECK_WITHOUT_BASE) mod
+        UncivGame.Current.pushScreen(UniqueBuilderScreen(combinedRuleset))
-            else RulesetCache.getComplexRuleset(linkedSetOf(mod.name), base)
-        UncivGame.Current.pushScreen(UniqueBuilderScreen(ruleset))
     }
 
     private fun autoUpdateUniques(screen: BaseScreen, mod: Ruleset, replaceableUniques: HashMap<String, String>) {

core/src/com/unciv/ui/screens/newgamescreen/ModCheckboxTable.kt

@@ -143,7 +143,7 @@ class ModCheckboxTable(
     /** Runs in parallel thread */ 
     private fun complexModCheckReturnsErrors(): Boolean {
         // Check over complete combination of selected mods
-        val complexModLinkCheck = RulesetCache.checkCombinedModLinks(mods, baseRulesetName)
+        val (_, complexModLinkCheck) = RulesetCache.checkCombinedModLinks(mods, baseRulesetName)
         if (!complexModLinkCheck.isWarnUser()){
             savedModcheckResult = null
             return false