From 3a80db2604fc5dc653839d1494d11cd262f9e815 Mon Sep 17 00:00:00 2001 From: SomeTroglodyte <63000004+SomeTroglodyte@users.noreply.github.com> Date: Sun, 28 Jan 2024 10:09:41 +0100 Subject: [PATCH] Unit actions dynamic paging (#11002) * Linting * Dynamic redistribution of buttons on cramped displays * Merge the default two pages if they're near empty * Keyboard bindings independent of button visibility * Remove caching todo - impossible unless we can guarantee no state change caused outside this class goes undetected - even full equality contract on MapUnit may not be enough --- .../ui/screens/worldscreen/WorldScreen.kt | 15 ++-- .../worldscreen/unit/actions/UnitActions.kt | 5 +- .../unit/actions/UnitActionsTable.kt | 87 +++++++++++++++---- .../unit/actions/UnitActionsUpgrade.kt | 22 ++--- 4 files changed, 89 insertions(+), 40 deletions(-) diff --git a/core/src/com/unciv/ui/screens/worldscreen/WorldScreen.kt b/core/src/com/unciv/ui/screens/worldscreen/WorldScreen.kt index f05041d754..8678135900 100644 --- a/core/src/com/unciv/ui/screens/worldscreen/WorldScreen.kt +++ b/core/src/com/unciv/ui/screens/worldscreen/WorldScreen.kt @@ -105,11 +105,11 @@ class WorldScreen( private val mapVisualization = MapVisualization(gameInfo, viewingCiv) // Floating Widgets going counter-clockwise - val topBar = WorldScreenTopBar(this) - private val techPolicyAndDiplomacy = TechPolicyDiplomacyButtons(this) + internal val topBar = WorldScreenTopBar(this) + internal val techPolicyAndDiplomacy = TechPolicyDiplomacyButtons(this) private val unitActionsTable = UnitActionsTable(this) /** Bottom left widget holding information about a selected unit or city */ - val bottomUnitTable = UnitTable(this) + internal val bottomUnitTable = UnitTable(this) private val battleTable = BattleTable(this) private val zoomController = ZoomButtonPair(mapHolder) internal val minimapWrapper = MinimapHolder(mapHolder) @@ -369,9 +369,6 @@ class WorldScreen( battleTable.update() displayTutorialTaskOnUpdate() - - unitActionsTable.update(bottomUnitTable.selectedUnit) - unitActionsTable.y = bottomUnitTable.height } mapHolder.resetArrows() @@ -401,6 +398,12 @@ class WorldScreen( if (techPolicyAndDiplomacy.update()) displayTutorial(TutorialTrigger.OtherCivEncountered) + if (uiEnabled) { + // UnitActionsTable measures geometry (its own y, techPolicyAndDiplomacy and fogOfWarButton), so call update this late + unitActionsTable.y = bottomUnitTable.height + unitActionsTable.update(bottomUnitTable.selectedUnit) + } + // If the game has ended, lets stop AutoPlay if (game.settings.autoPlay.isAutoPlaying() && !gameInfo.oneMoreTurnMode && (viewingCiv.isDefeated() || gameInfo.checkForVictory())) { 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 b6e4fe3243..228e5471be 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 @@ -13,10 +13,6 @@ import com.unciv.models.translations.tr 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.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. @@ -103,6 +99,7 @@ object UnitActions { /** Only for action types that wish to change their "More/Back" page position depending on context. * All others get a defaultPage statically from [UnitActionType]. + * Note the returned "page numbers" are treated as suggestions, buttons may get redistributed when screen space is scarce. */ private val actionTypeToPageGetter = linkedMapOf Int>( UnitActionType.Automate to { 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 57958104b6..ddf1dff77f 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 @@ -3,13 +3,13 @@ package com.unciv.ui.screens.worldscreen.unit.actions import com.badlogic.gdx.graphics.Color import com.badlogic.gdx.scenes.scene2d.ui.Button import com.badlogic.gdx.scenes.scene2d.ui.Table -import com.unciv.GUI import com.unciv.UncivGame import com.unciv.logic.map.mapunit.MapUnit import com.unciv.models.UnitAction import com.unciv.models.UnitActionType import com.unciv.models.UpgradeUnitAction import com.unciv.ui.components.extensions.disable +import com.unciv.ui.components.input.keyShortcuts import com.unciv.ui.components.input.onActivation import com.unciv.ui.components.input.onRightClick import com.unciv.ui.images.IconTextButton @@ -18,14 +18,24 @@ 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 buttonsPerPage = Int.MAX_VALUE + private var numPages = 2 private var shownForUnitHash = 0 companion object { - /** Maximum for how many pages there can be. */ + /** Maximum for how many pages there can be. ([minButtonsPerPage]-1)*[maxAllowedPages] + * is the upper bound for how many actions a unit can display. */ private const val maxAllowedPages = 10 + /** Lower bound for how many buttons to distribute per page, including navigation buttons. + * Affects really cramped displays. */ + // less than navigation buttons + 2 makes little sense, and setting it to 4 isn't necessary. + private const val minButtonsPerPage = 3 + /** Upper bound for how many buttons to distribute per page, including navigation buttons. + * Affects large displays, resulting in more map visible between the actions and tech/diplo/policy buttons. */ + private const val maxButtonsPerPage = 7 + /** Maximum number of buttons to present without paging, overriding page preferences (implementation currently limited to merging two pages) */ + private const val maxSinglePageButtons = 5 /** Padding between and to the left of the Buttons */ private const val padBetweenButtons = 2f } @@ -35,7 +45,7 @@ class UnitActionsTable(val worldScreen: WorldScreen) : Table() { } fun changePage(delta: Int, unit: MapUnit) { - if (delta == 0) return + if (delta == 0 || numPages <= 1) return currentPage = (currentPage + delta) % numPages update(unit) } @@ -51,18 +61,40 @@ class UnitActionsTable(val worldScreen: WorldScreen) : Table() { if (unit == null) return if (!worldScreen.canChangeState) return // No actions when it's not your turn or spectator! + numPages = 0 val pageActionBuckets = Array>(maxAllowedPages) { ArrayDeque() } + fun freeSlotsOnPage(page: Int) = buttonsPerPage - + pageActionBuckets[page].size - + (if (numPages > 1) 1 else 0) // room for the navigation buttons val (nextPageAction, previousPageAction) = UnitActions.getPagingActions(unit, this) val nextPageButton = getUnitActionButton(unit, nextPageAction) val previousPageButton = getUnitActionButton(unit, previousPageAction) + updateButtonsPerPage(nextPageButton) // Distribute sequentially into the buckets for (unitAction in UnitActions.getUnitActions(unit)) { - val actionPage = UnitActions.getActionDefaultPage(unit, unitAction.type) + var actionPage = UnitActions.getActionDefaultPage(unit, unitAction.type) + while (actionPage < maxAllowedPages && freeSlotsOnPage(actionPage) <= 0) + actionPage++ if (actionPage >= maxAllowedPages) break + if (actionPage >= numPages) numPages = actionPage + 1 pageActionBuckets[actionPage].addLast(unitAction) } + // Due to room reserved for paging buttons changing, buckets may now be too full + for (page in 0 until maxAllowedPages - 1) { + while (freeSlotsOnPage(page) < 0) { + val element = pageActionBuckets[page].removeLast() + pageActionBuckets[page + 1].addFirst(element) + if (numPages < page + 2) numPages = page + 2 + } + } + // Special case: Only the default two pages used and all actions would fit in one + if (numPages == 2 && buttonsPerPage >= maxSinglePageButtons && pageActionBuckets[0].size + pageActionBuckets[1].size <= maxSinglePageButtons) { + pageActionBuckets[0].addAll(pageActionBuckets[1]) + pageActionBuckets[1].clear() + numPages = 1 + } // clamp currentPage if (currentPage !in 0 until numPages) currentPage = 0 @@ -85,6 +117,26 @@ class UnitActionsTable(val worldScreen: WorldScreen) : Table() { if (currentPage < numPages - 1) add(nextPageButton) pack() + + // Bind all currently invisible actions to their keys + keyShortcuts.clear() + for (page in pageActionBuckets.indices) { + if (page == currentPage) continue // these are already done + for (unitAction in pageActionBuckets[page]) { + if (unitAction.action == null) continue + keyShortcuts.add(unitAction.type.binding) { + activateAction(unitAction, unit) + } + } + } + } + + private fun updateButtonsPerPage(button: Button) { + val upperLimit = worldScreen.techPolicyAndDiplomacy.y + val lowerLimit = this.y + val availableHeight = upperLimit - lowerLimit - padBetweenButtons + val buttonHeight = button.height + padBetweenButtons + buttonsPerPage = (availableHeight / buttonHeight).toInt().coerceIn(minButtonsPerPage, maxButtonsPerPage) } private fun getUnitActionButton(unit: MapUnit, unitAction: UnitAction): Button { @@ -104,19 +156,22 @@ class UnitActionsTable(val worldScreen: WorldScreen) : Table() { actionButton.disable() } else { actionButton.onActivation(unitAction.uncivSound, binding) { - unitAction.action.invoke() - GUI.setUpdateWorldOnNextRender() - // We keep the unit action/selection overlay from the previous unit open even when already selecting another unit - // so you need less clicks/touches to do things, but once we do an action with the new unit, we want to close this - // overlay, since the user definitely wants to interact with the new unit. - worldScreen.mapHolder.removeUnitActionOverlay() - if (UncivGame.Current.settings.autoUnitCycle - && (unit.isDestroyed || (unit.isMoving() && unit.currentMovement == 0f && unitAction.type.isSkippingToNextUnit) || (!unit.isMoving() && unitAction.type.isSkippingToNextUnit))) { - worldScreen.switchToNextUnit() - } + activateAction(unitAction, unit) } } return actionButton } + + private fun activateAction(unitAction: UnitAction, unit: MapUnit) { + unitAction.action!!.invoke() + worldScreen.shouldUpdate = true + // We keep the unit action/selection overlay from the previous unit open even when already selecting another unit + // so you need less clicks/touches to do things, but once we do an action with the new unit, we want to close this + // overlay, since the user definitely wants to interact with the new unit. + worldScreen.mapHolder.removeUnitActionOverlay() + if (!UncivGame.Current.settings.autoUnitCycle) return + if (unit.isDestroyed || unitAction.type.isSkippingToNextUnit && (unit.isMoving() && unit.currentMovement == 0f || !unit.isMoving())) + worldScreen.switchToNextUnit() + } } 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 c72be3680e..a96f6caa9d 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 @@ -10,25 +10,24 @@ import com.unciv.models.translations.tr object UnitActionsUpgrade { - /** Common implementation for [getUpgradeAction], [getFreeUpgradeAction] and [getAncientRuinsUpgradeAction] */ + /** Common implementation for `getUpgradeActions(unit)`, [getFreeUpgradeAction], [getAncientRuinsUpgradeAction] and [getUpgradeActionAnywhere] */ private fun getUpgradeActions( unit: MapUnit, isFree: Boolean, isSpecial: Boolean, isAnywhere: Boolean - ): Sequence { + ) = sequence { val unitTile = unit.getTile() val civInfo = unit.civ - val specialUpgradesTo = if (isSpecial) - unit.baseUnit().getMatchingUniques(UniqueType.RuinsUpgrade, StateForConditionals(civInfo, unit= unit)) + val specialUpgradesTo = if (isSpecial) + unit.baseUnit().getMatchingUniques(UniqueType.RuinsUpgrade, StateForConditionals(civInfo, unit = unit)) .map { it.params[0] }.firstOrNull() else null val upgradeUnits = if (specialUpgradesTo != null) sequenceOf(specialUpgradesTo) else unit.baseUnit.getUpgradeUnits(StateForConditionals(civInfo, unit = unit)) - if (upgradeUnits.none()) return emptySequence() // can't upgrade to anything - if (!isAnywhere && unitTile.getOwner() != civInfo) return emptySequence() + if (upgradeUnits.none()) return@sequence // can't upgrade to anything + if (!isAnywhere && unitTile.getOwner() != civInfo) return@sequence - var upgradeActions = emptySequence() for (upgradesTo in upgradeUnits){ val upgradedUnit = civInfo.getEquivalentUnit(upgradesTo) @@ -55,7 +54,7 @@ object UnitActionsUpgrade { "Upgrade to [${upgradedUnit.name}] ([$goldCostOfUpgrade] gold)" else "Upgrade to [${upgradedUnit.name}]\n([$goldCostOfUpgrade] gold, [$newResourceRequirementsString])" - upgradeActions += UpgradeUnitAction( + yield(UpgradeUnitAction( title = title, unitToUpgradeTo = upgradedUnit, goldCostOfUpgrade = goldCostOfUpgrade, @@ -64,10 +63,6 @@ object UnitActionsUpgrade { unit.destroy(destroyTransportedUnit = false) val newUnit = civInfo.units.placeUnitNearTile(unitTile.position, upgradedUnit) - /** We were UNABLE to place the new unit, which means that the unit failed to upgrade! - * The only known cause of this currently is "land units upgrading to water units" which fail to be placed. - */ - /** We were UNABLE to place the new unit, which means that the unit failed to upgrade! * The only known cause of this currently is "land units upgrading to water units" which fail to be placed. */ @@ -88,9 +83,8 @@ object UnitActionsUpgrade { && unit.upgrade.canUpgrade(unitToUpgradeTo = upgradedUnit) ) } - ) + )) } - return upgradeActions } fun getUpgradeActions(unit: MapUnit) =