From 8ab686cb1423628ffdfd0101e36967d1f138f1f5 Mon Sep 17 00:00:00 2001 From: Paul Pogonyshev Date: Sun, 29 May 2022 21:16:04 +0200 Subject: [PATCH] Improve suggestions when constructing an improvement is not possible (#6947) * Improve suggestions when constructing an improvement is not possible * Reimplement getImprovementBuildingProblems() as a generator function * Update documentation of getImprovementBuildingProblems(); forgotten in the last commit * ImprovementPickerScreen - tr * Rename a variable for clarity Co-authored-by: SomeTroglodyte <63000004+SomeTroglodyte@users.noreply.github.com> --- .../jsons/translations/German.properties | 4 ++ .../jsons/translations/template.properties | 4 ++ core/src/com/unciv/logic/map/TileInfo.kt | 68 ++++++++++++------- .../pickerscreens/ImprovementPickerScreen.kt | 59 ++++++++++++---- .../unciv/ui/worldscreen/unit/UnitActions.kt | 6 +- 5 files changed, 101 insertions(+), 40 deletions(-) diff --git a/android/assets/jsons/translations/German.properties b/android/assets/jsons/translations/German.properties index 90da1eda2d..2dc045933e 100644 --- a/android/assets/jsons/translations/German.properties +++ b/android/assets/jsons/translations/German.properties @@ -928,6 +928,10 @@ Provides [amount] [resource] = Stellt [amount] × [resource] zur Verfügung Replaces [improvement] = Ersetzt [improvement] Pick now! = Wähle jetzt! Remove [feature] first = Entferne zuerst [feature] +Research [tech] first = Erforsche [tech] +Have this tile close to your borders = Bringe deine Grenzen an das Feld +Have this tile inside your empire = Bringe das Feld in dein Territorium +Acquire more [resource] = Besorg dir mehr [resource] Build [building] = [building] bauen Train [unit] = [unit] ausbilden Produce [thingToProduce] = [thingToProduce] herstellen diff --git a/android/assets/jsons/translations/template.properties b/android/assets/jsons/translations/template.properties index 66cbe270d1..c5960f9174 100644 --- a/android/assets/jsons/translations/template.properties +++ b/android/assets/jsons/translations/template.properties @@ -931,6 +931,10 @@ Provides [amount] [resource] = Replaces [improvement] = Pick now! = Remove [feature] first = +Research [tech] first = +Have this tile close to your borders = +Have this tile inside your empire = +Acquire more [resource] = Build [building] = Train [unit] = Produce [thingToProduce] = diff --git a/core/src/com/unciv/logic/map/TileInfo.kt b/core/src/com/unciv/logic/map/TileInfo.kt index f8f8e26882..249b6e859b 100644 --- a/core/src/com/unciv/logic/map/TileInfo.kt +++ b/core/src/com/unciv/logic/map/TileInfo.kt @@ -569,9 +569,47 @@ open class TileInfo { } /** Returns true if the [improvement] can be built on this [TileInfo] */ - fun canBuildImprovement(improvement: TileImprovement, civInfo: CivilizationInfo): Boolean { + fun canBuildImprovement(improvement: TileImprovement, civInfo: CivilizationInfo): Boolean = getImprovementBuildingProblems(improvement, civInfo).none() + + enum class ImprovementBuildingProblem { + WrongCiv, MissingTech, Unbuildable, NotJustOutsideBorders, OutsideBorders, UnmetConditional, Obsolete, MissingResources, Other + } + + /** Generates a sequence of reasons that prevent building given [improvement]. + * If the sequence is empty, improvement can be built immediately. + */ + fun getImprovementBuildingProblems(improvement: TileImprovement, civInfo: CivilizationInfo, failFast: Boolean = false): Sequence = sequence { + val stateForConditionals = StateForConditionals(civInfo, tile=this@TileInfo) + + if (improvement.uniqueTo != null && improvement.uniqueTo != civInfo.civName) + yield(ImprovementBuildingProblem.WrongCiv) + if (improvement.techRequired != null && !civInfo.tech.isResearched(improvement.techRequired!!)) + yield(ImprovementBuildingProblem.MissingTech) + if (improvement.hasUnique(UniqueType.Unbuildable, stateForConditionals)) + yield(ImprovementBuildingProblem.Unbuildable) + + if (getOwner() != civInfo && !improvement.hasUnique(UniqueType.CanBuildOutsideBorders, stateForConditionals)) { + if (!improvement.hasUnique(UniqueType.CanBuildJustOutsideBorders, stateForConditionals)) + yield(ImprovementBuildingProblem.OutsideBorders) + else if (neighbors.none { it.getOwner() == civInfo }) + yield(ImprovementBuildingProblem.NotJustOutsideBorders) + } + + if (improvement.getMatchingUniques(UniqueType.OnlyAvailableWhen, StateForConditionals.IgnoreConditionals).any { + !it.conditionalsApply(stateForConditionals) + }) + yield(ImprovementBuildingProblem.UnmetConditional) + + if (improvement.getMatchingUniques(UniqueType.ObsoleteWith, stateForConditionals).any { + civInfo.tech.isResearched(it.params[0]) + }) + yield(ImprovementBuildingProblem.Obsolete) + + if (improvement.getMatchingUniques(UniqueType.ConsumesResources, stateForConditionals).any { + civInfo.getCivResourcesByName()[it.params[1]]!! < it.params[0].toInt() + }) + yield(ImprovementBuildingProblem.MissingResources) - val stateForConditionals = StateForConditionals(civInfo, tile=this) val knownFeatureRemovals = ruleset.tileImprovements.values .filter { rulesetImprovement -> rulesetImprovement.name.startsWith(Constants.remove) @@ -579,28 +617,10 @@ open class TileInfo { && (rulesetImprovement.techRequired == null || civInfo.tech.isResearched(rulesetImprovement.techRequired!!)) } - return when { - improvement.uniqueTo != null && improvement.uniqueTo != civInfo.civName -> false - improvement.techRequired != null && !civInfo.tech.isResearched(improvement.techRequired!!) -> false - improvement.hasUnique(UniqueType.Unbuildable, stateForConditionals) -> false - getOwner() != civInfo && !( - improvement.hasUnique(UniqueType.CanBuildOutsideBorders, stateForConditionals) - || ( - improvement.hasUnique(UniqueType.CanBuildJustOutsideBorders, stateForConditionals) - && neighbors.any { it.getOwner() == civInfo } - ) - ) -> false - improvement.getMatchingUniques(UniqueType.OnlyAvailableWhen, StateForConditionals.IgnoreConditionals).any { - !it.conditionalsApply(stateForConditionals) - } -> false - improvement.getMatchingUniques(UniqueType.ObsoleteWith, stateForConditionals).any { - civInfo.tech.isResearched(it.params[0]) - } -> false - improvement.getMatchingUniques(UniqueType.ConsumesResources, stateForConditionals).any { - civInfo.getCivResourcesByName()[it.params[1]]!! < it.params[0].toInt() - } -> false - else -> canImprovementBeBuiltHere(improvement, hasViewableResource(civInfo), knownFeatureRemovals, stateForConditionals) - } + if (!canImprovementBeBuiltHere(improvement, hasViewableResource(civInfo), knownFeatureRemovals, stateForConditionals)) + // There are way too many conditions in that functions, besides, they are not interesting + // at least for the current usecases. Improve if really needed. + yield(ImprovementBuildingProblem.Other) } /** Without regards to what CivInfo it is, a lot of the checks are just for the improvement on the tile. diff --git a/core/src/com/unciv/ui/pickerscreens/ImprovementPickerScreen.kt b/core/src/com/unciv/ui/pickerscreens/ImprovementPickerScreen.kt index 5247d34b5e..987cab3890 100644 --- a/core/src/com/unciv/ui/pickerscreens/ImprovementPickerScreen.kt +++ b/core/src/com/unciv/ui/pickerscreens/ImprovementPickerScreen.kt @@ -8,6 +8,7 @@ import com.unciv.UncivGame import com.unciv.logic.map.MapUnit import com.unciv.logic.map.TileInfo import com.unciv.models.ruleset.tile.TileImprovement +import com.unciv.models.ruleset.unique.UniqueType import com.unciv.models.stats.Stats import com.unciv.models.translations.tr import com.unciv.ui.images.ImageGetter @@ -23,6 +24,19 @@ class ImprovementPickerScreen( private val unit: MapUnit, private val onAccept: ()->Unit, ) : PickerScreen() { + + companion object { + /** Set of resolvable improvement building problems that this class knows how to report. */ + private val reportableProblems = setOf( + TileInfo.ImprovementBuildingProblem.MissingTech, + TileInfo.ImprovementBuildingProblem.NotJustOutsideBorders, + TileInfo.ImprovementBuildingProblem.OutsideBorders, + TileInfo.ImprovementBuildingProblem.MissingResources) + + /** Return true if we can report improvements associated with the [problems] (or there are no problems for it at all). */ + fun canReport(problems: Collection) = problems.all { it in reportableProblems } + } + private var selectedImprovement: TileImprovement? = null private val gameInfo = tileInfo.tileMap.gameInfo private val ruleSet = gameInfo.ruleSet @@ -62,9 +76,9 @@ class ImprovementPickerScreen( // clone tileInfo without "top" feature if it could be removed // Keep this copy around for speed - val tileInfoNoLast: TileInfo = tileInfo.clone() - if (Constants.remove + tileInfoNoLast.getLastTerrain().name in ruleSet.tileImprovements) { - tileInfoNoLast.removeTerrainFeature(tileInfoNoLast.getLastTerrain().name) + val tileInfoWithoutLastTerrain: TileInfo = tileInfo.clone() + if (Constants.remove + tileInfoWithoutLastTerrain.getLastTerrain().name in ruleSet.tileImprovements) { + tileInfoWithoutLastTerrain.removeTerrainFeature(tileInfoWithoutLastTerrain.getLastTerrain().name) } for (improvement in ruleSet.tileImprovements.values) { @@ -72,13 +86,16 @@ class ImprovementPickerScreen( // canBuildImprovement() would allow e.g. great improvements thus we need to exclude them - except cancel if (improvement.turnsToBuild == 0 && improvement.name != Constants.cancelImprovementOrder) continue if (improvement.name == tileInfo.improvement) continue // also checked by canImprovementBeBuiltHere, but after more expensive tests - if (!tileInfo.canBuildImprovement(improvement, currentPlayerCiv)) { - // if there is an improvement that could remove that terrain - if (!tileInfoNoLast.canBuildImprovement(improvement, currentPlayerCiv)) continue - suggestRemoval = true - } if (!unit.canBuildImprovement(improvement)) continue + var unbuildableBecause = tileInfo.getImprovementBuildingProblems(improvement, currentPlayerCiv).toSet() + if (!canReport(unbuildableBecause)) { + // Try after pretending to have removed the top terrain layer. + unbuildableBecause = tileInfoWithoutLastTerrain.getImprovementBuildingProblems(improvement, currentPlayerCiv).toSet() + if (!canReport(unbuildableBecause)) continue + else suggestRemoval = true + } + val image = ImageGetter.getImprovementIcon(improvement.name, 30f) // allow multiple key mappings to technologically supersede each other @@ -100,7 +117,7 @@ class ImprovementPickerScreen( var labelText = improvement.name.tr() val turnsToBuild = if (tileInfo.improvementInProgress == improvement.name) tileInfo.turnsToImprovement else improvement.getTurnsToBuild(currentPlayerCiv, unit) - + if (turnsToBuild > 0) labelText += " - $turnsToBuild${Fonts.turn}" val provideResource = tileInfo.hasViewableResource(currentPlayerCiv) && tileInfo.tileResource.isImprovedBy(improvement.name) if (provideResource) labelText += "\n" + "Provides [${tileInfo.resource}]".tr() @@ -109,8 +126,24 @@ class ImprovementPickerScreen( && improvement.name != Constants.cancelImprovementOrder) if (tileInfo.improvement != null && removeImprovement) labelText += "\n" + "Replaces [${tileInfo.improvement}]".tr() - val pickNow = when { - suggestRemoval -> "${Constants.remove}[${tileInfo.getLastTerrain().name}] first".toLabel() + val proposedSolutions = mutableListOf() + + if (suggestRemoval) + proposedSolutions.add("${Constants.remove}[${tileInfo.getLastTerrain().name}] first") + if (TileInfo.ImprovementBuildingProblem.MissingTech in unbuildableBecause) + proposedSolutions.add("Research [${improvement.techRequired}] first") + if (TileInfo.ImprovementBuildingProblem.NotJustOutsideBorders in unbuildableBecause) + proposedSolutions.add("Have this tile close to your borders") + if (TileInfo.ImprovementBuildingProblem.OutsideBorders in unbuildableBecause) + proposedSolutions.add("Have this tile inside your empire") + if (TileInfo.ImprovementBuildingProblem.MissingResources in unbuildableBecause) { + proposedSolutions.addAll(improvement.getMatchingUniques(UniqueType.ConsumesResources).filter { + currentPlayerCiv.getCivResourcesByName()[it.params[1]]!! < it.params[0].toInt() + }.map { "Acquire more [$it]" }) + } + + val explanationText = when { + proposedSolutions.any() -> proposedSolutions.joinToString("}\n{", "{", "}").toLabel() tileInfo.improvementInProgress == improvement.name -> "Current construction".toLabel() tileMarkedForCreatesOneImprovement -> null else -> "Pick now!".toLabel().onClick { accept(improvement) } @@ -140,7 +173,7 @@ class ImprovementPickerScreen( } if (improvement.name == tileInfo.improvementInProgress) improvementButton.color = Color.GREEN - if (suggestRemoval || tileMarkedForCreatesOneImprovement) { + if (proposedSolutions.isNotEmpty() || tileMarkedForCreatesOneImprovement) { improvementButton.disable() } else if (shortcutKey != null) { keyPressDispatcher[shortcutKey] = { accept(improvement) } @@ -148,7 +181,7 @@ class ImprovementPickerScreen( } regularImprovements.add(improvementButton) - regularImprovements.add(pickNow).padLeft(10f).fillY() + regularImprovements.add(explanationText).padLeft(10f).fillY() regularImprovements.row() } diff --git a/core/src/com/unciv/ui/worldscreen/unit/UnitActions.kt b/core/src/com/unciv/ui/worldscreen/unit/UnitActions.kt index 023fa14b7a..1059742536 100644 --- a/core/src/com/unciv/ui/worldscreen/unit/UnitActions.kt +++ b/core/src/com/unciv/ui/worldscreen/unit/UnitActions.kt @@ -394,10 +394,10 @@ object UnitActions { if (!unit.hasUniqueToBuildImprovements) return if (unit.isEmbarked()) return - val canConstruct = unit.currentMovement > 0 + val couldConstruct = unit.currentMovement > 0 && !tile.isCityCenter() && unit.civInfo.gameInfo.ruleSet.tileImprovements.values.any { - tile.canBuildImprovement(it, unit.civInfo) + ImprovementPickerScreen.canReport(tile.getImprovementBuildingProblems(it, unit.civInfo).toSet()) && unit.canBuildImprovement(it) } @@ -405,7 +405,7 @@ object UnitActions { isCurrentAction = unit.currentTile.hasImprovementInProgress(), action = { worldScreen.game.setScreen(ImprovementPickerScreen(tile, unit) { unitTable.selectUnit() }) - }.takeIf { canConstruct } + }.takeIf { couldConstruct } ) }