Modding: Validation for civilopediaText (#11491)

* Lint RulesetValidator

* Better reusable AtlasPreview

* Validate civilopediaText in all RulesetObjects

* Prepare Event and EventChoice having ICivilopediaText

* Activate events civilopediaText validation

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

@@ -28,6 +28,7 @@ import com.unciv.models.ruleset.validation.RulesetValidator
 import com.unciv.models.ruleset.validation.UniqueValidator
 import com.unciv.models.stats.INamed
 import com.unciv.models.translations.tr
+import com.unciv.ui.screens.civilopediascreen.ICivilopediaText
 import com.unciv.utils.Log
 import kotlin.collections.set
 
@@ -238,6 +239,8 @@ class Ruleset {
             // Victories is only INamed
     fun allIHasUniques(): Sequence<IHasUniques> =
             allRulesetObjects() + sequenceOf(modOptions)
+    fun allICivilopediaText(): Sequence<ICivilopediaText> =
+            allRulesetObjects() + events.values + events.values.flatMap { it.choices }
 
     fun load(folderHandle: FileHandle) {
         // Note: Most files are loaded using createHashmap, which sets originRuleset automatically.

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

@@ -2,7 +2,6 @@ package com.unciv.models.ruleset.validation
 
 import com.badlogic.gdx.Gdx
 import com.badlogic.gdx.graphics.Color
-import com.badlogic.gdx.graphics.g2d.TextureAtlas.TextureAtlasData
 import com.unciv.Constants
 import com.unciv.json.fromJsonFile
 import com.unciv.json.json
@@ -10,6 +9,7 @@ import com.unciv.logic.map.tile.RoadStatus
 import com.unciv.models.metadata.BaseRuleset
 import com.unciv.models.ruleset.BeliefType
 import com.unciv.models.ruleset.Building
+import com.unciv.models.ruleset.IRulesetObject
 import com.unciv.models.ruleset.Ruleset
 import com.unciv.models.ruleset.RulesetCache
 import com.unciv.models.ruleset.nation.Nation
@@ -20,14 +20,18 @@ import com.unciv.models.ruleset.unique.StateForConditionals
 import com.unciv.models.ruleset.unique.UniqueType
 import com.unciv.models.ruleset.unit.BaseUnit
 import com.unciv.models.ruleset.unit.Promotion
+import com.unciv.models.stats.INamed
 import com.unciv.models.stats.Stats
 import com.unciv.models.tilesets.TileSetCache
 import com.unciv.models.tilesets.TileSetConfig
+import com.unciv.ui.images.AtlasPreview
 
 class RulesetValidator(val ruleset: Ruleset) {
 
     private val uniqueValidator = UniqueValidator(ruleset)
 
+    private val textureNamesCache by lazy { AtlasPreview(ruleset) }
+
     fun getErrorList(tryFixUnknownUniques: Boolean = false): RulesetErrorList {
         // When no base ruleset is loaded - references cannot be checked
         if (!ruleset.modOptions.isBaseRuleset) return getNonBaseRulesetErrorList(tryFixUnknownUniques)
@@ -49,10 +53,10 @@ class RulesetValidator(val ruleset: Ruleset) {
         addPromotionErrorsRulesetInvariant(lines, tryFixUnknownUniques)
         addResourceErrorsRulesetInvariant(lines, tryFixUnknownUniques)
 
-        /**********************  **********************/
+        // Tileset tests - e.g. json configs complete and parseable
-        // e.g. json configs complete and parseable
+        checkTilesetSanity(lines)
-        // Check for mod or Civ_V_GnK to avoid running the same test twice (~200ms for the builtin assets)
+
-        if (ruleset.folderLocation != null) checkTilesetSanity(lines)
+        checkCivilopediaText(lines)
 
         return lines
     }
@@ -87,11 +91,14 @@ class RulesetValidator(val ruleset: Ruleset) {
         addEventErrors(lines, tryFixUnknownUniques)
         addCityStateTypeErrors(tryFixUnknownUniques, lines)
 
+        // Tileset tests - e.g. json configs complete and parseable
         // Check for mod or Civ_V_GnK to avoid running the same test twice (~200ms for the builtin assets)
         if (ruleset.folderLocation != null || ruleset.name == BaseRuleset.Civ_V_GnK.fullName) {
             checkTilesetSanity(lines)
         }
 
+        checkCivilopediaText(lines)
+
         return lines
     }
 
@@ -144,7 +151,7 @@ class RulesetValidator(val ruleset: Ruleset) {
 
     private fun addEventErrors(lines: RulesetErrorList,
                                tryFixUnknownUniques: Boolean) {
-        // A Difficulty is not a IHasUniques, so not suitable as sourceObject
+        // An Event is not a IHasUniques, so not suitable as sourceObject
         for (event in ruleset.events.values) {
             for (choice in event.choices) {
                 for (unique in choice.conditionObjects + choice.triggeredUniqueObjects)
@@ -819,21 +826,11 @@ class RulesetValidator(val ruleset: Ruleset) {
             lines.add("Fallback tileset invalid: ${unknownFallbacks.joinToString()}", RulesetErrorSeverity.Warning, sourceObject = null)
     }
 
-    private fun getTilesetNamesFromAtlases(): Set<String> {
+    private fun getTilesetNamesFromAtlases() =
-        // This partially duplicates code in ImageGetter.getAvailableTilesets, but we don't want to reload that singleton cache.
+        textureNamesCache
-
+            .filter { it.startsWith("TileSets/") && !it.contains("/Units/") }
-        // Our builtin rulesets have no folderLocation, in that case cheat and apply knowledge about
+            .map { it.split("/")[1] }
-        // where the builtin Tileset textures are (correct would be to parse Atlases.json):
-        val files = ruleset.folderLocation?.list("atlas")?.asSequence()
-            ?: sequenceOf(Gdx.files.internal("Tilesets.atlas"))
-        // Next, we need to cope with this running without GL context (unit test) - no TextureAtlas(file)
-        return files
-            .flatMap { file ->
-                TextureAtlasData(file, file.parent(), false).regions.asSequence()
-                    .filter { it.name.startsWith("TileSets/") && !it.name.contains("/Units/") }
-            }.map { it.name.split("/")[1] }
             .toSet()
-    }
 
     private fun checkPromotionCircularReferences(lines: RulesetErrorList) {
         fun recursiveCheck(history: HashSet<Promotion>, promotion: Promotion, level: Int) {
@@ -858,4 +855,21 @@ class RulesetValidator(val ruleset: Ruleset) {
             recursiveCheck(hashSetOf(), promotion, 0)
         }
     }
+
+    private fun checkCivilopediaText(lines: RulesetErrorList) {
+        for (sourceObject in ruleset.allICivilopediaText()) {
+            for ((index, line) in sourceObject.civilopediaText.withIndex()) {
+                for (error in line.unsupportedReasons(this)) {
+                    val nameText = (sourceObject as? INamed)?.name?.plus("'s ") ?: ""
+                    val text = "(${sourceObject::class.java.simpleName}) ${nameText}civilopediaText line ${index + 1}: $error"
+                    lines.add(text, RulesetErrorSeverity.WarningOptionsOnly, sourceObject as? IRulesetObject, null)
+                }
+            }
+        }
+    }
+
+    fun uncachedImageExists(name: String): Boolean {
+        if (ruleset.folderLocation == null) return false // Can't check in this case
+        return textureNamesCache.imageExists(name)
+    }
 }

core/src/com/unciv/ui/images/AtlasPreview.kt

@@ -0,0 +1,36 @@
+package com.unciv.ui.images
+
+import com.badlogic.gdx.Gdx
+import com.badlogic.gdx.graphics.g2d.TextureAtlas.TextureAtlasData
+import com.unciv.json.json
+import com.unciv.models.ruleset.Ruleset
+import com.unciv.utils.Log
+
+class AtlasPreview(ruleset: Ruleset) : Iterable<String> {
+    // This partially duplicates code in ImageGetter.getAvailableTilesets, but we don't want to reload that singleton cache.
+
+    private val regionNames = mutableSetOf<String>()
+
+    init {
+        // For builtin rulesets, the Atlases.json is right in internal root
+        val folder = ruleset.folderLocation ?: Gdx.files.internal(".")
+        val controlFile = folder.child("Atlases.json")
+        val fileNames = (if (controlFile.exists()) json().fromJson(Array<String>::class.java, controlFile)
+            else emptyArray()).toMutableSet()
+        if (ruleset.name.isNotEmpty())
+            fileNames += "game"  // Backwards compatibility - when packed by 4.9.15+ this is already in the control file
+        for (fileName in fileNames) {
+            val file = folder.child("$fileName.atlas")
+            if (!file.exists()) continue
+
+            // Next, we need to cope with this running without GL context (unit test) - no TextureAtlas(file)
+            val data = TextureAtlasData(file, file.parent(), false)
+            data.regions.mapTo(regionNames) { it.name }
+        }
+        Log.debug("Atlas preview for $ruleset: ${regionNames.size} entries.")
+    }
+
+    fun imageExists(name: String) = name in regionNames
+
+    override fun iterator(): Iterator<String> = regionNames.iterator()
+}

core/src/com/unciv/ui/screens/civilopediascreen/FormattedLine.kt

@@ -17,6 +17,7 @@ import com.unciv.models.metadata.BaseRuleset
 import com.unciv.models.ruleset.Ruleset
 import com.unciv.models.ruleset.RulesetCache
 import com.unciv.models.ruleset.unique.Unique
+import com.unciv.models.ruleset.validation.RulesetValidator
 import com.unciv.ui.components.extensions.getReadonlyPixmap
 import com.unciv.ui.components.extensions.toLabel
 import com.unciv.ui.components.widgets.ColorMarkupLabel
@@ -114,26 +115,58 @@ class FormattedLine (
     }
 
     /** Retrieves the parsed [Color] corresponding to the [color] property (String)*/
-    val displayColor: Color by lazy { parseColor() }
+    val displayColor: Color by lazy { parseColor() ?: defaultColor }
 
     /** Returns true if this formatted line will not display anything */
     fun isEmpty(): Boolean = text.isEmpty() && extraImage.isEmpty() &&
             !starred && icon.isEmpty() && link.isEmpty() && !separator
 
-    /** Self-check to potentially support the mod checker
+    /** Self-check to support the RulesetValidator
-     * @return `null` if no problems found, or multiline String naming problems.
+     * @return 0 or more Strings naming problems - all occurrences get the same severity upstream
      */
-    @Suppress("unused")
+    fun unsupportedReasons(validator: RulesetValidator) = sequence {
-    fun unsupportedReason(): String? {
+        if (hasNormalContent() && separator)
-        val reasons = sequence {
+            yield("separator and other options are incompatible")
-            if (text.isNotEmpty() && separator) yield("separator and text are incompatible")
+        if (link.isNotEmpty() && !(isValidInternalLink(link) || link.hasProtocol()))
-            if (extraImage.isNotEmpty() && link.isNotEmpty()) yield("extraImage and other options except imageSize are incompatible")
+            yield("link is invalid - use internal category/name format or a https:// URL")
-            if (header != 0 && size != Int.MIN_VALUE) yield("use either size or header but not both")
+        if (icon.isNotEmpty() && !isValidInternalLink(link))
-            // ...
+            yield("icon is invalid - use internal category/name format")
-        }
+        if (header != 0 && size != Int.MIN_VALUE)
-        return reasons.joinToString { "\n" }.takeIf { it.isNotEmpty() }
+            yield("use either size or header but not both")
+        if (header !in headerSizes.indices)
+            yield("header should be in the range 1..${headerSizes.size - 1}") // Not mentioning 0 is valid too - same as omitting it
+        if (size != Int.MIN_VALUE && size !in 1..100)  // arbitrary
+            yield("size is out of sensible range")
+        if (indent !in 0..100)  // arbitrary
+            yield("indent is out of sensible range")
+        if (color.isNotEmpty())
+            if (parseColor() == null)
+                yield("unknown color \"$color\"")
+            else if (text.isEmpty() && textToDisplay.isEmpty() && !starred && !separator)
+                yield("color set but nothing to apply it to")
+        if (iconCrossed && link.isEmpty() && icon.isEmpty())
+            yield("iconCrossed set without icon or link")
+        if (extraImage.isNotEmpty()) checkExtraImage(validator)
+        if (!imageSize.isNaN() && extraImage.isEmpty())
+            yield("imageSize is only valid for an extraImage")
     }
 
+    private suspend fun SequenceScope<String>.checkExtraImage(validator: RulesetValidator) {
+        if (hasNormalContent() || separator)
+            // not checking centered or padding - these may be implementable
+            yield("extraImage and other options except imageSize are incompatible")
+        // check image exists - but for textures from atlases we can't rely on ImageGetter having cached the appropriate combo???
+        if (ImageGetter.imageExists(extraImage)) return
+        if (ImageGetter.findExternalImage(extraImage) != null) return
+        if (validator.uncachedImageExists(extraImage)) return
+        yield("extraImage not found as either atlas texture or in ExtraImages folder")
+    }
+
+    /** Not one of the exceptions - empty, separator or extraImage. For validation, independent of [isEmpty] which looks for **visible** content */
+    private fun hasNormalContent() =
+        text.isNotEmpty() || link.isNotEmpty() || icon.isNotEmpty() || color.isNotEmpty() || size != Int.MIN_VALUE || header != 0 || starred
+    private fun isValidInternalLink(link: String) = link.matches(Regex("""^[^/]+/[^/]+$"""))
+
     /** Constants used by [FormattedLine] */
     companion object {
         /** Array of text sizes to translate the [header] attribute */
@@ -221,14 +254,14 @@ class FormattedLine (
         }
 
     /** Parse a json-supplied color string to [Color], defaults to [defaultColor]. */
-    private fun parseColor(): Color {
+    private fun parseColor(): Color? {
-        if (color.isEmpty()) return defaultColor
+        if (color.isEmpty()) return null
         if (color[0] == '#' && color.isHex(1,3)) {
             if (color.isHex(1,6)) return Color.valueOf(color)
             val hex6 = String(charArrayOf(color[1], color[1], color[2], color[2], color[3], color[3]))
             return Color.valueOf(hex6)
         }
-        return Colors.get(color.uppercase()) ?: defaultColor
+        return Colors.get(color.uppercase())
     }
 
     /** Used only as parameter to [FormattedLine.render] and [MarkupRenderer.render] */