From 1bc1f33dfd6f7035520066f3929e1b432add8c60 Mon Sep 17 00:00:00 2001 From: SomeTroglodyte <63000004+SomeTroglodyte@users.noreply.github.com> Date: Tue, 23 Apr 2024 23:00:13 +0200 Subject: [PATCH] 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 | 3 + .../ruleset/validation/RulesetValidator.kt | 54 ++++++++++------ core/src/com/unciv/ui/images/AtlasPreview.kt | 36 +++++++++++ .../civilopediascreen/FormattedLine.kt | 63 ++++++++++++++----- 4 files changed, 121 insertions(+), 35 deletions(-) create mode 100644 core/src/com/unciv/ui/images/AtlasPreview.kt diff --git a/core/src/com/unciv/models/ruleset/Ruleset.kt b/core/src/com/unciv/models/ruleset/Ruleset.kt index a850187530..379945941a 100644 --- a/core/src/com/unciv/models/ruleset/Ruleset.kt +++ b/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 = allRulesetObjects() + sequenceOf(modOptions) + fun allICivilopediaText(): Sequence = + allRulesetObjects() + events.values + events.values.flatMap { it.choices } fun load(folderHandle: FileHandle) { // Note: Most files are loaded using createHashmap, which sets originRuleset automatically. diff --git a/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt b/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt index 35718972d1..3adbb20ec8 100644 --- a/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt +++ b/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) - /********************** **********************/ - // 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) checkTilesetSanity(lines) + // Tileset tests - e.g. json configs complete and parseable + 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 { - // This partially duplicates code in ImageGetter.getAvailableTilesets, but we don't want to reload that singleton cache. - - // Our builtin rulesets have no folderLocation, in that case cheat and apply knowledge about - // 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] } + private fun getTilesetNamesFromAtlases() = + textureNamesCache + .filter { it.startsWith("TileSets/") && !it.contains("/Units/") } + .map { it.split("/")[1] } .toSet() - } private fun checkPromotionCircularReferences(lines: RulesetErrorList) { fun recursiveCheck(history: HashSet, 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) + } } diff --git a/core/src/com/unciv/ui/images/AtlasPreview.kt b/core/src/com/unciv/ui/images/AtlasPreview.kt new file mode 100644 index 0000000000..c0f4d9a07c --- /dev/null +++ b/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 { + // This partially duplicates code in ImageGetter.getAvailableTilesets, but we don't want to reload that singleton cache. + + private val regionNames = mutableSetOf() + + 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::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 = regionNames.iterator() +} diff --git a/core/src/com/unciv/ui/screens/civilopediascreen/FormattedLine.kt b/core/src/com/unciv/ui/screens/civilopediascreen/FormattedLine.kt index 586edff88e..bb4a1b6a90 100644 --- a/core/src/com/unciv/ui/screens/civilopediascreen/FormattedLine.kt +++ b/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 - * @return `null` if no problems found, or multiline String naming problems. + /** Self-check to support the RulesetValidator + * @return 0 or more Strings naming problems - all occurrences get the same severity upstream */ - @Suppress("unused") - fun unsupportedReason(): String? { - val reasons = sequence { - if (text.isNotEmpty() && separator) yield("separator and text are incompatible") - if (extraImage.isNotEmpty() && link.isNotEmpty()) yield("extraImage and other options except imageSize are incompatible") - if (header != 0 && size != Int.MIN_VALUE) yield("use either size or header but not both") - // ... - } - return reasons.joinToString { "\n" }.takeIf { it.isNotEmpty() } + fun unsupportedReasons(validator: RulesetValidator) = sequence { + if (hasNormalContent() && separator) + yield("separator and other options are incompatible") + if (link.isNotEmpty() && !(isValidInternalLink(link) || link.hasProtocol())) + yield("link is invalid - use internal category/name format or a https:// URL") + if (icon.isNotEmpty() && !isValidInternalLink(link)) + yield("icon is invalid - use internal category/name format") + if (header != 0 && size != Int.MIN_VALUE) + 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.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 { - if (color.isEmpty()) return defaultColor + private fun parseColor(): Color? { + 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] */