Re-show redundant tech prereq mod errors without impeding playability (#4046)

* Re-show redundant tech prereq mod errors without impeding playability

* Re-enable playability of mods with less severe errors - fun version

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

@@ -227,9 +227,35 @@ class Ruleset {
         return stringList.joinToString()
     }
 
-
+    /** Severity level of Mod RuleSet check */
-    fun checkModLinks(): String {
+    enum class CheckModLinksStatus {OK, Warning, Error}
+    /** Result of a Mod RuleSet check */
+    // essentially a named Pair with a few shortcuts
+    class CheckModLinksResult(val status: CheckModLinksStatus, val message: String) {
+        // Empty constructor just makes the Complex Mod Check on the new game screen shorter 
+        constructor(): this(CheckModLinksStatus.OK, "")
+        // Constructor that joins lines
+        constructor(status: CheckModLinksStatus, lines: ArrayList<String>):
+                this (status,
+                    lines.joinToString("\n"))
+        // Constructor that auto-determines severity
+        constructor(warningCount: Int, lines: ArrayList<String>):
+                this (
+                    when {
+                        lines.isEmpty() -> CheckModLinksStatus.OK
+                        lines.size == warningCount -> CheckModLinksStatus.Warning
+                        else -> CheckModLinksStatus.Error
+                    },
+                    lines)
+        // Allows $this in format strings 
+        override fun toString() = message
+        // Readability shortcuts
+        fun isError() = status == CheckModLinksStatus.Error
+        fun isNotOK() = status != CheckModLinksStatus.OK
+    }
+    fun checkModLinks(): CheckModLinksResult {
         val lines = ArrayList<String>()
+        var warningCount = 0
 
         // Checks for all mods
         for (unit in units.values) {
@@ -253,7 +279,7 @@ class Ruleset {
                 lines += "${building.name} must either have an explicit cost or reference an existing tech!"
         }
 
-        if (!modOptions.isBaseRuleset) return lines.joinToString("\n")
+        if (!modOptions.isBaseRuleset) return CheckModLinksResult(warningCount, lines)
 
 
         for (unit in units.values) {
@@ -336,13 +362,14 @@ class Ruleset {
                 }
 
                 if (tech.prerequisites.asSequence().filterNot { it == prereq }
-                        .any { getPrereqTree(it).contains(prereq) })
+                        .any { getPrereqTree(it).contains(prereq) }){
-                    println("No need to add $prereq as a prerequisite of ${tech.name} - it is already implicit from the other prerequisites!")
+                    lines += "No need to add $prereq as a prerequisite of ${tech.name} - it is already implicit from the other prerequisites!"
+                    warningCount++
+                }
             }
         }
 
-
+        return CheckModLinksResult(warningCount, lines)
-        return lines.joinToString("\n")
     }
 }
 

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

@@ -2,6 +2,7 @@ package com.unciv.ui.newgamescreen
 
 import com.badlogic.gdx.scenes.scene2d.ui.CheckBox
 import com.badlogic.gdx.scenes.scene2d.ui.Table
+import com.unciv.models.ruleset.Ruleset.CheckModLinksResult
 import com.unciv.models.ruleset.RulesetCache
 import com.unciv.models.translations.tr
 import com.unciv.ui.utils.CameraStageBaseScreen
@@ -22,35 +23,37 @@ class ModCheckboxTable(val mods:LinkedHashSet<String>, val screen: CameraStageBa
             checkBox.onChange {
                 if (checkBox.isChecked) {
                     val modLinkErrors = mod.checkModLinks()
-                    if (modLinkErrors != "") {
+                    if (modLinkErrors.isNotOK()) {
                         ToastPopup("The mod you selected is incorrectly defined!\n\n$modLinkErrors", screen)
-                        checkBox.isChecked = false
+                        if (modLinkErrors.isError()) {
-                        return@onChange
+                            checkBox.isChecked = false
+                            return@onChange
+                        }
                     }
 
                     val previousMods = mods.toList()
 
                     if (mod.modOptions.isBaseRuleset)
-                        for (oldBaseRuleset in previousMods) // so we don't get concurrent modification excpetions
+                        for (oldBaseRuleset in previousMods) // so we don't get concurrent modification exceptions
                             if (modRulesets.firstOrNull { it.name == oldBaseRuleset }?.modOptions?.isBaseRuleset == true)
                                 mods.remove(oldBaseRuleset)
                     mods.add(mod.name)
 
                     var isCompatibleWithCurrentRuleset = true
-                    var complexModLinkErrors = ""
+                    var complexModLinkCheck = CheckModLinksResult()
                     try {
                         val newRuleset = RulesetCache.getComplexRuleset(mods)
                         newRuleset.modOptions.isBaseRuleset = true // This is so the checkModLinks finds all connections
-                        complexModLinkErrors = newRuleset.checkModLinks()
+                        complexModLinkCheck = newRuleset.checkModLinks()
-                        if (complexModLinkErrors != "") isCompatibleWithCurrentRuleset = false
+                        isCompatibleWithCurrentRuleset = !complexModLinkCheck.isError()
                     } catch (x: Exception) {
                         // This happens if a building is dependent on a tech not in the base ruleset
-                        //  because newRuleset.updateBuildingCosts() in getComplexRulset() throws an error
+                        //  because newRuleset.updateBuildingCosts() in getComplexRuleset() throws an error
                         isCompatibleWithCurrentRuleset = false
                     }
 
                     if (!isCompatibleWithCurrentRuleset) {
-                        ToastPopup("The mod you selected is incompatible with the defined ruleset!\n\n$complexModLinkErrors", screen)
+                        ToastPopup("The mod you selected is incompatible with the defined ruleset!\n\n$complexModLinkCheck", screen)
                         checkBox.isChecked = false
                         mods.clear()
                         mods.addAll(previousMods)

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

@@ -249,11 +249,11 @@ class OptionsPopup(val previousScreen:CameraStageBaseScreen) : Popup(previousScr
             val lines = ArrayList<String>()
             for (mod in RulesetCache.values) {
                 val modLinks = mod.checkModLinks()
-                if (modLinks != "") {
+                if (modLinks.isNotOK()) {
                     lines += ""
                     lines += mod.name
                     lines += ""
-                    lines += modLinks
+                    lines += modLinks.message
                     lines += ""
                 }
             }

tests/src/com/unciv/testing/BasicTests.kt

@@ -81,8 +81,8 @@ class BasicTests {
     fun baseRulesetHasNoBugs() {
         ruleset.modOptions.isBaseRuleset=true
         val modCheck = ruleset.checkModLinks()
-        if(modCheck!="") println(modCheck)
+        if(modCheck.isNotOK()) println(modCheck)
-        Assert.assertTrue(modCheck == "")
+        Assert.assertFalse(modCheck.isNotOK())
     }
 
 }