Minimal support for Alpha Frontier-like Workers (#9690)

* Linting

* Cache hasUnique(UniqueType.CreateWaterImprovements)

* Move automateWorkBoats to WorkerAutomation - for readability and better chances to create synergies

* Add a chance for automated Alpha Frontier Workers to create Water improvements

* Reorg random271's Lemming code a little

* Tune Worker automation for buildable Water improvements

core/src/com/unciv/logic/automation/unit/SpecificUnitAutomation.kt

@@ -6,7 +6,6 @@ import com.unciv.logic.battle.Battle
 import com.unciv.logic.battle.GreatGeneralImplementation
 import com.unciv.logic.battle.MapUnitCombatant
 import com.unciv.logic.city.City
-import com.unciv.logic.civilization.Civilization
 import com.unciv.logic.civilization.diplomacy.DiplomaticModifiers
 import com.unciv.logic.map.mapunit.MapUnit
 import com.unciv.logic.map.tile.Tile
@@ -20,32 +19,6 @@ import com.unciv.ui.screens.worldscreen.unit.actions.UnitActionsReligion
 
 object SpecificUnitAutomation {
 
-    private fun hasWorkableSeaResource(tile: Tile, civInfo: Civilization): Boolean =
-            tile.isWater && tile.improvement == null && tile.hasViewableResource(civInfo)
-
-    fun automateWorkBoats(unit: MapUnit) {
-        val closestReachableResource = unit.civ.cities.asSequence()
-                .flatMap { city -> city.getWorkableTiles() }
-                .filter {
-                    hasWorkableSeaResource(it, unit.civ)
-                            && (unit.currentTile == it || unit.movement.canMoveTo(it))
-                }
-                .sortedBy { it.aerialDistanceTo(unit.currentTile) }
-                .firstOrNull { unit.movement.canReach(it) }
-
-        when (closestReachableResource) {
-            null -> UnitAutomation.tryExplore(unit)
-            else -> {
-                unit.movement.headTowards(closestReachableResource)
-
-                // could be either fishing boats or oil well
-                val isImprovable = closestReachableResource.tileResource.getImprovements().any()
-                if (isImprovable && unit.currentTile == closestReachableResource)
-                    UnitActions.getWaterImprovementAction(unit)?.action?.invoke()
-            }
-        }
-    }
-
     fun automateGreatGeneral(unit: MapUnit): Boolean {
         //try to follow nearby units. Do not garrison in city if possible
         val maxAffectedTroopsTile = GreatGeneralImplementation.getBestAffectedTroopsTile(unit)

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

@@ -251,6 +251,12 @@ object UnitAutomation {
         if (unit.cache.hasUniqueToBuildImprovements)
             return unit.civ.getWorkerAutomation().automateWorkerAction(unit, tilesWhereWeWillBeCaptured)
 
+        if (unit.cache.hasUniqueToCreateWaterImprovements){
+            if (!unit.civ.getWorkerAutomation().automateWorkBoats(unit))
+                tryExplore(unit)
+            return
+        }
+
         if (unit.hasUnique(UniqueType.MayFoundReligion)
                 && unit.civ.religionManager.religionState < ReligionState.Religion
                 && unit.civ.religionManager.mayFoundReligionAtAll(unit)
@@ -263,9 +269,6 @@ object UnitAutomation {
         )
             return SpecificUnitAutomation.enhanceReligion(unit)
 
-        if (unit.hasUnique(UniqueType.CreateWaterImprovements))
-            return SpecificUnitAutomation.automateWorkBoats(unit)
-
         // We try to add any unit in the capital we can, though that might not always be desirable
         // For now its a simple option to allow AI to win a science victory again
         if (unit.hasUnique(UniqueType.AddInCapital))
@@ -693,7 +696,7 @@ object UnitAutomation {
     }
 
     private fun tryTakeBackCapturedCity(unit: MapUnit): Boolean {
-        var capturedCities = unit.civ.getKnownCivs().asSequence()
+        var capturedCities = unit.civ.getKnownCivs() // This is a Sequence
                 .flatMap { it.cities.asSequence() }
                 .filter {
                     unit.civ.isAtWarWith(it.civ) &&

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

@@ -121,7 +121,7 @@ class WorkerAutomation(
         val currentTile = unit.getTile()
         val tileToWork = findTileToWork(unit, tilesWhereWeWillBeCaptured)
 
-        if (civInfo.getWorkerAutomation().getPriority(tileToWork) < 3) { // building roads is more important
+        if (getPriority(tileToWork) < 3) { // building roads is more important
             if (tryConnectingCities(unit)) return
         }
 
@@ -155,13 +155,18 @@ class WorkerAutomation(
             return
         }
 
-        if (currentTile.improvementInProgress == null && currentTile.isLand
+        if (currentTile.improvementInProgress == null && tileCanBeImproved(unit, currentTile)) {
-            && tileCanBeImproved(unit, currentTile)) {
             debug("WorkerAutomation: ${unit.label()} -> start improving $currentTile")
             return currentTile.startWorkingOnImprovement(chooseImprovement(unit, currentTile)!!, civInfo, unit)
         }
 
         if (currentTile.improvementInProgress != null) return // we're working!
+
+        if (unit.cache.hasUniqueToCreateWaterImprovements) {
+            // Support Alpha Frontier-Style Workers that _also_ have the "May create improvements on water resources" unique
+            if (automateWorkBoats(unit)) return
+        }
+
         if (tryConnectingCities(unit)) return //nothing to do, try again to connect cities
 
         val citiesToNumberOfUnimprovedTiles = HashMap<String, Int>()
@@ -267,7 +272,7 @@ class WorkerAutomation(
      */
     private fun findTileToWork(unit: MapUnit, tilesToAvoid: Set<Tile>): Tile {
         val currentTile = unit.getTile()
-        val workableTiles = currentTile.getTilesInDistance(4)
+        val workableTilesCenterFirst = currentTile.getTilesInDistance(4)
                 .filter {
                     it !in tilesToAvoid
                     && (it.civilianUnit == null || it == currentTile)
@@ -278,18 +283,29 @@ class WorkerAutomation(
                     && it.getTilesInDistance(3)  // don't work in range of enemy units
                         .none { tile -> tile.militaryUnit != null && tile.militaryUnit!!.civ.isAtWarWith(civInfo)}
                 }
-                .sortedByDescending { getPriority(it) }
 
         // Carthage can move through mountains, special case
-        // If there is a non-damage dealing tile available, move to that tile, otherwise move to the damage dealing tile
+        // If there are non-damage dealing tiles available, move to the best of those, otherwise move to the best damage dealing tile
-        // These are the expensive calculations (tileCanBeImproved, canReach), so we only apply these filters after everything else it done.
+        val workableTilesPrioritized = workableTilesCenterFirst
-        val selectedTile =
+                .sortedWith(
-            workableTiles.sortedByDescending { tile -> unit.getDamageFromTerrain(tile) <= 0 }.firstOrNull { unit.movement.canReach(it) && (tileCanBeImproved(unit, it) || it.isPillaged()) }
+                    compareBy<Tile> { unit.getDamageFromTerrain(it) > 0 } // Sort on Boolean puts false first
-                ?: return currentTile
+                        .thenByDescending { getPriority(it) }
+                )
 
-        return if ((!tileCanBeImproved(unit, currentTile) && !currentTile.isPillaged()) // current tile is unimprovable
+        // These are the expensive calculations (tileCanBeImproved, canReach), so we only apply these filters after everything else it done.
-                    || !workableTiles.contains(currentTile) // current tile is unworkable by city
+        val selectedTile = workableTilesPrioritized
-                    || getPriority(selectedTile) > getPriority(currentTile))  // current tile is less important
+            .firstOrNull { unit.movement.canReach(it) && (tileCanBeImproved(unit, it) || it.isPillaged()) }
+            ?: return currentTile
+
+        // Note: workableTiles is a Sequence, and we oiginally used workableTiles.contains for the second
+        // test, which looks like a second potentially deep iteration of it, after just being iterated
+        // for selectedTile. But TileMap.getTilesInDistanceRange iterates its range forward, meaning
+        // currentTile is always the very first entry of the _unsorted_ Sequence - if it is still
+        // contained at all and not dropped by the filters - which is the point here.
+        return if ( currentTile == selectedTile  // No choice
+                || (!tileCanBeImproved(unit, currentTile) && !currentTile.isPillaged()) // current tile is unimprovable
+                || workableTilesCenterFirst.firstOrNull() != currentTile  // current tile is unworkable by city
+                || getPriority(selectedTile) > getPriority(currentTile))  // current tile is less important
             selectedTile
         else currentTile
     }
@@ -299,8 +315,12 @@ class WorkerAutomation(
      * (but does not check whether the ruleset contains any unit capable of it)
      */
     private fun tileCanBeImproved(unit: MapUnit, tile: Tile): Boolean {
-        if (!tile.isLand || tile.isImpassible() || tile.isCityCenter())
+        //todo This is wrong but works for Alpha Frontier, because the unit has both:
-            return false
+        // It should test for the build over time ability, but this tests the create and die ability
+        if (!tile.isLand && !unit.cache.hasUniqueToCreateWaterImprovements) return false
+        // Allow outlandish mods having non-road improvements on Mountains
+        if (tile.isImpassible() && !unit.cache.canPassThroughImpassableTiles) return false
+        if (tile.isCityCenter()) return false
 
         val city = tile.getCity()
         if (city == null || city.civ != civInfo)
@@ -518,4 +538,43 @@ class WorkerAutomation(
         return distanceBetweenCities + 2 > distanceToEnemy + distanceToOurCity
     }
 
+    private fun hasWorkableSeaResource(tile: Tile, civInfo: Civilization): Boolean =
+        tile.isWater && tile.improvement == null && tile.hasViewableResource(civInfo)
+
+    /** Try improving a Water Resource
+     *
+     *  No logic to avoid capture by enemies yet!
+     *
+     *  @return Whether any progress was made (improved a tile or at least moved towards an opportunity)
+     */
+    fun automateWorkBoats(unit: MapUnit): Boolean {
+        val closestReachableResource = unit.civ.cities.asSequence()
+            .flatMap { city -> city.getWorkableTiles() }
+            .filter {
+                hasWorkableSeaResource(it, unit.civ)
+                    && (unit.currentTile == it || unit.movement.canMoveTo(it))
+            }
+            .sortedBy { it.aerialDistanceTo(unit.currentTile) }
+            .firstOrNull { unit.movement.canReach(it) }
+            ?: return false
+
+        // could be either fishing boats or oil well
+        val isImprovable = closestReachableResource.tileResource.getImprovements().any()
+        if (!isImprovable) return false
+
+        unit.movement.headTowards(closestReachableResource)
+        if (unit.currentTile != closestReachableResource) return true // moving counts as progress
+
+        // We know we have the CreateWaterImprovements, but not whether
+        // all conditionals succeed with a current StateForConditionals(civ, unit)
+        // todo: Not necessarily the optimal flow: Be optimistic and head towards,
+        //       then when arrived and the conditionals say "no" do something else instead?
+        val action = UnitActions.getWaterImprovementAction(unit)
+            ?: return false
+
+        // If action.action is null that means only transient reasons prevent the improvement -
+        // report progress and hope next run it will work.
+        action.action?.invoke()
+        return true
+    }
 }

core/src/com/unciv/logic/map/mapunit/MapUnitCache.kt

@@ -65,6 +65,7 @@ class MapUnitCache(private val mapUnit: MapUnit) {
     var paradropRange = 0
 
     var hasUniqueToBuildImprovements = false    // not canBuildImprovements to avoid confusion
+    var hasUniqueToCreateWaterImprovements = false
 
     var hasStrengthBonusInRadiusUnique = false
 
@@ -113,6 +114,8 @@ class MapUnitCache(private val mapUnit: MapUnit) {
         )
 
         hasUniqueToBuildImprovements = mapUnit.hasUnique(UniqueType.BuildImprovements)
+        hasUniqueToCreateWaterImprovements = mapUnit.hasUnique(UniqueType.CreateWaterImprovements)
+
         canEnterForeignTerrain = mapUnit.hasUnique(UniqueType.CanEnterForeignTiles)
                 || mapUnit.hasUnique(UniqueType.CanEnterForeignTilesButLosesReligiousStrength)
 

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

@@ -180,11 +180,11 @@ object UnitActions {
       */
     fun getFoundCityAction(unit: MapUnit, tile: Tile): UnitAction? {
         val unique = unit.getMatchingUniques(UniqueType.FoundCity)
-            .filter { it.conditionals.none { it.type == UniqueType.UnitActionExtraLimitedTimes } }
+            .filter { unique -> unique.conditionals.none { it.type == UniqueType.UnitActionExtraLimitedTimes } }
             .firstOrNull()
         if (unique == null || tile.isWater || tile.isImpassible()) return null
         // Spain should still be able to build Conquistadors in a one city challenge - but can't settle them
-        if (unit.civ.isOneCityChallenger() && unit.civ.hasEverOwnedOriginalCapital == true) return null
+        if (unit.civ.isOneCityChallenger() && unit.civ.hasEverOwnedOriginalCapital) return null
         if (usagesLeft(unit, unique)==0) return null
 
         if (unit.currentMovement <= 0 || !tile.canBeSettled())
@@ -745,7 +745,7 @@ object UnitActions {
     fun getMaxUsages(unit: MapUnit, actionUnique: Unique): Int? {
         val extraTimes = unit.getMatchingUniques(actionUnique.type!!)
             .filter { it.text.removeConditionals() == actionUnique.text.removeConditionals() }
-            .flatMap { it.conditionals.filter { it.type == UniqueType.UnitActionExtraLimitedTimes } }
+            .flatMap { unique -> unique.conditionals.filter { it.type == UniqueType.UnitActionExtraLimitedTimes } }
             .map { it.params[0].toInt() }
             .sum()