diff --git a/core/src/com/unciv/models/ruleset/Ruleset.kt b/core/src/com/unciv/models/ruleset/Ruleset.kt index 3bec02430e..8ecd358131 100644 --- a/core/src/com/unciv/models/ruleset/Ruleset.kt +++ b/core/src/com/unciv/models/ruleset/Ruleset.kt @@ -148,6 +148,9 @@ class Ruleset { fun clone(): Ruleset { val newRuleset = Ruleset() newRuleset.add(this) + // Make sure the clone is recognizable - e.g. startNewGame fallback when a base mod was removed needs this + newRuleset.name = name + newRuleset.modOptions.isBaseRuleset = modOptions.isBaseRuleset return newRuleset } @@ -439,25 +442,26 @@ class Ruleset { // Add objects that might not be present in base ruleset mods, but are required if (modOptions.isBaseRuleset) { + val fallbackRuleset by lazy { RulesetCache.getVanillaRuleset() } // clone at most once // This one should be temporary if (unitTypes.isEmpty()) { - unitTypes.putAll(RulesetCache.getVanillaRuleset().unitTypes) + unitTypes.putAll(fallbackRuleset.unitTypes) } // These should be permanent if (ruinRewards.isEmpty()) - ruinRewards.putAll(RulesetCache.getVanillaRuleset().ruinRewards) + ruinRewards.putAll(fallbackRuleset.ruinRewards) if (globalUniques.uniques.isEmpty()) { - globalUniques = RulesetCache.getVanillaRuleset().globalUniques + globalUniques = fallbackRuleset.globalUniques } // If we have no victories, add all the default victories - if (victories.isEmpty()) victories.putAll(RulesetCache.getVanillaRuleset().victories) + if (victories.isEmpty()) victories.putAll(fallbackRuleset.victories) - if (speeds.isEmpty()) speeds.putAll(RulesetCache.getVanillaRuleset().speeds) + if (speeds.isEmpty()) speeds.putAll(fallbackRuleset.speeds) if (cityStateTypes.isEmpty()) - for (cityStateType in RulesetCache.getVanillaRuleset().cityStateTypes.values) + for (cityStateType in fallbackRuleset.cityStateTypes.values) cityStateTypes[cityStateType.name] = CityStateType().apply { name = cityStateType.name color = cityStateType.color diff --git a/core/src/com/unciv/models/ruleset/Speed.kt b/core/src/com/unciv/models/ruleset/Speed.kt index 0fe282831c..7e779b77ee 100644 --- a/core/src/com/unciv/models/ruleset/Speed.kt +++ b/core/src/com/unciv/models/ruleset/Speed.kt @@ -29,6 +29,7 @@ class Speed : RulesetObject(), IsPartOfGameInfoSerialization { var startYear: Float = -4000f var turns: ArrayList> = ArrayList() + data class YearsPerTurn(val yearInterval: Float, val untilTurn: Int) val yearsPerTurn: ArrayList by lazy { ArrayList().apply { turns.forEach { this.add(YearsPerTurn(it["yearsPerTurn"]!!, it["untilTurn"]!!.toInt())) } @@ -83,8 +84,3 @@ class Speed : RulesetObject(), IsPartOfGameInfoSerialization { fun numTotalTurns(): Int = yearsPerTurn.last().untilTurn } - -class YearsPerTurn(yearsPerTurn: Float, turnsPerIncrement: Int) { - var yearInterval: Float = yearsPerTurn - var untilTurn: Int = turnsPerIncrement -} diff --git a/core/src/com/unciv/models/ruleset/validation/BaseRulesetValidator.kt b/core/src/com/unciv/models/ruleset/validation/BaseRulesetValidator.kt index 46791aebc4..2d511edcf2 100644 --- a/core/src/com/unciv/models/ruleset/validation/BaseRulesetValidator.kt +++ b/core/src/com/unciv/models/ruleset/validation/BaseRulesetValidator.kt @@ -1,19 +1,18 @@ package com.unciv.models.ruleset.validation import com.unciv.Constants -import com.unciv.logic.map.tile.RoadStatus -import com.unciv.models.ruleset.BeliefType +import com.unciv.models.ruleset.Building import com.unciv.models.ruleset.MilestoneType import com.unciv.models.ruleset.Policy import com.unciv.models.ruleset.Ruleset import com.unciv.models.ruleset.RulesetCache +import com.unciv.models.ruleset.nation.Nation import com.unciv.models.ruleset.tile.TerrainType import com.unciv.models.ruleset.unique.IHasUniques 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.ruleset.unit.UnitMovementType import com.unciv.models.stats.Stats /** @@ -32,55 +31,37 @@ internal class BaseRulesetValidator( * value a Set of its prerequisites including indirect ones */ private val prereqsHashMap = HashMap>() - override fun addBeliefErrors(lines: RulesetErrorList) { - super.addBeliefErrors(lines) - - for (belief in ruleset.beliefs.values) { - if (belief.type == BeliefType.Any || belief.type == BeliefType.None) - lines.add("${belief.name} type is ${belief.type}, which is not allowed!", sourceObject = belief) - uniqueValidator.checkUniques(belief, lines, true, tryFixUnknownUniques) - } + init { + // The `UniqueValidator.checkUntypedUnique` filtering Unique test ("X not found in Unciv's unique types, and is not used as a filtering unique") + // should not complain when running the RulesetInvariant version, because an Extension Mod may e.g. define additional "Aircraft" and the _use_ of the + // filtering Unique only exists in the Base Ruleset. But here we *do* want the test, and it needs its cache filled, and that is not done automatically. + uniqueValidator.populateFilteringUniqueHashsets() } - override fun addBuildingErrors(lines: RulesetErrorList) { - // No super.addBuildingErrors(lines): included in the loop below + override fun checkBuilding(building: Building, lines: RulesetErrorList) { + super.checkBuilding(building, lines) - for (building in ruleset.buildings.values) { - addBuildingErrorRulesetInvariant(building, lines) - - for (requiredTech: String in building.requiredTechs()) - if (!ruleset.technologies.containsKey(requiredTech)) - lines.add("${building.name} requires tech $requiredTech which does not exist!", sourceObject = building) - for (specialistName in building.specialistSlots.keys) - if (!ruleset.specialists.containsKey(specialistName)) - lines.add("${building.name} provides specialist $specialistName which does not exist!", sourceObject = building) - for (resource in building.getResourceRequirementsPerTurn(StateForConditionals.IgnoreConditionals).keys) - if (!ruleset.tileResources.containsKey(resource)) - lines.add("${building.name} requires resource $resource which does not exist!", sourceObject = building) - if (building.replaces != null && !ruleset.buildings.containsKey(building.replaces!!)) - lines.add("${building.name} replaces ${building.replaces} which does not exist!", sourceObject = building) - if (building.requiredBuilding != null && !ruleset.buildings.containsKey(building.requiredBuilding!!)) - lines.add("${building.name} requires ${building.requiredBuilding} which does not exist!", sourceObject = building) - - checkUniqueToMisspelling(building, building.uniqueTo, lines) - uniqueValidator.checkUniques(building, lines, true, tryFixUnknownUniques) - } - } - - override fun addCityStateTypeErrors(lines: RulesetErrorList) { - super.addCityStateTypeErrors(lines) - - for (cityStateType in ruleset.cityStateTypes.values) { - for (unique in cityStateType.allyBonusUniqueMap.getAllUniques() + cityStateType.friendBonusUniqueMap.getAllUniques()) { - val errors = uniqueValidator.checkUnique( - unique, - tryFixUnknownUniques, - null, - true + for ((gppName, _) in building.greatPersonPoints) + if (!ruleset.units.containsKey(gppName)) + lines.add( + "Building ${building.name} has greatPersonPoints for $gppName, which is not a unit in the ruleset!", + RulesetErrorSeverity.Warning, building ) - lines.addAll(errors) - } - } + for (requiredTech: String in building.requiredTechs()) + if (!ruleset.technologies.containsKey(requiredTech)) + lines.add("${building.name} requires tech $requiredTech which does not exist!", sourceObject = building) + for (specialistName in building.specialistSlots.keys) + if (!ruleset.specialists.containsKey(specialistName)) + lines.add("${building.name} provides specialist $specialistName which does not exist!", sourceObject = building) + for (resource in building.getResourceRequirementsPerTurn(StateForConditionals.IgnoreConditionals).keys) + if (!ruleset.tileResources.containsKey(resource)) + lines.add("${building.name} requires resource $resource which does not exist!", sourceObject = building) + if (building.replaces != null && !ruleset.buildings.containsKey(building.replaces!!)) + lines.add("${building.name} replaces ${building.replaces} which does not exist!", sourceObject = building) + if (building.requiredBuilding != null && !ruleset.buildings.containsKey(building.requiredBuilding!!)) + lines.add("${building.name} requires ${building.requiredBuilding} which does not exist!", sourceObject = building) + + checkUniqueToMisspelling(building, building.uniqueTo, lines) } override fun addDifficultyErrors(lines: RulesetErrorList) { @@ -91,6 +72,12 @@ internal class BaseRulesetValidator( for (unitName in difficulty.aiCityStateBonusStartingUnits + difficulty.aiMajorCivBonusStartingUnits + difficulty.playerBonusStartingUnits) if (unitName != Constants.eraSpecificUnit && !ruleset.units.containsKey(unitName)) lines.add("Difficulty ${difficulty.name} contains starting unit $unitName which does not exist!", sourceObject = null) + if (difficulty.aiDifficultyLevel != null && !ruleset.difficulties.containsKey(difficulty.aiDifficultyLevel)) + lines.add("Difficulty ${difficulty.name} contains aiDifficultyLevel ${difficulty.aiDifficultyLevel} which does not exist!", + RulesetErrorSeverity.Warning, sourceObject = null) + for (tech in difficulty.aiFreeTechs) + if (!ruleset.technologies.containsKey(tech)) + lines.add("Difficulty ${difficulty.name} contains AI free tech $tech which does not exist!", sourceObject = null) } } @@ -116,9 +103,7 @@ internal class BaseRulesetValidator( if (building !in ruleset.buildings) lines.add("Nonexistent building $building built by settlers when starting in ${era.name}", sourceObject = era) // todo the whole 'starting unit' thing needs to be redone, there's no reason we can't have a single list containing all the starting units. - if (era.startingSettlerUnit !in ruleset.units - && ruleset.units.values.none { it.isCityFounder() } - ) + if (era.startingSettlerUnit !in ruleset.units && ruleset.units.values.none { it.isCityFounder() }) lines.add("Nonexistent unit ${era.startingSettlerUnit} marked as starting unit when starting in ${era.name}", sourceObject = era) if (era.startingWorkerCount != 0 && era.startingWorkerUnit !in ruleset.units && ruleset.units.values.none { it.hasUnique(UniqueType.BuildImprovements) } @@ -129,35 +114,6 @@ internal class BaseRulesetValidator( || allDifficultiesStartingUnits.contains(Constants.eraSpecificUnit) if (grantsStartingMilitaryUnit && era.startingMilitaryUnit !in ruleset.units) lines.add("Nonexistent unit ${era.startingMilitaryUnit} marked as starting unit when starting in ${era.name}", sourceObject = era) - if (era.researchAgreementCost < 0 || era.startingSettlerCount < 0 || era.startingWorkerCount < 0 || era.startingMilitaryUnitCount < 0 || era.startingGold < 0 || era.startingCulture < 0) - lines.add("Unexpected negative number found while parsing era ${era.name}", sourceObject = era) - if (era.settlerPopulation <= 0) - lines.add("Population in cities from settlers must be strictly positive! Found value ${era.settlerPopulation} for era ${era.name}", sourceObject = era) - - if (era.allyBonus.isNotEmpty()) - lines.add( - "Era ${era.name} contains city-state bonuses. City-state bonuses are now defined in CityStateType.json", - RulesetErrorSeverity.WarningOptionsOnly, era - ) - if (era.friendBonus.isNotEmpty()) - lines.add( - "Era ${era.name} contains city-state bonuses. City-state bonuses are now defined in CityStateType.json", - RulesetErrorSeverity.WarningOptionsOnly, era - ) - - uniqueValidator.checkUniques(era, lines, true, tryFixUnknownUniques) - } - } - - override fun addEventErrors(lines: RulesetErrorList) { - super.addEventErrors(lines) - - // An Event is not a IHasUniques, so not suitable as sourceObject - for (event in ruleset.events.values) { - for (choice in event.choices) { - uniqueValidator.checkUniques(choice, lines, true, tryFixUnknownUniques) - } - uniqueValidator.checkUniques(event, lines, true, tryFixUnknownUniques) } } @@ -169,68 +125,34 @@ internal class BaseRulesetValidator( lines.add("${improvement.name} requires tech ${improvement.techRequired} which does not exist!", sourceObject = improvement) if (improvement.replaces != null && !ruleset.tileImprovements.containsKey(improvement.replaces)) lines.add("${improvement.name} replaces ${improvement.replaces} which does not exist!", sourceObject = improvement) - if (improvement.replaces != null && improvement.uniqueTo == null) - lines.add("${improvement.name} should replace ${improvement.replaces} but does not have uniqueTo assigned!") + checkUniqueToMisspelling(improvement, improvement.uniqueTo, lines) + for (terrain in improvement.terrainsCanBeBuiltOn) if (!ruleset.terrains.containsKey(terrain) && terrain != "Land" && terrain != "Water") lines.add("${improvement.name} can be built on terrain $terrain which does not exist!", sourceObject = improvement) - if (improvement.terrainsCanBeBuiltOn.isEmpty() - && !improvement.hasUnique(UniqueType.CanOnlyImproveResource) - && !improvement.hasUnique(UniqueType.Unbuildable) - && !improvement.name.startsWith(Constants.remove) - && improvement.name !in RoadStatus.entries.map { it.removeAction } - && improvement.name != Constants.cancelImprovementOrder - ) { - lines.add( - "${improvement.name} has an empty `terrainsCanBeBuiltOn`, isn't allowed to only improve resources. As such it isn't buildable! Either give this the unique \"Unbuildable\", \"Can only be built to improve a resource\", or add \"Land\", \"Water\" or any other value to `terrainsCanBeBuiltOn`.", - RulesetErrorSeverity.Warning, improvement - ) - } - for (unique in improvement.uniqueObjects - .filter { it.type == UniqueType.PillageYieldRandom || it.type == UniqueType.PillageYieldFixed }) { - if (!Stats.isStats(unique.params[0])) continue - val params = Stats.parse(unique.params[0]) - if (params.values.any { it < 0 }) lines.add( - "${improvement.name} cannot have a negative value for a pillage yield!", - RulesetErrorSeverity.Error, improvement - ) - } - - val hasPillageUnique = improvement.hasUnique(UniqueType.PillageYieldRandom, StateForConditionals.IgnoreConditionals) - || improvement.hasUnique(UniqueType.PillageYieldFixed, StateForConditionals.IgnoreConditionals) - if (hasPillageUnique && improvement.hasUnique(UniqueType.Unpillagable, StateForConditionals.IgnoreConditionals)) { - lines.add( - "${improvement.name} has both an `Unpillagable` unique type and a `PillageYieldRandom` or `PillageYieldFixed` unique type!", - RulesetErrorSeverity.Warning, improvement - ) - } - uniqueValidator.checkUniques(improvement, lines, true, tryFixUnknownUniques) } } override fun addModOptionsErrors(lines: RulesetErrorList) { super.addModOptionsErrors(lines) + // `ruleset` can be a true base ruleset or a combined one when we're checking an extension mod together with a base. + // In the combined case, don't complain about ModRequires! + if (ruleset.name.isEmpty() && ruleset.mods.size > 1) return + for (unique in ruleset.modOptions.getMatchingUniques(UniqueType.ModRequires)) { lines.add("Mod option '${unique.text}' is invalid for a base ruleset.", sourceObject = null) } } - override fun addNationErrors(lines: RulesetErrorList) { - // No super.addNationErrors(lines), included in loop below - for (nation in ruleset.nations.values) { - addNationErrorRulesetInvariant(nation, lines) - - uniqueValidator.checkUniques(nation, lines, true, tryFixUnknownUniques) - - if (nation.preferredVictoryType != Constants.neutralVictoryType && nation.preferredVictoryType !in ruleset.victories) - lines.add("${nation.name}'s preferredVictoryType is ${nation.preferredVictoryType} which does not exist!", sourceObject = nation) - if (nation.cityStateType != null && nation.cityStateType !in ruleset.cityStateTypes) - lines.add("${nation.name} is of city-state type ${nation.cityStateType} which does not exist!", sourceObject = nation) - if (nation.favoredReligion != null && nation.favoredReligion !in ruleset.religions) - lines.add("${nation.name} has ${nation.favoredReligion} as their favored religion, which does not exist!", sourceObject = nation) - } + override fun checkNation(nation: Nation, lines: RulesetErrorList) { + if (nation.preferredVictoryType != Constants.neutralVictoryType && nation.preferredVictoryType !in ruleset.victories) + lines.add("${nation.name}'s preferredVictoryType is ${nation.preferredVictoryType} which does not exist!", sourceObject = nation) + if (nation.cityStateType != null && nation.cityStateType !in ruleset.cityStateTypes) + lines.add("${nation.name} is of city-state type ${nation.cityStateType} which does not exist!", sourceObject = nation) + if (nation.favoredReligion != null && nation.favoredReligion !in ruleset.religions) + lines.add("${nation.name} has ${nation.favoredReligion} as their favored religion, which does not exist!", sourceObject = nation) } override fun addPersonalityErrors(lines: RulesetErrorList) { @@ -240,7 +162,7 @@ internal class BaseRulesetValidator( if (personality.preferredVictoryType != Constants.neutralVictoryType && personality.preferredVictoryType !in ruleset.victories) { lines.add("Preferred victory type ${personality.preferredVictoryType} does not exist in ruleset", - RulesetErrorSeverity.Warning, sourceObject = personality,) + RulesetErrorSeverity.Warning, sourceObject = personality) } } } @@ -252,8 +174,6 @@ internal class BaseRulesetValidator( for (prereq in policy.requires ?: emptyList()) if (!ruleset.policies.containsKey(prereq)) lines.add("${policy.name} requires policy $prereq which does not exist!", sourceObject = policy) - - uniqueValidator.checkUniques(policy, lines, true, tryFixUnknownUniques) } for (branch in ruleset.policyBranches.values) { @@ -271,37 +191,33 @@ internal class BaseRulesetValidator( } } - for (policy in ruleset.policyBranches.values.flatMap { it.policies + it }) if (policy != ruleset.policies[policy.name]) lines.add("More than one policy with the name ${policy.name} exists!", sourceObject = policy) - } override fun addPromotionErrors(lines: RulesetErrorList) { - // No super.addPromotionErrors(lines): included below - //TODO except the contrast check - for (promotion in ruleset.unitPromotions.values) { - addPromotionErrorRulesetInvariant(promotion, lines) - - // These are warning as of 3.17.5 to not break existing mods and give them time to correct, should be upgraded to error in the future - for (prereq in promotion.prerequisites) - if (!ruleset.unitPromotions.containsKey(prereq)) - lines.add( - "${promotion.name} requires promotion $prereq which does not exist!", - RulesetErrorSeverity.Warning, promotion - ) - for (unitType in promotion.unitTypes) checkUnitType(unitType) { - lines.add( - "${promotion.name} references unit type $unitType, which does not exist!", - RulesetErrorSeverity.Warning, promotion - ) - } - uniqueValidator.checkUniques(promotion, lines, true, tryFixUnknownUniques) - } + super.addPromotionErrors(lines) checkPromotionCircularReferences(lines) } + override fun checkPromotion(promotion: Promotion, lines: RulesetErrorList) { + super.checkPromotion(promotion, lines) + + for (prereq in promotion.prerequisites) + if (!ruleset.unitPromotions.containsKey(prereq)) + lines.add( + "${promotion.name} requires promotion $prereq which does not exist!", + RulesetErrorSeverity.ErrorOptionsOnly, promotion + ) + for (unitType in promotion.unitTypes) checkUnitType(unitType) { + lines.add( + "${promotion.name} references unit type $unitType, which does not exist!", + RulesetErrorSeverity.ErrorOptionsOnly, promotion + ) + } + } + private fun checkPromotionCircularReferences(lines: RulesetErrorList) { fun recursiveCheck(history: HashSet, promotion: Promotion, level: Int) { if (promotion in history) { @@ -327,8 +243,6 @@ internal class BaseRulesetValidator( } override fun addResourceErrors(lines: RulesetErrorList) { - // No super.addResourceErrors(lines), included below - for (resource in ruleset.tileResources.values) { if (resource.revealedBy != null && !ruleset.technologies.containsKey(resource.revealedBy!!)) lines.add("${resource.name} revealed by tech ${resource.revealedBy} which does not exist!", sourceObject = resource) @@ -340,20 +254,17 @@ internal class BaseRulesetValidator( for (terrain in resource.terrainsCanBeFoundOn) if (!ruleset.terrains.containsKey(terrain)) lines.add("${resource.name} can be found on terrain $terrain which does not exist!", sourceObject = resource) - uniqueValidator.checkUniques(resource, lines, true, tryFixUnknownUniques) } + super.addResourceErrors(lines) } override fun addRuinsErrors(lines: RulesetErrorList) { super.addRuinsErrors(lines) for (reward in ruleset.ruinRewards.values) { - @Suppress("KotlinConstantConditions") // data is read from json, so any assumptions may be wrong - if (reward.weight < 0) lines.add("${reward.name} has a negative weight, which is not allowed!", sourceObject = reward) for (difficulty in reward.excludedDifficulties) if (!ruleset.difficulties.containsKey(difficulty)) lines.add("${reward.name} references difficulty ${difficulty}, which does not exist!", sourceObject = reward) - uniqueValidator.checkUniques(reward, lines, true, tryFixUnknownUniques) } } @@ -362,29 +273,17 @@ internal class BaseRulesetValidator( // Specialist is not a IHasUniques and unsuitable as sourceObject for (specialist in ruleset.specialists.values) { - for (gpp in specialist.greatPersonPoints) - if (gpp.key !in ruleset.units) + for ((gppName, _) in specialist.greatPersonPoints) + if (gppName !in ruleset.units) lines.add( - "Specialist ${specialist.name} has greatPersonPoints for ${gpp.key}, which is not a unit in the ruleset!", + "Specialist ${specialist.name} has greatPersonPoints for $gppName, which is not a unit in the ruleset!", RulesetErrorSeverity.Warning, sourceObject = null ) } } - override fun addSpeedErrors(lines: RulesetErrorList) { - super.addSpeedErrors(lines) - - for (speed in ruleset.speeds.values) { - if (speed.modifier < 0f) - lines.add("Negative speed modifier for game speed ${speed.name}", sourceObject = speed) - if (speed.yearsPerTurn.isEmpty()) - lines.add("Empty turn increment list for game speed ${speed.name}", sourceObject = speed) - } - } - override fun addTechErrors(lines: RulesetErrorList) { - // No super.addTechErrors(lines) or we would duplicate the checkUniques - //TODO missing `row < 1` check -> unify + super.addTechErrors(lines) for (tech in ruleset.technologies.values) { for (prereq in tech.prerequisites) { @@ -394,7 +293,7 @@ internal class BaseRulesetValidator( if (tech.prerequisites.any { it != prereq && 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, tech + RulesetErrorSeverity.WarningOptionsOnly, tech ) } @@ -403,13 +302,15 @@ internal class BaseRulesetValidator( } if (tech.era() !in ruleset.eras) lines.add("Unknown era ${tech.era()} referenced in column of tech ${tech.name}", sourceObject = tech) - uniqueValidator.checkUniques(tech, lines, true, tryFixUnknownUniques) + + for (otherTech in ruleset.technologies.values) { + if (tech.name > otherTech.name && otherTech.column?.columnNumber == tech.column?.columnNumber && otherTech.row == tech.row) + lines.add("${tech.name} is in the same row and column as ${otherTech.name}!", sourceObject = tech) + } } } override fun addTerrainErrors(lines: RulesetErrorList) { - super.addTerrainErrors(lines) - if (ruleset.terrains.values.none { it.type == TerrainType.Land && !it.impassable && !it.hasUnique( UniqueType.NoNaturalGeneration) }) lines.add("No passable land terrains exist!", sourceObject = null) @@ -430,26 +331,21 @@ internal class BaseRulesetValidator( // See https://github.com/hackedpassword/Z2/blob/main/HybridTileTech.md for a clever exploit lines.add("${terrain.name} turns into terrain ${terrain.turnsInto} which is not a base terrain!", RulesetErrorSeverity.Warning, terrain) } - uniqueValidator.checkUniques(terrain, lines, true, tryFixUnknownUniques) } + + super.addTerrainErrors(lines) } override fun addUnitErrors(lines: RulesetErrorList) { - // No super.addUnitErrors(lines), included below - if (ruleset.units.values.none { it.isCityFounder() }) lines.add("No city-founding units in ruleset!", sourceObject = null) - - for (unit in ruleset.units.values) { - checkUnitRulesetInvariant(unit, lines) - checkUnit(unit, lines) - uniqueValidator.checkUniques(unit, lines, true, tryFixUnknownUniques) - checkUniqueToMisspelling(unit, unit.uniqueTo, lines) - } + super.addUnitErrors(lines) } /** Collects all RulesetSpecific checks for a BaseUnit */ - private fun checkUnit(unit: BaseUnit, lines: RulesetErrorList) { + override fun checkUnit(unit: BaseUnit, lines: RulesetErrorList) { + super.checkUnit(unit, lines) + for (requiredTech: String in unit.requiredTechs()) if (!ruleset.technologies.containsKey(requiredTech)) lines.add("${unit.name} requires tech $requiredTech which does not exist!", sourceObject = unit) @@ -498,17 +394,8 @@ internal class BaseRulesetValidator( RulesetErrorSeverity.WarningOptionsOnly, unit) } } - } - override fun addUnitTypeErrors(lines: RulesetErrorList) { - super.addUnitTypeErrors(lines) - - val unitMovementTypes = UnitMovementType.entries.map { it.name }.toSet() - for (unitType in ruleset.unitTypes.values) { - if (unitType.movementType !in unitMovementTypes) - lines.add("Unit type ${unitType.name} has an invalid movement type ${unitType.movementType}", sourceObject = unitType) - uniqueValidator.checkUniques(unitType, lines, true, tryFixUnknownUniques) - } + checkUniqueToMisspelling(unit, unit.uniqueTo, lines) } override fun addVictoryTypeErrors(lines: RulesetErrorList) { @@ -524,11 +411,6 @@ internal class BaseRulesetValidator( ) 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, sourceObject = null - ) if (milestone.type in listOf(MilestoneType.BuiltBuilding, MilestoneType.BuildingBuiltGlobally) && milestone.params[0] !in ruleset.buildings) lines.add( @@ -536,13 +418,6 @@ internal class BaseRulesetValidator( 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, sourceObject = null - ) } } diff --git a/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt b/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt index 66aed84d01..f4acbe2f15 100644 --- a/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt +++ b/core/src/com/unciv/models/ruleset/validation/RulesetValidator.kt @@ -7,6 +7,8 @@ import com.unciv.Constants import com.unciv.UncivGame import com.unciv.json.fromJsonFile import com.unciv.json.json +import com.unciv.logic.map.tile.RoadStatus +import com.unciv.models.ruleset.BeliefType import com.unciv.models.ruleset.Building import com.unciv.models.ruleset.IRulesetObject import com.unciv.models.ruleset.Ruleset @@ -24,8 +26,10 @@ import com.unciv.models.ruleset.unique.UniqueTarget 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.ruleset.unit.UnitMovementType import com.unciv.models.ruleset.validation.RulesetValidator.Companion.create 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 @@ -56,7 +60,7 @@ import com.unciv.ui.images.PortraitPromotion */ open class RulesetValidator protected constructor( protected val ruleset: Ruleset, - protected val tryFixUnknownUniques: Boolean + private val tryFixUnknownUniques: Boolean ) { /** `true` for a [BaseRulesetValidator] instance, `false` for a [RulesetValidator] instance. */ private val reportRulesetSpecificErrors = ruleset.modOptions.isBaseRuleset @@ -125,40 +129,82 @@ open class RulesetValidator protected constructor( //region RulesetObject-specific handlers - protected open fun addBeliefErrors(lines: RulesetErrorList) {} - - protected open fun addBuildingErrors(lines: RulesetErrorList) { - for (building in ruleset.buildings.values) { - addBuildingErrorRulesetInvariant(building, lines) - uniqueValidator.checkUniques(building, lines, false, tryFixUnknownUniques) + protected open fun addBeliefErrors(lines: RulesetErrorList) { + for (belief in ruleset.beliefs.values) { + if (belief.type == BeliefType.Any || belief.type == BeliefType.None) + lines.add("${belief.name} type is ${belief.type}, which is not allowed!", sourceObject = belief) + uniqueValidator.checkUniques(belief, lines, reportRulesetSpecificErrors, tryFixUnknownUniques) } } - protected fun addBuildingErrorRulesetInvariant(building: Building, lines: RulesetErrorList) { - if (building.requiredTechs().none() && building.cost == -1 && !building.hasUnique( - UniqueType.Unbuildable - ) - ) + protected open fun addBuildingErrors(lines: RulesetErrorList) { + for (building in ruleset.buildings.values) { + checkBuilding(building, lines) + uniqueValidator.checkUniques(building, lines, reportRulesetSpecificErrors, tryFixUnknownUniques) + } + } + + protected open fun checkBuilding(building: Building, lines: RulesetErrorList) { + if (building.requiredTechs().none() && 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, building ) - for (gpp in building.greatPersonPoints) - if (gpp.key !in ruleset.units) - lines.add( - "Building ${building.name} has greatPersonPoints for ${gpp.key}, which is not a unit in the ruleset!", - RulesetErrorSeverity.Warning, building - ) - if (building.replaces != null && building.uniqueTo == null) lines.add("${building.name} should replace ${building.replaces} but does not have uniqueTo assigned!") } - protected open fun addCityStateTypeErrors(lines: RulesetErrorList) {} - protected open fun addDifficultyErrors(lines: RulesetErrorList) {} - protected open fun addEraErrors(lines: RulesetErrorList) {} - protected open fun addEventErrors(lines: RulesetErrorList) {} + protected open fun addCityStateTypeErrors(lines: RulesetErrorList) { + for (cityStateType in ruleset.cityStateTypes.values) { + for (unique in cityStateType.allyBonusUniqueMap.getAllUniques() + cityStateType.friendBonusUniqueMap.getAllUniques()) { + val errors = uniqueValidator.checkUnique(unique, tryFixUnknownUniques, null, reportRulesetSpecificErrors) + lines.addAll(errors) + } + } + } + + protected open fun addDifficultyErrors(lines: RulesetErrorList) { + for (difficulty in ruleset.difficulties.values) { + if (difficulty.aiBuildingCostModifier < 0 || difficulty.aiBuildingMaintenanceModifier < 0 || difficulty.aiCityGrowthModifier < 0 || + difficulty.aiUnhappinessModifier < 0 || difficulty.aiUnitCostModifier < 0 || difficulty.aiUnitMaintenanceModifier < 0 || + difficulty.aiUnitSupplyModifier < 0 || difficulty.aiWonderCostModifier < 0 || + difficulty.buildingCostModifier < 0 || difficulty.policyCostModifier < 0 || difficulty.researchCostModifier < 0 || + difficulty.unhappinessModifier < 0 || difficulty.unitCostModifier < 0) + lines.add("Difficulty ${difficulty.name} contains one or more negative modifier(s)!", sourceObject = null) + if (difficulty.turnBarbariansCanEnterPlayerTiles < 0) + lines.add("Difficulty ${difficulty.name} has a negative turnBarbariansCanEnterPlayerTiles!", + RulesetErrorSeverity.Warning, sourceObject = null) + } + } + + protected open fun addEraErrors(lines: RulesetErrorList) { + for (era in ruleset.eras.values) { + if (era.researchAgreementCost < 0 || era.startingSettlerCount < 0 || era.startingWorkerCount < 0 || + era.startingMilitaryUnitCount < 0 || era.startingGold < 0 || era.startingCulture < 0) + lines.add("Unexpected negative number found while parsing era ${era.name}", sourceObject = era) + if (era.settlerPopulation <= 0) + lines.add("Population in cities from settlers must be strictly positive! Found value ${era.settlerPopulation} for era ${era.name}", sourceObject = era) + + if (era.allyBonus.isNotEmpty() || era.friendBonus.isNotEmpty()) + lines.add( + "Era ${era.name} contains city-state bonuses. City-state bonuses are now defined in CityStateType.json", + RulesetErrorSeverity.WarningOptionsOnly, era + ) + + uniqueValidator.checkUniques(era, lines, reportRulesetSpecificErrors, tryFixUnknownUniques) + } + } + + protected open fun addEventErrors(lines: RulesetErrorList) { + // An Event is not a IHasUniques, so not suitable as sourceObject + for (event in ruleset.events.values) { + for (choice in event.choices) { + uniqueValidator.checkUniques(choice, lines, reportRulesetSpecificErrors, tryFixUnknownUniques) + } + uniqueValidator.checkUniques(event, lines, reportRulesetSpecificErrors, tryFixUnknownUniques) + } + } protected open fun addGlobalUniqueErrors(lines: RulesetErrorList) { uniqueValidator.checkUniques(ruleset.globalUniques, lines, reportRulesetSpecificErrors, tryFixUnknownUniques) @@ -183,13 +229,64 @@ open class RulesetValidator protected constructor( } } - protected open fun addImprovementErrors(lines: RulesetErrorList) {} + protected open fun addImprovementErrors(lines: RulesetErrorList) { + for (improvement in ruleset.tileImprovements.values) { + if (improvement.replaces != null && improvement.uniqueTo == null) + lines.add("${improvement.name} should replace ${improvement.replaces} but does not have uniqueTo assigned!") + if (improvement.terrainsCanBeBuiltOn.isEmpty() + && !improvement.hasUnique(UniqueType.CanOnlyImproveResource) + && !improvement.hasUnique(UniqueType.Unbuildable) + && !improvement.name.startsWith(Constants.remove) + && improvement.name !in RoadStatus.entries.map { it.removeAction } + && improvement.name != Constants.cancelImprovementOrder + ) { + lines.add( + "${improvement.name} has an empty `terrainsCanBeBuiltOn`, isn't allowed to only improve resources. As such it isn't buildable! Either give this the unique \"Unbuildable\", \"Can only be built to improve a resource\", or add \"Land\", \"Water\" or any other value to `terrainsCanBeBuiltOn`.", + RulesetErrorSeverity.Warning, improvement + ) + } + + for (unique in improvement.uniqueObjects + .filter { it.type == UniqueType.PillageYieldRandom || it.type == UniqueType.PillageYieldFixed }) { + if (!Stats.isStats(unique.params[0])) continue + val params = Stats.parse(unique.params[0]) + if (params.values.any { it < 0 }) lines.add( + "${improvement.name} cannot have a negative value for a pillage yield!", + RulesetErrorSeverity.Error, improvement + ) + } + + val hasPillageUnique = improvement.hasUnique(UniqueType.PillageYieldRandom, StateForConditionals.IgnoreConditionals) + || improvement.hasUnique(UniqueType.PillageYieldFixed, StateForConditionals.IgnoreConditionals) + if (hasPillageUnique && improvement.hasUnique(UniqueType.Unpillagable, StateForConditionals.IgnoreConditionals)) { + lines.add( + "${improvement.name} has both an `Unpillagable` unique type and a `PillageYieldRandom` or `PillageYieldFixed` unique type!", + RulesetErrorSeverity.Warning, improvement + ) + } + + uniqueValidator.checkUniques(improvement, lines, reportRulesetSpecificErrors, tryFixUnknownUniques) + } + } protected open fun addModOptionsErrors(lines: RulesetErrorList) { // Basic Unique validation (type, target, parameters) should always run. // Using reportRulesetSpecificErrors=true as ModOptions never should use Uniques depending on objects from a base ruleset anyway. uniqueValidator.checkUniques(ruleset.modOptions, lines, reportRulesetSpecificErrors = true, tryFixUnknownUniques) + //TODO: More thorough checks. Here I picked just those where bad values might endanger stability. + val constants = ruleset.modOptions.constants + if (constants.cityExpandRange !in 1..100) + lines.add("Invalid ModConstant 'cityExpandRange'.", sourceObject = null) + if (constants.cityWorkRange !in 1..100) + lines.add("Invalid ModConstant 'cityWorkRange'.", sourceObject = null) + if (constants.minimalCityDistance < 1) + lines.add("Invalid ModConstant 'minimalCityDistance'.", sourceObject = null) + if (constants.minimalCityDistanceOnDifferentContinents < 1) + lines.add("Invalid ModConstant 'minimalCityDistanceOnDifferentContinents'.", sourceObject = null) + if (constants.baseCityBombardRange < 1) + lines.add("Invalid ModConstant 'baseCityBombardRange'.", sourceObject = null) + if (ruleset.name.isBlank()) return // The rest of these tests don't make sense for combined rulesets val audioVisualUniqueTypes = setOf( @@ -220,12 +317,12 @@ open class RulesetValidator protected constructor( protected open fun addNationErrors(lines: RulesetErrorList) { for (nation in ruleset.nations.values) { - addNationErrorRulesetInvariant(nation, lines) - uniqueValidator.checkUniques(nation, lines, false, tryFixUnknownUniques) + checkNation(nation, lines) + uniqueValidator.checkUniques(nation, lines, reportRulesetSpecificErrors, tryFixUnknownUniques) } } - protected fun addNationErrorRulesetInvariant(nation: Nation, lines: RulesetErrorList) { + protected open fun checkNation(nation: Nation, lines: RulesetErrorList) { if (nation.cities.isEmpty() && !nation.isSpectator && !nation.isBarbarian) { lines.add("${nation.name} can settle cities, but has no city names!", sourceObject = nation) } @@ -233,19 +330,29 @@ open class RulesetValidator protected constructor( checkContrasts(nation.getInnerColor(), nation.getOuterColor(), nation, lines) } - protected open fun addPersonalityErrors(lines: RulesetErrorList) {} - protected open fun addPolicyErrors(lines: RulesetErrorList) {} - - protected open fun addPromotionErrors(lines: RulesetErrorList) { - for (promotion in ruleset.unitPromotions.values) { - uniqueValidator.checkUniques(promotion, lines, false, tryFixUnknownUniques) - checkContrasts(promotion.innerColorObject ?: PortraitPromotion.defaultInnerColor, - promotion.outerColorObject ?: PortraitPromotion.defaultOuterColor, promotion, lines) - addPromotionErrorRulesetInvariant(promotion, lines) + protected open fun addPersonalityErrors(lines: RulesetErrorList) { + for (personality in ruleset.personalities.values) { + if (personality.uniques.isNotEmpty()) + lines.add("Personality Uniques are not supported", RulesetErrorSeverity.Warning, personality) } } - protected fun addPromotionErrorRulesetInvariant(promotion: Promotion, lines: RulesetErrorList) { + protected open fun addPolicyErrors(lines: RulesetErrorList) { + for (policy in ruleset.policies.values) { + uniqueValidator.checkUniques(policy, lines, reportRulesetSpecificErrors, tryFixUnknownUniques) + } + } + + protected open fun addPromotionErrors(lines: RulesetErrorList) { + for (promotion in ruleset.unitPromotions.values) { + uniqueValidator.checkUniques(promotion, lines, reportRulesetSpecificErrors, tryFixUnknownUniques) + checkContrasts(promotion.innerColorObject ?: PortraitPromotion.defaultInnerColor, + promotion.outerColorObject ?: PortraitPromotion.defaultOuterColor, promotion, lines) + checkPromotion(promotion, lines) + } + } + + protected open fun checkPromotion(promotion: Promotion, lines: RulesetErrorList) { if (promotion.row < -1) lines.add("Promotion ${promotion.name} has invalid row value: ${promotion.row}", sourceObject = promotion) if (promotion.column < 0) lines.add("Promotion ${promotion.name} has invalid column value: ${promotion.column}", sourceObject = promotion) if (promotion.row == -1) return @@ -256,18 +363,49 @@ open class RulesetValidator protected constructor( protected open fun addResourceErrors(lines: RulesetErrorList) { for (resource in ruleset.tileResources.values) { - uniqueValidator.checkUniques(resource, lines, false, tryFixUnknownUniques) + uniqueValidator.checkUniques(resource, lines, reportRulesetSpecificErrors, tryFixUnknownUniques) + } + } + + protected open fun addRuinsErrors(lines: RulesetErrorList) { + for (reward in ruleset.ruinRewards.values) { + @Suppress("KotlinConstantConditions") // data is read from json, so any assumptions may be wrong + if (reward.weight < 0) lines.add("${reward.name} has a negative weight, which is not allowed!", sourceObject = reward) + uniqueValidator.checkUniques(reward, lines, reportRulesetSpecificErrors, tryFixUnknownUniques) } } - protected open fun addRuinsErrors(lines: RulesetErrorList) {} protected open fun addSpecialistErrors(lines: RulesetErrorList) {} - protected open fun addSpeedErrors(lines: RulesetErrorList) {} + + protected open fun addSpeedErrors(lines: RulesetErrorList) { + for (speed in ruleset.speeds.values) { + if (speed.modifier < 0f || speed.barbarianModifier < 0f || speed.cultureCostModifier < 0f || speed.faithCostModifier < 0f || + speed.goldCostModifier < 0f || speed.goldGiftModifier < 0f || speed.goldenAgeLengthModifier < 0f || + speed.improvementBuildLengthModifier < 0f || speed.productionCostModifier < 0f || speed.scienceCostModifier < 0f) + lines.add("One or more negative speed modifier(s) for game speed ${speed.name}", sourceObject = speed) + if (speed.dealDuration < 1 || speed.peaceDealDuration < 1) + lines.add("Deal durations must be positive", sourceObject = speed) + if (speed.religiousPressureAdjacentCity < 0) + lines.add("'religiousPressureAdjacentCity' must not be negative", sourceObject = speed) + if (speed.yearsPerTurn.isEmpty()) + lines.add("Empty turn increment list for game speed ${speed.name}", sourceObject = speed) + var lastTurn = 0 + for ((yearInterval, untilTurn) in speed.yearsPerTurn) { + if (yearInterval <= 0f) + lines.add("Negative year interval $yearInterval in turn increment list", sourceObject = speed) + if (untilTurn <= lastTurn) + lines.add("The 'untilTurn' field in the turn increment list must be monotonously increasing, but $untilTurn is <= $lastTurn", sourceObject = speed) + lastTurn = untilTurn + } + if (speed.uniques.isNotEmpty()) + lines.add("Speed Uniques are not supported", RulesetErrorSeverity.Warning, speed) + } + } protected open fun addTechErrors(lines: RulesetErrorList) { for (tech in ruleset.technologies.values) { if (tech.row < 1) lines.add("Tech ${tech.name} has a row value below 1: ${tech.row}", sourceObject = tech) - uniqueValidator.checkUniques(tech, lines, false, tryFixUnknownUniques) + uniqueValidator.checkUniques(tech, lines, reportRulesetSpecificErrors, tryFixUnknownUniques) } } @@ -295,27 +433,24 @@ open class RulesetValidator protected constructor( RulesetErrorSeverity.Warning, sourceObject = null ) } - - for (tech in ruleset.technologies.values) { - for (otherTech in ruleset.technologies.values) { - if (tech != otherTech && otherTech.column?.columnNumber == tech.column?.columnNumber && otherTech.row == tech.row) - lines.add("${tech.name} is in the same row and column as ${otherTech.name}!", sourceObject = tech) - } - } } - protected open fun addTerrainErrors(lines: RulesetErrorList) {} + protected open fun addTerrainErrors(lines: RulesetErrorList) { + for (terrain in ruleset.terrains.values) { + uniqueValidator.checkUniques(terrain, lines, reportRulesetSpecificErrors, tryFixUnknownUniques) + } + } protected open fun addUnitErrors(lines: RulesetErrorList) { for (unit in ruleset.units.values) { - checkUnitRulesetInvariant(unit, lines) - uniqueValidator.checkUniques(unit, lines, false, tryFixUnknownUniques) + checkUnit(unit, lines) + uniqueValidator.checkUniques(unit, lines, reportRulesetSpecificErrors, tryFixUnknownUniques) } } - protected fun checkUnitRulesetInvariant(unit: BaseUnit, lines: RulesetErrorList) { + protected open fun checkUnit(unit: BaseUnit, lines: RulesetErrorList) { for (upgradesTo in unit.getUpgradeUnits(StateForConditionals.IgnoreConditionals)) { - if (upgradesTo == unit.name || (upgradesTo == unit.replaces)) + if (upgradesTo == unit.name || upgradesTo == unit.replaces) lines.add("${unit.name} upgrades to itself!", sourceObject = unit) } @@ -326,8 +461,34 @@ open class RulesetValidator protected constructor( lines.add("${unit.name} is a military unit but has no assigned strength!", sourceObject = unit) } - protected open fun addUnitTypeErrors(lines: RulesetErrorList) {} - protected open fun addVictoryTypeErrors(lines: RulesetErrorList) {} + protected open fun addUnitTypeErrors(lines: RulesetErrorList) { + val unitMovementTypes = UnitMovementType.entries.map { it.name }.toSet() + for (unitType in ruleset.unitTypes.values) { + if (unitType.movementType !in unitMovementTypes) + lines.add("Unit type ${unitType.name} has an invalid movement type ${unitType.movementType}", sourceObject = unitType) + uniqueValidator.checkUniques(unitType, lines, reportRulesetSpecificErrors, tryFixUnknownUniques) + } + } + + protected open fun addVictoryTypeErrors(lines: RulesetErrorList) { + // Victory and Milestone aren't IHasUniques and are unsuitable as sourceObject + for (victoryType in ruleset.victories.values) { + 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, sourceObject = null + ) + } + + for (otherVictory in ruleset.victories.values) + if (otherVictory.name > victoryType.name && otherVictory.milestones == victoryType.milestones) + lines.add( + "Victory types ${victoryType.name} and ${otherVictory.name} have the same requirements!", + RulesetErrorSeverity.Warning, sourceObject = null + ) + } + } //endregion //region General helpers @@ -548,10 +709,7 @@ open class RulesetValidator protected constructor( .toSet() /* This is public because `FormattedLine` does its own checking and needs the textureNamesCache test */ - fun uncachedImageExists(name: String): Boolean { - if (ruleset.folderLocation == null) return false // Can't check in this case - return textureNamesCache.imageExists(name) - } + fun uncachedImageExists(name: String) = textureNamesCache.imageExists(name) //endregion diff --git a/core/src/com/unciv/models/ruleset/validation/UniqueValidator.kt b/core/src/com/unciv/models/ruleset/validation/UniqueValidator.kt index 97b1c1239b..a73a464ea3 100644 --- a/core/src/com/unciv/models/ruleset/validation/UniqueValidator.kt +++ b/core/src/com/unciv/models/ruleset/validation/UniqueValidator.kt @@ -52,7 +52,7 @@ class UniqueValidator(val ruleset: Ruleset) { lines.addAll(errors) } } - + private val performanceHeavyConditionals = setOf(UniqueType.ConditionalNeighborTiles, UniqueType.ConditionalAdjacentTo, UniqueType.ConditionalNotAdjacentTo ) @@ -64,10 +64,10 @@ class UniqueValidator(val ruleset: Ruleset) { reportRulesetSpecificErrors: Boolean ): RulesetErrorList { val prefix by lazy { getUniqueContainerPrefix(uniqueContainer) + "\"${unique.text}\"" } - if (unique.type == null) return checkUntypedUnique(unique, tryFixUnknownUniques, uniqueContainer, prefix) + if (unique.type == null) return checkUntypedUnique(unique, tryFixUnknownUniques, uniqueContainer, prefix, reportRulesetSpecificErrors) val rulesetErrors = RulesetErrorList(ruleset) - + if (uniqueContainer != null && !(unique.type.canAcceptUniqueTarget(uniqueContainer.getUniqueTarget()) || // "for X turns" effectively turns a global unique into a trigger @@ -86,14 +86,14 @@ class UniqueValidator(val ruleset: Ruleset) { " ${complianceError.acceptableParameterTypes.joinToString(" or ") { it.parameterName }} !", complianceError.errorSeverity.getRulesetErrorSeverity(), uniqueContainer, unique ) - + addExpressionParseErrors(complianceError, rulesetErrors, uniqueContainer, unique) } for (conditional in unique.modifiers) { addConditionalErrors(conditional, rulesetErrors, prefix, unique, uniqueContainer, reportRulesetSpecificErrors) } - + val conditionals = unique.modifiers.filter { it.type?.canAcceptUniqueTarget(UniqueTarget.Conditional) == true } if (conditionals.size > 1){ val lastCheapConditional = conditionals.lastOrNull { it.type !in performanceHeavyConditionals } @@ -105,7 +105,7 @@ class UniqueValidator(val ruleset: Ruleset) { "For performance, consider switching their locations.", RulesetErrorSeverity.WarningOptionsOnly, uniqueContainer, unique) } } - + if (unique.type in MapUnitCache.UnitMovementUniques && unique.modifiers.any { it.type != UniqueType.ConditionalOurUnit || it.params[0] !in Constants.all } @@ -134,7 +134,7 @@ class UniqueValidator(val ruleset: Ruleset) { unique: Unique ) { if (!complianceError.acceptableParameterTypes.contains(UniqueParameterType.Countable)) return - + val parseError = Expressions.getParsingError(complianceError.parameterName) if (parseError != null) { val marker = "HEREāž”" @@ -147,7 +147,7 @@ class UniqueValidator(val ruleset: Ruleset) { rulesetErrors.add(text, RulesetErrorSeverity.WarningOptionsOnly, uniqueContainer, unique) return } - + val countableErrors = Expressions.getCountableErrors(complianceError.parameterName, ruleset) if (countableErrors.isNotEmpty()) { val text = "\"${complianceError.parameterName}\" was parsed as an expression, but has the following errors with this ruleset:" + @@ -225,14 +225,14 @@ class UniqueValidator(val ruleset: Ruleset) { " which references a citywide resource. This is not a valid conditional for a resource uniques, " + "as it causes a recursive evaluation loop.", RulesetErrorSeverity.Error, uniqueContainer, unique) - + // Find resource uniques with countable parameters in conditionals, that depend on citywide resources // This too leads to an endless loop if (unique.type in resourceUniques) for ((index, param) in conditional.params.withIndex()){ if (ruleset.tileResources[param]?.isCityWide != true) continue if (unique.type!!.parameterTypeMap.getOrNull(index)?.contains(UniqueParameterType.Countable) != true) continue - + rulesetErrors.add( "$prefix contains the modifier \"${conditional.text}\"," + " which references a citywide resource as a countable." + @@ -253,7 +253,7 @@ class UniqueValidator(val ruleset: Ruleset) { " ${complianceError.acceptableParameterTypes.joinToString(" or ") { it.parameterName }} !", complianceError.errorSeverity.getRulesetErrorSeverity(), uniqueContainer, unique ) - + addExpressionParseErrors(complianceError, rulesetErrors, uniqueContainer, unique) } @@ -313,7 +313,7 @@ class UniqueValidator(val ruleset: Ruleset) { } val acceptableParamTypes = unique.type.parameterTypeMap[index] if (acceptableParamTypes.size == 0) continue // This is a deprecated parameter type, don't bother checking it - + val errorTypesForAcceptableParameters = acceptableParamTypes.map { getParamTypeErrorSeverityCached(it, param) } if (errorTypesForAcceptableParameters.any { it == null }) continue // This matches one of the types! @@ -322,7 +322,7 @@ class UniqueValidator(val ruleset: Ruleset) { continue // This is a filtering param, and the unique it's filtering for actually exists, no problem here! val leastSevereWarning = errorTypesForAcceptableParameters.minByOrNull { it!!.ordinal } - if (leastSevereWarning == null) + if (leastSevereWarning == null) throw Exception("Unique ${unique.text} from mod ${ruleset.name} is acting strangely - please open a bug report") errorList += UniqueComplianceError(param, acceptableParamTypes, leastSevereWarning) } @@ -342,7 +342,13 @@ class UniqueValidator(val ruleset: Ruleset) { return severity } - private fun checkUntypedUnique(unique: Unique, tryFixUnknownUniques: Boolean, uniqueContainer: IHasUniques?, prefix: String): RulesetErrorList { + private fun checkUntypedUnique( + unique: Unique, + tryFixUnknownUniques: Boolean, + uniqueContainer: IHasUniques?, + prefix: String, + reportRulesetSpecificErrors: Boolean + ): RulesetErrorList { // Malformed conditional is always bad if (unique.text.count { it == '<' } != unique.text.count { it == '>' }) return RulesetErrorList.of( @@ -351,7 +357,8 @@ class UniqueValidator(val ruleset: Ruleset) { ) // Support purely filtering Uniques without actual implementation - if (isFilteringUniqueAllowed(unique)) return RulesetErrorList() + if (isFilteringUniqueAllowed(unique, reportRulesetSpecificErrors)) return RulesetErrorList() + if (tryFixUnknownUniques) { val fixes = tryFixUnknownUnique(unique, uniqueContainer, prefix) if (fixes.isNotEmpty()) return fixes @@ -364,10 +371,11 @@ class UniqueValidator(val ruleset: Ruleset) { ) } - private fun isFilteringUniqueAllowed(unique: Unique): Boolean { + private fun isFilteringUniqueAllowed(unique: Unique, reportRulesetSpecificErrors: Boolean): Boolean { // Isolate this decision, to allow easy change of approach // This says: Must have no conditionals or parameters, and is used in any "filtering" parameter of another Unique if (unique.modifiers.isNotEmpty() || unique.params.isNotEmpty()) return false + if (!reportRulesetSpecificErrors) return true // Don't report unless checking a complete Ruleset return unique.text in allUniqueParameters // referenced at least once from elsewhere } diff --git a/core/src/com/unciv/ui/images/AtlasPreview.kt b/core/src/com/unciv/ui/images/AtlasPreview.kt index 955b207d44..a6fd2f0a2b 100644 --- a/core/src/com/unciv/ui/images/AtlasPreview.kt +++ b/core/src/com/unciv/ui/images/AtlasPreview.kt @@ -6,6 +6,7 @@ import com.badlogic.gdx.graphics.Pixmap import com.badlogic.gdx.graphics.g2d.TextureAtlas.TextureAtlasData import com.unciv.json.json import com.unciv.models.ruleset.Ruleset +import com.unciv.models.ruleset.RulesetCache import com.unciv.models.ruleset.validation.RulesetErrorList import com.unciv.models.ruleset.validation.RulesetErrorSeverity import com.unciv.utils.Log @@ -13,7 +14,7 @@ import java.io.File /** * This extracts all texture names from all atlases of a Ruleset. - * - Weak point: For combined rulesets, this always loads the builtin assets. + * - For combined rulesets, this loads assets for all component rulesets that are present in RulesetCache * - Used by RulesetValidator to check texture names without relying on ImageGetter * - Doubles as integrity checker and detects: * - Atlases.json names an atlas that does not exist @@ -28,6 +29,19 @@ class AtlasPreview(ruleset: Ruleset, errorList: RulesetErrorList) : Iterable() init { + if (ruleset.name.isNotEmpty()) loadSingleRuleset(ruleset, errorList) + else loadComplexRuleset(ruleset, errorList) + Log.debug("Atlas preview for $ruleset: ${regionNames.size} entries.") + } + + private fun loadComplexRuleset(ruleset: Ruleset, errorList: RulesetErrorList) { + for (modName in ruleset.mods) { + val componentRuleset = RulesetCache[modName] ?: continue + loadSingleRuleset(componentRuleset, errorList) + } + } + + private fun loadSingleRuleset(ruleset: Ruleset, errorList: RulesetErrorList) { // For builtin rulesets, the Atlases.json is right in internal root val folder = ruleset.folder() val controlFile = folder.child("Atlases.json") @@ -53,7 +67,6 @@ class AtlasPreview(ruleset: Ruleset, errorList: RulesetErrorList) : Iterable { if (!controlFileExists) return emptyArray() - + // Type checker doesn't know that fromJson can return null if the file is empty, treat as an empty array val fileNames = json().fromJson(Array::class.java, controlFile) ?: emptyArray() if (fileNames.isEmpty()) errorList.add("Atlases.json is empty", RulesetErrorSeverity.Warning)