From 8576f16c6ea77a10d740f4d77160cb376e2cdc9a Mon Sep 17 00:00:00 2001 From: SomeTroglodyte <63000004+SomeTroglodyte@users.noreply.github.com> Date: Tue, 8 Aug 2023 23:47:14 +0200 Subject: [PATCH] Ruleset validator: Tilesets (#9881) * Defensive Tileset loading * Deleting a mod should do the same cleanup as downloading a new one * Extensive RulesetValidator checks on Tileset mod integrity --- core/src/com/unciv/Constants.kt | 2 + .../ruleset/validation/RulesetValidator.kt | 156 ++++++++++++------ .../com/unciv/models/tilesets/TileSetCache.kt | 22 ++- .../unciv/models/tilesets/TileSetConfig.kt | 6 +- .../screens/modmanager/ModManagementScreen.kt | 3 +- 5 files changed, 128 insertions(+), 61 deletions(-) diff --git a/core/src/com/unciv/Constants.kt b/core/src/com/unciv/Constants.kt index bbe5e2b2f1..1cf560801f 100644 --- a/core/src/com/unciv/Constants.kt +++ b/core/src/com/unciv/Constants.kt @@ -90,6 +90,8 @@ object Constants { const val uncivXyzServer = "https://uncivserver.xyz" const val defaultTileset = "HexaRealm" + /** Default for TileSetConfig.fallbackTileSet - Don't change unless you've also moved the crosshatch, borders, and arrows as well */ + const val defaultFallbackTileset = "FantasyHex" const val defaultUnitset = "AbsoluteUnits" const val defaultSkin = "Minimal" diff --git a/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt b/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt index b8c7eb16b9..40f98c2c2c 100644 --- a/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt +++ b/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt @@ -1,8 +1,13 @@ 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 import com.unciv.logic.map.tile.RoadStatus +import com.unciv.models.metadata.BaseRuleset import com.unciv.models.ruleset.IRulesetObject import com.unciv.models.ruleset.Ruleset import com.unciv.models.ruleset.RulesetCache @@ -17,6 +22,8 @@ import com.unciv.models.ruleset.unique.UniqueType 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 class RulesetValidator(val ruleset: Ruleset) { @@ -67,23 +74,20 @@ class RulesetValidator(val ruleset: Ruleset) { for (techColumn in ruleset.techColumns){ if (techColumn.columnNumber < 0) - lines+= "Tech Column number ${techColumn.columnNumber} is negative" + lines += "Tech Column number ${techColumn.columnNumber} is negative" if (techColumn.buildingCost == -1) lines.add("Tech Column number ${techColumn.columnNumber} has no explicit building cost", - RulesetErrorSeverity.Warning - ) + RulesetErrorSeverity.Warning) if (techColumn.wonderCost == -1) lines.add("Tech Column number ${techColumn.columnNumber} has no explicit wonder cost", - RulesetErrorSeverity.Warning - ) + RulesetErrorSeverity.Warning) } for (building in ruleset.buildings.values) { if (building.requiredTech == null && building.cost == -1 && !building.hasUnique( UniqueType.Unbuildable)) lines.add("${building.name} is buildable and therefore should either have an explicit cost or reference an existing tech!", - RulesetErrorSeverity.Warning - ) + RulesetErrorSeverity.Warning) checkUniques(building, lines, rulesetInvariant, tryFixUnknownUniques) @@ -148,6 +152,13 @@ class RulesetValidator(val ruleset: Ruleset) { checkUniques(resource, lines, rulesetInvariant, tryFixUnknownUniques) } + /********************** 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) + } + // Quit here when no base ruleset is loaded - references cannot be checked if (!ruleset.modOptions.isBaseRuleset) return lines @@ -199,8 +210,7 @@ class RulesetValidator(val ruleset: Ruleset) { unit.isCivilian() && !unit.isGreatPersonOfType("War")) { lines.add("${unit.name} can place improvement $improvementName which has no stats, preventing unit automation!", - RulesetErrorSeverity.Warning - ) + RulesetErrorSeverity.Warning) } } @@ -295,8 +305,7 @@ class RulesetValidator(val ruleset: Ruleset) { if (tech.prerequisites.asSequence().filterNot { it == prereq } .any { getPrereqTree(it).contains(prereq) }){ lines.add("No need to add $prereq as a prerequisite of ${tech.name} - it is already implicit from the other prerequisites!", - RulesetErrorSeverity.Warning - ) + RulesetErrorSeverity.Warning) } if (getPrereqTree(prereq).contains(tech.name)) @@ -343,13 +352,10 @@ class RulesetValidator(val ruleset: Ruleset) { if (era.allyBonus.isNotEmpty()) lines.add("Era ${era.name} contains city-state bonuses. City-state bonuses are now defined in CityStateType.json", - RulesetErrorSeverity.WarningOptionsOnly - ) + RulesetErrorSeverity.WarningOptionsOnly) if (era.friendBonus.isNotEmpty()) lines.add("Era ${era.name} contains city-state bonuses. City-state bonuses are now defined in CityStateType.json", - RulesetErrorSeverity.WarningOptionsOnly - ) - + RulesetErrorSeverity.WarningOptionsOnly) checkUniques(era, lines, rulesetSpecific, tryFixUnknownUniques) } @@ -403,13 +409,11 @@ class RulesetValidator(val ruleset: Ruleset) { for (prereq in promotion.prerequisites) if (!ruleset.unitPromotions.containsKey(prereq)) lines.add("${promotion.name} requires promotion $prereq which does not exist!", - RulesetErrorSeverity.Warning - ) + RulesetErrorSeverity.Warning) for (unitType in promotion.unitTypes) if (!ruleset.unitTypes.containsKey(unitType) && (ruleset.unitTypes.isNotEmpty() || !vanillaRuleset.unitTypes.containsKey(unitType))) lines.add("${promotion.name} references unit type $unitType, which does not exist!", - RulesetErrorSeverity.Warning - ) + RulesetErrorSeverity.Warning) checkUniques(promotion, lines, rulesetSpecific, tryFixUnknownUniques) checkPromotionCircularReferences(lines) } @@ -422,18 +426,15 @@ class RulesetValidator(val ruleset: Ruleset) { for (requiredUnit in victoryType.requiredSpaceshipParts) if (!ruleset.units.contains(requiredUnit)) lines.add("Victory type ${victoryType.name} requires adding the non-existant unit $requiredUnit to the capital to win!", - RulesetErrorSeverity.Warning - ) + RulesetErrorSeverity.Warning) for (milestone in victoryType.milestoneObjects) if (milestone.type == null) lines.add("Victory type ${victoryType.name} has milestone ${milestone.uniqueDescription} that is of an unknown type!", - RulesetErrorSeverity.Error - ) + RulesetErrorSeverity.Error) for (victory in ruleset.victories.values) if (victory.name != victoryType.name && victory.milestones == victoryType.milestones) lines.add("Victory types ${victoryType.name} and ${victory.name} have the same requirements!", - RulesetErrorSeverity.Warning - ) + RulesetErrorSeverity.Warning) } for (difficulty in ruleset.difficulties.values) { @@ -457,12 +458,73 @@ class RulesetValidator(val ruleset: Ruleset) { return lines } + private fun checkTilesetSanity(lines: RulesetErrorList) { + val tilesetConfigFolder = (ruleset.folderLocation ?: Gdx.files.internal("")).child("jsons\\TileSets") + if (!tilesetConfigFolder.exists()) return + + val configTilesets = mutableSetOf() + val allFallbacks = mutableSetOf() + val folderContent = tilesetConfigFolder.list() + var folderContentBad = false + + for (file in folderContent) { + if (file.isDirectory || file.extension() != "json") { folderContentBad = true; continue } + // All json files should be parseable + try { + val config = json().fromJsonFile(TileSetConfig::class.java, file) + configTilesets += file.nameWithoutExtension().removeSuffix("Config") + if (config.fallbackTileSet?.isNotEmpty() == true) + allFallbacks.add(config.fallbackTileSet!!) + } catch (ex: Exception) { + // Our fromJsonFile wrapper already intercepts Exceptions and gives them a generalized message, so go a level deeper for useful details (like "unmatched brace") + lines.add("Tileset config '${file.name()}' cannot be loaded (${ex.cause?.message})", RulesetErrorSeverity.Warning) + } + } + + // Folder should not contain subdirectories, non-json files, or be empty + if (folderContentBad) + lines.add("The Mod tileset config folder contains non-json files or subdirectories", RulesetErrorSeverity.Warning) + if (configTilesets.isEmpty()) + lines.add("The Mod tileset config folder contains no json files", RulesetErrorSeverity.Warning) + + // There should be atlas images corresponding to each json name + val atlasTilesets = getTilesetNamesFromAtlases() + val configOnlyTilesets = configTilesets - atlasTilesets + if (configOnlyTilesets.isNotEmpty()) + lines.add("Mod has no graphics for configured tilesets: ${configOnlyTilesets.joinToString()}", RulesetErrorSeverity.Warning) + + // For all atlas images matching "TileSets/*" there should be a json + val atlasOnlyTilesets = atlasTilesets - configTilesets + if (atlasOnlyTilesets.isNotEmpty()) + lines.add("Mod has no configuration for tileset graphics: ${atlasOnlyTilesets.joinToString()}", RulesetErrorSeverity.Warning) + + // All fallbacks should exist (default added because TileSetCache is not loaded when running as unit test) + val unknownFallbacks = allFallbacks - TileSetCache.keys - Constants.defaultFallbackTileset + if (unknownFallbacks.isNotEmpty()) + lines.add("Fallback tileset invalid: ${unknownFallbacks.joinToString()}", RulesetErrorSeverity.Warning) + } + + 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] } + .toSet() + } + private fun checkPromotionCircularReferences(lines: RulesetErrorList) { fun recursiveCheck(history: LinkedHashSet, promotion: Promotion, level: Int) { if (promotion in history) { lines.add("Circular Reference in Promotions: ${history.joinToString("→") { it.name }}→${promotion.name}", - RulesetErrorSeverity.Warning - ) + RulesetErrorSeverity.Warning) return } if (level > 99) return @@ -513,13 +575,11 @@ class RulesetValidator(val ruleset: Ruleset) { val typeComplianceErrors = unique.type.getComplianceErrors(unique, ruleset) for (complianceError in typeComplianceErrors) { if (complianceError.errorSeverity <= severityToReport) - rulesetErrors.add( - RulesetError("$prefix unique \"${unique.text}\" contains parameter ${complianceError.parameterName}," + + rulesetErrors.add(RulesetError("$prefix unique \"${unique.text}\" contains parameter ${complianceError.parameterName}," + " which does not fit parameter type" + " ${complianceError.acceptableParameterTypes.joinToString(" or ") { it.parameterName }} !", complianceError.errorSeverity.getRulesetErrorSeverity(severityToReport) - ) - ) + )) } for (conditional in unique.conditionals) { @@ -533,20 +593,17 @@ class RulesetValidator(val ruleset: Ruleset) { if (conditional.type.targetTypes.none { it.modifierType != UniqueTarget.ModifierType.None }) rulesetErrors.add("$prefix unique \"${unique.text}\" contains the conditional \"${conditional.text}\"," + " which is a Unique type not allowed as conditional or trigger.", - RulesetErrorSeverity.Warning - ) + RulesetErrorSeverity.Warning) val conditionalComplianceErrors = conditional.type.getComplianceErrors(conditional, ruleset) for (complianceError in conditionalComplianceErrors) { if (complianceError.errorSeverity == severityToReport) - rulesetErrors.add( - RulesetError( "$prefix unique \"${unique.text}\" contains the conditional \"${conditional.text}\"." + + rulesetErrors.add(RulesetError( "$prefix 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.errorSeverity.getRulesetErrorSeverity(severityToReport) - ) - ) + )) } } } @@ -577,12 +634,9 @@ class RulesetValidator(val ruleset: Ruleset) { private fun checkUntypedUnique(unique: Unique, tryFixUnknownUniques: Boolean, prefix: String ): List { // Malformed conditional is always bad if (unique.text.count { it == '<' } != unique.text.count { it == '>' }) - return listOf( - RulesetError( + return listOf(RulesetError( "$prefix unique \"${unique.text}\" contains mismatched conditional braces!", - RulesetErrorSeverity.Warning - ) - ) + RulesetErrorSeverity.Warning)) // Support purely filtering Uniques without actual implementation if (isFilteringUniqueAllowed(unique)) return emptyList() @@ -593,12 +647,9 @@ class RulesetValidator(val ruleset: Ruleset) { if (RulesetCache.modCheckerAllowUntypedUniques) return emptyList() - return listOf( - RulesetError( - "$prefix unique \"${unique.text}\" not found in Unciv's unique types.", - RulesetErrorSeverity.WarningOptionsOnly - ) - ) + return listOf(RulesetError( + "$prefix unique \"${unique.text}\" not found in Unciv's unique types.", + RulesetErrorSeverity.WarningOptionsOnly)) } private fun isFilteringUniqueAllowed(unique: Unique): Boolean { @@ -619,12 +670,9 @@ class RulesetValidator(val ruleset: Ruleset) { similarUniques.filter { it.placeholderText == unique.placeholderText } return when { // This should only ever happen if a bug is or has been introduced that prevents Unique.type from being set for a valid UniqueType, I think.\ - equalUniques.isNotEmpty() -> listOf( - RulesetError( + equalUniques.isNotEmpty() -> listOf(RulesetError( "$prefix unique \"${unique.text}\" looks like it should be fine, but for some reason isn't recognized.", - RulesetErrorSeverity.OK - ) - ) + RulesetErrorSeverity.OK)) similarUniques.isNotEmpty() -> { val text = diff --git a/core/src/com/unciv/models/tilesets/TileSetCache.kt b/core/src/com/unciv/models/tilesets/TileSetCache.kt index 238a0f42c6..8554670bd7 100644 --- a/core/src/com/unciv/models/tilesets/TileSetCache.kt +++ b/core/src/com/unciv/models/tilesets/TileSetCache.kt @@ -38,7 +38,13 @@ object TileSetCache : HashMap() { } } - fun loadTileSetConfigs(consoleMode: Boolean = false){ + /** Load the json config files and do an initial [assembleTileSetConfigs] run without explicit mods. + * + * Runs from UncivGame.create without exception handling, therefore should not be vulnerable to broken mods. + * (The critical point is json parsing in loadConfigFiles, which is now hardened) + * Also runs from ModManagementScreen after downloading a new mod or deleting one. + */ + fun loadTileSetConfigs(consoleMode: Boolean = false) { clear() @@ -48,7 +54,7 @@ object TileSetCache : HashMap() { FileHandle("jsons/TileSets").list().asSequence() else ImageGetter.getAvailableTilesets() - .map { Gdx.files.internal("jsons/TileSets/$it.json")} + .map { Gdx.files.internal("jsons/TileSets/$it.json") } .filter { it.exists() } loadConfigFiles(internalFiles, TileSet.DEFAULT) @@ -73,8 +79,18 @@ object TileSetCache : HashMap() { private fun loadConfigFiles(files: Sequence, configId: String) { for (configFile in files) { + // jsons/TileSets shouldn't have subfolders, but if a mad modder has one, don't crash (file.readString would throw): + if (configFile.isDirectory) continue + // `files` is an unfiltered directory listing - ignore chaff + if (configFile.extension() != "json") continue + + val config = try { + json().fromJsonFile(TileSetConfig::class.java, configFile) + } catch (_: Exception) { + continue // Ignore jsons that don't load + } + val name = configFile.nameWithoutExtension().removeSuffix("Config") - val config = json().fromJsonFile(TileSetConfig::class.java, configFile) val tileset = get(name) ?: TileSet(name) tileset.cacheConfigFromMod(configId, config) set(name, tileset) diff --git a/core/src/com/unciv/models/tilesets/TileSetConfig.kt b/core/src/com/unciv/models/tilesets/TileSetConfig.kt index 8c3061deb8..fdab0a8d4a 100644 --- a/core/src/com/unciv/models/tilesets/TileSetConfig.kt +++ b/core/src/com/unciv/models/tilesets/TileSetConfig.kt @@ -1,15 +1,15 @@ package com.unciv.models.tilesets import com.badlogic.gdx.graphics.Color +import com.unciv.Constants class TileSetConfig { var useColorAsBaseTerrain = false var useSummaryImages = false var unexploredTileColor: Color = Color.DARK_GRAY var fogOfWarColor: Color = Color.BLACK - /** Name of the tileset to use when this one is missing images. Null to disable. - * Don't change unless you've also moved the crosshatch, borders, and arrows as well */ - var fallbackTileSet: String? = "FantasyHex" + /** Name of the tileset to use when this one is missing images. Null to disable. */ + var fallbackTileSet: String? = Constants.defaultFallbackTileset /** Scale factor for hex images, with hex center as origin. */ var tileScale: Float = 1f var tileScales: HashMap = HashMap() diff --git a/core/src/com/unciv/ui/screens/modmanager/ModManagementScreen.kt b/core/src/com/unciv/ui/screens/modmanager/ModManagementScreen.kt index b32b86f949..b6b4b6c0e7 100644 --- a/core/src/com/unciv/ui/screens/modmanager/ModManagementScreen.kt +++ b/core/src/com/unciv/ui/screens/modmanager/ModManagementScreen.kt @@ -576,7 +576,7 @@ class ModManagementScreen( val repoName = modFolder.name() // repo.name still has the replaced "-"'s ToastPopup("[$repoName] Downloaded!", this@ModManagementScreen) RulesetCache.loadRulesets() - TileSetCache.loadTileSetConfigs(false) + TileSetCache.loadTileSetConfigs() UncivGame.Current.translations.tryReadTranslationForCurrentLanguage() RulesetCache[repoName]?.let { installedModInfo[repoName] = ModUIData(it) @@ -720,6 +720,7 @@ class ModManagementScreen( private fun deleteMod(mod: Ruleset) { mod.folderLocation!!.deleteDirectory() RulesetCache.loadRulesets() + TileSetCache.loadTileSetConfigs() installedModInfo.remove(mod.name) refreshInstalledModTable() }