From 2ca7ed154166e40cd119a316bd8589286317e022 Mon Sep 17 00:00:00 2001 From: Paul Pogonyshev Date: Mon, 23 May 2022 22:08:22 +0200 Subject: [PATCH] Resolve misc. issues with commit 93d9fe9cc --- .../logic/civilization/CivilizationInfo.kt | 73 ++++++++++--------- .../unciv/ui/worldscreen/unit/UnitActions.kt | 26 +++---- 2 files changed, 51 insertions(+), 48 deletions(-) diff --git a/core/src/com/unciv/logic/civilization/CivilizationInfo.kt b/core/src/com/unciv/logic/civilization/CivilizationInfo.kt index 6eb33714ee..39ad4a2d06 100644 --- a/core/src/com/unciv/logic/civilization/CivilizationInfo.kt +++ b/core/src/com/unciv/logic/civilization/CivilizationInfo.kt @@ -74,6 +74,12 @@ class CivilizationInfo { @Transient private var units = listOf() + /** + * Index in the unit list above of the unit that is potentially due and is next up for button "Next unit". + */ + @Transient + private var nextPotentiallyDueAt = 0 + @Transient var viewableTiles = setOf() @@ -211,13 +217,6 @@ class CivilizationInfo { */ var attacksSinceTurnStart = ArrayList() - /** - * Queue of all civilization units that have their actions possibly pending. Internally - * might include e.g. non-idle units, so must be filtered before being given to any outside code. - * (Should be small enough that using ArrayList is fine.) - */ - @Transient - private var dueUnits = mutableListOf() var hasMovedAutomatedUnits = false @Transient @@ -271,7 +270,6 @@ class CivilizationInfo { toReturn.totalCultureForContests = totalCultureForContests toReturn.totalFaithForContests = totalFaithForContests toReturn.attacksSinceTurnStart = attacksSinceTurnStart.copy() - toReturn.dueUnits = dueUnits.toMutableList() toReturn.hasMovedAutomatedUnits = hasMovedAutomatedUnits return toReturn } @@ -456,14 +454,18 @@ class CivilizationInfo { fun getCivUnits(): Sequence = units.asSequence() fun getCivGreatPeople(): Sequence = getCivUnits().filter { mapUnit -> mapUnit.isGreatPerson() } + // Similar to getCivUnits(), but the returned list is rotated so that the + // 'nextPotentiallyDueAt' unit is first here. + private fun getCivUnitsStartingAtNexDue() = units.subList(nextPotentiallyDueAt, units.size) + units.subList(0, nextPotentiallyDueAt) + fun addUnit(mapUnit: MapUnit, updateCivInfo: Boolean = true) { - val newList = ArrayList(units) + // Since we create a new list anyway, also rearrange existing units so that + // 'nextPotentiallyDueAt' becomes 0. This way new units are always last to be due + // (can be changed as wanted, just have a predictable place). + var newList = ArrayList(getCivUnitsStartingAtNexDue()) newList.add(mapUnit) units = newList - - // Make sure it is initialized. - getDueUnits() - dueUnits.add(mapUnit) + nextPotentiallyDueAt = 0 if (updateCivInfo) { // Not relevant when updating TileInfo transients, since some info of the civ itself isn't yet available, @@ -474,46 +476,47 @@ class CivilizationInfo { } fun removeUnit(mapUnit: MapUnit) { - val newList = ArrayList(units) + // See comment in addUnit(). + var newList = ArrayList(getCivUnitsStartingAtNexDue()) newList.remove(mapUnit) units = newList - dueUnits.remove(mapUnit) + nextPotentiallyDueAt = 0 + updateStatsForNextTurn() // unit upkeep updateDetailedCivResources() } fun getIdleUnits() = getCivUnits().filter { it.isIdle() } - // Drop all units that are not really 'due' anymore. We do it here to avoid caring how and where it happened. - // Internal side effect: if 'dueUnits' has never been initialized (new game, load game), do it here. - fun getDueUnits(): List { - if (dueUnits.none()) - dueUnits.addAll(units) - return dueUnits.filter { it.due && it.isIdle() } - } + fun getDueUnits(): List = getCivUnitsStartingAtNexDue().filter { it.due && it.isIdle() } fun shouldGoToDueUnit() = UncivGame.Current.settings.checkForDueUnits && getDueUnits().any() // Callers should consider if cycleThroughDueUnits() is not a better choice. fun getNextDueUnit() = getDueUnits().firstOrNull() - fun cycleThroughDueUnits(unitToSkip: MapUnit?): MapUnit? { - var realDueUnits = getDueUnits(); - if (realDueUnits.any()) { - var unit = realDueUnits.first(); - // We shift the unit to the back of the queue. However, the caller may clear its 'due' state if it wants. - dueUnits.remove(unit); - dueUnits.add(unit); + // Return the next due unit, but preferably not 'unitToSkip': this is returned only if it is the only remaining due unit. + fun cycleThroughDueUnits(unitToSkip: MapUnit? = null): MapUnit? { + var returnAt = nextPotentiallyDueAt; + var fallbackAt = -1; - if (unit == unitToSkip && realDueUnits.size > 1) { - unit = realDueUnits[1]; - dueUnits.remove(unit); - dueUnits.add(unit); + do { + if (units[returnAt].due && units[returnAt].isIdle()) { + if (units[returnAt] != unitToSkip) { + nextPotentiallyDueAt = (returnAt + 1) % units.size + return units[returnAt] + } + else fallbackAt = returnAt } - return unit; + returnAt = (returnAt + 1) % units.size + } while (returnAt != nextPotentiallyDueAt) + + if (fallbackAt >= 0) { + nextPotentiallyDueAt = (fallbackAt + 1) % units.size + return units[fallbackAt] } - else return null; + else return null } //endregion diff --git a/core/src/com/unciv/ui/worldscreen/unit/UnitActions.kt b/core/src/com/unciv/ui/worldscreen/unit/UnitActions.kt index 5a0af1a15c..150b888f79 100644 --- a/core/src/com/unciv/ui/worldscreen/unit/UnitActions.kt +++ b/core/src/com/unciv/ui/worldscreen/unit/UnitActions.kt @@ -68,10 +68,10 @@ object UnitActions { addTriggerUniqueActions(unit, actionList) addAddInCapitalAction(unit, actionList, tile) - addToggleActionsAction(unit, actionList, unitTable) - addWaitAction(unit, actionList, worldScreen); + addToggleActionsAction(unit, actionList, unitTable) + return actionList } @@ -822,17 +822,6 @@ object UnitActions { } } - private fun addToggleActionsAction(unit: MapUnit, actionList: ArrayList, unitTable: UnitTable) { - actionList += UnitAction( - type = if (unit.showAdditionalActions) UnitActionType.HideAdditionalActions - else UnitActionType.ShowAdditionalActions, - action = { - unit.showAdditionalActions = !unit.showAdditionalActions - unitTable.update() - } - ) - } - private fun addWaitAction(unit: MapUnit, actionList: ArrayList, worldScreen: WorldScreen) { if (!unit.isIdle()) return if (worldScreen.viewingCiv.getDueUnits().filter { it != unit }.none()) return @@ -844,4 +833,15 @@ object UnitActions { } ) } + + private fun addToggleActionsAction(unit: MapUnit, actionList: ArrayList, unitTable: UnitTable) { + actionList += UnitAction( + type = if (unit.showAdditionalActions) UnitActionType.HideAdditionalActions + else UnitActionType.ShowAdditionalActions, + action = { + unit.showAdditionalActions = !unit.showAdditionalActions + unitTable.update() + } + ) + } }