Further split between different road automations, some cleanup of the roadTo automation (but not enough, still pretty bad)

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

@@ -29,7 +29,7 @@ object CivilianUnitAutomation {
             return SpecificUnitAutomation.automateSettlerActions(unit, dangerousTiles)
 
         if (unit.isAutomatingRoadConnection())
-            return unit.civ.getWorkerAutomation().roadAutomation.automateConnectRoad(unit, dangerousTiles)
+            return unit.civ.getWorkerAutomation().roadToAutomation.automateConnectRoad(unit, dangerousTiles)
 
         if (unit.cache.hasUniqueToBuildImprovements)
             return unit.civ.getWorkerAutomation().automateWorkerAction(unit, dangerousTiles)

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

@@ -4,28 +4,22 @@ import com.badlogic.gdx.math.Vector2
 import com.unciv.UncivGame
 import com.unciv.logic.city.City
 import com.unciv.logic.civilization.Civilization
-import com.unciv.logic.civilization.NotificationCategory
-import com.unciv.logic.civilization.NotificationIcon
 import com.unciv.logic.map.BFS
 import com.unciv.logic.map.HexMath
-import com.unciv.logic.map.MapPathing
 import com.unciv.logic.map.mapunit.MapUnit
 import com.unciv.logic.map.tile.RoadStatus
 import com.unciv.logic.map.tile.Tile
 import com.unciv.utils.Log
 import com.unciv.utils.debug
 
-
 private object WorkerAutomationConst {
     /** BFS max size is determined by the aerial distance of two cities to connect, padded with this */
     // two tiles longer than the distance to the nearest connected city should be enough as the 'reach' of a BFS is increased by blocked tiles
     const val maxBfsReachPadding = 2
 }
 
-class RoadAutomation(val civInfo: Civilization, cachedForTurn:Int, cloningSource: RoadAutomation? = null) {
+/** Responsible for the "connect cities" automation as part of worker automation */
-
+class RoadBetweenCitiesAutomation(val civInfo: Civilization, cachedForTurn:Int, cloningSource: RoadBetweenCitiesAutomation? = null) {
-    //region Cache
-    private val ruleSet = civInfo.gameInfo.ruleset
 
     /** Caches BFS by city locations (cities needing connecting).
      *
@@ -36,6 +30,7 @@ class RoadAutomation(val civInfo: Civilization, cachedForTurn:Int, cloningSource
     //todo: If BFS were to deal in vectors instead of Tiles, we could copy this on cloning
     private val bfsCache = HashMap<Vector2, BFS>()
 
+
     /** Caches road to build for connecting cities unless option is off or ruleset removed all roads */
     internal val bestRoadAvailable: RoadStatus =
         cloningSource?.bestRoadAvailable ?:
@@ -45,31 +40,6 @@ class RoadAutomation(val civInfo: Civilization, cachedForTurn:Int, cloningSource
             RoadStatus.None
         else civInfo.tech.getBestRoadAvailable()
 
-    /** Same as above, but ignores the option */
-    private val actualBestRoadAvailable: RoadStatus = civInfo.tech.getBestRoadAvailable()
-
-    /** Civ-wide list of unconnected Cities, sorted by closest to capital first */
-    private val citiesThatNeedConnecting: List<City> by lazy {
-        val result = civInfo.cities.asSequence()
-            .filter {
-                civInfo.getCapital() != null
-                    && it.population.population > 3
-                    && !it.isCapital() && !it.isBeingRazed // Cities being razed should not be connected.
-                    && !it.cityStats.isConnectedToCapital(bestRoadAvailable)
-            }.sortedBy {
-                it.getCenterTile().aerialDistanceTo(civInfo.getCapital()!!.getCenterTile())
-            }.toList()
-        if (Log.shouldLog()) {
-            debug("WorkerAutomation citiesThatNeedConnecting for ${civInfo.civName} turn $cachedForTurn:")
-            if (result.isEmpty())
-                debug("\tempty")
-            else result.forEach {
-                debug("\t${it.name}")
-            }
-        }
-        result
-    }
-
     /** Civ-wide list of _connected_ Cities, unsorted */
     private val tilesOfConnectedCities: List<Tile> by lazy {
         val result = civInfo.cities.asSequence()
@@ -93,136 +63,6 @@ class RoadAutomation(val civInfo: Civilization, cachedForTurn:Int, cloningSource
     /** Hashmap of all cached tiles in each list in [roadsToConnectCitiesCache] */
     internal val tilesOfRoadsToConnectCities: HashMap<Tile, City> = HashMap()
 
-    //endregion
-
-    //region Functions
-    /**
-     * Automate the process of connecting a road between two points.
-     * Current thoughts:
-     * Will be a special case of MapUnit.automated property
-     * Unit has new attributes startTile endTile
-     * - We will progress towards the end path sequentially, taking absolute least distance w/o regard for movement cost
-     * - Cancel upon risk of capture
-     * - Cancel upon blocked
-     * - End automation upon finish
-     */
-    // TODO: Caching
-    // TODO: Hide the automate road button if road is not unlocked
-    fun automateConnectRoad(unit: MapUnit, tilesWhereWeWillBeCaptured: Set<Tile>){
-        if (actualBestRoadAvailable == RoadStatus.None) return
-
-        var currentTile = unit.getTile()
-
-        /** Reset side effects from automation, return worker to non-automated state*/
-        fun stopAndCleanAutomation(){
-            unit.automated = false
-            unit.action = null
-            unit.automatedRoadConnectionDestination = null
-            unit.automatedRoadConnectionPath = null
-            currentTile.stopWorkingOnImprovement()
-        }
-
-        if (unit.automatedRoadConnectionDestination == null){
-            stopAndCleanAutomation()
-            return
-        }
-
-        /** Conditions for whether it is acceptable to build a road on this tile */
-        fun shouldBuildRoadOnTile(tile: Tile): Boolean {
-            return !tile.isCityCenter() // Can't build road on city tiles
-                // Special case for civs that treat forest/jungles as roads (inside their territory). We shouldn't build if railroads aren't unlocked.
-                && !(tile.hasConnection(unit.civ) && actualBestRoadAvailable == RoadStatus.Road)
-                // Build (upgrade) if possible
-                && tile.roadStatus != actualBestRoadAvailable
-                // Build if the road is pillaged
-                || tile.roadIsPillaged
-        }
-
-        val destinationTile = unit.civ.gameInfo.tileMap[unit.automatedRoadConnectionDestination!!]
-
-        var pathToDest: List<Vector2>? = unit.automatedRoadConnectionPath
-
-        // The path does not exist, create it
-        if (pathToDest == null) {
-            val foundPath: List<Tile>? = MapPathing.getRoadPath(unit, currentTile, destinationTile)
-            if (foundPath == null) {
-                Log.debug("WorkerAutomation: $unit -> connect road failed")
-                stopAndCleanAutomation()
-                unit.civ.addNotification("Connect road failed!", currentTile.position, NotificationCategory.Units, NotificationIcon.Construction)
-                return
-            }
-
-            pathToDest = foundPath // Convert to a list of positions for serialization
-                .map { it.position }
-
-            unit.automatedRoadConnectionPath = pathToDest
-            debug("WorkerAutomation: $unit -> found connect road path to destination tile: %s, %s", destinationTile, pathToDest)
-        }
-
-        val currTileIndex = pathToDest.indexOf(currentTile.position)
-
-        // The worker was somehow moved off its path, cancel the action
-        if (currTileIndex == -1) {
-            Log.debug("$unit -> was moved off its connect road path. Operation cancelled.")
-            stopAndCleanAutomation()
-            unit.civ.addNotification("Connect road cancelled!", currentTile.position, NotificationCategory.Units, unit.name)
-            return
-        }
-
-        /* Can not build a road on this tile, try to move on.
-        * The worker should search for the next furthest tile in the path that:
-        * - It can move to
-        * - Can be improved/upgraded
-        * */
-        if (unit.currentMovement > 0 && !shouldBuildRoadOnTile(currentTile)) {
-            if (currTileIndex == pathToDest.size - 1) { // The last tile in the path is unbuildable or has a road.
-                stopAndCleanAutomation()
-                unit.civ.addNotification("Connect road completed!", currentTile.position, NotificationCategory.Units, unit.name)
-                return
-            }
-
-            if (currTileIndex < pathToDest.size - 1) { // Try to move to the next tile in the path
-                val tileMap = unit.civ.gameInfo.tileMap
-                var nextTile: Tile = currentTile
-
-                // Create a new list with tiles where the index is greater than currTileIndex
-                val futureTiles = pathToDest.asSequence()
-                    .dropWhile { it != unit.currentTile.position }
-                    .drop(1)
-                    .map { tileMap[it] }
-
-
-
-                for (futureTile in futureTiles) { // Find the furthest tile we can reach in this turn, move to, and does not have a road
-                    if (unit.movement.canReachInCurrentTurn(futureTile) && unit.movement.canMoveTo(futureTile)) { // We can at least move to this tile
-                        nextTile = futureTile
-                        if (shouldBuildRoadOnTile(futureTile)) {
-                            break // Stop on this tile
-                        }
-                    }
-                }
-
-                unit.movement.moveToTile(nextTile)
-                currentTile = unit.getTile()
-            }
-        }
-
-        // We need to check current movement again after we've (potentially) moved
-        if (unit.currentMovement > 0) {
-            // Repair pillaged roads first
-            if (currentTile.roadStatus != RoadStatus.None && currentTile.roadIsPillaged){
-                currentTile.setRepaired()
-                return
-            }
-            if (shouldBuildRoadOnTile(currentTile) && currentTile.improvementInProgress != actualBestRoadAvailable.name) {
-                val improvement = actualBestRoadAvailable.improvement(ruleSet)!!
-                currentTile.startWorkingOnImprovement(improvement, civInfo, unit)
-                return
-            }
-        }
-    }
-
-
 
     /**
      * Uses a cache to find and return the connection to make that is associated with a city.
@@ -265,6 +105,25 @@ class RoadAutomation(val civInfo: Civilization, cachedForTurn:Int, cloningSource
     }
 
 
+    /** Civ-wide list of unconnected Cities, sorted by closest to capital first */
+    private val citiesThatNeedConnecting: List<City> by lazy {
+        val result = civInfo.cities.asSequence()
+            .filter {
+                civInfo.getCapital() != null
+                    && it.population.population > 3
+                    && !it.isCapital() && !it.isBeingRazed // Cities being razed should not be connected.
+                    && !it.cityStats.isConnectedToCapital(bestRoadAvailable)
+            }.sortedBy {
+                it.getCenterTile().aerialDistanceTo(civInfo.getCapital()!!.getCenterTile())
+            }.toList()
+        if (Log.shouldLog()) {
+            debug("WorkerAutomation citiesThatNeedConnecting for ${civInfo.civName} turn $cachedForTurn:")
+            if (result.isEmpty()) debug("\tempty")
+            else result.forEach { debug("\t${it.name}") }
+        }
+        result
+    }
+
     /**
      * Most importantly builds the cache so that [chooseImprovement] knows later what tiles a road should be built on
      * Returns a list of all the cities close by that this worker may want to connect
@@ -315,10 +174,9 @@ class RoadAutomation(val civInfo: Civilization, cachedForTurn:Int, cloningSource
             unit.movement.headTowards(bestTileToConstructRoadOn)
         if (unit.currentMovement > 0 && bestTileToConstructRoadOn == currentTile
             && currentTile.improvementInProgress != bestRoadAvailable.name) {
-            val improvement = bestRoadAvailable.improvement(ruleSet)!!
+            val improvement = bestRoadAvailable.improvement(civInfo.gameInfo.ruleset)!!
             bestTileToConstructRoadOn.startWorkingOnImprovement(improvement, civInfo, unit)
         }
         return true
     }
-    //endregion
 }

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

@@ -0,0 +1,148 @@
+package com.unciv.logic.automation.unit
+
+import com.badlogic.gdx.math.Vector2
+import com.unciv.logic.civilization.Civilization
+import com.unciv.logic.civilization.NotificationCategory
+import com.unciv.logic.civilization.NotificationIcon
+import com.unciv.logic.map.MapPathing
+import com.unciv.logic.map.mapunit.MapUnit
+import com.unciv.logic.map.tile.RoadStatus
+import com.unciv.logic.map.tile.Tile
+import com.unciv.utils.Log
+import com.unciv.utils.debug
+
+
+/** Responsible for automation the "build road to" action
+ * This is *pretty bad code* overall and needs to be cleaned up */
+class RoadToAutomation(val civInfo: Civilization) {
+
+    private val actualBestRoadAvailable: RoadStatus = civInfo.tech.getBestRoadAvailable()
+
+
+    /**
+     * Automate the process of connecting a road between two points.
+     * Current thoughts:
+     * Will be a special case of MapUnit.automated property
+     * Unit has new attributes startTile endTile
+     * - We will progress towards the end path sequentially, taking absolute least distance w/o regard for movement cost
+     * - Cancel upon risk of capture
+     * - Cancel upon blocked
+     * - End automation upon finish
+     */
+    // TODO: Caching
+    // TODO: Hide the automate road button if road is not unlocked
+    fun automateConnectRoad(unit: MapUnit, tilesWhereWeWillBeCaptured: Set<Tile>){
+        if (actualBestRoadAvailable == RoadStatus.None) return
+
+        var currentTile = unit.getTile()
+
+
+        if (unit.automatedRoadConnectionDestination == null){
+            stopAndCleanAutomation(unit)
+            return
+        }
+
+
+        val destinationTile = unit.civ.gameInfo.tileMap[unit.automatedRoadConnectionDestination!!]
+
+        var pathToDest: List<Vector2>? = unit.automatedRoadConnectionPath
+
+        // The path does not exist, create it
+        if (pathToDest == null) {
+            val foundPath: List<Tile>? = MapPathing.getRoadPath(unit, currentTile, destinationTile)
+            if (foundPath == null) {
+                Log.debug("WorkerAutomation: $unit -> connect road failed")
+                stopAndCleanAutomation(unit)
+                unit.civ.addNotification("Connect road failed!", currentTile.position, NotificationCategory.Units, NotificationIcon.Construction)
+                return
+            }
+
+            pathToDest = foundPath // Convert to a list of positions for serialization
+                .map { it.position }
+
+            unit.automatedRoadConnectionPath = pathToDest
+            debug("WorkerAutomation: $unit -> found connect road path to destination tile: %s, %s", destinationTile, pathToDest)
+        }
+
+        val currTileIndex = pathToDest.indexOf(currentTile.position)
+
+        // The worker was somehow moved off its path, cancel the action
+        if (currTileIndex == -1) {
+            Log.debug("$unit -> was moved off its connect road path. Operation cancelled.")
+            stopAndCleanAutomation(unit)
+            unit.civ.addNotification("Connect road cancelled!", currentTile.position, NotificationCategory.Units, unit.name)
+            return
+        }
+
+        /* Can not build a road on this tile, try to move on.
+        * The worker should search for the next furthest tile in the path that:
+        * - It can move to
+        * - Can be improved/upgraded
+        * */
+        if (unit.currentMovement > 0 && !shouldBuildRoadOnTile(currentTile)) {
+            if (currTileIndex == pathToDest.size - 1) { // The last tile in the path is unbuildable or has a road.
+                stopAndCleanAutomation(unit)
+                unit.civ.addNotification("Connect road completed!", currentTile.position, NotificationCategory.Units, unit.name)
+                return
+            }
+
+            if (currTileIndex < pathToDest.size - 1) { // Try to move to the next tile in the path
+                val tileMap = unit.civ.gameInfo.tileMap
+                var nextTile: Tile = currentTile
+
+                // Create a new list with tiles where the index is greater than currTileIndex
+                val futureTiles = pathToDest.asSequence()
+                    .dropWhile { it != unit.currentTile.position }
+                    .drop(1)
+                    .map { tileMap[it] }
+
+
+
+                for (futureTile in futureTiles) { // Find the furthest tile we can reach in this turn, move to, and does not have a road
+                    if (unit.movement.canReachInCurrentTurn(futureTile) && unit.movement.canMoveTo(futureTile)) { // We can at least move to this tile
+                        nextTile = futureTile
+                        if (shouldBuildRoadOnTile(futureTile)) {
+                            break // Stop on this tile
+                        }
+                    }
+                }
+
+                unit.movement.moveToTile(nextTile)
+                currentTile = unit.getTile()
+            }
+        }
+
+        // We need to check current movement again after we've (potentially) moved
+        if (unit.currentMovement > 0) {
+            // Repair pillaged roads first
+            if (currentTile.roadStatus != RoadStatus.None && currentTile.roadIsPillaged){
+                currentTile.setRepaired()
+                return
+            }
+            if (shouldBuildRoadOnTile(currentTile) && currentTile.improvementInProgress != actualBestRoadAvailable.name) {
+                val improvement = actualBestRoadAvailable.improvement(civInfo.gameInfo.ruleset)!!
+                currentTile.startWorkingOnImprovement(improvement, civInfo, unit)
+                return
+            }
+        }
+    }
+
+    /** Reset side effects from automation, return worker to non-automated state*/
+    fun stopAndCleanAutomation(unit: MapUnit){
+        unit.automated = false
+        unit.action = null
+        unit.automatedRoadConnectionDestination = null
+        unit.automatedRoadConnectionPath = null
+        unit.currentTile.stopWorkingOnImprovement()
+    }
+
+
+    /** Conditions for whether it is acceptable to build a road on this tile */
+    fun shouldBuildRoadOnTile(tile: Tile): Boolean {
+        if (tile.roadIsPillaged) return true
+        return !tile.isCityCenter() // Can't build road on city tiles
+            // Special case for civs that treat forest/jungles as roads (inside their territory). We shouldn't build if railroads aren't unlocked.
+            && !(tile.hasConnection(civInfo) && actualBestRoadAvailable == RoadStatus.Road)
+            && tile.roadStatus != actualBestRoadAvailable // Build (upgrade) if possible
+    }
+}

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

@@ -39,7 +39,9 @@ class WorkerAutomation(
 ) {
     ///////////////////////////////////////// Cached data /////////////////////////////////////////
 
-    val roadAutomation:RoadAutomation = RoadAutomation(civInfo, cachedForTurn, cloningSource?.roadAutomation)
+    val roadToAutomation:RoadToAutomation = RoadToAutomation(civInfo)
+    val roadBetweenCitiesAutomation:RoadBetweenCitiesAutomation = RoadBetweenCitiesAutomation(civInfo, cachedForTurn, cloningSource?.roadBetweenCitiesAutomation)
+
     private val ruleSet = civInfo.gameInfo.ruleset
 
 
@@ -70,7 +72,7 @@ class WorkerAutomation(
     fun automateWorkerAction(unit: MapUnit, dangerousTiles: HashSet<Tile>) {
         val currentTile = unit.getTile()
         // Must be called before any getPriority checks to guarantee the local road cache is processed
-        val citiesToConnect = roadAutomation.getNearbyCitiesToConnect(unit)
+        val citiesToConnect = roadBetweenCitiesAutomation.getNearbyCitiesToConnect(unit)
         // Shortcut, we are working a good tile (like resource) and don't need to check for other tiles to work
         if (!dangerousTiles.contains(currentTile) && getFullPriority(unit.getTile(), unit) >= 10
             && currentTile.improvementInProgress != null) {
@@ -149,7 +151,7 @@ class WorkerAutomation(
         }
 
         // Nothing to do, try again to connect cities
-        if (civInfo.stats.statsForNextTurn.gold > 10 && roadAutomation.tryConnectingCities(unit, citiesToConnect)) return
+        if (civInfo.stats.statsForNextTurn.gold > 10 && roadBetweenCitiesAutomation.tryConnectingCities(unit, citiesToConnect)) return
 
 
         debug("WorkerAutomation: %s -> nothing to do", unit.toString())
@@ -230,7 +232,7 @@ class WorkerAutomation(
                 && !civInfo.hasResource(tile.resource!!))
                 priority += 2
         }
-        if (tile in roadAutomation.tilesOfRoadsToConnectCities) priority += when {
+        if (tile in roadBetweenCitiesAutomation.tilesOfRoadsToConnectCities) priority += when {
                 civInfo.stats.statsForNextTurn.gold <= 5 -> 0
                 civInfo.stats.statsForNextTurn.gold <= 10 -> 1
                 civInfo.stats.statsForNextTurn.gold <= 30 -> 2
@@ -369,15 +371,15 @@ class WorkerAutomation(
         val improvement = ruleSet.tileImprovements[improvementName]!!
 
         // Add the value of roads if we want to build it here
-        if (improvement.isRoad() && roadAutomation.bestRoadAvailable.improvement(ruleSet) == improvement
+        if (improvement.isRoad() && roadBetweenCitiesAutomation.bestRoadAvailable.improvement(ruleSet) == improvement
-            && tile in roadAutomation.tilesOfRoadsToConnectCities) {
+            && tile in roadBetweenCitiesAutomation.tilesOfRoadsToConnectCities) {
             var value = 1f
-            val city = roadAutomation.tilesOfRoadsToConnectCities[tile]!!
+            val city = roadBetweenCitiesAutomation.tilesOfRoadsToConnectCities[tile]!!
             if (civInfo.stats.statsForNextTurn.gold >= 20)
             // Bigger cities have a higher priority to connect
                 value += (city.population.population - 3) * .3f
             // Higher priority if we are closer to connecting the city
-            value += (5 - roadAutomation.roadsToConnectCitiesCache[city]!!.size).coerceAtLeast(0)
+            value += (5 - roadBetweenCitiesAutomation.roadsToConnectCitiesCache[city]!!.size).coerceAtLeast(0)
             return value
         }