diff --git a/core/src/com/unciv/logic/automation/unit/SpecificUnitAutomation.kt b/core/src/com/unciv/logic/automation/unit/SpecificUnitAutomation.kt index f47a04eaf4..f13830dec4 100644 --- a/core/src/com/unciv/logic/automation/unit/SpecificUnitAutomation.kt +++ b/core/src/com/unciv/logic/automation/unit/SpecificUnitAutomation.kt @@ -313,7 +313,6 @@ object SpecificUnitAutomation { 0, wonderToHurry.name ) - unit.showAdditionalActions = false // make sure getUnitActions doesn't skip the important ones return UnitActions.invokeUnitAction(unit, UnitActionType.HurryBuilding) || UnitActions.invokeUnitAction(unit, UnitActionType.HurryWonder) } diff --git a/core/src/com/unciv/logic/map/BFS.kt b/core/src/com/unciv/logic/map/BFS.kt index 80ff1f7753..628bbe2a5d 100644 --- a/core/src/com/unciv/logic/map/BFS.kt +++ b/core/src/com/unciv/logic/map/BFS.kt @@ -46,6 +46,8 @@ class BFS( * and to the yet-to-be-processed set. * * Will do nothing when [hasEnded] returns `true` + * + * @return The Tile that was checked, or `null` if there was nothing to do */ fun nextStep(): Tile? { if (tilesReached.size >= maxSize) { tilesToCheck.clear(); return null } diff --git a/core/src/com/unciv/logic/map/mapunit/MapUnit.kt b/core/src/com/unciv/logic/map/mapunit/MapUnit.kt index fbe5e9e6d0..15a2d27e4d 100644 --- a/core/src/com/unciv/logic/map/mapunit/MapUnit.kt +++ b/core/src/com/unciv/logic/map/mapunit/MapUnit.kt @@ -122,9 +122,6 @@ class MapUnit : IsPartOfGameInfoSerialization { @Transient var viewableTiles = HashSet() - @Transient - var showAdditionalActions: Boolean = false - //endregion /** @@ -931,7 +928,6 @@ class MapUnit : IsPartOfGameInfoSerialization { } fun actionsOnDeselect() { - showAdditionalActions = false if (isPreparingParadrop() || isPreparingAirSweep()) action = null } diff --git a/core/src/com/unciv/models/UnitAction.kt b/core/src/com/unciv/models/UnitAction.kt index 76d9bad18d..3fccabd07a 100644 --- a/core/src/com/unciv/models/UnitAction.kt +++ b/core/src/com/unciv/models/UnitAction.kt @@ -93,10 +93,12 @@ enum class UnitActionType( val imageGetter: (()-> Actor)?, binding: KeyboardBinding? = null, val isSkippingToNextUnit: Boolean = true, - val uncivSound: UncivSound = UncivSound.Click + val uncivSound: UncivSound = UncivSound.Click, + /** UI "page" preference, 0-based - Dynamic overrides to this are in `UnitActions.actionTypeToPageGetter` */ + val defaultPage: Int ) { SwapUnits("Swap units", - { ImageGetter.getUnitActionPortrait("Swap") }, false), + { ImageGetter.getUnitActionPortrait("Swap") }, false, defaultPage = 1), Automate("Automate", { ImageGetter.getUnitActionPortrait("Automate") }), ConnectRoad("Connect road", @@ -106,7 +108,7 @@ enum class UnitActionType( StopMovement("Stop movement", { ImageGetter.getUnitActionPortrait("StopMove") }, false), ShowUnitDestination("Show unit destination", - { ImageGetter.getUnitActionPortrait("ShowUnitDestination")}, false), + { ImageGetter.getUnitActionPortrait("ShowUnitDestination")}, false, defaultPage = 1), Sleep("Sleep", { ImageGetter.getUnitActionPortrait("Sleep") }), SleepUntilHealed("Sleep until healed", @@ -164,24 +166,24 @@ enum class UnitActionType( EnhanceReligion("Enhance a Religion", { ImageGetter.getUnitActionPortrait("EnhanceReligion") }, UncivSound.Choir), DisbandUnit("Disband unit", - { ImageGetter.getUnitActionPortrait("DisbandUnit") }, false), + { ImageGetter.getUnitActionPortrait("DisbandUnit") }, false, defaultPage = 1), GiftUnit("Gift unit", - { ImageGetter.getUnitActionPortrait("Present") }, UncivSound.Silent), + { ImageGetter.getUnitActionPortrait("Present") }, UncivSound.Silent, defaultPage = 1), Wait("Wait", { ImageGetter.getUnitActionPortrait("Wait") }, UncivSound.Silent), ShowAdditionalActions("Show more", { ImageGetter.getUnitActionPortrait("ShowMore") }, false), HideAdditionalActions("Back", - { ImageGetter.getUnitActionPortrait("HideMore") }, false), + { ImageGetter.getUnitActionPortrait("HideMore") }, false, defaultPage = 1), AddInCapital( "Add in capital", { ImageGetter.getUnitActionPortrait("AddInCapital")}, UncivSound.Chimes), ; // Allow shorter initializations - constructor(value: String, imageGetter: (() -> Actor)?, uncivSound: UncivSound = UncivSound.Click) - : this(value, imageGetter, null, true, uncivSound) - constructor(value: String, imageGetter: (() -> Actor)?, isSkippingToNextUnit: Boolean = true, uncivSound: UncivSound = UncivSound.Click) - : this(value, imageGetter, null, isSkippingToNextUnit, uncivSound) + constructor(value: String, imageGetter: (() -> Actor)?, uncivSound: UncivSound = UncivSound.Click, defaultPage: Int = 0) + : this(value, imageGetter, null, true, uncivSound, defaultPage) + constructor(value: String, imageGetter: (() -> Actor)?, isSkippingToNextUnit: Boolean = true, uncivSound: UncivSound = UncivSound.Click, defaultPage: Int = 0) + : this(value, imageGetter, null, isSkippingToNextUnit, uncivSound, defaultPage) val binding: KeyboardBinding = binding ?: diff --git a/core/src/com/unciv/models/ruleset/unique/Unique.kt b/core/src/com/unciv/models/ruleset/unique/Unique.kt index 8a4812f9b0..ef40651aef 100644 --- a/core/src/com/unciv/models/ruleset/unique/Unique.kt +++ b/core/src/com/unciv/models/ruleset/unique/Unique.kt @@ -66,7 +66,7 @@ class Unique(val text: String, val sourceObjectType: UniqueTarget? = null, val s fun conditionalsApply(state: StateForConditionals = StateForConditionals()): Boolean { if (state.ignoreConditionals) return true // Always allow Timed conditional uniques. They are managed elsewhere - if (conditionals.any{ it.isOfType(UniqueType.ConditionalTimedUnique) }) return true + if (conditionals.any { it.isOfType(UniqueType.ConditionalTimedUnique) }) return true for (condition in conditionals) { if (!conditionalApplies(condition, state)) return false } diff --git a/core/src/com/unciv/ui/screens/worldscreen/WorldScreen.kt b/core/src/com/unciv/ui/screens/worldscreen/WorldScreen.kt index 2439cc22f7..c9b71f9426 100644 --- a/core/src/com/unciv/ui/screens/worldscreen/WorldScreen.kt +++ b/core/src/com/unciv/ui/screens/worldscreen/WorldScreen.kt @@ -5,7 +5,6 @@ import com.badlogic.gdx.Gdx import com.badlogic.gdx.Input import com.badlogic.gdx.math.Vector2 import com.badlogic.gdx.scenes.scene2d.ui.Table -import com.badlogic.gdx.scenes.scene2d.ui.TextButton import com.badlogic.gdx.utils.Align import com.unciv.Constants import com.unciv.UncivGame @@ -26,10 +25,7 @@ import com.unciv.models.ruleset.tile.ResourceType import com.unciv.models.ruleset.unique.UniqueType import com.unciv.ui.components.extensions.centerX import com.unciv.ui.components.extensions.darken -import com.unciv.ui.components.extensions.isEnabled -import com.unciv.ui.components.extensions.setFontSize import com.unciv.ui.components.extensions.toLabel -import com.unciv.ui.components.extensions.toTextButton import com.unciv.ui.components.input.KeyCharAndCode import com.unciv.ui.components.input.KeyShortcutDispatcherVeto import com.unciv.ui.components.input.KeyboardBinding 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 684e1a085d..b6e4fe3243 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,49 +10,69 @@ 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.getActionDefaultPage +import com.unciv.ui.screens.worldscreen.unit.actions.UnitActions.getPagingActions 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] + * API used by UI: [getUnitActions] without `unitActionType` parameter, [getActionDefaultPage], [getPagingActions] + * API used by Automation: [invokeUnitAction] + * API used by unit tests: [getUnitActions] with `unitActionType` parameter + * Note on unit test use: Some UnitAction factories access GUI helpers that crash from a unit test. + * Avoid testing actions that need WorldScreen context, and migrate any un-mapped ones you need to `actionTypeToFunctions`. */ object UnitActions { - fun getUnitActions(unit: MapUnit): Sequence { - return if (unit.showAdditionalActions) getAdditionalActions(unit) - else getNormalActions(unit) - } - /** * 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 */ + * @return whether the action was invoked + */ fun invokeUnitAction(unit: MapUnit, unitActionType: UnitActionType): Boolean { - 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 + val internalAction = + getUnitActions(unit, unitActionType) + .firstOrNull { it.action != null } // If there's more than one, take the first enabled one. + ?.action ?: return false internalAction.invoke() return true } - private val actionTypeToFunctions = linkedMapOf Sequence>( + /** + * Get all currently possible instances of [UnitAction] for [unit]. + */ + fun getUnitActions(unit: MapUnit) = sequence { + val tile = unit.getTile() + + // Actions standardized with a directly callable invokeUnitAction + for (getActionsFunction in actionTypeToFunctions.values) + yieldAll(getActionsFunction(unit, tile)) + + // Actions not migrated to actionTypeToFunctions + addUnmappedUnitActions(unit) + } + + /** + * Get all instances of [UnitAction] of the [unitActionType] type for [unit]. + * + * Includes optimization for direct creation of the needed instance type, falls back to enumerating [getUnitActions] to look for the given type. + */ + fun getUnitActions(unit: MapUnit, unitActionType: UnitActionType) = + if (unitActionType in actionTypeToFunctions) + actionTypeToFunctions[unitActionType]!! // we have a mapped getter... + .invoke(unit, unit.getTile()) // ...call it to get a collection... + else sequence { + addUnmappedUnitActions(unit) // No mapped getter: Enumerate all... + }.filter { it.type == unitActionType } // ...and take ones matching the type. + + private val actionTypeToFunctions = linkedMapOf Sequence>( // Determined by unit uniques UnitActionType.Transform to UnitActionsFromUniques::getTransformActions, UnitActionType.Paradrop to UnitActionsFromUniques::getParadropActions, @@ -73,22 +93,47 @@ object UnitActions { UnitActionType.SpreadReligion to UnitActionsReligion::getSpreadReligionActions, UnitActionType.RemoveHeresy to UnitActionsReligion::getRemoveHeresyActions, UnitActionType.TriggerUnique to UnitActionsFromUniques::getTriggerUniqueActions, - UnitActionType.AddInCapital to UnitActionsFromUniques::getAddInCapitalActions + UnitActionType.AddInCapital to UnitActionsFromUniques::getAddInCapitalActions, + UnitActionType.GiftUnit to UnitActions::getGiftActions ) - private fun shouldAutomationBePrimaryAction(unit:MapUnit) = unit.cache.hasUniqueToBuildImprovements || unit.hasUnique(UniqueType.AutomationPrimaryAction) + /** Gets the preferred "page" to display a [UnitAction] of type [unitActionType] on, possibly dynamic depending on the state or situation [unit] is in. */ + fun getActionDefaultPage(unit: MapUnit, unitActionType: UnitActionType) = + actionTypeToPageGetter[unitActionType]?.invoke(unit) ?: unitActionType.defaultPage - private fun getNormalActions(unit: MapUnit) = sequence { + /** Only for action types that wish to change their "More/Back" page position depending on context. + * All others get a defaultPage statically from [UnitActionType]. + */ + private val actionTypeToPageGetter = linkedMapOf Int>( + UnitActionType.Automate to { unit -> + if (unit.cache.hasUniqueToBuildImprovements || unit.hasUnique(UniqueType.AutomationPrimaryAction)) 0 else 1 + }, + UnitActionType.Fortify to { unit -> + // Fortify moves to second page if current action is FortifyUntilHealed or if unit is wounded and it's not already the current action + if (unit.isFortifyingUntilHealed() || unit.health < 100 && !(unit.isFortified() && !unit.isActionUntilHealed())) 1 else 0 + }, + UnitActionType.FortifyUntilHealed to { unit -> + // FortifyUntilHealed only moves to the second page if Fortify is the current action + if (unit.isFortified() && !unit.isActionUntilHealed()) 1 else 0 + }, + UnitActionType.Sleep to { unit -> + // Sleep moves to second page if current action is SleepUntilHealed or if unit is wounded and it's not already the current action + if (unit.isSleepingUntilHealed() || unit.health < 100 && !(unit.isSleeping() && !unit.isActionUntilHealed())) 1 else 0 + }, + UnitActionType.SleepUntilHealed to { unit -> + // SleepUntilHealed only moves to the second page if Sleep is the current action + if (unit.isSleeping() && !unit.isActionUntilHealed()) 1 else 0 + }, + UnitActionType.Explore to { unit -> + if (unit.isCivilian()) 1 else 0 + }, + ) + + private suspend fun SequenceScope.addUnmappedUnitActions(unit: MapUnit) { val tile = unit.getTile() - // Actions standardized with a directly callable invokeUnitAction - for (getActionsFunction in actionTypeToFunctions.values) - yieldAll(getActionsFunction(unit, tile)) - // General actions - - if (shouldAutomationBePrimaryAction(unit)) - addAutomateActions(unit) + addAutomateActions(unit) if (unit.isMoving()) yield(UnitAction(UnitActionType.StopMovement) { unit.action = null }) if (unit.isExploring()) @@ -104,39 +149,24 @@ object UnitActions { yieldAll(UnitActionsPillage.getPillageActions(unit, tile)) addSleepActions(unit, tile) - addSleepUntilHealedActions(unit, tile) + addFortifyActions(unit) - addFortifyActions(unit, false) - - if (unit.isMilitary()) - addExplorationActions(unit) + addExplorationActions(unit) addWaitAction(unit) - addToggleActionsAction(unit) - } - - private fun getAdditionalActions(unit: MapUnit) = sequence { + // From here we have actions defaulting to the second page if (unit.isMoving()) { yield(UnitAction(UnitActionType.ShowUnitDestination) { GUI.getMap().setCenterPosition(unit.getMovementDestination().position, true) }) } - addFortifyActions(unit, true) - if (!shouldAutomationBePrimaryAction(unit)) - addAutomateActions(unit) addSwapAction(unit) addDisbandAction(unit) - addGiftAction(unit, unit.getTile()) - if (unit.isCivilian()) - addExplorationActions(unit) - - addToggleActionsAction(unit) } private suspend fun SequenceScope.addSwapAction(unit: MapUnit) { - val worldScreen = GUI.getWorldScreen() // Air units cannot swap if (unit.baseUnit.movesLikeAirUnits()) return // Disable unit swapping if multiple units are selected. It would make little sense. @@ -145,6 +175,7 @@ object UnitActions { // have the visual bug that the tile overlays for the eligible swap locations are drawn for // /all/ selected units instead of only the first one. This could be fixed, but again, // swapping makes little sense for multiselect anyway. + val worldScreen = GUI.getWorldScreen() 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 @@ -160,24 +191,25 @@ object UnitActions { } private suspend fun SequenceScope.addDisbandAction(unit: MapUnit) { - val worldScreen = GUI.getWorldScreen() - 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() - else "Do you really want to disband this unit?".tr() - ConfirmPopup(worldScreen, disbandText, "Disband unit") { - unit.disband() - unit.civ.updateStatsForNextTurn() // less upkeep! - GUI.setUpdateWorldOnNextRender() - if (GUI.getSettings().autoUnitCycle) - worldScreen.switchToNextUnit() - }.open() - } - }.takeIf { unit.currentMovement > 0 })) + yield(UnitAction(type = UnitActionType.DisbandUnit, + action = { + val worldScreen = GUI.getWorldScreen() + if (!worldScreen.hasOpenPopups()) { + val disbandText = if (unit.currentTile.getOwner() == unit.civ) + "Disband this unit for [${unit.baseUnit.getDisbandGold(unit.civ)}] gold?".tr() + else "Do you really want to disband this unit?".tr() + ConfirmPopup(worldScreen, disbandText, "Disband unit") { + unit.disband() + unit.civ.updateStatsForNextTurn() // less upkeep! + GUI.setUpdateWorldOnNextRender() + if (GUI.getSettings().autoUnitCycle) + worldScreen.switchToNextUnit() + }.open() + } + }.takeIf { unit.currentMovement > 0 } + )) } - 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 @@ -197,9 +229,8 @@ object UnitActions { }) } - - private suspend fun SequenceScope.addFortifyActions(unit: MapUnit, showingAdditionalActions: Boolean) { - if (unit.isFortified() && !showingAdditionalActions) { + private suspend fun SequenceScope.addFortifyActions(unit: MapUnit) { + if (unit.isFortified()) { yield(UnitAction( type = if (unit.isActionUntilHealed()) UnitActionType.FortifyUntilHealed else @@ -210,37 +241,27 @@ object UnitActions { return } - if (!unit.canFortify()) return - if (unit.currentMovement == 0f) return + if (!unit.canFortify() || unit.currentMovement == 0f) return - val isFortified = unit.isFortified() - val isDamaged = unit.health < 100 + yield(UnitAction(UnitActionType.Fortify, + action = { unit.fortify() }.takeIf { !unit.isFortified() || unit.isFortifyingUntilHealed() } + )) - if (isDamaged && !showingAdditionalActions && unit.rankTileForHealing(unit.currentTile) != 0) - yield(UnitAction(UnitActionType.FortifyUntilHealed, - action = { unit.fortifyUntilHealed() }.takeIf { !unit.isFortifyingUntilHealed() } - )) - else if (isDamaged || !showingAdditionalActions) - yield(UnitAction(UnitActionType.Fortify, - action = { unit.fortify() }.takeIf { !isFortified } - )) - } - - private fun shouldHaveSleepAction(unit: MapUnit, tile: Tile): Boolean { - if (unit.isFortified() || unit.canFortify() || unit.currentMovement == 0f) return false - return !(tile.hasImprovementInProgress() - && unit.canBuildImprovement(tile.getTileImprovementInProgress()!!)) - } - 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() } + if (unit.health == 100) return + yield(UnitAction(UnitActionType.FortifyUntilHealed, + action = { unit.fortifyUntilHealed() } + .takeIf { !unit.isFortifyingUntilHealed() && unit.canHealInCurrentTile() } )) } - private suspend fun SequenceScope.addSleepUntilHealedActions(unit: MapUnit, tile: Tile) { - if (!shouldHaveSleepAction(unit, tile)) return + private suspend fun SequenceScope.addSleepActions(unit: MapUnit, tile: Tile) { + if (unit.isFortified() || unit.canFortify() || unit.currentMovement == 0f) return + if (tile.hasImprovementInProgress() && unit.canBuildImprovement(tile.getTileImprovementInProgress()!!)) return + + yield(UnitAction(UnitActionType.Sleep, + action = { unit.action = UnitActionType.Sleep.value }.takeIf { !unit.isSleeping() || unit.isSleepingUntilHealed() } + )) + if (unit.health == 100) return yield(UnitAction(UnitActionType.SleepUntilHealed, action = { unit.action = UnitActionType.SleepUntilHealed.value } @@ -248,17 +269,13 @@ object UnitActions { )) } - private suspend fun SequenceScope.addGiftAction(unit: MapUnit, tile: Tile) { - yieldIfNotNull(getGiftAction(unit, tile)) - } - - fun getGiftAction(unit: MapUnit, tile: Tile): UnitAction? { + private fun getGiftActions(unit: MapUnit, tile: Tile) = sequence { val recipient = tile.getOwner() // We need to be in another civs territory. - if (recipient == null || recipient.isCurrentPlayer()) return null + if (recipient == null || recipient.isCurrentPlayer()) return@sequence if (recipient.isCityState()) { - if (recipient.isAtWarWith(unit.civ)) return null // No gifts to enemy CS + if (recipient.isAtWarWith(unit.civ)) return@sequence // No gifts to enemy CS // City States only take military units (and units specifically allowed by uniques) if (!unit.isMilitary() && unit.getMatchingUniques( @@ -266,16 +283,18 @@ object UnitActions { checkCivInfoUniques = true ) .none { unit.matchesFilter(it.params[1]) } - ) return null + ) return@sequence } // If gifting to major civ they need to be friendly - else if (!tile.isFriendlyTerritory(unit.civ)) return null + else if (!tile.isFriendlyTerritory(unit.civ)) return@sequence // Transported units can't be gifted - if (unit.isTransported) return null + if (unit.isTransported) return@sequence - if (unit.currentMovement <= 0) - return UnitAction(UnitActionType.GiftUnit, action = null) + if (unit.currentMovement <= 0) { + yield(UnitAction(UnitActionType.GiftUnit, action = null)) + return@sequence + } val giftAction = { if (recipient.isCityState()) { @@ -300,8 +319,7 @@ object UnitActions { unit.gift(recipient) GUI.setUpdateWorldOnNextRender() } - - return UnitAction(UnitActionType.GiftUnit, action = giftAction) + yield(UnitAction(UnitActionType.GiftUnit, action = giftAction)) } private suspend fun SequenceScope.addAutomateActions(unit: MapUnit) { @@ -327,15 +345,18 @@ object UnitActions { )) } - 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() - } - )) + /** + * Creates the "paging" [UnitAction]s for: + * - [first][Pair.first] - [UnitActionType.ShowAdditionalActions] (page forward) + * - [second][Pair.second] - [UnitActionType.HideAdditionalActions] (page back) + * + * These are not returned as part of [getUnitActions]! + */ + // This function is here for historic reasons, and to keep UnitActionType implementations closer together. + // The code might move to UnitActionsTable altogether, no big difference. + internal fun getPagingActions(unit: MapUnit, actionsTable: UnitActionsTable): Pair { + return UnitAction(UnitActionType.ShowAdditionalActions) { actionsTable.changePage(1, unit) } to + UnitAction(UnitActionType.HideAdditionalActions) { actionsTable.changePage(-1, unit) } } } 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 b195d85fc1..57958104b6 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,19 +17,57 @@ import com.unciv.ui.popups.UnitUpgradeMenu import com.unciv.ui.screens.worldscreen.WorldScreen class UnitActionsTable(val worldScreen: WorldScreen) : Table() { + /** Distribute UnitActions on "pages" */ + // todo since this runs surprisingly often - some caching? Does it even need to do anything if unit and currentPage are the same? + private var currentPage = 0 + private val numPages = 2 //todo static for now + private var shownForUnitHash = 0 + companion object { + /** Maximum for how many pages there can be. */ + private const val maxAllowedPages = 10 /** Padding between and to the left of the Buttons */ private const val padBetweenButtons = 2f } + init { defaults().left().padLeft(padBetweenButtons).padBottom(padBetweenButtons) } + fun changePage(delta: Int, unit: MapUnit) { + if (delta == 0) return + currentPage = (currentPage + delta) % numPages + update(unit) + } + fun update(unit: MapUnit?) { + val newUnitHash = unit?.hashCode() ?: 0 + if (shownForUnitHash != newUnitHash) { + currentPage = 0 + shownForUnitHash = newUnitHash + } + clear() if (unit == null) return if (!worldScreen.canChangeState) return // No actions when it's not your turn or spectator! + + val pageActionBuckets = Array>(maxAllowedPages) { ArrayDeque() } + + val (nextPageAction, previousPageAction) = UnitActions.getPagingActions(unit, this) + val nextPageButton = getUnitActionButton(unit, nextPageAction) + val previousPageButton = getUnitActionButton(unit, previousPageAction) + + // Distribute sequentially into the buckets for (unitAction in UnitActions.getUnitActions(unit)) { + val actionPage = UnitActions.getActionDefaultPage(unit, unitAction.type) + if (actionPage >= maxAllowedPages) break + pageActionBuckets[actionPage].addLast(unitAction) + } + + // clamp currentPage + if (currentPage !in 0 until numPages) currentPage = 0 + // actually show the buttons of the currentPage + for (unitAction in pageActionBuckets[currentPage]) { val button = getUnitActionButton(unit, unitAction) if (unitAction is UpgradeUnitAction) { button.onRightClick { @@ -38,12 +76,17 @@ class UnitActionsTable(val worldScreen: WorldScreen) : Table() { } } } - add(button).row() + add(button).colspan(2).row() } + + // show page navigation + if (currentPage > 0) + add(previousPageButton) + if (currentPage < numPages - 1) + add(nextPageButton) pack() } - private fun getUnitActionButton(unit: MapUnit, unitAction: UnitAction): Button { val icon = unitAction.getIcon() // If peripheral keyboard not detected, hotkeys will not be displayed @@ -74,6 +117,6 @@ class UnitActionsTable(val worldScreen: WorldScreen) : Table() { } } - return actionButton + return actionButton } } diff --git a/tests/src/com/unciv/uniques/UnitUniquesTests.kt b/tests/src/com/unciv/uniques/UnitUniquesTests.kt index a3fc16178c..ce4f96becd 100644 --- a/tests/src/com/unciv/uniques/UnitUniquesTests.kt +++ b/tests/src/com/unciv/uniques/UnitUniquesTests.kt @@ -2,6 +2,7 @@ package com.unciv.uniques import com.badlogic.gdx.math.Vector2 import com.unciv.logic.map.mapunit.UnitTurnManager +import com.unciv.models.UnitActionType import com.unciv.models.ruleset.unique.UniqueType import com.unciv.models.translations.fillPlaceholders import com.unciv.testing.GdxTestRunner @@ -42,7 +43,8 @@ class UnitUniquesTests { val greatPerson = game.addUnit("Great Scientist", mainCiv, unitTile) // then - val giftAction = UnitActions.getGiftAction(greatPerson, unitTile) + val giftAction = UnitActions.getUnitActions(greatPerson, UnitActionType.GiftUnit) + .firstOrNull { it.action != null } // This tests that the action should be enabled, too Assert.assertNotNull("Great Person should have a gift action", giftAction) }