Generalized building of improvements (#4252)

* Generalized building of improvements

* Readded support for the deprecated unique "Can build improvements on tiles"

* Small code quality changes

* Implemented requested chagnes

android/assets/jsons/Civ V - Vanilla/Units.json

@@ -7,7 +7,7 @@
 		"name": "Worker",
 		"unitType": "Civilian",
 		"movement": 2,
-		"uniques": ["Can build improvements on tiles"],
+		"uniques": ["Can build [Land] improvements on tiles"],
 		"cost": 70
 	},
 	{
@@ -372,10 +372,9 @@
 		"upgradesTo": "Longswordsman",
 		"obsoleteTech": "Gunpowder",
 		"requiredResource": "Iron",
-		"uniques": ["Can construct roads"],
+		"uniques": ["Can build [Road] improvements on tiles", "Can build [Fort] improvements on tiles"],
 		"hurryCostModifier": 20,
 		"attackSound": "metalhit"
-		// can construct fort (if required for fort tech is researched)
 	},
 	{
 		"name": "Mohawk Warrior",

core/src/com/unciv/Constants.kt

@@ -2,7 +2,10 @@ package com.unciv
 
 object Constants {
     const val worker = "Worker"
+    const val canBuildImprovements = "Can build [] improvements on tiles"
+    // Deprecated as of 3.15.5
         const val workerUnique = "Can build improvements on tiles"
+    //
     const val settler = "Settler"
     const val settlerUnique = "Founds a new city"
 

core/src/com/unciv/logic/automation/ConstructionAutomation.kt

@@ -11,8 +11,8 @@ import com.unciv.logic.map.BFS
 import com.unciv.models.ruleset.Building
 import com.unciv.models.ruleset.VictoryType
 import com.unciv.models.stats.Stat
+import com.unciv.models.translations.equalsPlaceholderText
 import kotlin.math.min
-import kotlin.math.roundToInt
 import kotlin.math.sqrt
 
 class ConstructionAutomation(val cityConstructions: CityConstructions){
@@ -27,7 +27,8 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
 
     val civUnits = civInfo.getCivUnits()
     val militaryUnits = civUnits.count { !it.type.isCivilian() }
-    val workers = civUnits.count { it.hasUnique(Constants.workerUnique) }.toFloat()
+    // Constants.workerUnique deprecated since 3.15.5
+    val workers = civUnits.count { (it.hasUnique(Constants.canBuildImprovements) || it.hasUnique(Constants.workerUnique)) && it.type.isCivilian() }.toFloat()
     val cities = civInfo.cities.size
     val allTechsAreResearched = civInfo.tech.getNumberOfTechsResearched() >= civInfo.gameInfo.ruleSet.technologies.size
 
@@ -138,13 +139,18 @@ class ConstructionAutomation(val cityConstructions: CityConstructions){
 
     private fun addWorkerChoice() {
         val workerEquivalents = civInfo.gameInfo.ruleSet.units.values
-                .filter { it.uniques.contains(Constants.workerUnique) && it.isBuildable(cityConstructions) }
+                .filter { it.uniques.any {
+                    // Constants.workerUnique deprecated since 3.15.5
+                        unique -> unique.equalsPlaceholderText(Constants.canBuildImprovements) || unique.equalsPlaceholderText(Constants.workerUnique) 
+                } && it.isBuildable(cityConstructions) }
         if (workerEquivalents.isEmpty()) return // for mods with no worker units
-        if (civInfo.getIdleUnits().any { it.action == Constants.unitActionAutomation && it.hasUnique(Constants.workerUnique) })
+        // Constants.workerUnique deprecated since 3.15.5
+        if (civInfo.getIdleUnits().any { it.action == Constants.unitActionAutomation && (it.hasUnique(Constants.canBuildImprovements) || it.hasUnique(Constants.workerUnique)) })
             return // If we have automated workers who have no work to do then it's silly to construct new workers.
 
         val citiesCountedTowardsWorkers = min(5, cities) // above 5 cities, extra cities won't make us want more workers
-        if (workers < citiesCountedTowardsWorkers * 0.6f && civUnits.none { it.hasUnique(Constants.workerUnique) && it.isIdle() }) {
+        // Constants.workerUnique deprecated since 3.15.5
+        if (workers < citiesCountedTowardsWorkers * 0.6f && civUnits.none { (it.hasUnique(Constants.canBuildImprovements) || it.hasUnique(Constants.workerUnique)) && it.isIdle() }) {
             var modifier = citiesCountedTowardsWorkers / (workers + 0.1f)
             if (!cityIsOverAverageProduction) modifier /= 5 // higher production cities will deal with this
             addChoice(relativeCostEffectiveness, workerEquivalents.minByOrNull { it.cost }!!.name, modifier)

core/src/com/unciv/logic/automation/UnitAutomation.kt

@@ -89,10 +89,11 @@ object UnitAutomation {
             if (unit.hasUnique(Constants.settlerUnique))
                 return SpecificUnitAutomation.automateSettlerActions(unit)
 
-            if (unit.hasUnique(Constants.workerUnique))
+            // Constants.workerUnique deprecated since 3.15.5
+            if (unit.hasUnique(Constants.canBuildImprovements) || unit.hasUnique(Constants.workerUnique))
                 return WorkerAutomation(unit).automateWorkerAction()
 
-            if (unit.name == "Work Boats")
+            if (unit.name == "Work Boats") // This is really not modular
                 return SpecificUnitAutomation.automateWorkBoats(unit)
 
             if (unit.hasUnique("Bonus for units in 2 tile radius 15%"))

core/src/com/unciv/logic/automation/WorkerAutomation.kt

@@ -15,7 +15,7 @@ class WorkerAutomation(val unit: MapUnit) {
         val enemyUnitsInWalkingDistance = unit.movement.getDistanceToTiles().keys
                 .filter { UnitAutomation.containsEnemyMilitaryUnit(unit, it) }
 
-        if (enemyUnitsInWalkingDistance.isNotEmpty()) return UnitAutomation.runAway(unit)
+        if (enemyUnitsInWalkingDistance.isNotEmpty() && !unit.type.isMilitary()) return UnitAutomation.runAway(unit)
 
         val currentTile = unit.getTile()
         val tileToWork = findTileToWork()
@@ -63,7 +63,6 @@ class WorkerAutomation(val unit: MapUnit) {
         //Player can choose not to auto-build roads & railroads.
         if (unit.civInfo.isPlayerCivilization() && !UncivGame.Current.settings.autoBuildingRoads)
             return false
-
         val targetRoad = unit.civInfo.tech.getBestRoadAvailable()
 
         val citiesThatNeedConnecting = unit.civInfo.cities.asSequence()
@@ -152,15 +151,14 @@ class WorkerAutomation(val unit: MapUnit) {
             return false
 
         if (tile.improvement == null) {
-            if (tile.improvementInProgress != null) return true
+            if (tile.improvementInProgress != null && unit.canBuildImprovement(tile.getTileImprovementInProgress()!!, tile)) return true
             val chosenImprovement = chooseImprovement(tile, civInfo)
-            if (chosenImprovement != null && tile.canBuildImprovement(chosenImprovement, civInfo)) return true
+            if (chosenImprovement != null && tile.canBuildImprovement(chosenImprovement, civInfo) && unit.canBuildImprovement(chosenImprovement, tile)) return true
         } else if (!tile.containsGreatImprovement() && tile.hasViewableResource(civInfo)
                 && tile.getTileResource().improvement != tile.improvement
                 && chooseImprovement(tile, civInfo) // if the chosen improvement is not null and buildable
-                        .let { it != null && tile.canBuildImprovement(it, civInfo) })
+                    .let { it != null && tile.canBuildImprovement(it, civInfo) && unit.canBuildImprovement(it, tile)})
             return true
-
         return false // couldn't find anything to construct here
     }
 
@@ -204,7 +202,9 @@ class WorkerAutomation(val unit: MapUnit) {
             // While AI sucks in strategical placement of forts, allow a human does it manually
             !civInfo.isPlayerCivilization() && evaluateFortPlacement(tile, civInfo, false) -> Constants.fort
             // I think we can assume that the unique improvement is better
-            uniqueImprovement != null && tile.canBuildImprovement(uniqueImprovement, civInfo) -> uniqueImprovement.name
+            uniqueImprovement != null && tile.canBuildImprovement(uniqueImprovement, civInfo) 
+                && unit.canBuildImprovement(uniqueImprovement, tile) -> 
+                    uniqueImprovement.name
 
             tile.terrainFeatures.contains("Fallout") -> "Remove Fallout"
             tile.terrainFeatures.contains(Constants.marsh) -> "Remove Marsh"

core/src/com/unciv/logic/map/MapUnit.kt

@@ -10,6 +10,7 @@ import com.unciv.logic.civilization.LocationAction
 import com.unciv.logic.civilization.NotificationIcon
 import com.unciv.models.ruleset.Ruleset
 import com.unciv.models.ruleset.Unique
+import com.unciv.models.ruleset.tile.TileImprovement
 import com.unciv.models.ruleset.unit.BaseUnit
 import com.unciv.models.ruleset.unit.UnitType
 import java.text.DecimalFormat
@@ -257,8 +258,13 @@ class MapUnit {
 
     fun isIdle(): Boolean {
         if (currentMovement == 0f) return false
-        if (hasUnique(Constants.workerUnique) && getTile().improvementInProgress != null) return false
+        // Constants.workerUnique deprecated since 3.15.5
+        if (getTile().improvementInProgress != null 
+            && canBuildImprovement(getTile().getTileImprovementInProgress()!!)) 
+                return false
+        // unique "Can construct roads" deprecated since 3.15.5
             if (hasUnique("Can construct roads") && currentTile.improvementInProgress == "Road") return false
+        //
         if (isFortified()) return false
         if (action == Constants.unitActionExplore || isSleeping()
             || action == Constants.unitActionAutomation || isMoving()
@@ -540,12 +546,16 @@ class MapUnit {
     fun endTurn() {
         doAction()
 
-        if (currentMovement > 0 && hasUnique(Constants.workerUnique)
+        if (currentMovement > 0 && 
-            && getTile().improvementInProgress != null
+            // Constants.workerUnique deprecated since 3.15.5
+            getTile().improvementInProgress != null
+            && canBuildImprovement(getTile().getTileImprovementInProgress()!!)
         ) workOnImprovement()
+        // unique "Can construct roads" deprecated since 3.15.4
             if (currentMovement > 0 && hasUnique("Can construct roads")
                 && currentTile.improvementInProgress == "Road"
             ) workOnImprovement()
+        //
         if (currentMovement == getMaxMovement().toFloat() && isFortified()) {
             val currentTurnsFortified = getFortificationTurns()
             if (currentTurnsFortified < 2)
@@ -913,5 +923,11 @@ class MapUnit {
         }
     }
 
+    fun canBuildImprovement(improvement: TileImprovement, tile: TileInfo = currentTile): Boolean {
+        // Constants.workerUnique deprecated since 3.15.5
+        val matchingUniques = getMatchingUniques(Constants.canBuildImprovements) + getMatchingUniques(Constants.workerUnique)
+        return matchingUniques.any { improvement.matchesFilter(it.params[0]) || tile.matchesTerrainFilter(it.params[0]) }
+    }
+
     //endregion
 }

core/src/com/unciv/logic/map/TileInfo.kt

@@ -154,6 +154,7 @@ open class TileInfo {
     fun isImpassible() = getLastTerrain().impassable
 
     fun getTileImprovement(): TileImprovement? = if (improvement == null) null else ruleset.tileImprovements[improvement!!]
+    fun getTileImprovementInProgress(): TileImprovement? = if (improvementInProgress == null) null else ruleset.tileImprovements[improvementInProgress!!]
 
 
     // This is for performance - since we access the neighbors of a tile ALL THE TIME,

core/src/com/unciv/models/UnitAction.kt

@@ -26,7 +26,9 @@ enum class UnitActionType(val value: String) {
     SetUp("Set up"),
     FoundCity("Found city"),
     ConstructImprovement("Construct improvement"),
+    // Deprecated since 3.15.4
         ConstructRoad("Construct road"),
+    //
     Create("Create"),
     HurryResearch("Hurry Research"),
     StartGoldenAge("Start Golden Age"),

core/src/com/unciv/models/ruleset/tile/TileImprovement.kt

@@ -99,7 +99,8 @@ class TileImprovement : NamedStats() {
         return when (filter) {
             name -> true
             "All" -> true
-            "Great Improvement" -> isGreatImprovement()
+            "All Road" -> name == "road" || name == "railroad"
+            "Great Improvement", "Great" -> isGreatImprovement()
             else -> false
         }
     }

core/src/com/unciv/ui/pickerscreens/ImprovementPickerScreen.kt

@@ -8,6 +8,7 @@ import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.badlogic.gdx.utils.Align
 import com.unciv.Constants
 import com.unciv.UncivGame
+import com.unciv.logic.map.MapUnit
 import com.unciv.logic.map.RoadStatus
 import com.unciv.logic.map.TileInfo
 import com.unciv.models.ruleset.tile.TileImprovement
@@ -17,7 +18,7 @@ import com.unciv.ui.utils.*
 import com.unciv.ui.utils.StaticTooltip.Companion.addStaticTip
 import kotlin.math.round
 
-class ImprovementPickerScreen(val tileInfo: TileInfo, val onAccept: ()->Unit) : PickerScreen() {
+class ImprovementPickerScreen(val tileInfo: TileInfo, unit: MapUnit, val onAccept: ()->Unit) : PickerScreen() {
     private var selectedImprovement: TileImprovement? = null
     private val gameInfo = tileInfo.tileMap.gameInfo
     private val ruleSet = gameInfo.ruleSet
@@ -58,6 +59,7 @@ class ImprovementPickerScreen(val tileInfo: TileInfo, val onAccept: ()->Unit) :
             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)) continue
+            if (!unit.canBuildImprovement(improvement)) continue
 
             val improvementButtonTable = Table()
 

core/src/com/unciv/ui/worldscreen/WorldScreen.kt

@@ -478,7 +478,12 @@ class WorldScreen(val gameInfo: GameInfo, val viewingCiv:CivilizationInfo) : Cam
 
         displayTutorial(Tutorial.InjuredUnits) { gameInfo.getCurrentPlayerCivilization().getCivUnits().any { it.health < 100 } }
 
-        displayTutorial(Tutorial.Workers) { gameInfo.getCurrentPlayerCivilization().getCivUnits().any { it.hasUnique(Constants.workerUnique) } }
+        displayTutorial(Tutorial.Workers) { 
+            gameInfo.getCurrentPlayerCivilization().getCivUnits().any { 
+                (it.hasUnique(Constants.canBuildImprovements) || it.hasUnique(Constants.workerUnique)) 
+                    && it.type.isCivilian() 
+            } 
+        }
     }
 
     private fun updateDiplomacyButton(civInfo: CivilizationInfo) {

core/src/com/unciv/ui/worldscreen/unit/UnitActions.kt

@@ -32,8 +32,8 @@ object UnitActions {
 
         if (unit.isMoving()) actionList += UnitAction(UnitActionType.StopMovement) { unit.action = null }
 
-        val workingOnImprovement = unit.hasUnique("Can build improvements on tiles")
+        // Constants.workerUnique deprecated since 3.15.5
-                && unit.currentTile.hasImprovementInProgress()
+        val workingOnImprovement = unit.currentTile.hasImprovementInProgress() && unit.canBuildImprovement(unit.currentTile.getTileImprovementInProgress()!!)
         if (!unit.isFortified() && !unit.canFortify() && unit.currentMovement > 0 && !workingOnImprovement) {
             addSleepActions(actionList, unit, unitTable)
         }
@@ -58,7 +58,9 @@ object UnitActions {
         addSetupAction(unit, actionList)
         addFoundCityAction(unit, actionList, tile)
         addWorkerActions(unit, actionList, tile, worldScreen, unitTable)
+        // Deprecated since 3.15.4
             addConstructRoadsAction(unit, tile, actionList)
+        //
         addCreateWaterImprovements(unit, actionList)
         addGreatPersonActions(unit, actionList, tile)
         actionList += getImprovementConstructionActions(unit, tile)
@@ -122,6 +124,7 @@ object UnitActions {
         return null
     }
     
+    // This entire function is deprecated since 3.15.4, as the 'can construct roads' unique is deprecated
         private fun addConstructRoadsAction(unit: MapUnit, tile: TileInfo, actionList: ArrayList<UnitAction>) {
             val improvement = RoadStatus.Road.improvement(unit.civInfo.gameInfo.ruleSet) ?: return
             if (unit.hasUnique("Can construct roads")
@@ -135,6 +138,7 @@ object UnitActions {
                             tile.turnsToImprovement = improvement.getTurnsToBuild(unit.civInfo)
                         }.takeIf { unit.currentMovement > 0 })
         }
+    //
 
     private fun addFoundCityAction(unit: MapUnit, actionList: ArrayList<UnitAction>, tile: TileInfo) {
         val getFoundCityAction = getFoundCityAction(unit, tile)
@@ -330,7 +334,8 @@ object UnitActions {
     }
 
     private fun addWorkerActions(unit: MapUnit, actionList: ArrayList<UnitAction>, tile: TileInfo, worldScreen: WorldScreen, unitTable: UnitTable) {
-        if (!unit.hasUnique("Can build improvements on tiles")) return
+        // Constants.workerUnique deprecated since 3.15.5
+        if (!unit.hasUnique(Constants.canBuildImprovements) && !unit.hasUnique(Constants.workerUnique)) return
 
         // Allow automate/unautomate when embarked, but not building improvements - see #1963
         if (Constants.unitActionAutomation == unit.action) {
@@ -347,12 +352,12 @@ object UnitActions {
 
         val canConstruct = unit.currentMovement > 0
                 && !tile.isCityCenter()
-                && unit.civInfo.gameInfo.ruleSet.tileImprovements.values.any { tile.canBuildImprovement(it, unit.civInfo) }
+                && unit.civInfo.gameInfo.ruleSet.tileImprovements.values.any { tile.canBuildImprovement(it, unit.civInfo) && unit.canBuildImprovement(it) }
         
         actionList += UnitAction(UnitActionType.ConstructImprovement,
                 isCurrentAction = unit.currentTile.hasImprovementInProgress(),
                 action = {
-                    worldScreen.game.setScreen(ImprovementPickerScreen(tile) { unitTable.selectUnit() })
+                    worldScreen.game.setScreen(ImprovementPickerScreen(tile, unit) { unitTable.selectUnit() })
                 }.takeIf { canConstruct })
     }
 
@@ -418,7 +423,6 @@ object UnitActions {
         }
     }
     
-
     fun getImprovementConstructionActions(unit: MapUnit, tile: TileInfo): ArrayList<UnitAction> {
         val finalActions = ArrayList<UnitAction>()
         for (unique in unit.getMatchingUniques("Can construct []")) {
@@ -442,6 +446,9 @@ object UnitActions {
                         city.cityStats.update()
                         city.civInfo.updateDetailedCivResources()
                     }
+                    // Why is this here? How do we now the unit is actually a great person?
+                    // What if in some mod some unit can construct a certain type of improvement using the "Can construct []" unique?
+                    // That unit does not need to be a great person at all, and yet it would trigger mausoleum of halicarnassus (?) here.
                     addGoldPerGreatPersonUsage(unit.civInfo)
                     unit.destroy()
                 }.takeIf {