Support more freely modded Worker-like units (#6339)

* TileImprovementTime UniqueType supports UniqueTarget.Unit

* Reduce UniqueType.ConstructImprovementConsumingUnit hardcoded Great People behaviour

* Some linting

Co-authored-by: Yair Morgenstern <yairm210@hotmail.com>

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

@@ -146,7 +146,7 @@ class WorkerAutomation(
             && tileCanBeImproved(unit, currentTile)) {
             if (WorkerAutomationConst.consoleOutput)
                 println("WorkerAutomation: ${unit.label()} -> start improving $currentTile")
-            return currentTile.startWorkingOnImprovement(chooseImprovement(unit, currentTile)!!, civInfo)
+            return currentTile.startWorkingOnImprovement(chooseImprovement(unit, currentTile)!!, civInfo, unit)
         }
 
         if (currentTile.improvementInProgress != null) return // we're working!
@@ -206,7 +206,7 @@ class WorkerAutomation(
                 BFS(toConnectTile, isCandidateTilePredicate).apply {
                     maxSize = HexMath.getNumberOfTilesInHexagon(
                         WorkerAutomationConst.maxBfsReachPadding +
-                        tilesOfConnectedCities.map { it.aerialDistanceTo(toConnectTile) }.minOrNull()!!
+                            tilesOfConnectedCities.minOf { it.aerialDistanceTo(toConnectTile) }
                     )
                     bfsCache[toConnectTile.position] = this@apply
                 }
@@ -234,7 +234,7 @@ class WorkerAutomation(
                     if (unit.currentMovement > 0 && currentTile == tileToConstructRoadOn
                         && currentTile.improvementInProgress != bestRoadAvailable.name) {
                         val improvement = bestRoadAvailable.improvement(ruleSet)!!
-                        tileToConstructRoadOn.startWorkingOnImprovement(improvement, civInfo)
+                        tileToConstructRoadOn.startWorkingOnImprovement(improvement, civInfo, unit)
                     }
                     if (WorkerAutomationConst.consoleOutput)
                         println("WorkerAutomation: ${unit.label()} -> connect city ${bfs.startingPoint.getCity()?.name} to ${cityTile.getCity()!!.name} on $tileToConstructRoadOn")

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

@@ -193,12 +193,12 @@ open class TileInfo {
     // We have to .toList() so that the values are stored together once for caching,
     // and the toSequence so that aggregations (like neighbors.flatMap{it.units} don't take up their own space
 
-    /** Returns the left shared neighbor of [this] and [neighbor] (relative to the view direction [this]->[neighbor]), or null if there is no such tile. */
+    /** Returns the left shared neighbor of `this` and [neighbor] (relative to the view direction `this`->[neighbor]), or null if there is no such tile. */
     fun getLeftSharedNeighbor(neighbor: TileInfo): TileInfo? {
         return tileMap.getClockPositionNeighborTile(this,(tileMap.getNeighborTileClockPosition(this, neighbor) - 2) % 12)
     }
 
-    /** Returns the right shared neighbor [this] and [neighbor] (relative to the view direction [this]->[neighbor]), or null if there is no such tile. */
+    /** Returns the right shared neighbor of `this` and [neighbor] (relative to the view direction `this`->[neighbor]), or null if there is no such tile. */
     fun getRightSharedNeighbor(neighbor: TileInfo): TileInfo? {
         return tileMap.getClockPositionNeighborTile(this,(tileMap.getNeighborTileClockPosition(this, neighbor) + 2) % 12)
     }
@@ -263,7 +263,7 @@ open class TileInfo {
     fun getTileStats(city: CityInfo?, observingCiv: CivilizationInfo?): Stats {
         var stats = getBaseTerrain().cloneStats()
 
-        val stateForConditionals = StateForConditionals(civInfo = observingCiv, cityInfo = city, tile = this);
+        val stateForConditionals = StateForConditionals(civInfo = observingCiv, cityInfo = city, tile = this)
 
         for (terrainFeatureBase in terrainFeatureObjects) {
             when {
@@ -897,9 +897,10 @@ open class TileInfo {
         }
     }
 
-    fun startWorkingOnImprovement(improvement: TileImprovement, civInfo: CivilizationInfo) {
+    fun startWorkingOnImprovement(improvement: TileImprovement, civInfo: CivilizationInfo, unit: MapUnit) {
         improvementInProgress = improvement.name
-        turnsToImprovement = if (civInfo.gameInfo.gameParameters.godMode) 1 else improvement.getTurnsToBuild(civInfo)
+        turnsToImprovement = if (civInfo.gameInfo.gameParameters.godMode) 1
+            else improvement.getTurnsToBuild(civInfo, unit)
     }
 
     fun stopWorkingOnImprovement() {

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

@@ -2,10 +2,12 @@ package com.unciv.models.ruleset.tile
 
 import com.unciv.UncivGame
 import com.unciv.logic.civilization.CivilizationInfo
+import com.unciv.logic.map.MapUnit
 import com.unciv.logic.map.RoadStatus
 import com.unciv.models.ruleset.Belief
 import com.unciv.models.ruleset.Ruleset
 import com.unciv.models.ruleset.RulesetStatsObject
+import com.unciv.models.ruleset.unique.StateForConditionals
 import com.unciv.models.ruleset.unique.UniqueTarget
 import com.unciv.models.ruleset.unique.UniqueType
 import com.unciv.models.translations.tr
@@ -24,14 +26,14 @@ class TileImprovement : RulesetStatsObject() {
     val turnsToBuild: Int = 0 // This is the base cost.
 
 
-    fun getTurnsToBuild(civInfo: CivilizationInfo): Int {
+    fun getTurnsToBuild(civInfo: CivilizationInfo, unit: MapUnit?): Int {
-        var realTurnsToBuild = turnsToBuild.toFloat() * civInfo.gameInfo.gameParameters.gameSpeed.modifier
+        val state = StateForConditionals(civInfo, unit = unit)
-        for (unique in civInfo.getMatchingUniques(UniqueType.TileImprovementTime)) {
+        val uniques = civInfo.getMatchingUniques(UniqueType.TileImprovementTime, state) +
-            realTurnsToBuild *= unique.params[0].toPercent()
+            (unit?.getMatchingUniques(UniqueType.TileImprovementTime, state) ?: sequenceOf())
-        }
+        return uniques.fold(turnsToBuild.toFloat() * civInfo.gameInfo.gameParameters.gameSpeed.modifier) {
+            it, unique -> it * unique.params[0].toPercent()
+        }.roundToInt().coerceAtLeast(1)
         // In some weird cases it was possible for something to take 0 turns, leading to it instead never finishing
-        if (realTurnsToBuild < 1) realTurnsToBuild = 1f
-        return realTurnsToBuild.roundToInt()
     }
 
     fun getDescription(ruleset: Ruleset): String {

core/src/com/unciv/models/ruleset/unique/UniqueType.kt

@@ -242,6 +242,7 @@ enum class UniqueType(val text: String, vararg targets: UniqueTarget, val flags:
 
     // Acts as a trigger - this should be generalized somehow but the current setup does not allow this
     // It would currently mean cycling through EVERY unique type to find ones with a specific conditional...
+    @Suppress("SpellCheckingInspection")  // Not worth fixing
     RecieveFreeUnitWhenDiscoveringTech("Receive free [baseUnitFilter] when you discover [tech]", UniqueTarget.Global),
 
     EnablesOpenBorders("Enables Open Borders agreements", UniqueTarget.Global),
@@ -253,7 +254,7 @@ enum class UniqueType(val text: String, vararg targets: UniqueTarget, val flags:
 
     BetterDefensiveBuildings("[amount]% City Strength from defensive buildings", UniqueTarget.Global),
 
-    TileImprovementTime("[amount]% tile improvement construction time", UniqueTarget.Global),
+    TileImprovementTime("[amount]% tile improvement construction time", UniqueTarget.Global, UniqueTarget.Unit),
     PercentGoldFromTradeMissions("[amount]% Gold from Great Merchant trade missions", UniqueTarget.Global),
     // Todo: Lowercase the 'U' of 'Units' in this unique
     CityHealingUnits("[mapUnitFilter] Units adjacent to this city heal [amount] HP per turn when healing", UniqueTarget.Global, UniqueTarget.FollowerBelief),
@@ -274,7 +275,6 @@ enum class UniqueType(val text: String, vararg targets: UniqueTarget, val flags:
     EmbarkAndEnterOcean("Can embark and move over Coasts and Oceans immediately", UniqueTarget.Global),
 
     PopulationLossFromNukes("Population loss from nuclear attacks [amount]% [cityFilter]", UniqueTarget.Global),
-    
     NaturalReligionSpreadStrength("[amount]% Natural religion spread [cityFilter]", UniqueTarget.FollowerBelief, UniqueTarget.Global),
     ReligionSpreadDistance("Religion naturally spreads to cities [amount] tiles away", UniqueTarget.Global, UniqueTarget.FollowerBelief),
 

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

@@ -16,7 +16,11 @@ import com.unciv.ui.utils.*
 import com.unciv.ui.utils.UncivTooltip.Companion.addTooltip
 import kotlin.math.roundToInt
 
-class ImprovementPickerScreen(val tileInfo: TileInfo, unit: MapUnit, val onAccept: ()->Unit) : PickerScreen() {
+class ImprovementPickerScreen(
+    private val tileInfo: TileInfo,
+    private val unit: MapUnit,
+    private val onAccept: ()->Unit
+) : PickerScreen() {
     private var selectedImprovement: TileImprovement? = null
     private val gameInfo = tileInfo.tileMap.gameInfo
     private val ruleSet = gameInfo.ruleSet
@@ -32,8 +36,8 @@ class ImprovementPickerScreen(val tileInfo: TileInfo, unit: MapUnit, val onAccep
             // no onAccept() - Worker can stay selected
         } else {
             if (improvement.name != tileInfo.improvementInProgress)
-                tileInfo.startWorkingOnImprovement(improvement, currentPlayerCiv)
+                tileInfo.startWorkingOnImprovement(improvement, currentPlayerCiv, unit)
-            if (tileInfo.civilianUnit != null) tileInfo.civilianUnit!!.action = null // this is to "wake up" the worker if it's sleeping
+            unit.action = null // this is to "wake up" the worker if it's sleeping
             onAccept()
         }
         game.setWorldScreen()
@@ -54,23 +58,20 @@ class ImprovementPickerScreen(val tileInfo: TileInfo, unit: MapUnit, val onAccep
 
         // clone tileInfo without "top" feature if it could be removed
         // Keep this copy around for speed
-        val tileInfoNoLast:TileInfo = tileInfo.clone()
+        val tileInfoNoLast: TileInfo = tileInfo.clone()
-        if (ruleSet.tileImprovements.any { it.key == Constants.remove + tileInfoNoLast.getLastTerrain().name }) {
+        if (Constants.remove + tileInfoNoLast.getLastTerrain().name in ruleSet.tileImprovements) {
             tileInfoNoLast.removeTerrainFeature(tileInfoNoLast.getLastTerrain().name)
         }
 
         for (improvement in ruleSet.tileImprovements.values) {
-            var suggestRemoval:Boolean = false
+            var suggestRemoval = false
             // 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)) {
+                if (!tileInfoNoLast.canBuildImprovement(improvement, currentPlayerCiv)) continue
-                    suggestRemoval = true
+                suggestRemoval = true
-                } else {
-                    continue
-                }
             }
             if (!unit.canBuildImprovement(improvement)) continue
 
@@ -94,7 +95,7 @@ class ImprovementPickerScreen(val tileInfo: TileInfo, unit: MapUnit, val onAccep
 
             var labelText = improvement.name.tr()
             val turnsToBuild = if (tileInfo.improvementInProgress == improvement.name) tileInfo.turnsToImprovement
-            else improvement.getTurnsToBuild(currentPlayerCiv)
+                else improvement.getTurnsToBuild(currentPlayerCiv, unit)
             if (turnsToBuild > 0) labelText += " - $turnsToBuild${Fonts.turn}"
             val provideResource = tileInfo.hasViewableResource(currentPlayerCiv) && tileInfo.tileResource.improvement == improvement.name
             if (provideResource) labelText += "\n" + "Provides [${tileInfo.resource}]".tr()
@@ -104,12 +105,11 @@ class ImprovementPickerScreen(val tileInfo: TileInfo, unit: MapUnit, val onAccep
                     && improvement.name != Constants.cancelImprovementOrder)
             if (tileInfo.improvement != null && removeImprovement) labelText += "\n" + "Replaces [${tileInfo.improvement}]".tr()
 
-            val pickNow = if (suggestRemoval)
+            val pickNow = when {
-                (Constants.remove + "[" + tileInfo.getLastTerrain().name + "] first").toLabel()
+                suggestRemoval -> "${Constants.remove}[${tileInfo.getLastTerrain().name}] first".toLabel()
-            else if (tileInfo.improvementInProgress != improvement.name)
+                tileInfo.improvementInProgress != improvement.name -> "Pick now!".toLabel().onClick { accept(improvement) }
-                "Pick now!".toLabel().onClick { accept(improvement) }
+                else -> "Current construction".toLabel()
-            else
+            }
-                "Current construction".toLabel()
 
             val statIcons = getStatIconsTable(provideResource, removeImprovement)
 

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

@@ -635,8 +635,14 @@ object UnitActions {
                 title = "Create [$improvementName]",
                 action = {
                     val unitTile = unit.getTile()
-                    for (terrainFeature in tile.terrainFeatures.filter { unitTile.ruleset.tileImprovements.containsKey("Remove $it") })
+                    unitTile.setTerrainFeatures(
-                        unitTile.removeTerrainFeature(terrainFeature)// remove forest/jungle/marsh
+                        // 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.improvement = improvementName
                     unitTile.improvementInProgress = null
                     unitTile.turnsToImprovement = 0
@@ -647,7 +653,8 @@ object UnitActions {
                         city.cityStats.update()
                         city.civInfo.updateDetailedCivResources()
                     }
-                    addStatsPerGreatPersonUsage(unit)
+                    if (unit.isGreatPerson())
+                        addStatsPerGreatPersonUsage(unit)
                     unit.destroy()
                 }.takeIf {
                     resourcesAvailable
@@ -703,7 +710,7 @@ object UnitActions {
             otherCiv.addNotification("[${unit.civInfo}] has stolen your territory!", unit.currentTile.position, unit.civInfo.civName, NotificationIcon.War)
     }
 
-    fun addStatsPerGreatPersonUsage(unit: MapUnit) {
+    private fun addStatsPerGreatPersonUsage(unit: MapUnit) {
         if (!unit.isGreatPerson()) return
 
         val civInfo = unit.civInfo
@@ -782,7 +789,7 @@ object UnitActions {
         if (getGiftAction != null) actionList += getGiftAction
     }
 
-    fun getGiftAction(unit: MapUnit, tile: TileInfo): UnitAction? {
+    private fun getGiftAction(unit: MapUnit, tile: TileInfo): UnitAction? {
         val recipient = tile.getOwner()
         // We need to be in another civs territory.
         if (recipient == null || recipient.isCurrentPlayer()) return null
@@ -824,7 +831,7 @@ object UnitActions {
         return UnitAction(UnitActionType.GiftUnit, action = giftAction)
     }
     
-    fun addTriggerUniqueActions(unit: MapUnit, actionList: ArrayList<UnitAction>){
+    private fun addTriggerUniqueActions(unit: MapUnit, actionList: ArrayList<UnitAction>){
         for (unique in unit.getUniques()) {
             if (!unique.conditionals.any { it.type == UniqueType.ConditionalConsumeUnit }) continue
             val unitAction = UnitAction(type = UnitActionType.TriggerUnique, unique.text){