From 77c12fcae4e3ad908a3503d737e35627e0345e4d Mon Sep 17 00:00:00 2001 From: SomeTroglodyte <63000004+SomeTroglodyte@users.noreply.github.com> Date: Sun, 21 Jan 2024 18:14:54 +0100 Subject: [PATCH] Unit actions builders use sequence (#10966) * Change actionTypeToFunctions signature to use Sequence * Linting and a little left pad for the unit action buttons --- .../src/com/unciv/ui/images/IconTextButton.kt | 8 +- .../worldscreen/unit/actions/UnitActions.kt | 199 +++++++++--------- .../unit/actions/UnitActionsFromUniques.kt | 84 ++++---- .../unit/actions/UnitActionsGreatPerson.kt | 21 +- .../unit/actions/UnitActionsPillage.kt | 11 +- .../unit/actions/UnitActionsReligion.kt | 46 ++-- .../unit/actions/UnitActionsTable.kt | 9 +- .../unit/actions/UnitActionsUpgrade.kt | 10 +- .../src/com/unciv/uniques/UnitUniquesTests.kt | 6 +- 9 files changed, 197 insertions(+), 197 deletions(-) diff --git a/core/src/com/unciv/ui/images/IconTextButton.kt b/core/src/com/unciv/ui/images/IconTextButton.kt index 394cd77880..5340c0f857 100644 --- a/core/src/com/unciv/ui/images/IconTextButton.kt +++ b/core/src/com/unciv/ui/images/IconTextButton.kt @@ -15,7 +15,7 @@ import com.unciv.ui.screens.basescreen.BaseScreen * * @param text Text of the button. * @property icon If non-null, [Actor] instance for icon left of the label. - * @param fontSize Text size for [String.toLabel]. + * @param fontSize Text size for [String.toLabel]. Also used to size the [icon]. * @param fontColor Text colour for [String.toLabel]. */ open class IconTextButton( @@ -24,13 +24,13 @@ open class IconTextButton( fontSize: Int = Constants.defaultFontSize, fontColor: Color = Color.WHITE ): Button(BaseScreen.skin) { - /** [Label] instance produced by and with content and formatting as specified to [String.toLabel]. */ + /** [Label] instance produced by, and with content and formatting as specified in [String.toLabel]. */ val label = text.toLabel(fontColor, fontSize, hideIcons = true) // Since by definition we already have an icon - /** Table cell containing the [icon] if any, or `null`. */ + /** Table cell containing the [icon] if any, or `null` (that is, when no [icon] was supplied, the Cell will exist but have no Actor). */ val iconCell: Cell = if (icon != null) { val size = fontSize.toFloat() - icon.setSize(size,size) + icon.setSize(size, size) icon.setOrigin(Align.center) add(icon).size(size).padRight(size / 3) } else { 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 b24819a3d4..254db9ed91 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 @@ -10,29 +10,49 @@ import com.unciv.models.UnitAction import com.unciv.models.UnitActionType import com.unciv.models.ruleset.unique.UniqueType import com.unciv.models.translations.tr +import com.unciv.ui.components.extensions.yieldIfNotNull import com.unciv.ui.popups.ConfirmPopup import com.unciv.ui.popups.hasOpenPopups import com.unciv.ui.screens.pickerscreens.PromotionPickerScreen +import com.unciv.ui.screens.worldscreen.unit.actions.UnitActions.getGiftAction +import com.unciv.ui.screens.worldscreen.unit.actions.UnitActions.getUnitActions +import com.unciv.ui.screens.worldscreen.unit.actions.UnitActions.invokeUnitAction +/** + * Manages creation of [UnitAction] instances. + * + * API for UI: [getUnitActions] + * API for Automation: [invokeUnitAction] + * API for unit tests: [getGiftAction] + */ object UnitActions { - fun getUnitActions(unit: MapUnit): List { + fun getUnitActions(unit: MapUnit): Sequence { return if (unit.showAdditionalActions) getAdditionalActions(unit) else getNormalActions(unit) } - /** Returns whether the action was invoked */ + /** + * Get an instance of [UnitAction] of the [unitActionType] type for [unit] and execute its [action][UnitAction.action], if enabled. + * + * Includes optimization for direct creation of the needed instance type, falls back to enumerating [getUnitActions] to look for the given type. + * + * @return whether the action was invoked */ fun invokeUnitAction(unit: MapUnit, unitActionType: UnitActionType): Boolean { - val unitAction = if (unitActionType in actionTypeToFunctions) actionTypeToFunctions[unitActionType]!!.invoke(unit, unit.getTile()) - .firstOrNull { it.action != null } - else getNormalActions(unit).firstOrNull { it.type == unitActionType && it.action != null } - ?: getAdditionalActions(unit).firstOrNull { it.type == unitActionType && it.action != null } + val unitAction = + if (unitActionType in actionTypeToFunctions) + actionTypeToFunctions[unitActionType]!! // we have a mapped getter... + .invoke(unit, unit.getTile()) // ...call it to get a collection... + .firstOrNull { it.action != null } // ...then take the first enabled one. + else + (getNormalActions(unit) + getAdditionalActions(unit)) // No mapped getter: Enumerate all... + .firstOrNull { it.type == unitActionType && it.action != null } // ...and take first enabled one to match the type. val internalAction = unitAction?.action ?: return false internalAction.invoke() return true } - private val actionTypeToFunctions = linkedMapOf Iterable>( + private val actionTypeToFunctions = linkedMapOf Sequence>( // Determined by unit uniques UnitActionType.Transform to UnitActionsFromUniques::getTransformActions, UnitActionType.Paradrop to UnitActionsFromUniques::getParadropActions, @@ -50,77 +70,72 @@ object UnitActions { UnitActionType.FoundReligion to UnitActionsReligion::getFoundReligionActions, UnitActionType.EnhanceReligion to UnitActionsReligion::getEnhanceReligionActions, UnitActionType.CreateImprovement to UnitActionsFromUniques::getImprovementCreationActions, - UnitActionType.SpreadReligion to UnitActionsReligion::addSpreadReligionActions, + UnitActionType.SpreadReligion to UnitActionsReligion::getSpreadReligionActions, UnitActionType.RemoveHeresy to UnitActionsReligion::getRemoveHeresyActions, UnitActionType.TriggerUnique to UnitActionsFromUniques::getTriggerUniqueActions, UnitActionType.AddInCapital to UnitActionsFromUniques::getAddInCapitalActions ) - fun shouldAutomationBePrimaryAction(unit:MapUnit) = unit.cache.hasUniqueToBuildImprovements || unit.hasUnique(UniqueType.AutomationPrimaryAction) + private fun shouldAutomationBePrimaryAction(unit:MapUnit) = unit.cache.hasUniqueToBuildImprovements || unit.hasUnique(UniqueType.AutomationPrimaryAction) - private fun getNormalActions(unit: MapUnit): List { + private fun getNormalActions(unit: MapUnit) = sequence { val tile = unit.getTile() - val actionList = ArrayList() + // Actions standardized with a directly callable invokeUnitAction for (getActionsFunction in actionTypeToFunctions.values) - actionList.addAll(getActionsFunction(unit, tile)) + yieldAll(getActionsFunction(unit, tile)) // General actions if (shouldAutomationBePrimaryAction(unit)) - actionList += getAutomateActions(unit, unit.currentTile) + addAutomateActions(unit) if (unit.isMoving()) - actionList += UnitAction(UnitActionType.StopMovement) { unit.action = null } + yield(UnitAction(UnitActionType.StopMovement) { unit.action = null }) if (unit.isExploring()) - actionList += UnitAction(UnitActionType.StopExploration) { unit.action = null } + yield(UnitAction(UnitActionType.StopExploration) { unit.action = null }) if (unit.isAutomated()) - actionList += UnitAction(UnitActionType.StopAutomation) { + yield(UnitAction(UnitActionType.StopAutomation) { unit.action = null unit.automated = false - } + }) - actionList += getPromoteActions(unit, unit.currentTile) - actionList += UnitActionsUpgrade.getUnitUpgradeActions(unit, unit.currentTile) - actionList += UnitActionsPillage.getPillageActions(unit, unit.currentTile) + addPromoteActions(unit) + yieldAll(UnitActionsUpgrade.getUnitUpgradeActions(unit, tile)) + yieldAll(UnitActionsPillage.getPillageActions(unit, tile)) - actionList += getSleepActions(unit, tile) - actionList += getSleepUntilHealedActions(unit, tile) + addSleepActions(unit, tile) + addSleepUntilHealedActions(unit, tile) - addFortifyActions(actionList, unit, false) + addFortifyActions(unit, false) - if (unit.isMilitary()) actionList += getExplorationActions(unit, unit.currentTile) + if (unit.isMilitary()) + addExplorationActions(unit) - addWaitAction(unit, actionList) + addWaitAction(unit) - addToggleActionsAction(unit, actionList) - - return actionList + addToggleActionsAction(unit) } - private fun getAdditionalActions(unit: MapUnit): List { - val tile = unit.getTile() - val actionList = ArrayList() - + private fun getAdditionalActions(unit: MapUnit) = sequence { if (unit.isMoving()) { - actionList += UnitAction(UnitActionType.ShowUnitDestination) { + yield(UnitAction(UnitActionType.ShowUnitDestination) { GUI.getMap().setCenterPosition(unit.getMovementDestination().position, true) - } + }) } - addFortifyActions(actionList, unit, true) + addFortifyActions(unit, true) if (!shouldAutomationBePrimaryAction(unit)) - actionList += getAutomateActions(unit, unit.currentTile) + addAutomateActions(unit) - addSwapAction(unit, actionList) - addDisbandAction(actionList, unit) - addGiftAction(unit, actionList, tile) - if (unit.isCivilian()) actionList += getExplorationActions(unit, unit.currentTile) + addSwapAction(unit) + addDisbandAction(unit) + addGiftAction(unit, unit.getTile()) + if (unit.isCivilian()) + addExplorationActions(unit) - addToggleActionsAction(unit, actionList) - - return actionList + addToggleActionsAction(unit) } - private fun addSwapAction(unit: MapUnit, actionList: ArrayList) { + private suspend fun SequenceScope.addSwapAction(unit: MapUnit) { val worldScreen = GUI.getWorldScreen() // Air units cannot swap if (unit.baseUnit.movesLikeAirUnits()) return @@ -133,7 +148,7 @@ object UnitActions { if (worldScreen.bottomUnitTable.selectedUnits.size > 1) return // Only show the swap action if there is at least one possible swap movement if (unit.movement.getUnitSwappableTiles().none()) return - actionList += UnitAction( + yield(UnitAction( type = UnitActionType.SwapUnits, isCurrentAction = worldScreen.bottomUnitTable.selectedUnitIsSwapping, action = { @@ -141,12 +156,12 @@ object UnitActions { !worldScreen.bottomUnitTable.selectedUnitIsSwapping worldScreen.shouldUpdate = true } - ) + )) } - private fun addDisbandAction(actionList: ArrayList, unit: MapUnit) { + private suspend fun SequenceScope.addDisbandAction(unit: MapUnit) { val worldScreen = GUI.getWorldScreen() - actionList += UnitAction(type = UnitActionType.DisbandUnit, action = { + yield(UnitAction(type = UnitActionType.DisbandUnit, action = { if (!worldScreen.hasOpenPopups()) { val disbandText = if (unit.currentTile.getOwner() == unit.civ) "Disband this unit for [${unit.baseUnit.getDisbandGold(unit.civ)}] gold?".tr() @@ -159,43 +174,39 @@ object UnitActions { worldScreen.switchToNextUnit() }.open() } - }.takeIf { unit.currentMovement > 0 }) + }.takeIf { unit.currentMovement > 0 })) } - private fun getPromoteActions(unit: MapUnit, tile: Tile): List { - if (unit.isCivilian() || !unit.promotions.canBePromoted()) return listOf() + private suspend fun SequenceScope.addPromoteActions(unit: MapUnit) { + if (unit.isCivilian() || !unit.promotions.canBePromoted()) return // promotion does not consume movement points, but is not allowed if a unit has exhausted its movement or has attacked - return listOf(UnitAction(UnitActionType.Promote, + yield(UnitAction(UnitActionType.Promote, action = { UncivGame.Current.pushScreen(PromotionPickerScreen(unit)) }.takeIf { unit.currentMovement > 0 && unit.attacksThisTurn == 0 } )) } - private fun getExplorationActions(unit: MapUnit, tile: Tile): List { - if (unit.baseUnit.movesLikeAirUnits()) return listOf() - if (unit.isExploring()) return listOf() - return listOf(UnitAction(UnitActionType.Explore) { + private suspend fun SequenceScope.addExplorationActions(unit: MapUnit) { + if (unit.baseUnit.movesLikeAirUnits()) return + if (unit.isExploring()) return + yield(UnitAction(UnitActionType.Explore) { unit.action = UnitActionType.Explore.value if (unit.currentMovement > 0) UnitAutomation.automatedExplore(unit) }) } - private fun addFortifyActions( - actionList: ArrayList, - unit: MapUnit, - showingAdditionalActions: Boolean - ) { + private suspend fun SequenceScope.addFortifyActions(unit: MapUnit, showingAdditionalActions: Boolean) { if (unit.isFortified() && !showingAdditionalActions) { - actionList += UnitAction( + yield(UnitAction( type = if (unit.isActionUntilHealed()) UnitActionType.FortifyUntilHealed else UnitActionType.Fortify, isCurrentAction = true, title = "${"Fortification".tr()} ${unit.getFortificationTurns() * 20}%" - ) + )) return } @@ -206,40 +217,39 @@ object UnitActions { val isDamaged = unit.health < 100 if (isDamaged && !showingAdditionalActions && unit.rankTileForHealing(unit.currentTile) != 0) - actionList += UnitAction(UnitActionType.FortifyUntilHealed, - action = { unit.fortifyUntilHealed() }.takeIf { !unit.isFortifyingUntilHealed() }) + yield(UnitAction(UnitActionType.FortifyUntilHealed, + action = { unit.fortifyUntilHealed() }.takeIf { !unit.isFortifyingUntilHealed() } + )) else if (isDamaged || !showingAdditionalActions) - actionList += UnitAction(UnitActionType.Fortify, - action = { unit.fortify() }.takeIf { !isFortified }) + yield(UnitAction(UnitActionType.Fortify, + action = { unit.fortify() }.takeIf { !isFortified } + )) } - fun shouldHaveSleepAction(unit: MapUnit, tile: Tile): Boolean { + private fun shouldHaveSleepAction(unit: MapUnit, tile: Tile): Boolean { if (unit.isFortified() || unit.canFortify() || unit.currentMovement == 0f) return false - if (tile.hasImprovementInProgress() - && unit.canBuildImprovement(tile.getTileImprovementInProgress()!!) - ) return false - return true + return !(tile.hasImprovementInProgress() + && unit.canBuildImprovement(tile.getTileImprovementInProgress()!!)) } - private fun getSleepActions(unit: MapUnit, tile: Tile): List { - if (!shouldHaveSleepAction(unit, tile)) return listOf() - if (unit.health < 100) return listOf() - return listOf(UnitAction(UnitActionType.Sleep, + private suspend fun SequenceScope.addSleepActions(unit: MapUnit, tile: Tile) { + if (!shouldHaveSleepAction(unit, tile)) return + if (unit.health < 100) return + yield(UnitAction(UnitActionType.Sleep, action = { unit.action = UnitActionType.Sleep.value }.takeIf { !unit.isSleeping() } )) } - private fun getSleepUntilHealedActions(unit: MapUnit, tile: Tile): List { - if (!shouldHaveSleepAction(unit, tile)) return listOf() - if (unit.health == 100) return listOf() - return listOf(UnitAction(UnitActionType.SleepUntilHealed, + private suspend fun SequenceScope.addSleepUntilHealedActions(unit: MapUnit, tile: Tile) { + if (!shouldHaveSleepAction(unit, tile)) return + if (unit.health == 100) return + yield(UnitAction(UnitActionType.SleepUntilHealed, action = { unit.action = UnitActionType.SleepUntilHealed.value } .takeIf { !unit.isSleepingUntilHealed() && unit.canHealInCurrentTile() } )) } - private fun addGiftAction(unit: MapUnit, actionList: ArrayList, tile: Tile) { - val getGiftAction = getGiftAction(unit, tile) - if (getGiftAction != null) actionList += getGiftAction + private suspend fun SequenceScope.addGiftAction(unit: MapUnit, tile: Tile) { + yieldIfNotNull(getGiftAction(unit, tile)) } fun getGiftAction(unit: MapUnit, tile: Tile): UnitAction? { @@ -294,13 +304,9 @@ object UnitActions { return UnitAction(UnitActionType.GiftUnit, action = giftAction) } - private fun getAutomateActions( - unit: MapUnit, - tile: Tile - ): List { - - if (unit.isAutomated()) return listOf() - return listOf(UnitAction(UnitActionType.Automate, + private suspend fun SequenceScope.addAutomateActions(unit: MapUnit) { + if (unit.isAutomated()) return + yield(UnitAction(UnitActionType.Automate, isCurrentAction = unit.isAutomated(), action = { // Temporary, for compatibility - we want games serialized *moving through old versions* to come out the other end with units still automated @@ -311,26 +317,25 @@ object UnitActions { )) } - private fun addWaitAction(unit: MapUnit, actionList: ArrayList) { - actionList += UnitAction( + private suspend fun SequenceScope.addWaitAction(unit: MapUnit) { + yield(UnitAction( type = UnitActionType.Wait, action = { unit.due = false GUI.getWorldScreen().switchToNextUnit() } - ) + )) } - private fun addToggleActionsAction(unit: MapUnit, actionList: ArrayList) { - actionList += UnitAction( + private suspend fun SequenceScope.addToggleActionsAction(unit: MapUnit) { + yield(UnitAction( type = if (unit.showAdditionalActions) UnitActionType.HideAdditionalActions else UnitActionType.ShowAdditionalActions, action = { unit.showAdditionalActions = !unit.showAdditionalActions GUI.getWorldScreen().bottomUnitTable.update() } - ) + )) } - } diff --git a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsFromUniques.kt b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsFromUniques.kt index d7ad7d8382..471ae19a01 100644 --- a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsFromUniques.kt +++ b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsFromUniques.kt @@ -24,12 +24,10 @@ import com.unciv.ui.components.fonts.Fonts import com.unciv.ui.popups.ConfirmPopup import com.unciv.ui.screens.pickerscreens.ImprovementPickerScreen +@Suppress("UNUSED_PARAMETER") // These methods are used as references in UnitActions.actionTypeToFunctions and need identical signature object UnitActionsFromUniques { - fun getFoundCityActions(unit: MapUnit, tile: Tile): List { - val getFoundCityAction = getFoundCityAction(unit, tile) ?: return emptyList() - return listOf(getFoundCityAction) - } + internal fun getFoundCityActions(unit: MapUnit, tile: Tile) = sequenceOf(getFoundCityAction(unit, tile)).filterNotNull() /** Produce a [UnitAction] for founding a city. * @param unit The unit to do the founding. @@ -39,7 +37,7 @@ object UnitActionsFromUniques { * The [action][UnitAction.action] field will be null if the action cannot be done here and now * (no movement left, too close to another city). */ - fun getFoundCityAction(unit: MapUnit, tile: Tile): UnitAction? { + internal fun getFoundCityAction(unit: MapUnit, tile: Tile): UnitAction? { val unique = UnitActionModifiers.getUsableUnitActionUniques(unit, UniqueType.FoundCity) .firstOrNull() ?: return null @@ -117,10 +115,10 @@ object UnitActionsFromUniques { return if(leadersWePromisedNotToSettleNear.isEmpty()) null else leadersWePromisedNotToSettleNear.joinToString(", ") } - fun getSetupActions(unit: MapUnit, tile: Tile): List { - if (!unit.hasUnique(UniqueType.MustSetUp) || unit.isEmbarked()) return emptyList() + internal fun getSetupActions(unit: MapUnit, tile: Tile): Sequence { + if (!unit.hasUnique(UniqueType.MustSetUp) || unit.isEmbarked()) return emptySequence() val isSetUp = unit.isSetUpForSiege() - return listOf(UnitAction(UnitActionType.SetUp, + return sequenceOf(UnitAction(UnitActionType.SetUp, isCurrentAction = isSetUp, action = { unit.action = UnitActionType.SetUp.value @@ -129,12 +127,12 @@ object UnitActionsFromUniques { ) } - fun getParadropActions(unit: MapUnit, tile: Tile): List { + internal fun getParadropActions(unit: MapUnit, tile: Tile): Sequence { val paradropUniques = unit.getMatchingUniques(UniqueType.MayParadrop) - if (!paradropUniques.any() || unit.isEmbarked()) return emptyList() + if (!paradropUniques.any() || unit.isEmbarked()) return emptySequence() unit.cache.paradropRange = paradropUniques.maxOfOrNull { it.params[0] }!!.toInt() - return listOf(UnitAction(UnitActionType.Paradrop, + return sequenceOf(UnitAction(UnitActionType.Paradrop, isCurrentAction = unit.isPreparingParadrop(), action = { if (unit.isPreparingParadrop()) unit.action = null @@ -147,11 +145,11 @@ object UnitActionsFromUniques { ) } - fun getAirSweepActions(unit: MapUnit, tile: Tile): List { + internal fun getAirSweepActions(unit: MapUnit, tile: Tile): Sequence { val airsweepUniques = unit.getMatchingUniques(UniqueType.CanAirsweep) - if (!airsweepUniques.any()) return emptyList() - return listOf(UnitAction(UnitActionType.AirSweep, + if (!airsweepUniques.any()) return emptySequence() + return sequenceOf(UnitAction(UnitActionType.AirSweep, isCurrentAction = unit.isPreparingAirSweep(), action = { if (unit.isPreparingAirSweep()) unit.action = null @@ -161,7 +159,8 @@ object UnitActionsFromUniques { } )) } - fun getTriggerUniqueActions(unit: MapUnit, tile: Tile) = sequence { + + internal fun getTriggerUniqueActions(unit: MapUnit, tile: Tile) = sequence { for (unique in unit.getUniques()) { // not a unit action if (unique.conditionals.none { it.type?.targetTypes?.contains(UniqueTarget.UnitActionModifier) == true }) continue @@ -183,11 +182,11 @@ object UnitActionsFromUniques { UnitActionModifiers.activateSideEffects(unit, unique) }) } - }.asIterable() + } - fun getAddInCapitalActions(unit: MapUnit, tile: Tile): List { - if (!unit.hasUnique(UniqueType.AddInCapital)) return listOf() - return listOf(UnitAction(UnitActionType.AddInCapital, + internal fun getAddInCapitalActions(unit: MapUnit, tile: Tile): Sequence { + if (!unit.hasUnique(UniqueType.AddInCapital)) return emptySequence() + return sequenceOf(UnitAction(UnitActionType.AddInCapital, title = "Add to [${ unit.getMatchingUniques(UniqueType.AddInCapital).first().params[0] }]", @@ -201,13 +200,13 @@ object UnitActionsFromUniques { )) } - fun getImprovementCreationActions(unit: MapUnit, tile: Tile) = sequence { + internal fun getImprovementCreationActions(unit: MapUnit, tile: Tile) = sequence { val waterImprovementAction = getWaterImprovementAction(unit, tile) if (waterImprovementAction != null) yield(waterImprovementAction) yieldAll(getImprovementConstructionActionsFromGeneralUnique(unit, tile)) - }.asIterable() + } - fun getWaterImprovementAction(unit: MapUnit, tile: Tile): UnitAction? { + private fun getWaterImprovementAction(unit: MapUnit, tile: Tile): UnitAction? { if (!tile.isWater || !unit.hasUnique(UniqueType.CreateWaterImprovements) || tile.resource == null) return null val improvementName = tile.tileResource.getImprovingImprovement(tile, unit.civ) ?: return null @@ -221,8 +220,8 @@ object UnitActionsFromUniques { }.takeIf { unit.currentMovement > 0 }) } - fun getImprovementConstructionActionsFromGeneralUnique(unit: MapUnit, tile: Tile): ArrayList { - val finalActions = ArrayList() + // Not internal: Used in SpecificUnitAutomation + fun getImprovementConstructionActionsFromGeneralUnique(unit: MapUnit, tile: Tile) = sequence { val uniquesToCheck = UnitActionModifiers.getUsableUnitActionUniques(unit, UniqueType.ConstructImprovementInstantly) val civResources = unit.civ.getCivResourcesByName() @@ -242,7 +241,7 @@ object UnitActionsFromUniques { (civResources[improvementUnique.params[1]] ?: 0) < improvementUnique.params[0].toInt() } - finalActions += UnitAction(UnitActionType.CreateImprovement, + yield(UnitAction(UnitActionType.CreateImprovement, title = UnitActionModifiers.actionTextWithSideEffects( "Create [${improvement.name}]", unique, @@ -263,13 +262,13 @@ object UnitActionsFromUniques { // not pretty, but users *can* remove the building from the city queue an thus clear this: && !tile.isMarkedForCreatesOneImprovement() && !tile.isImpassible() // Not 100% sure that this check is necessary... - }) + } + )) } } - return finalActions } - fun getConnectRoadActions(unit: MapUnit, tile: Tile) = sequence { + internal fun getConnectRoadActions(unit: MapUnit, tile: Tile) = sequence { if (!unit.hasUnique(UniqueType.BuildImprovements)) return@sequence val unitCivBestRoad = unit.civ.tech.getBestRoadAvailable() if (unitCivBestRoad == RoadStatus.None) return@sequence @@ -294,16 +293,14 @@ object UnitActionsFromUniques { } ) ) - }.asIterable() + } - fun getTransformActions( - unit: MapUnit, tile: Tile - ): ArrayList { + internal fun getTransformActions(unit: MapUnit, tile: Tile) = sequence { val unitTile = unit.getTile() val civInfo = unit.civ val stateForConditionals = StateForConditionals(unit = unit, civInfo = civInfo, tile = unitTile) - val transformList = ArrayList() + for (unique in unit.getMatchingUniques(UniqueType.CanTransform, stateForConditionals)) { val unitToTransformTo = civInfo.getEquivalentUnit(unique.params[0]) @@ -329,7 +326,7 @@ object UnitActionsFromUniques { "Transform to [${unitToTransformTo.name}]" else "Transform to [${unitToTransformTo.name}]\n([$newResourceRequirementsString])" - transformList.add(UnitAction(UnitActionType.Transform, + yield(UnitAction(UnitActionType.Transform, title = title, action = { unit.destroy() @@ -356,14 +353,10 @@ object UnitActionsFromUniques { } )) } - return transformList } - fun getBuildingImprovementsActions( - unit: MapUnit, - tile: Tile - ): List { - if (!unit.cache.hasUniqueToBuildImprovements) return emptyList() + internal fun getBuildingImprovementsActions(unit: MapUnit, tile: Tile): Sequence { + if (!unit.cache.hasUniqueToBuildImprovements) return emptySequence() val couldConstruct = unit.currentMovement > 0 && !tile.isCityCenter() @@ -377,7 +370,7 @@ object UnitActionsFromUniques { && unit.canBuildImprovement(it) } - return listOf(UnitAction(UnitActionType.ConstructImprovement, + return sequenceOf(UnitAction(UnitActionType.ConstructImprovement, isCurrentAction = tile.hasImprovementInProgress(), action = { GUI.pushScreen(ImprovementPickerScreen(tile, unit) { @@ -388,7 +381,7 @@ object UnitActionsFromUniques { )) } - fun getRepairTurns(unit: MapUnit): Int { + internal fun getRepairTurns(unit: MapUnit): Int { val tile = unit.currentTile if (!tile.isPillaged()) return 0 if (tile.improvementInProgress == Constants.repair) return tile.turnsToImprovement @@ -401,10 +394,9 @@ object UnitActionsFromUniques { return repairTurns } - fun getRepairActions(unit: MapUnit, tile: Tile): List { - val repairAction = getRepairAction(unit) ?: return emptyList() - return listOf(repairAction) - } + internal fun getRepairActions(unit: MapUnit, tile: Tile) = sequenceOf(getRepairAction(unit)).filterNotNull() + + // Public - used in WorkerAutomation fun getRepairAction(unit: MapUnit) : UnitAction? { if (!unit.currentTile.ruleset.tileImprovements.containsKey(Constants.repair)) return null if (!unit.cache.hasUniqueToBuildImprovements) return null diff --git a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsGreatPerson.kt b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsGreatPerson.kt index fd18075dbd..05ebd3a773 100644 --- a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsGreatPerson.kt +++ b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsGreatPerson.kt @@ -11,9 +11,10 @@ import com.unciv.models.ruleset.unique.UniqueType import com.unciv.ui.components.extensions.toPercent import kotlin.math.min +@Suppress("UNUSED_PARAMETER") // references need to have the signature expected by UnitActions.actionTypeToFunctions object UnitActionsGreatPerson { - fun getHurryResearchActions(unit:MapUnit, tile: Tile) = sequence { + internal fun getHurryResearchActions(unit:MapUnit, tile: Tile) = sequence { for (unique in unit.getMatchingUniques(UniqueType.CanHurryResearch)){ yield(UnitAction( UnitActionType.HurryResearch, @@ -27,9 +28,9 @@ object UnitActionsGreatPerson { } )) } - }.asIterable() + } - fun getHurryPolicyActions(unit:MapUnit, tile: Tile) = sequence { + internal fun getHurryPolicyActions(unit:MapUnit, tile: Tile) = sequence { for (unique in unit.getMatchingUniques(UniqueType.CanHurryPolicy)){ yield(UnitAction( UnitActionType.HurryPolicy, @@ -39,9 +40,9 @@ object UnitActionsGreatPerson { }.takeIf {unit.currentMovement > 0} )) } - }.asIterable() + } - fun getHurryWonderActions(unit: MapUnit, tile: Tile) = sequence { + internal fun getHurryWonderActions(unit: MapUnit, tile: Tile) = sequence { for (unique in unit.getMatchingUniques(UniqueType.CanSpeedupWonderConstruction)) { val canHurryWonder = if (!tile.isCityCenter()) false @@ -61,9 +62,9 @@ object UnitActionsGreatPerson { }.takeIf { unit.currentMovement > 0 && canHurryWonder } )) } - }.asIterable() + } - fun getHurryBuildingActions(unit:MapUnit, tile: Tile) = sequence { + internal fun getHurryBuildingActions(unit:MapUnit, tile: Tile) = sequence { for (unique in unit.getMatchingUniques(UniqueType.CanSpeedupConstruction)) { if (!tile.isCityCenter()) { yield(UnitAction(UnitActionType.HurryBuilding, action = null)) @@ -94,9 +95,9 @@ object UnitActionsGreatPerson { }.takeIf { unit.currentMovement > 0 && canHurryConstruction } )) } - }.asIterable() + } - fun getConductTradeMissionActions(unit:MapUnit, tile: Tile) = sequence { + internal fun getConductTradeMissionActions(unit:MapUnit, tile: Tile) = sequence { for (unique in unit.getMatchingUniques(UniqueType.CanTradeWithCityStateForGoldAndInfluence)) { val canConductTradeMission = tile.owningCity?.civ?.isCityState() == true && tile.owningCity?.civ != unit.civ @@ -120,5 +121,5 @@ object UnitActionsGreatPerson { }.takeIf { unit.currentMovement > 0 && canConductTradeMission } )) } - }.asIterable() + } } diff --git a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsPillage.kt b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsPillage.kt index b3ff042440..bbe8daef71 100644 --- a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsPillage.kt +++ b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsPillage.kt @@ -16,12 +16,12 @@ import kotlin.random.Random object UnitActionsPillage { - fun getPillageActions(unit: MapUnit, tile: Tile): List { + internal fun getPillageActions(unit: MapUnit, tile: Tile): Sequence { val pillageAction = getPillageAction(unit, tile) - ?: return listOf() + ?: return emptySequence() if (pillageAction.action == null || unit.civ.isAI() || (unit.civ.isHuman() && UncivGame.Current.settings.autoPlay.isAutoPlaying())) - return listOf(pillageAction) - else return listOf(UnitAction(UnitActionType.Pillage, pillageAction.title) { + return sequenceOf(pillageAction) + else return sequenceOf(UnitAction(UnitActionType.Pillage, pillageAction.title) { val pillageText = "Are you sure you want to pillage this [${tile.getImprovementToPillageName()!!}]?" ConfirmPopup( GUI.getWorldScreen(), @@ -35,7 +35,7 @@ object UnitActionsPillage { }) } - fun getPillageAction(unit: MapUnit, tile: Tile): UnitAction? { + internal fun getPillageAction(unit: MapUnit, tile: Tile): UnitAction? { val improvementName = unit.currentTile.getImprovementToPillageName() if (unit.isCivilian() || improvementName == null || tile.getOwner() == unit.civ) return null return UnitAction( @@ -112,6 +112,7 @@ object UnitActionsPillage { globalPillageYield.notify("") } + // Public - used in UnitAutomation fun canPillage(unit: MapUnit, tile: Tile): Boolean { if (unit.isTransported) return false if (!tile.canPillageTile()) return false diff --git a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsReligion.kt b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsReligion.kt index 9b03624611..652a1992ac 100644 --- a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsReligion.kt +++ b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsReligion.kt @@ -12,18 +12,17 @@ import com.unciv.ui.components.extensions.toPercent object UnitActionsReligion { - - internal fun getFoundReligionActions(unit: MapUnit, tile:Tile): List { - if (!unit.civ.religionManager.mayFoundReligionAtAll()) return listOf() + internal fun getFoundReligionActions(unit: MapUnit, tile:Tile): Sequence { + if (!unit.civ.religionManager.mayFoundReligionAtAll()) return emptySequence() val unique = UnitActionModifiers.getUsableUnitActionUniques(unit, UniqueType.MayFoundReligion) - .firstOrNull() ?: return listOf() + .firstOrNull() ?: return emptySequence() val hasActionModifiers = unique.conditionals.any { it.type?.targetTypes?.contains( UniqueTarget.UnitActionModifier ) == true } - return listOf(UnitAction( + return sequenceOf(UnitAction( UnitActionType.FoundReligion, if (hasActionModifiers) UnitActionModifiers.actionTextWithSideEffects( @@ -41,18 +40,18 @@ object UnitActionsReligion { )) } - internal fun getEnhanceReligionActions(unit: MapUnit, tile: Tile): List { - if (!unit.civ.religionManager.mayEnhanceReligionAtAll()) return listOf() + internal fun getEnhanceReligionActions(unit: MapUnit, tile: Tile): Sequence { + if (!unit.civ.religionManager.mayEnhanceReligionAtAll()) return emptySequence() val unique = UnitActionModifiers.getUsableUnitActionUniques(unit, UniqueType.MayEnhanceReligion) - .firstOrNull() ?: return listOf() + .firstOrNull() ?: return emptySequence() val hasActionModifiers = unique.conditionals.any { it.type?.targetTypes?.contains( UniqueTarget.UnitActionModifier ) == true } val baseTitle = "Enhance [${unit.civ.religionManager.religion!!.getReligionDisplayName()}]" - return listOf(UnitAction( + return sequenceOf(UnitAction( UnitActionType.EnhanceReligion, title = if (hasActionModifiers) UnitActionModifiers.actionTextWithSideEffects( baseTitle, @@ -77,16 +76,17 @@ object UnitActionsReligion { return pressureAdded.toInt() } - fun addSpreadReligionActions(unit: MapUnit, tile: Tile): List { - if (!unit.civ.religionManager.maySpreadReligionAtAll(unit)) return listOf() - val city = tile.getCity() ?: return listOf() + internal fun getSpreadReligionActions(unit: MapUnit, tile: Tile): Sequence { + if (!unit.civ.religionManager.maySpreadReligionAtAll(unit)) return emptySequence() + val city = tile.getCity() ?: return emptySequence() - val newStyleUnique = UnitActionModifiers.getUsableUnitActionUniques(unit, UniqueType.CanSpreadReligion).firstOrNull() ?: return emptyList() + val newStyleUnique = UnitActionModifiers.getUsableUnitActionUniques(unit, UniqueType.CanSpreadReligion) + .firstOrNull() ?: return emptySequence() val title = UnitActionModifiers.actionTextWithSideEffects("Spread [${unit.getReligionDisplayName()!!}]", newStyleUnique, unit) - return listOf(UnitAction( + return sequenceOf(UnitAction( UnitActionType.SpreadReligion, title = title, action = { @@ -103,26 +103,26 @@ object UnitActionsReligion { )) } - internal fun getRemoveHeresyActions(unit: MapUnit, tile: Tile): List { - if (!unit.civ.gameInfo.isReligionEnabled()) return listOf() - val religion = unit.civ.gameInfo.religions[unit.religion] ?: return listOf() - if (religion.isPantheon()) return listOf() + internal fun getRemoveHeresyActions(unit: MapUnit, tile: Tile): Sequence { + if (!unit.civ.gameInfo.isReligionEnabled()) return emptySequence() + val religion = unit.civ.gameInfo.religions[unit.religion] ?: return emptySequence() + if (religion.isPantheon()) return emptySequence() - val city = tile.getCity() ?: return listOf() - if (city.civ != unit.civ) return listOf() + val city = tile.getCity() ?: return emptySequence() + if (city.civ != unit.civ) return emptySequence() // Only allow the action if the city actually has any foreign religion // This will almost be always due to pressure from cities close-by - if (city.religion.getPressures().none { it.key != unit.religion!! }) return listOf() + if (city.religion.getPressures().none { it.key != unit.religion!! }) return emptySequence() val newStyleUnique = UnitActionModifiers.getUsableUnitActionUniques(unit, UniqueType.CanRemoveHeresy).firstOrNull() val hasNewStyleAbility = newStyleUnique != null - if (!hasNewStyleAbility) return listOf() + if (!hasNewStyleAbility) return emptySequence() val title = UnitActionModifiers.actionTextWithSideEffects("Remove Heresy", newStyleUnique!!, unit) - return listOf(UnitAction( + return sequenceOf(UnitAction( UnitActionType.RemoveHeresy, title = title, action = { diff --git a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsTable.kt b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsTable.kt index 10563161ee..b195d85fc1 100644 --- a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsTable.kt +++ b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsTable.kt @@ -17,6 +17,13 @@ import com.unciv.ui.popups.UnitUpgradeMenu import com.unciv.ui.screens.worldscreen.WorldScreen class UnitActionsTable(val worldScreen: WorldScreen) : Table() { + companion object { + /** Padding between and to the left of the Buttons */ + private const val padBetweenButtons = 2f + } + init { + defaults().left().padLeft(padBetweenButtons).padBottom(padBetweenButtons) + } fun update(unit: MapUnit?) { clear() @@ -31,7 +38,7 @@ class UnitActionsTable(val worldScreen: WorldScreen) : Table() { } } } - add(button).left().padBottom(2f).row() + add(button).row() } pack() } diff --git a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsUpgrade.kt b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsUpgrade.kt index 5349adcbd4..b1597cdbfe 100644 --- a/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsUpgrade.kt +++ b/core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActionsUpgrade.kt @@ -11,14 +11,8 @@ import com.unciv.models.translations.tr object UnitActionsUpgrade { - internal fun getUnitUpgradeActions( - unit: MapUnit, - tile: Tile - ): List { - val upgradeAction = getUpgradeAction(unit) - if (upgradeAction != null) return listOf(upgradeAction) - return listOf() - } + @Suppress("UNUSED_PARAMETER") // reference needs to have this signature + internal fun getUnitUpgradeActions(unit: MapUnit, tile: Tile) = sequenceOf(getUpgradeAction(unit)).filterNotNull() /** Common implementation for [getUpgradeAction], [getFreeUpgradeAction] and [getAncientRuinsUpgradeAction] */ private fun getUpgradeAction( diff --git a/tests/src/com/unciv/uniques/UnitUniquesTests.kt b/tests/src/com/unciv/uniques/UnitUniquesTests.kt index f8491ab21b..a3fc16178c 100644 --- a/tests/src/com/unciv/uniques/UnitUniquesTests.kt +++ b/tests/src/com/unciv/uniques/UnitUniquesTests.kt @@ -48,7 +48,7 @@ class UnitUniquesTests { } @Test - fun CanConstructResourceRequiringImprovement() { + fun canConstructResourceRequiringImprovement() { // Do this early so the uniqueObjects lazy is still un-triggered val improvement = game.ruleset.tileImprovements["Manufactory"]!! val requireUnique = UniqueType.ConsumesResources.text.fillPlaceholders("3", "Iron") @@ -73,7 +73,7 @@ class UnitUniquesTests { return }.filter { it.action != null } Assert.assertTrue("Great Engineer should NOT be able to create a Manufactory modded to require Iron with 0 Iron", - actionsWithoutIron.isEmpty()) + actionsWithoutIron.none()) // Supply Iron val ironTile = game.getTile(Vector2(0f,1f)) @@ -91,7 +91,7 @@ class UnitUniquesTests { 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()) + actionsWithIron.none()) } @Test