diff --git a/core/src/com/unciv/logic/automation/unit/SpecificUnitAutomation.kt b/core/src/com/unciv/logic/automation/unit/SpecificUnitAutomation.kt index 488e99bcc2..02e7254b0a 100644 --- a/core/src/com/unciv/logic/automation/unit/SpecificUnitAutomation.kt +++ b/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) diff --git a/core/src/com/unciv/logic/automation/unit/UnitAutomation.kt b/core/src/com/unciv/logic/automation/unit/UnitAutomation.kt index 623812fa3b..d05cb3608f 100644 --- a/core/src/com/unciv/logic/automation/unit/UnitAutomation.kt +++ b/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) && diff --git a/core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt b/core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt index 53d8454f62..87d3a4b906 100644 --- a/core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt +++ b/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 - && tileCanBeImproved(unit, currentTile)) { + if (currentTile.improvementInProgress == null && 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() @@ -267,7 +272,7 @@ class WorkerAutomation( */ private fun findTileToWork(unit: MapUnit, tilesToAvoid: Set): 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 - // These are the expensive calculations (tileCanBeImproved, canReach), so we only apply these filters after everything else it done. - val selectedTile = - workableTiles.sortedByDescending { tile -> unit.getDamageFromTerrain(tile) <= 0 }.firstOrNull { unit.movement.canReach(it) && (tileCanBeImproved(unit, it) || it.isPillaged()) } - ?: return currentTile + // If there are non-damage dealing tiles available, move to the best of those, otherwise move to the best damage dealing tile + val workableTilesPrioritized = workableTilesCenterFirst + .sortedWith( + compareBy { unit.getDamageFromTerrain(it) > 0 } // Sort on Boolean puts false first + .thenByDescending { getPriority(it) } + ) - return if ((!tileCanBeImproved(unit, currentTile) && !currentTile.isPillaged()) // current tile is unimprovable - || !workableTiles.contains(currentTile) // current tile is unworkable by city - || getPriority(selectedTile) > getPriority(currentTile)) // current tile is less important + // These are the expensive calculations (tileCanBeImproved, canReach), so we only apply these filters after everything else it done. + val selectedTile = workableTilesPrioritized + .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()) - return false + //todo This is wrong but works for Alpha Frontier, because the unit has both: + // 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 + } } diff --git a/core/src/com/unciv/logic/map/mapunit/MapUnitCache.kt b/core/src/com/unciv/logic/map/mapunit/MapUnitCache.kt index 367505965f..bf5ac1779b 100644 --- a/core/src/com/unciv/logic/map/mapunit/MapUnitCache.kt +++ b/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) diff --git a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActions.kt b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActions.kt index ffc6b4ab2e..d37bcb9867 100644 --- a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActions.kt +++ b/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()