Standardized 'create improvement' uniques - need to remember to deprecate the water improvements!

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

@@ -52,13 +52,13 @@ object SpecificUnitAutomation {
         if (tileToSteal != null) {
             unit.movement.headTowards(tileToSteal)
             if (unit.currentMovement > 0 && unit.currentTile == tileToSteal)
-                UnitActionsFromUniques.getImprovementConstructionActions(unit, unit.currentTile).firstOrNull()?.action?.invoke()
+                UnitActionsFromUniques.getImprovementConstructionActionsFromGeneralUnique(unit, unit.currentTile).firstOrNull()?.action?.invoke()
             return true
         }
 
         // try to build a citadel for defensive purposes
         if (unit.civ.getWorkerAutomation().evaluateFortPlacement(unit.currentTile, true)) {
-            UnitActionsFromUniques.getImprovementConstructionActions(unit, unit.currentTile).firstOrNull()?.action?.invoke()
+            UnitActionsFromUniques.getImprovementConstructionActionsFromGeneralUnique(unit, unit.currentTile).firstOrNull()?.action?.invoke()
             return true
         }
         return false
@@ -92,7 +92,7 @@ object SpecificUnitAutomation {
         }
         unit.movement.headTowards(tileForCitadel)
         if (unit.currentMovement > 0 && unit.currentTile == tileForCitadel)
-            UnitActionsFromUniques.getImprovementConstructionActions(unit, unit.currentTile)
+            UnitActionsFromUniques.getImprovementConstructionActionsFromGeneralUnique(unit, unit.currentTile)
                 .firstOrNull()?.action?.invoke()
     }
 
@@ -131,7 +131,7 @@ object SpecificUnitAutomation {
 
 
         // If the tile we are currently on is close to the best tile, then lets just settle here instead
-        if (bestTilesInfo.tileRankMap.containsKey(unit.getTile()) 
+        if (bestTilesInfo.tileRankMap.containsKey(unit.getTile())
                 && (bestTilesInfo.bestTile == null || bestTilesInfo.tileRankMap[unit.getTile()]!! >= bestTilesInfo.tileRankMap[bestTilesInfo.bestTile]!! - 10)) {
                 bestCityLocation = unit.getTile()
         }
@@ -242,7 +242,7 @@ object SpecificUnitAutomation {
             if (unit.currentTile == chosenTile) {
                 if (unit.currentTile.isPillaged())
                     UnitActions.invokeUnitAction(unit, UnitActionType.Repair)
-                else UnitActions.invokeUnitAction(unit, UnitActionType.Create)
+                else UnitActions.invokeUnitAction(unit, UnitActionType.CreateImprovement)
                 return true
             }
             return unitTileBeforeMovement != unit.currentTile

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

@@ -25,7 +25,7 @@ open class UnitAction(
         if (type.imageGetter != null)
             return type.imageGetter.invoke()
         return when (type) {
-            UnitActionType.Create -> {
+            UnitActionType.CreateImprovement -> {
                 ImageGetter.getImprovementPortrait(title.getPlaceholderParameters()[0])
             }
             UnitActionType.SpreadReligion -> {
@@ -139,7 +139,7 @@ enum class UnitActionType(
         { ImageGetter.getUnitActionPortrait("ConstructImprovement") }, false),
     Repair(Constants.repair,
         { ImageGetter.getUnitActionPortrait("Repair") }, UncivSound.Construction),
-    Create("Create",
+    CreateImprovement("Create",
         null, false, UncivSound.Chimes),
     HurryResearch("{Hurry Research} (${Fonts.death})",
         { ImageGetter.getUnitActionPortrait("HurryResearch") }, UncivSound.Chimes),

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

@@ -322,14 +322,13 @@ enum class UniqueType(
 
     FoundCity("Founds a new city", UniqueTarget.UnitAction),
     ConstructImprovementInstantly("Can instantly construct a [improvementFilter] improvement", UniqueTarget.UnitAction),
+    CreateWaterImprovements("May create improvements on water resources", UniqueTarget.Unit),
+    BuildImprovements("Can build [improvementFilter/terrainFilter] improvements on tiles", UniqueTarget.Unit),
     CanSpreadReligion("Can Spread Religion", UniqueTarget.UnitAction),
     CanRemoveHeresy("Can remove other religions from cities", UniqueTarget.UnitAction),
     MayFoundReligion("May found a religion", UniqueTarget.UnitAction),
     MayEnhanceReligion("May enhance a religion", UniqueTarget.UnitAction),
 
-    BuildImprovements("Can build [improvementFilter/terrainFilter] improvements on tiles", UniqueTarget.Unit),
-    CreateWaterImprovements("May create improvements on water resources", UniqueTarget.Unit),
-
     AddInCapital("Can be added to [comment] in the Capital", UniqueTarget.Unit),
     PreventSpreadingReligion("Prevents spreading of religion to the city it is next to", UniqueTarget.Unit),
     RemoveOtherReligions("Removes other religions when spreading religion", UniqueTarget.Unit),

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

@@ -46,7 +46,8 @@ object UnitActions {
         UnitActionType.HurryBuilding to UnitActionsGreatPerson::getHurryBuildingActions,
         UnitActionType.ConductTradeMission to UnitActionsGreatPerson::getConductTradeMissionActions,
         UnitActionType.FoundReligion to UnitActionsReligion::getFoundReligionActions,
-        UnitActionType.EnhanceReligion to UnitActionsReligion::getEnhanceReligionActions
+        UnitActionType.EnhanceReligion to UnitActionsReligion::getEnhanceReligionActions,
+        UnitActionType.CreateImprovement to UnitActionsFromUniques::getImprovementCreationActions
     )
 
     private fun getNormalActions(unit: MapUnit): List<UnitAction> {
@@ -57,8 +58,6 @@ object UnitActions {
             actionList.addAll(getActionsFunction(unit, tile))
 
         // Determined by unit uniques
-        UnitActionsFromUniques.addCreateWaterImprovements(unit, actionList)
-        actionList += UnitActionsFromUniques.getImprovementConstructionActions(unit, tile)
         UnitActionsReligion.addSpreadReligionActions(unit, actionList)
         UnitActionsReligion.addRemoveHeresyActions(unit, actionList)
 

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

@@ -26,28 +26,6 @@ import com.unciv.ui.screens.pickerscreens.ImprovementPickerScreen
 
 object UnitActionsFromUniques {
 
-    fun addCreateWaterImprovements(unit: MapUnit, actionList: ArrayList<UnitAction>) {
-        val waterImprovementAction = getWaterImprovementAction(unit)
-        if (waterImprovementAction != null) actionList += waterImprovementAction
-    }
-
-    fun getWaterImprovementAction(unit: MapUnit): UnitAction? {
-        val tile = unit.currentTile
-        if (!tile.isWater || !unit.hasUnique(UniqueType.CreateWaterImprovements) || tile.resource == null) return null
-
-        val improvementName = tile.tileResource.getImprovingImprovement(tile, unit.civ) ?: return null
-        val improvement = tile.ruleset.tileImprovements[improvementName] ?: return null
-        if (!tile.improvementFunctions.canBuildImprovement(improvement, unit.civ)) return null
-
-        return UnitAction(UnitActionType.Create, "Create [$improvementName]",
-            action = {
-                tile.changeImprovement(improvementName, unit.civ, unit)
-                unit.destroy()  // Modders may wish for a nondestructive way, but that should be another Unique
-            }.takeIf { unit.currentMovement > 0 })
-    }
-
-
-
     fun getFoundCityActions(unit: MapUnit, tile: Tile): List<UnitAction> {
         val getFoundCityAction = getFoundCityAction(unit, tile) ?: return emptyList()
         return listOf(getFoundCityAction)
@@ -225,7 +203,29 @@ object UnitActionsFromUniques {
         actionList += getAddInCapitalAction(unit, tile)
     }
 
-    fun getImprovementConstructionActions(unit: MapUnit, tile: Tile): ArrayList<UnitAction> {
+
+    fun getImprovementCreationActions(unit: MapUnit, tile: Tile) = sequence {
+        val waterImprovementAction = getWaterImprovementAction(unit)
+        if (waterImprovementAction != null) yield(waterImprovementAction)
+        yieldAll(getImprovementConstructionActionsFromGeneralUnique(unit, tile))
+    }.asIterable()
+
+    fun getWaterImprovementAction(unit: MapUnit): UnitAction? {
+        val tile = unit.currentTile
+        if (!tile.isWater || !unit.hasUnique(UniqueType.CreateWaterImprovements) || tile.resource == null) return null
+
+        val improvementName = tile.tileResource.getImprovingImprovement(tile, unit.civ) ?: return null
+        val improvement = tile.ruleset.tileImprovements[improvementName] ?: return null
+        if (!tile.improvementFunctions.canBuildImprovement(improvement, unit.civ)) return null
+
+        return UnitAction(UnitActionType.CreateImprovement, "Create [$improvementName]",
+            action = {
+                tile.changeImprovement(improvementName, unit.civ, unit)
+                unit.destroy()  // Modders may wish for a nondestructive way, but that should be another Unique
+            }.takeIf { unit.currentMovement > 0 })
+    }
+
+    fun getImprovementConstructionActionsFromGeneralUnique(unit: MapUnit, tile: Tile): ArrayList<UnitAction> {
         val finalActions = ArrayList<UnitAction>()
         val uniquesToCheck = UnitActionModifiers.getUsableUnitActionUniques(unit, UniqueType.ConstructImprovementInstantly)
 
@@ -248,7 +248,7 @@ object UnitActionsFromUniques {
                     (civResources[improvementUnique.params[1]] ?: 0) < improvementUnique.params[0].toInt()
             }
 
-            finalActions += UnitAction(UnitActionType.Create,
+            finalActions += UnitAction(UnitActionType.CreateImprovement,
                 title = UnitActionModifiers.actionTextWithSideEffects(
                     "Create [$improvementName]",
                     unique,

tests/src/com/unciv/uniques/UnitUniquesTests.kt

@@ -66,7 +66,7 @@ class UnitUniquesTests {
         val unit = game.addUnit("Great Engineer", civ, unitTile)
         unit.currentMovement = unit.baseUnit.movement.toFloat()  // Required!
         val actionsWithoutIron = try {
-            UnitActionsFromUniques.getImprovementConstructionActions(unit, unitTile)
+            UnitActionsFromUniques.getImprovementConstructionActionsFromGeneralUnique(unit, unitTile)
         } catch (ex: Throwable) {
             // Give that IndexOutOfBoundsException a nicer name
             Assert.fail("getImprovementConstructionActions throws Exception ${ex.javaClass.simpleName}")
@@ -88,7 +88,7 @@ class UnitUniquesTests {
         Assert.assertTrue("Test preparation failed to add Iron to Civ resources", ironAvailable >= 3)
 
         // See if that same Engineer could create a Manufactory NOW
-        val actionsWithIron = UnitActionsFromUniques.getImprovementConstructionActions(unit, unitTile)
+        val actionsWithIron = UnitActionsFromUniques.getImprovementConstructionActionsFromGeneralUnique(unit, unitTile)
             .filter { it.action != null }
         Assert.assertFalse("Great Engineer SHOULD be able to create a Manufactory modded to require Iron once Iron is available",
             actionsWithIron.isEmpty())