From 7079619fe2af227d1abd166f8d66a0463cf78a81 Mon Sep 17 00:00:00 2001 From: Xander Lenstra <71121390+xlenstra@users.noreply.github.com> Date: Tue, 17 May 2022 19:04:49 +0200 Subject: [PATCH] Great improvements can again be constructed on forest (#6823) * Great improvements can again be constructed on forest This PR fixes a bug where great improvements couldn't be build on forests/marshes/jungles/etc. It does so by creating a unique which specifically allows for removing features, and checking for that. Additionally, we only remove these features when we have the tech to remove them. For example, you can no longer plonk an Academy down over a forest without having researched mining. * Missed the file for vanilla * Reviews * Fixed logic --- .../TileImprovements.json | 13 +++---- .../Civ V - Vanilla/TileImprovements.json | 13 +++---- core/src/com/unciv/logic/map/TileInfo.kt | 36 +++++++++++++++---- .../models/ruleset/tile/TileImprovement.kt | 19 +++++++++- .../unciv/models/ruleset/unique/UniqueType.kt | 3 +- .../unciv/ui/worldscreen/unit/UnitActions.kt | 8 ----- docs/Modders/uniques.md | 5 ++- 7 files changed, 67 insertions(+), 30 deletions(-) diff --git a/android/assets/jsons/Civ V - Gods & Kings/TileImprovements.json b/android/assets/jsons/Civ V - Gods & Kings/TileImprovements.json index 79a5331775..ab941aa384 100644 --- a/android/assets/jsons/Civ V - Gods & Kings/TileImprovements.json +++ b/android/assets/jsons/Civ V - Gods & Kings/TileImprovements.json @@ -181,31 +181,31 @@ "name": "Academy", "terrainsCanBeBuiltOn": ["Land"], "science": 8, - "uniques": ["Great Improvement", "[+2 Science] ", "[+2 Science] "] + "uniques": ["Great Improvement", "[+2 Science] ", "[+2 Science] ", "Removes removable features when built"] }, { "name": "Landmark", "terrainsCanBeBuiltOn": ["Land"], "culture": 6, - "uniques": ["Great Improvement"] + "uniques": ["Great Improvement", "Removes removable features when built"] }, { "name": "Manufactory", "terrainsCanBeBuiltOn": ["Land"], "production": 4, - "uniques": ["Great Improvement", "[+1 Production] "] + "uniques": ["Great Improvement", "[+1 Production] ", "Removes removable features when built"] }, { "name": "Customs house", "terrainsCanBeBuiltOn": ["Land"], "gold": 4, - "uniques": ["Great Improvement", "[+1 Gold] "] + "uniques": ["Great Improvement", "[+1 Gold] ", "Removes removable features when built"] }, { "name": "Holy site", "terrainsCanBeBuiltOn": ["Land"], "faith": 6, - "uniques": ["Great Improvement"] + "uniques": ["Great Improvement", "Removes removable features when built"] }, { "name": "Citadel", @@ -215,7 +215,8 @@ "Gives a defensive bonus of [100]%", "Adjacent enemy units ending their turn take [30] damage", "Can be built just outside your borders", - "Constructing it will take over the tiles around it and assign them to your closest city" + "Constructing it will take over the tiles around it and assign them to your closest city", + "Removes removable features when built", ] }, diff --git a/android/assets/jsons/Civ V - Vanilla/TileImprovements.json b/android/assets/jsons/Civ V - Vanilla/TileImprovements.json index d5880e5982..bfd003fc50 100644 --- a/android/assets/jsons/Civ V - Vanilla/TileImprovements.json +++ b/android/assets/jsons/Civ V - Vanilla/TileImprovements.json @@ -181,31 +181,31 @@ "name": "Academy", "terrainsCanBeBuiltOn": ["Land"], "science": 8, - "uniques": ["Great Improvement", "[+2 Science] "] + "uniques": ["Great Improvement", "[+2 Science] ", "Removes removable features when built"] }, { "name": "Landmark", "terrainsCanBeBuiltOn": ["Land"], "culture": 6, - "uniques": ["Great Improvement"] + "uniques": ["Great Improvement", "Removes removable features when built"] }, { "name": "Manufactory", "terrainsCanBeBuiltOn": ["Land"], "production": 4, - "uniques": ["Great Improvement", "[+1 Production] "] + "uniques": ["Great Improvement", "[+1 Production] ", "Removes removable features when built"] }, { "name": "Customs house", "terrainsCanBeBuiltOn": ["Land"], "gold": 4, - "uniques": ["Great Improvement", "[+1 Gold] "] + "uniques": ["Great Improvement", "[+1 Gold] ", "Removes removable features when built"] }, { "name": "Holy site", "terrainsCanBeBuiltOn": ["Land"], "faith": 6, - "uniques": ["Great Improvement"] + "uniques": ["Great Improvement", "Removes removable features when built"] }, { "name": "Citadel", @@ -215,7 +215,8 @@ "Gives a defensive bonus of [100]%", "Adjacent enemy units ending their turn take [30] damage", "Can be built just outside your borders", - "Constructing it will take over the tiles around it and assign them to your closest city" + "Constructing it will take over the tiles around it and assign them to your closest city", + "Removes removable features when built", ] }, diff --git a/core/src/com/unciv/logic/map/TileInfo.kt b/core/src/com/unciv/logic/map/TileInfo.kt index e0d107e797..8348b1b7ff 100644 --- a/core/src/com/unciv/logic/map/TileInfo.kt +++ b/core/src/com/unciv/logic/map/TileInfo.kt @@ -491,13 +491,33 @@ open class TileInfo { /** Returns true if the [improvement] can be built on this [TileInfo] */ fun canBuildImprovement(improvement: TileImprovement, civInfo: CivilizationInfo): Boolean { + + fun TileImprovement.canBeBuildOnThisUnbuildableTerrain(civInfo: CivilizationInfo, stateForConditionals: StateForConditionals): Boolean { + val topTerrain = getLastTerrain() + // We can build if we are specifically allowed to build on this terrain + if (isAllowedOnFeature(topTerrain.name)) return true + + // Otherwise, we can if this improvement removes the top terrain + if (!hasUnique(UniqueType.RemovesFeaturesIfBuilt, stateForConditionals)) return false + val removeAction = ruleset.tileImprovements[Constants.remove + topTerrain.name] ?: return false + // and we have the tech to remove that top terrain + if (removeAction.techRequired != null && !civInfo.tech.isResearched(removeAction.techRequired!!)) return false + // and we can build it on the tile without the top terrain + val clonedTile = this@TileInfo.clone() + clonedTile.removeTerrainFeature(topTerrain.name) + return clonedTile.canBuildImprovement(this, civInfo) + } + val stateForConditionals = StateForConditionals(civInfo, tile=this) + + 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) - || ( // citadel can be built only next to or within own borders + || ( improvement.hasUnique(UniqueType.CanBuildJustOutsideBorders, stateForConditionals) && neighbors.any { it.getOwner() == civInfo } ) @@ -511,10 +531,11 @@ open class TileInfo { improvement.getMatchingUniques(UniqueType.ConsumesResources, stateForConditionals).any { civInfo.getCivResourcesByName()[it.params[1]]!! < it.params[0].toInt() } -> false - improvement.hasUnique(UniqueType.Unbuildable, stateForConditionals) -> false + getLastTerrain().unbuildable && !improvement.canBeBuildOnThisUnbuildableTerrain(civInfo, stateForConditionals) -> false else -> canImprovementBeBuiltHere(improvement, hasViewableResource(civInfo), stateForConditionals) } } + /** Without regards to what CivInfo it is, a lot of the checks are just for the improvement on the tile. * Doubles as a check for the map editor. @@ -524,12 +545,13 @@ open class TileInfo { resourceIsVisible: Boolean = resource != null, stateForConditionals: StateForConditionals = StateForConditionals(tile=this) ): Boolean { - val topTerrain = getLastTerrain() return when { improvement.name == this.improvement -> false isCityCenter() -> false + // First we handle a few special improvements + // Can only cancel if there is actually an improvement being built improvement.name == Constants.cancelImprovementOrder -> (this.improvementInProgress != null) // Can only remove roads if that road is actually there @@ -565,26 +587,26 @@ open class TileInfo { !isAdjacentTo(it.params[0]) } -> false - // Can't build on unbuildable terrains - EXCEPT when specifically allowed to - topTerrain.unbuildable && !improvement.isAllowedOnFeature(topTerrain.name) -> false - // Can't build it if it is only allowed to improve resources and it doesn't improve this resource improvement.hasUnique(UniqueType.CanOnlyImproveResource, stateForConditionals) && ( !resourceIsVisible || !tileResource.isImprovedBy(improvement.name) ) -> false // At this point we know this is a normal improvement and that there is no reason not to allow it to be built. + // Lastly we check if the improvement may be built on this terrain or resource - improvement.canBeBuiltOn(topTerrain.name) -> true + improvement.canBeBuiltOn(getLastTerrain().name) -> true isLand && improvement.canBeBuiltOn("Land") -> true isWater && improvement.canBeBuiltOn("Water") -> true // DO NOT reverse this &&. isAdjacentToFreshwater() is a lazy which calls a function, and reversing it breaks the tests. improvement.hasUnique(UniqueType.ImprovementBuildableByFreshWater, stateForConditionals) && isAdjacentTo(Constants.freshWater) -> true + // I don't particularly like this check, but it is required to build mines on non-hill resources resourceIsVisible && tileResource.isImprovedBy(improvement.name) -> true // DEPRECATED since 4.0.14, REMOVE SOON: isLand && improvement.terrainsCanBeBuiltOn.isEmpty() && !improvement.hasUnique(UniqueType.CanOnlyImproveResource) -> true + // No reason this improvement should be built here, so can't build it else -> false } } diff --git a/core/src/com/unciv/models/ruleset/tile/TileImprovement.kt b/core/src/com/unciv/models/ruleset/tile/TileImprovement.kt index 39d5b684ea..f43374c001 100644 --- a/core/src/com/unciv/models/ruleset/tile/TileImprovement.kt +++ b/core/src/com/unciv/models/ruleset/tile/TileImprovement.kt @@ -1,5 +1,6 @@ package com.unciv.models.ruleset.tile +import com.unciv.Constants import com.unciv.UncivGame import com.unciv.logic.civilization.CivilizationInfo import com.unciv.logic.map.MapUnit @@ -72,15 +73,31 @@ class TileImprovement : RulesetStatsObject() { } fun handleImprovementCompletion(builder: MapUnit) { + val tile = builder.getTile() if (hasUnique(UniqueType.TakesOverAdjacentTiles)) UnitActions.takeOverTilesAround(builder) - if (builder.getTile().resource != null) { + if (tile.resource != null) { val city = builder.getTile().getCity() if (city != null) { city.cityStats.update() city.civInfo.updateDetailedCivResources() } } + if (hasUnique(UniqueType.RemovesFeaturesIfBuilt)) { + // Remove terrainFeatures that a Worker can remove + // and that aren't explicitly allowed under the improvement + val removableTerrainFeatures = tile.terrainFeatures.filter { feature -> + val removingAction = "${Constants.remove}$feature" + + removingAction in tile.ruleset.tileImprovements + && !isAllowedOnFeature(feature) + && tile.ruleset.tileImprovements[removingAction]!!.let { + it.techRequired == null || builder.civInfo.tech.isResearched(it.techRequired!!) + } + } + + tile.setTerrainFeatures(tile.terrainFeatures.filterNot { it in removableTerrainFeatures }) + } } /** diff --git a/core/src/com/unciv/models/ruleset/unique/UniqueType.kt b/core/src/com/unciv/models/ruleset/unique/UniqueType.kt index 75481bb8f1..a2913f31bb 100644 --- a/core/src/com/unciv/models/ruleset/unique/UniqueType.kt +++ b/core/src/com/unciv/models/ruleset/unique/UniqueType.kt @@ -620,8 +620,9 @@ enum class UniqueType(val text: String, vararg targets: UniqueTarget, val flags: CanBuildJustOutsideBorders("Can be built just outside your borders", UniqueTarget.Improvement), CanOnlyBeBuiltOnTile("Can only be built on [tileFilter] tiles", UniqueTarget.Improvement), CannotBuildOnTile("Cannot be built on [tileFilter] tiles", UniqueTarget.Improvement), - NoFeatureRemovalNeeded("Does not need removal of [tileFilter]", UniqueTarget.Improvement), CanOnlyImproveResource("Can only be built to improve a resource", UniqueTarget.Improvement), + NoFeatureRemovalNeeded("Does not need removal of [tileFilter]", UniqueTarget.Improvement), + RemovesFeaturesIfBuilt("Removes removable features when built", UniqueTarget.Improvement), DefensiveBonus("Gives a defensive bonus of [relativeAmount]%", UniqueTarget.Improvement), ImprovementMaintenance("Costs [amount] gold per turn when in your territory", UniqueTarget.Improvement), // Unused diff --git a/core/src/com/unciv/ui/worldscreen/unit/UnitActions.kt b/core/src/com/unciv/ui/worldscreen/unit/UnitActions.kt index 9e9a41094f..6d4983b5f9 100644 --- a/core/src/com/unciv/ui/worldscreen/unit/UnitActions.kt +++ b/core/src/com/unciv/ui/worldscreen/unit/UnitActions.kt @@ -644,14 +644,6 @@ object UnitActions { title = "Create [$improvementName]", action = { val unitTile = unit.getTile() - unitTile.setTerrainFeatures( - // Remove terrainFeatures that a Worker can remove - // and that aren't explicitly allowed under the improvement - unitTile.terrainFeatures.filter { - "Remove $it" !in unitTile.ruleset.tileImprovements || - it in improvement.terrainsCanBeBuiltOn - } - ) unitTile.removeCreatesOneImprovementMarker() unitTile.improvement = improvementName unitTile.stopWorkingOnImprovement() diff --git a/docs/Modders/uniques.md b/docs/Modders/uniques.md index b608407740..16a13f56ec 100644 --- a/docs/Modders/uniques.md +++ b/docs/Modders/uniques.md @@ -1430,12 +1430,15 @@ Simple unique parameters are explained by mouseover. Complex parameters are expl Applicable to: Improvement +??? example "Can only be built to improve a resource" + Applicable to: Improvement + ??? example "Does not need removal of [tileFilter]" Example: "Does not need removal of [Farm]" Applicable to: Improvement -??? example "Can only be built to improve a resource" +??? example "Removes removable features when built" Applicable to: Improvement ??? example "Gives a defensive bonus of [relativeAmount]%"