Prevent mod conflicts better (#9586)

* Tighten mod check severity and selectivity for unit-producing triggered Uniques

* Prettify display of mod check results by suppressing dupes and hiding conditionals from tr()

* Extra confirmation to play with errors, colors, improved handling of mod checkboxes

* Tweaks to improved mod checking in new game

android/assets/jsons/translations/template.properties

@@ -439,11 +439,17 @@ No victory conditions were selected! =
 Mods: = 
 Extension mods = 
 Base ruleset: = 
+# Note - do not translate the colour names between «». Changing them works if you know what you're doing.
 The mod you selected is incorrectly defined! = 
+The mod you selected is «RED»incorrectly defined!«» = 
 The mod combination you selected is incorrectly defined! = 
+The mod combination you selected is «RED»incorrectly defined!«» = 
 The mod combination you selected has problems. = 
 You can play it, but don't expect everything to work! = 
+The mod combination you selected «GOLD»has problems«». = 
+You can play it, but «GOLDENROD»don't expect everything to work!«» = 
 This base ruleset is not compatible with the previously selected\nextension mods. They have been disabled. = 
+Are you really sure you want to play with the following known problems? = 
 Base Ruleset = 
 [amount] Techs = 
 [amount] Nations = 

core/src/com/unciv/models/metadata/GameParameters.kt

@@ -50,6 +50,8 @@ class GameParameters : IsPartOfGameInfoSerialization { // Default values are the
 
     var maxTurns = 500
 
+    var acceptedModCheckErrors = ""
+
     fun clone(): GameParameters {
         val parameters = GameParameters()
         parameters.difficulty = difficulty
@@ -80,6 +82,7 @@ class GameParameters : IsPartOfGameInfoSerialization { // Default values are the
         parameters.baseRuleset = baseRuleset
         parameters.mods = LinkedHashSet(mods)
         parameters.maxTurns = maxTurns
+        parameters.acceptedModCheckErrors = acceptedModCheckErrors
         return parameters
     }
 

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

@@ -31,6 +31,7 @@ class RulesetValidator(val ruleset: Ruleset) {
 
         val lines = RulesetErrorList()
 
+        /********************** Ruleset Invariant Part **********************/
         // Checks for all mods - only those that can succeed without loading a base ruleset
         // When not checking the entire ruleset, we can only really detect ruleset-invariant errors in uniques
 
@@ -128,6 +129,8 @@ class RulesetValidator(val ruleset: Ruleset) {
         // Quit here when no base ruleset is loaded - references cannot be checked
         if (!ruleset.modOptions.isBaseRuleset) return lines
 
+        /********************** Ruleset Specific Part **********************/
+
         val vanillaRuleset = RulesetCache.getVanillaRuleset()  // for UnitTypes fallback
 
 
@@ -484,12 +487,11 @@ class RulesetValidator(val ruleset: Ruleset) {
 
         val typeComplianceErrors = unique.type.getComplianceErrors(unique, ruleset)
         for (complianceError in typeComplianceErrors) {
-            // TODO: Make this Error eventually, this is Not Good
             if (complianceError.errorSeverity <= severityToReport)
                 rulesetErrors.add(RulesetError("$name's unique \"${unique.text}\" contains parameter ${complianceError.parameterName}," +
                         " which does not fit parameter type" +
                         " ${complianceError.acceptableParameterTypes.joinToString(" or ") { it.parameterName }} !",
-                    RulesetErrorSeverity.Warning
+                    complianceError.errorSeverity.getRulesetErrorSeverity(severityToReport)
                 ))
         }
 
@@ -505,9 +507,11 @@ class RulesetValidator(val ruleset: Ruleset) {
                         conditional.type.getComplianceErrors(conditional, ruleset)
                 for (complianceError in conditionalComplianceErrors) {
                     if (complianceError.errorSeverity == severityToReport)
-                        rulesetErrors += "$name's unique \"${unique.text}\" contains the conditional \"${conditional.text}\"." +
+                        rulesetErrors.add(RulesetError( "$name's unique \"${unique.text}\" contains the conditional \"${conditional.text}\"." +
                                 " This contains the parameter ${complianceError.parameterName} which does not fit parameter type" +
-                                " ${complianceError.acceptableParameterTypes.joinToString(" or ") { it.parameterName }} !"
+                                " ${complianceError.acceptableParameterTypes.joinToString(" or ") { it.parameterName }} !",
+                            complianceError.errorSeverity.getRulesetErrorSeverity(severityToReport)
+                        ))
                 }
             }
         }
@@ -538,6 +542,7 @@ class RulesetValidator(val ruleset: Ruleset) {
 
 
 class RulesetError(val text:String, val errorSeverityToReport: RulesetErrorSeverity)
+
 enum class RulesetErrorSeverity(val color: Color) {
     OK(Color.GREEN),
     WarningOptionsOnly(Color.YELLOW),
@@ -554,6 +559,23 @@ class RulesetErrorList : ArrayList<RulesetError>() {
         add(RulesetError(text, errorSeverityToReport))
     }
 
+    override fun add(element: RulesetError): Boolean {
+        // Suppress duplicates due to the double run of some checks for invariant/specific,
+        // Without changing collection type or making RulesetError obey the equality contract
+        val existing = firstOrNull { it.text == element.text }
+            ?: return super.add(element)
+        if (existing.errorSeverityToReport >= element.errorSeverityToReport) return false
+        remove(existing)
+        return super.add(element)
+    }
+
+    override fun addAll(elements: Collection<RulesetError>): Boolean {
+        var result = false
+        for (element in elements)
+            if (add(element)) result = true
+        return result
+    }
+
     fun getFinalSeverity(): RulesetErrorSeverity {
         if (isEmpty()) return RulesetErrorSeverity.OK
         return this.maxOf { it.errorSeverityToReport }
@@ -571,5 +593,10 @@ class RulesetErrorList : ArrayList<RulesetError>() {
     fun getErrorText(filter: (RulesetError)->Boolean) =
             filter(filter)
                 .sortedByDescending { it.errorSeverityToReport }
-                .joinToString("\n") { it.errorSeverityToReport.name + ": " + it.text }
+                .joinToString("\n") {
+                    it.errorSeverityToReport.name + ": " +
+                        // This will go through tr(), unavoidably, which will move the conditionals
+                        // out of place. Prevent via kludge:
+                        it.text.replace('<','〈').replace('>','〉')
+                }
 }

core/src/com/unciv/models/ruleset/unique/UniqueParameterType.kt

@@ -125,12 +125,12 @@ enum class UniqueParameterType(
         override fun getTranslationWriterStringsForOutput() = knownValues
     },
 
-    /** Only used by [BaseUnitFilter] */
+    /** Used by [BaseUnitFilter] and e.g. [UniqueType.OneTimeFreeUnit] */
     UnitName("unit", "Musketman") {
         override fun getErrorSeverity(parameterText: String, ruleset: Ruleset):
                 UniqueType.UniqueComplianceErrorSeverity? {
             if (ruleset.units.containsKey(parameterText)) return null
-            return UniqueType.UniqueComplianceErrorSeverity.WarningOnly
+            return UniqueType.UniqueComplianceErrorSeverity.RulesetSpecific  // OneTimeFreeUnitRuins crashes with a bad parameter
         }
     },
 
@@ -250,7 +250,7 @@ enum class UniqueParameterType(
             parameterText != "All" && getErrorSeverity(parameterText, ruleset) == null
     },
 
-    /** Implemented by [PopulationManager.getPopulationFilterAmount][com.unciv.logic.city.CityPopulationManager.getPopulationFilterAmount] */
+    /** Implemented by [PopulationManager.getPopulationFilterAmount][com.unciv.logic.city.managers.CityPopulationManager.getPopulationFilterAmount] */
     PopulationFilter("populationFilter", "Followers of this Religion", null, "Population Filters") {
         private val knownValues = setOf("Population", "Specialists", "Unemployed", "Followers of the Majority Religion", "Followers of this Religion")
         override fun getErrorSeverity(

core/src/com/unciv/models/ruleset/unique/UniqueType.kt

@@ -2,6 +2,7 @@ package com.unciv.models.ruleset.unique
 
 import com.unciv.Constants
 import com.unciv.models.ruleset.Ruleset
+import com.unciv.models.ruleset.RulesetErrorSeverity
 import com.unciv.models.translations.getPlaceholderParameters
 import com.unciv.models.translations.getPlaceholderText
 
@@ -732,9 +733,9 @@ enum class UniqueType(val text: String, vararg targets: UniqueTarget, val flags:
     ///////////////////////////////////////// region TRIGGERED ONE-TIME /////////////////////////////////////////
 
 
-    OneTimeFreeUnit("Free [baseUnitFilter] appears", UniqueTarget.Triggerable),  // used in Policies, Buildings
+    OneTimeFreeUnit("Free [unit] appears", UniqueTarget.Triggerable),  // used in Policies, Buildings
-    OneTimeAmountFreeUnits("[amount] free [baseUnitFilter] units appear", UniqueTarget.Triggerable), // used in Buildings
+    OneTimeAmountFreeUnits("[amount] free [unit] units appear", UniqueTarget.Triggerable), // used in Buildings
-    OneTimeFreeUnitRuins("Free [baseUnitFilter] found in the ruins", UniqueTarget.Ruins), // Differs from "Free [] appears" in that it spawns near the ruins instead of in a city
+    OneTimeFreeUnitRuins("Free [unit] found in the ruins", UniqueTarget.Ruins), // Differs from "Free [] appears" in that it spawns near the ruins instead of in a city
     OneTimeFreePolicy("Free Social Policy", UniqueTarget.Triggerable), // used in Buildings
     OneTimeAmountFreePolicies("[amount] Free Social Policies", UniqueTarget.Triggerable),  // Not used in Vanilla
     OneTimeEnterGoldenAge("Empire enters golden age", UniqueTarget.Triggerable),  // used in Policies, Buildings
@@ -1205,15 +1206,32 @@ enum class UniqueType(val text: String, vararg targets: UniqueTarget, val flags:
     enum class UniqueComplianceErrorSeverity {
 
         /** This is for filters that can also potentially accept free text, like UnitFilter and TileFilter */
-        WarningOnly,
+        WarningOnly {
+            override fun getRulesetErrorSeverity(severityToReport: UniqueComplianceErrorSeverity) =
+                RulesetErrorSeverity.WarningOptionsOnly
+        },
 
         /** This is a problem like "unit/resource/tech name doesn't exist in ruleset" - definite bug */
-        RulesetSpecific,
+        RulesetSpecific {
-
+            // Report Warning on the first pass of RulesetValidator only, where mods are checked standalone
+            // but upgrade to error when the econd pass asks, which runs only for combined or base rulesets.
+            override fun getRulesetErrorSeverity(severityToReport: UniqueComplianceErrorSeverity) =
+                RulesetErrorSeverity.Warning
+        },
 
         /** This is a problem like "numbers don't parse", "stat isn't stat", "city filter not applicable" */
-        RulesetInvariant
+        RulesetInvariant {
+            override fun getRulesetErrorSeverity(severityToReport: UniqueComplianceErrorSeverity) =
+                RulesetErrorSeverity.Error
+        },
+        ;
 
+        /** Done as function instead of property so we can in the future upgrade severities depending
+         *  on the [RulesetValidator] "pass": [severityToReport]==[RulesetInvariant] means it's the
+         *  first pass that also runs for extension mods without a base mixed in; the complex check
+         *  runs with [severityToReport]==[RulesetSpecific].
+         */
+        abstract fun getRulesetErrorSeverity(severityToReport: UniqueComplianceErrorSeverity): RulesetErrorSeverity
     }
 
     /** Maps uncompliant parameters to their required types */

core/src/com/unciv/ui/popups/ToastPopup.kt

@@ -1,6 +1,8 @@
 package com.unciv.ui.popups
 
 import com.badlogic.gdx.scenes.scene2d.Stage
+import com.badlogic.gdx.utils.Align
+import com.unciv.ui.components.ColorMarkupLabel
 import com.unciv.ui.components.input.onClick
 import com.unciv.ui.screens.basescreen.BaseScreen
 import com.unciv.utils.Concurrency
@@ -10,6 +12,8 @@ import kotlinx.coroutines.delay
 /**
  * This is an unobtrusive popup which will close itself after a given amount of time.
  * Default time is two seconds (in milliseconds)
+ *
+ * Note: Supports color markup via [ColorMarkupLabel], using «» instead of Gdx's [].
  */
 class ToastPopup (message: String, stageToShowOn: Stage, val time: Long = 2000) : Popup(stageToShowOn){
 
@@ -20,7 +24,11 @@ class ToastPopup (message: String, stageToShowOn: Stage, val time: Long = 2000)
         setFillParent(false)
         onClick { close() }  // or `touchable = Touchable.disabled` so you can operate what's behind
 
-        addGoodSizedLabel(message)
+        add(ColorMarkupLabel(message).apply {
+            wrap = true
+            setAlignment(Align.center)
+        }).width(stageToShowOn.width / 2)
+
         open()
         //move it to the top so its not in the middle of the screen
         //have to be done after open() because open() centers the popup

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

@@ -0,0 +1,48 @@
+package com.unciv.ui.screens.newgamescreen
+
+import com.badlogic.gdx.utils.Align
+import com.unciv.Constants
+import com.unciv.ui.components.ColorMarkupLabel
+import com.unciv.ui.popups.ConfirmPopup
+import com.unciv.ui.popups.closeAllPopups
+import com.unciv.ui.screens.basescreen.BaseScreen
+
+internal class AcceptModErrorsPopup(
+    screen: BaseScreen,
+    modCheckResult: String,
+    restoreDefault: () -> Unit,
+    action: () -> Unit
+) : ConfirmPopup(
+    screen,
+    question = "",  // do coloured label instead
+    confirmText = "Accept",
+    isConfirmPositive = false,
+    restoreDefault = restoreDefault,
+    action = action
+) {
+    init {
+        clickBehindToClose = false
+        row()  // skip the empty question label
+        val maxRowWidth = screen.stage.width * 0.9f - 50f  // total padding is 2*(20+5)
+        getScrollPane()?.setScrollingDisabled(true, false)
+
+        // Note - using the version of ColorMarkupLabel that supports «color» but it was too garish.
+        val question = "Are you really sure you want to play with the following known problems?"
+        val label1 = ColorMarkupLabel(question, Constants.headingFontSize)
+        val wrapWidth = label1.prefWidth.coerceIn(maxRowWidth / 2, maxRowWidth)
+        label1.setAlignment(Align.center)
+        if (label1.prefWidth > wrapWidth) {
+            label1.wrap = true
+            add(label1).width(wrapWidth).padBottom(15f).row()
+        } else add(label1).padBottom(15f).row()
+
+        val warnings = modCheckResult.replace("Error:", "«RED»Error«»:")
+            .replace("Warning:","«GOLD»Warning«»:")
+        val label2 = ColorMarkupLabel(warnings)
+        label2.wrap = true
+        add(label2).width(wrapWidth)
+
+        screen.closeAllPopups()  // Toasts too
+        open(true)
+    }
+}

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

@@ -8,6 +8,7 @@ import com.badlogic.gdx.utils.Align
 import com.unciv.Constants
 import com.unciv.UncivGame
 import com.unciv.logic.civilization.PlayerType
+import com.unciv.models.metadata.BaseRuleset
 import com.unciv.models.metadata.GameParameters
 import com.unciv.models.metadata.Player
 import com.unciv.models.ruleset.RulesetCache
@@ -18,20 +19,19 @@ import com.unciv.ui.audio.MusicMood
 import com.unciv.ui.audio.MusicTrackChooserFlags
 import com.unciv.ui.components.AutoScrollPane
 import com.unciv.ui.components.ExpanderTab
-import com.unciv.ui.components.input.KeyCharAndCode
 import com.unciv.ui.components.UncivSlider
-import com.unciv.ui.components.input.keyShortcuts
-import com.unciv.ui.components.input.onActivation
-import com.unciv.ui.components.input.onChange
-import com.unciv.ui.components.input.onClick
 import com.unciv.ui.components.extensions.pad
 import com.unciv.ui.components.extensions.toCheckBox
 import com.unciv.ui.components.extensions.toImageButton
 import com.unciv.ui.components.extensions.toLabel
 import com.unciv.ui.components.extensions.toTextButton
+import com.unciv.ui.components.input.KeyCharAndCode
+import com.unciv.ui.components.input.keyShortcuts
+import com.unciv.ui.components.input.onActivation
+import com.unciv.ui.components.input.onChange
+import com.unciv.ui.components.input.onClick
 import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.popups.Popup
-import com.unciv.ui.popups.ToastPopup
 import com.unciv.ui.screens.basescreen.BaseScreen
 import com.unciv.ui.screens.multiplayerscreens.MultiplayerHelpers
 import kotlin.reflect.KMutableProperty0
@@ -45,30 +45,31 @@ class GameOptionsTable(
     var gameParameters = previousScreen.gameSetupInfo.gameParameters
     val ruleset = previousScreen.ruleset
     var locked = false
-    var modCheckboxes: ModCheckboxTable? = null
+
-        private set
+    /** Holds the UI for the Extension Mods
+     *
+     *  Attention: This Widget is a little tricky due to the UI changes to support portrait mode:
+     *  *  With `isPortrait==false`, this Table will **contain** `modCheckboxes`
+     *  *  With `isPortrait==true`, this Table will **only initialize** `modCheckboxes` and [NewGameScreen] will fetch and place it.
+     *
+     *  The second reason this is public: [NewGameScreen] accesses [ModCheckboxTable.savedModcheckResult] for display.
+     */
+    val modCheckboxes = getModCheckboxes(isPortrait = isPortrait)
+
     // Remember this so we can unselect it when the pool dialog returns an empty pool
     private var randomNationsPoolCheckbox: CheckBox? = null
+    // Allow resetting base ruleset from outside
+    private var baseRulesetSelectBox: TranslatedSelectBox? = null
 
     init {
-        getGameOptionsTable()
         background = BaseScreen.skinStrings.getUiBackground("NewGameScreen/GameOptionsTable", tintColor = BaseScreen.skinStrings.skinConfig.clearColor)
+        top()
+        defaults().pad(5f)
+        update()
     }
 
     fun update() {
         clear()
-        getGameOptionsTable()
-    }
-
-    private fun getGameOptionsTable() {
-        top()
-        defaults().pad(5f)
-
-        // We assign this first to make sure addBaseRulesetSelectBox doesn't reference a null object
-        modCheckboxes =
-            if (isPortrait)
-                getModCheckboxes(isPortrait = true)
-            else getModCheckboxes()
 
         add(Table().apply {
             defaults().pad(5f)
@@ -271,7 +272,6 @@ class GameOptionsTable(
     ) {
         if (maxValue < minValue) return
 
-        @Suppress("JoinDeclarationAndAssignment")  // it's a forward declaration!
         lateinit var maxSlider: UncivSlider  // lateinit safe because the closure won't use it until the user operates a slider
         val minSlider = UncivSlider(minValue.toFloat(), maxValue.toFloat(), 1f, initial = minField.get().toFloat()) {
             val newMin = it.toInt()
@@ -341,7 +341,7 @@ class GameOptionsTable(
         return slider
     }
 
-    private fun Table.addSelectBox(text: String, values: Collection<String>, initialState: String, onChange: (newValue: String) -> String?) {
+    private fun Table.addSelectBox(text: String, values: Collection<String>, initialState: String, onChange: (newValue: String) -> String?): TranslatedSelectBox {
         add(text.toLabel(hideIcons = true)).left()
         val selectBox = TranslatedSelectBox(values, initialState, BaseScreen.skin)
         selectBox.isDisabled = locked
@@ -351,6 +351,7 @@ class GameOptionsTable(
         }
         onChange(selectBox.selected.value)
         add(selectBox).fillX().row()
+        return selectBox
     }
 
     private fun Table.addDifficultySelectBox() {
@@ -359,50 +360,36 @@ class GameOptionsTable(
     }
 
     private fun Table.addBaseRulesetSelectBox() {
-        val sortedBaseRulesets = RulesetCache.getSortedBaseRulesets()
+        fun onBaseRulesetSelected(newBaseRuleset: String): String? {
-        if (sortedBaseRulesets.size < 2) return
-
-        addSelectBox(
-            "{Base Ruleset}:",
-            sortedBaseRulesets,
-            gameParameters.baseRuleset
-        ) { newBaseRuleset ->
             val previousSelection = gameParameters.baseRuleset
-            if (newBaseRuleset == gameParameters.baseRuleset) return@addSelectBox null
+            if (newBaseRuleset == previousSelection) return null
 
             // Check if this mod is well-defined
             val baseRulesetErrors = RulesetCache[newBaseRuleset]!!.checkModLinks()
             if (baseRulesetErrors.isError()) {
-                val toastMessage = "The mod you selected is incorrectly defined!".tr() + "\n\n${baseRulesetErrors.getErrorText()}"
+                baseRulesetErrors.showWarnOrErrorToast(previousScreen as BaseScreen)
-                ToastPopup(toastMessage, previousScreen as BaseScreen, 5000L)
+                return previousSelection
-                return@addSelectBox previousSelection
             }
 
             // If so, add it to the current ruleset
             gameParameters.baseRuleset = newBaseRuleset
             onChooseMod(newBaseRuleset)
 
-            // Check if the ruleset in it's entirety is still well-defined
+            // Check if the ruleset in its entirety is still well-defined
             val modLinkErrors = ruleset.checkModLinks()
             if (modLinkErrors.isError()) {
-                gameParameters.mods.clear()
+                modCheckboxes.disableAllCheckboxes()  // also clears gameParameters.mods
                 reloadRuleset()
-                val toastMessage =
+            }
-                    "This base ruleset is not compatible with the previously selected\nextension mods. They have been disabled.".tr()
+            modLinkErrors.showWarnOrErrorToast(previousScreen as BaseScreen)
-                ToastPopup(toastMessage, previousScreen as BaseScreen, 5000L)
 
-                modCheckboxes!!.disableAllCheckboxes()
+            modCheckboxes.setBaseRuleset(newBaseRuleset)
-            } else if (modLinkErrors.isWarnUser()) {
+            return null
-                val toastMessage =
-                    "{The mod combination you selected has problems.}\n{You can play it, but don't expect everything to work!}".tr() +
-                    "\n\n${modLinkErrors.getErrorText()}"
-                ToastPopup(toastMessage, previousScreen as BaseScreen, 5000L)
         }
 
-            modCheckboxes!!.setBaseRuleset(newBaseRuleset)
+        val sortedBaseRulesets = RulesetCache.getSortedBaseRulesets()
-
+        if (sortedBaseRulesets.size < 2) return
-            null
+        baseRulesetSelectBox = addSelectBox("{Base Ruleset}:", sortedBaseRulesets, gameParameters.baseRuleset, ::onBaseRulesetSelected)
-        }
     }
 
     private fun Table.addGameSpeedSelectBox() {
@@ -442,6 +429,15 @@ class GameOptionsTable(
         add(victoryConditionsTable).colspan(2).row()
     }
 
+    fun resetRuleset() {
+        val rulesetName = BaseRuleset.Civ_V_GnK.fullName
+        gameParameters.baseRuleset = rulesetName
+        modCheckboxes.setBaseRuleset(rulesetName)
+        modCheckboxes.disableAllCheckboxes()
+        baseRulesetSelectBox?.setSelected(rulesetName)
+        reloadRuleset()
+    }
+
     private fun reloadRuleset() {
         ruleset.clear()
         val newRuleset = RulesetCache.getComplexRuleset(gameParameters)

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

@@ -5,21 +5,19 @@ import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.badlogic.gdx.scenes.scene2d.utils.ChangeListener
 import com.unciv.models.ruleset.Ruleset
 import com.unciv.models.ruleset.RulesetCache
-import com.unciv.models.ruleset.RulesetErrorList
-import com.unciv.models.translations.tr
-import com.unciv.ui.popups.ToastPopup
-import com.unciv.ui.screens.basescreen.BaseScreen
 import com.unciv.ui.components.ExpanderTab
-import com.unciv.ui.components.input.onChange
 import com.unciv.ui.components.extensions.pad
 import com.unciv.ui.components.extensions.toCheckBox
+import com.unciv.ui.components.input.onChange
+import com.unciv.ui.popups.ToastPopup
+import com.unciv.ui.screens.basescreen.BaseScreen
 
 /**
  * A widget containing one expander for extension mods.
  * Manages compatibility checks, warns or prevents incompatibilities.
  *
  * @param mods In/out set of active mods, modified in place
- * @param baseRuleset The selected base Ruleset //todo clarify
+ * @param baseRuleset The selected base Ruleset, only for running mod checks against. Use [setBaseRuleset] to change on the fly.
  * @param screen Parent screen, used only to show [ToastPopup]s
  * @param isPortrait Used only for minor layout tweaks, arrangement is always vertical
  * @param onUpdate Callback, parameter is the mod name, called after any checks that may prevent mod selection succeed.
@@ -29,14 +27,21 @@ class ModCheckboxTable(
     private var baseRuleset: String,
     private val screen: BaseScreen,
     isPortrait: Boolean = false,
-    onUpdate: (String) -> Unit
+    private val onUpdate: (String) -> Unit
 ): Table() {
-    private val modRulesets = RulesetCache.values.filter { it.name != "" && !it.modOptions.isBaseRuleset}
-    private var lastToast: ToastPopup? = null
     private val extensionRulesetModButtons = ArrayList<CheckBox>()
 
-    init {
+    /** Saved result from any complex mod check unless the causing selection has already been reverted.
+     *  In other words, this can contain the text for an "Error" level check only if the Widget was
+     *  initialized with such an invalid mod combination.
+     *  This Widget reverts User changes that cause an Error severity immediately and this field is nulled.
+     */
+    var savedModcheckResult: String? = null
 
+    private var disableChangeEvents = false
+
+    init {
+        val modRulesets = RulesetCache.values.filter { it.name != "" && !it.modOptions.isBaseRuleset}
         for (mod in modRulesets.sortedBy { it.name }) {
             val checkBox = mod.name.toCheckBox(mod.name in mods)
             checkBox.onChange {
@@ -57,25 +62,29 @@ class ModCheckboxTable(
                     it.add(checkbox).row()
                 }
             }).pad(10f).padTop(padTop).growX().row()
+
+            runComplexModCheck()
         }
     }
 
     fun setBaseRuleset(newBaseRuleset: String) { baseRuleset = newBaseRuleset }
     fun disableAllCheckboxes() {
+        disableChangeEvents = true
         for (checkBox in extensionRulesetModButtons) {
             checkBox.isChecked = false
         }
+        mods.clear()
+        disableChangeEvents = false
+        onUpdate("-")  // should match no mod
     }
 
-
+    private fun runComplexModCheck(): Boolean {
-    private fun popupToastError(rulesetErrorList: RulesetErrorList) {
+        // Check over complete combination of selected mods
-        val initialText =
+        val complexModLinkCheck = RulesetCache.checkCombinedModLinks(mods, baseRuleset)
-            if (rulesetErrorList.isError()) "The mod combination you selected is incorrectly defined!".tr()
+        if (!complexModLinkCheck.isWarnUser()) return false
-            else "{The mod combination you selected has problems.}\n{You can play it, but don't expect everything to work!}".tr()
+        savedModcheckResult = complexModLinkCheck.getErrorText()
-        val toastMessage =  "$initialText\n\n${rulesetErrorList.getErrorText()}"
+        complexModLinkCheck.showWarnOrErrorToast(screen)
-
+        return complexModLinkCheck.isError()
-        lastToast?.close()
-        lastToast = ToastPopup(toastMessage, screen, 5000L)
     }
 
     private fun checkBoxChanged(
@@ -83,14 +92,13 @@ class ModCheckboxTable(
         changeEvent: ChangeListener.ChangeEvent,
         mod: Ruleset
     ): Boolean {
+        if (disableChangeEvents) return false
+
         if (checkBox.isChecked) {
             // First the quick standalone check
             val modLinkErrors = mod.checkModLinks()
             if (modLinkErrors.isError()) {
-                lastToast?.close()
+                modLinkErrors.showWarnOrErrorToast(screen)
-                val toastMessage =
-                    "The mod you selected is incorrectly defined!".tr() + "\n\n${modLinkErrors.getErrorText()}"
-                lastToast = ToastPopup(toastMessage, screen, 5000L)
                 changeEvent.cancel() // Cancel event to reset to previous state - see Button.setChecked()
                 return false
             }
@@ -98,15 +106,12 @@ class ModCheckboxTable(
             mods.add(mod.name)
 
             // Check over complete combination of selected mods
-            val complexModLinkCheck = RulesetCache.checkCombinedModLinks(mods, baseRuleset)
+            if (runComplexModCheck()) {
-            if (complexModLinkCheck.isWarnUser()) {
-                popupToastError(complexModLinkCheck)
-                if (complexModLinkCheck.isError()) {
                 changeEvent.cancel() // Cancel event to reset to previous state - see Button.setChecked()
                 mods.remove(mod.name)
+                savedModcheckResult = null  // we just fixed it
                 return false
             }
-            }
 
         } else {
             /**
@@ -115,15 +120,13 @@ class ModCheckboxTable(
              */
 
             mods.remove(mod.name)
-            val complexModLinkCheck = RulesetCache.checkCombinedModLinks(mods, baseRuleset)
+
-            if (complexModLinkCheck.isWarnUser()) {
+            if (runComplexModCheck()) {
-                popupToastError(complexModLinkCheck)
-                if (complexModLinkCheck.isError()) {
                 changeEvent.cancel() // Cancel event to reset to previous state - see Button.setChecked()
                 mods.add(mod.name)
+                savedModcheckResult = null  // we just fixed it
                 return false
             }
-            }
 
         }
 

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

@@ -0,0 +1,23 @@
+package com.unciv.ui.screens.newgamescreen
+
+import com.unciv.models.ruleset.RulesetErrorList
+import com.unciv.models.translations.tr
+import com.unciv.ui.popups.ToastPopup
+import com.unciv.ui.popups.popups
+import com.unciv.ui.screens.basescreen.BaseScreen
+
+/**
+ * Show a [ToastPopup] for this if severity is at least [isWarnUser][RulesetErrorList.isWarnUser].
+ *
+ * Adds an appropriate header to [getErrorText][RulesetErrorList.getErrorText],
+ * exists mainly to centralize those strings.
+ */
+fun RulesetErrorList.showWarnOrErrorToast(screen: BaseScreen) {
+    if (!isWarnUser()) return
+    val headerText =
+        if (isError()) "The mod combination you selected is «RED»incorrectly defined!«»"
+        else "{The mod combination you selected «GOLD»has problems«».}\n{You can play it, but «GOLDENROD»don't expect everything to work!«»}"
+    val toastMessage = headerText.tr() + "\n\n{" + getErrorText() + "}"
+    for (oldToast in screen.popups.filterIsInstance<ToastPopup>()) { oldToast.close() }
+    ToastPopup(toastMessage, screen, 5000L)
+}

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

@@ -20,17 +20,17 @@ import com.unciv.models.metadata.GameSetupInfo
 import com.unciv.models.ruleset.RulesetCache
 import com.unciv.models.translations.tr
 import com.unciv.ui.components.ExpanderTab
-import com.unciv.ui.components.input.KeyCharAndCode
 import com.unciv.ui.components.extensions.addSeparator
 import com.unciv.ui.components.extensions.addSeparatorVertical
 import com.unciv.ui.components.extensions.disable
 import com.unciv.ui.components.extensions.enable
-import com.unciv.ui.components.input.keyShortcuts
-import com.unciv.ui.components.input.onActivation
-import com.unciv.ui.components.input.onClick
 import com.unciv.ui.components.extensions.pad
 import com.unciv.ui.components.extensions.toLabel
 import com.unciv.ui.components.extensions.toTextButton
+import com.unciv.ui.components.input.KeyCharAndCode
+import com.unciv.ui.components.input.keyShortcuts
+import com.unciv.ui.components.input.onActivation
+import com.unciv.ui.components.input.onClick
 import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.popups.ConfirmPopup
 import com.unciv.ui.popups.Popup
@@ -38,8 +38,8 @@ import com.unciv.ui.popups.ToastPopup
 import com.unciv.ui.screens.basescreen.BaseScreen
 import com.unciv.ui.screens.basescreen.RecreateOnResize
 import com.unciv.ui.screens.pickerscreens.PickerScreen
-import com.unciv.utils.Log
 import com.unciv.utils.Concurrency
+import com.unciv.utils.Log
 import com.unciv.utils.launchOnGLThread
 import kotlinx.coroutines.coroutineScope
 import java.net.URL
@@ -79,17 +79,17 @@ class NewGameScreen(
             updatePlayerPickerRandomLabel = { playerPickerTable.updateRandomNumberLabel() }
         )
         mapOptionsTable = MapOptionsTable(this)
-        pickerPane.closeButton.onActivation {
+        closeButton.onActivation {
             mapOptionsTable.cancelBackgroundJobs()
             game.popScreen()
         }
-        pickerPane.closeButton.keyShortcuts.add(KeyCharAndCode.BACK)
+        closeButton.keyShortcuts.add(KeyCharAndCode.BACK)
 
         if (isPortrait) initPortrait()
         else initLandscape()
 
-        pickerPane.bottomTable.background = skinStrings.getUiBackground("NewGameScreen/BottomTable", tintColor = skinStrings.skinConfig.clearColor)
+        bottomTable.background = skinStrings.getUiBackground("NewGameScreen/BottomTable", tintColor = skinStrings.skinConfig.clearColor)
-        pickerPane.topTable.background = skinStrings.getUiBackground("NewGameScreen/TopTable", tintColor = skinStrings.skinConfig.clearColor)
+        topTable.background = skinStrings.getUiBackground("NewGameScreen/TopTable", tintColor = skinStrings.skinConfig.clearColor)
 
         if (UncivGame.Current.settings.lastGameSetup != null) {
             rightSideGroup.addActorAt(0, VerticalGroup().padBottom(5f))
@@ -166,6 +166,20 @@ class NewGameScreen(
             return
         }
 
+        val modCheckResult = newGameOptionsTable.modCheckboxes.savedModcheckResult
+        newGameOptionsTable.modCheckboxes.savedModcheckResult = null
+        if (modCheckResult != null) {
+            AcceptModErrorsPopup(
+                this, modCheckResult,
+                restoreDefault = { newGameOptionsTable.resetRuleset() },
+                action = {
+                    gameSetupInfo.gameParameters.acceptedModCheckErrors = modCheckResult
+                    onStartGameClicked()
+                }
+            )
+            return
+        }
+
         Gdx.input.inputProcessor = null // remove input processing - nothing will be clicked!
 
         if (mapOptionsTable.mapTypeSelectBox.selected.value == MapGeneratedMainType.custom) {

core/src/com/unciv/ui/screens/pickerscreens/PickerPane.kt

@@ -20,7 +20,8 @@ import com.unciv.ui.components.extensions.toTextButton
 class PickerPane(
     disableScroll: Boolean = false,
 ) : Table() {
-    /** The close button on the lower left of [bottomTable], see [setDefaultCloseAction] */
+    /** The close button on the lower left of [bottomTable], see [PickerScreen.setDefaultCloseAction].
+     *  Note if you don't use that helper, you'll need to do both click and keyboard support yourself. */
     val closeButton = Constants.close.toTextButton()
     /** A scrollable wrapped Label you can use to show descriptions in the [bottomTable], starts empty */
     val descriptionLabel = "".toLabel()

core/src/com/unciv/ui/screens/pickerscreens/PickerScreen.kt

@@ -20,6 +20,8 @@ open class PickerScreen(disableScroll: Boolean = false) : BaseScreen() {
 
     /** @see PickerPane.topTable */
     val topTable by pickerPane::topTable
+    /** @see PickerPane.bottomTable */
+    val bottomTable by pickerPane::bottomTable
     /** @see PickerPane.scrollPane */
     val scrollPane by pickerPane::scrollPane
     /** @see PickerPane.splitPane */

docs/Modders/uniques.md

@@ -1,13 +1,13 @@
 # Uniques
 Simple unique parameters are explained by mouseover. Complex parameters are explained in [Unique parameter types](../Unique-parameters)
 ## Triggerable uniques
-??? example  "Free [baseUnitFilter] appears"
+??? example  "Free [unit] appears"
-	Example: "Free [Melee] appears"
+	Example: "Free [Musketman] appears"
 
 	Applicable to: Triggerable
 
-??? example  "[amount] free [baseUnitFilter] units appear"
+??? example  "[amount] free [unit] units appear"
-	Example: "[3] free [Melee] units appear"
+	Example: "[3] free [Musketman] units appear"
 
 	Applicable to: Triggerable
 
@@ -1655,8 +1655,8 @@ Simple unique parameters are explained by mouseover. Complex parameters are expl
 	Applicable to: Resource
 
 ## Ruins uniques
-??? example  "Free [baseUnitFilter] found in the ruins"
+??? example  "Free [unit] found in the ruins"
-	Example: "Free [Melee] found in the ruins"
+	Example: "Free [Musketman] found in the ruins"
 
 	Applicable to: Ruins