diff --git a/core/src/com/unciv/logic/GameInfo.kt b/core/src/com/unciv/logic/GameInfo.kt index ebea1af04a..47469cbea5 100644 --- a/core/src/com/unciv/logic/GameInfo.kt +++ b/core/src/com/unciv/logic/GameInfo.kt @@ -19,6 +19,7 @@ import com.unciv.logic.city.City import com.unciv.logic.civilization.Civilization import com.unciv.logic.civilization.CivilizationInfoPreview import com.unciv.logic.civilization.LocationAction +import com.unciv.logic.civilization.MapUnitAction import com.unciv.logic.civilization.Notification import com.unciv.logic.civilization.NotificationCategory import com.unciv.logic.civilization.NotificationIcon @@ -510,10 +511,10 @@ class GameInfo : IsPartOfGameInfoSerialization, HasGameInfoSerializationVersion private fun addBombardNotification(thisPlayer: Civilization, cities: List) { if (cities.size < 3) { for (city in cities) - thisPlayer.addNotification("Your city [${city.name}] can bombard the enemy!", city.location, NotificationCategory.War, NotificationIcon.City, NotificationIcon.Crosshair) + thisPlayer.addNotification("Your city [${city.name}] can bombard the enemy!", MapUnitAction(city.location), NotificationCategory.War, NotificationIcon.City, NotificationIcon.Crosshair) } else { - val positions = cities.asSequence().map { it.location } - thisPlayer.addNotification("[${cities.size}] of your cities can bombard the enemy!", LocationAction(positions), NotificationCategory.War, NotificationIcon.City, NotificationIcon.Crosshair) + val notificationActions = cities.asSequence().map { MapUnitAction(it.location) } + thisPlayer.addNotification("[${cities.size}] of your cities can bombard the enemy!", notificationActions, NotificationCategory.War, NotificationIcon.City, NotificationIcon.Crosshair) } } diff --git a/core/src/com/unciv/logic/automation/unit/RoadToAutomation.kt b/core/src/com/unciv/logic/automation/unit/RoadToAutomation.kt index ce93d65018..29aafc742b 100644 --- a/core/src/com/unciv/logic/automation/unit/RoadToAutomation.kt +++ b/core/src/com/unciv/logic/automation/unit/RoadToAutomation.kt @@ -2,6 +2,7 @@ package com.unciv.logic.automation.unit import com.badlogic.gdx.math.Vector2 import com.unciv.logic.civilization.Civilization +import com.unciv.logic.civilization.MapUnitAction import com.unciv.logic.civilization.NotificationCategory import com.unciv.logic.civilization.NotificationIcon import com.unciv.logic.map.MapPathing @@ -54,7 +55,7 @@ class RoadToAutomation(val civInfo: Civilization) { if (foundPath == null) { Log.debug("WorkerAutomation: $unit -> connect road failed") stopAndCleanAutomation(unit) - unit.civ.addNotification("Connect road failed!", currentTile.position, NotificationCategory.Units, NotificationIcon.Construction) + unit.civ.addNotification("Connect road failed!", MapUnitAction(unit), NotificationCategory.Units, NotificationIcon.Construction) return } @@ -71,7 +72,7 @@ class RoadToAutomation(val civInfo: Civilization) { if (currTileIndex == -1) { Log.debug("$unit -> was moved off its connect road path. Operation cancelled.") stopAndCleanAutomation(unit) - unit.civ.addNotification("Connect road cancelled!", currentTile.position, NotificationCategory.Units, unit.name) + unit.civ.addNotification("Connect road cancelled!", MapUnitAction(unit), NotificationCategory.Units, unit.name) return } @@ -83,7 +84,7 @@ class RoadToAutomation(val civInfo: Civilization) { if (unit.currentMovement > 0 && !shouldBuildRoadOnTile(currentTile)) { if (currTileIndex == pathToDest.size - 1) { // The last tile in the path is unbuildable or has a road. stopAndCleanAutomation(unit) - unit.civ.addNotification("Connect road completed!", currentTile.position, NotificationCategory.Units, unit.name) + unit.civ.addNotification("Connect road completed!", MapUnitAction(unit), NotificationCategory.Units, unit.name) return } diff --git a/core/src/com/unciv/logic/automation/unit/UnitAutomation.kt b/core/src/com/unciv/logic/automation/unit/UnitAutomation.kt index 09fd63194e..35faf08fb0 100644 --- a/core/src/com/unciv/logic/automation/unit/UnitAutomation.kt +++ b/core/src/com/unciv/logic/automation/unit/UnitAutomation.kt @@ -12,6 +12,7 @@ import com.unciv.logic.battle.MapUnitCombatant import com.unciv.logic.battle.TargetHelper import com.unciv.logic.city.City import com.unciv.logic.civilization.Civilization +import com.unciv.logic.civilization.MapUnitAction import com.unciv.logic.civilization.NotificationCategory import com.unciv.logic.civilization.diplomacy.DiplomacyFlags import com.unciv.logic.civilization.diplomacy.DiplomaticStatus @@ -514,7 +515,7 @@ object UnitAutomation { val hostileCivs = civInfo.getKnownCivs().filter { it.isAtWarWith(civInfo) || hasPreparationFlag(it) }.toSet() val closeCities = civInfo.threatManager.getNeighboringCitiesOfOtherCivs().filter { it.second.civ in hostileCivs } - val closestDistance = closeCities.minOfOrNull { it.first.getCenterTile().aerialDistanceTo(it.second.getCenterTile()) } + val closestDistance = closeCities.minOfOrNull { it.first.getCenterTile().aerialDistanceTo(it.second.getCenterTile()) } ?: return false val citiesToDefend = closeCities.filter { it.first.getCenterTile().aerialDistanceTo(it.second.getCenterTile()) <= closestDistance + 2 } .map { it.first } @@ -716,7 +717,7 @@ object UnitAutomation { if (tryGoToRuinAndEncampment(unit) && (unit.currentMovement == 0f || unit.isDestroyed)) return if (unit.health < 80 && tryHealUnit(unit)) return if (tryExplore(unit)) return - unit.civ.addNotification("${unit.shortDisplayName()} finished exploring.", unit.currentTile.position, NotificationCategory.Units, unit.name, "OtherIcons/Sleep") + unit.civ.addNotification("${unit.shortDisplayName()} finished exploring.", MapUnitAction(unit), NotificationCategory.Units, unit.name, "OtherIcons/Sleep") unit.action = null } diff --git a/core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt b/core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt index d12f51069a..19448781f4 100644 --- a/core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt +++ b/core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt @@ -7,6 +7,7 @@ import com.unciv.logic.automation.ThreatLevel import com.unciv.logic.automation.civilization.NextTurnAutomation import com.unciv.logic.automation.unit.UnitAutomation.wander import com.unciv.logic.civilization.Civilization +import com.unciv.logic.civilization.MapUnitAction import com.unciv.logic.civilization.NotificationCategory import com.unciv.logic.map.mapunit.MapUnit import com.unciv.logic.map.tile.Tile @@ -163,7 +164,7 @@ class WorkerAutomation( debug("WorkerAutomation: %s -> nothing to do", unit.toString()) - unit.civ.addNotification("${unit.shortDisplayName()} has no work to do.", currentTile.position, NotificationCategory.Units, unit.name, "OtherIcons/Sleep") + unit.civ.addNotification("${unit.shortDisplayName()} has no work to do.", MapUnitAction(unit), NotificationCategory.Units, unit.name, "OtherIcons/Sleep") // Idle CS units should wander so they don't obstruct players so much if (unit.civ.isCityState) diff --git a/core/src/com/unciv/logic/battle/BattleUnitCapture.kt b/core/src/com/unciv/logic/battle/BattleUnitCapture.kt index e840d2de21..c62e7ed2af 100644 --- a/core/src/com/unciv/logic/battle/BattleUnitCapture.kt +++ b/core/src/com/unciv/logic/battle/BattleUnitCapture.kt @@ -4,6 +4,7 @@ import com.badlogic.gdx.math.Vector2 import com.unciv.Constants import com.unciv.logic.civilization.AlertType import com.unciv.logic.civilization.Civilization +import com.unciv.logic.civilization.MapUnitAction import com.unciv.logic.civilization.NotificationCategory import com.unciv.logic.civilization.NotificationIcon import com.unciv.logic.civilization.PlayerType @@ -98,7 +99,7 @@ object BattleUnitCapture { val addedUnit = attacker.getCivInfo().units.placeUnitNearTile(defenderTile.position, defender.getName()) ?: return false addedUnit.currentMovement = 0f addedUnit.health = 50 - attacker.getCivInfo().addNotification("An enemy [${defender.getName()}] has joined us!", addedUnit.getTile().position, NotificationCategory.War, defender.getName()) + attacker.getCivInfo().addNotification("An enemy [${defender.getName()}] has joined us!", MapUnitAction(addedUnit), NotificationCategory.War, defender.getName()) defender.getCivInfo().addNotification( "An enemy [${attacker.getName()}] has captured our [${defender.getName()}]", diff --git a/core/src/com/unciv/logic/civilization/NotificationActions.kt b/core/src/com/unciv/logic/civilization/NotificationActions.kt index 39587ab2d4..9a5b8bf150 100644 --- a/core/src/com/unciv/logic/civilization/NotificationActions.kt +++ b/core/src/com/unciv/logic/civilization/NotificationActions.kt @@ -3,6 +3,7 @@ package com.unciv.logic.civilization import com.badlogic.gdx.math.Vector2 import com.badlogic.gdx.utils.Json import com.badlogic.gdx.utils.JsonValue +import com.unciv.Constants import com.unciv.logic.IsPartOfGameInfoSerialization import com.unciv.logic.city.City import com.unciv.logic.map.mapunit.MapUnit @@ -108,24 +109,29 @@ class MayaLongCountAction : NotificationAction { } } -/** A notification action that shows and selects units on the map. +/** A notification action that shows **and selects** things on the map. * - * Saves and serializes only the location. Activation will select the tile which will select any unit - * on it or cycle through selections if this NotificationAction is the only one on the Notification. + * Saves and serializes only the location and optionally a MapUnit id. + * When an id is present, activation will select that unit if it is still in the given position - otherwise, it will behave like LocationAction. + * Without id, activation will select the tile which will select any unit on it or cycle through selections if this NotificationAction is the only one on the Notification. * When the unit has been moved away, activation still shows the tile and not the unit. - * - * As MapUnits do not have any persistent ID differentiating them from other units of same Civ and BaseUnit, - * this cannot be done significantly better. Should someone add a persisted UUID to MapUnit, please change this too. + * Activation without unit id also works for cities, selecting them - so a bombard is one click less. */ -class MapUnitAction(private val location: Vector2 = Vector2.Zero) : NotificationAction { - constructor(unit: MapUnit) : this(unit.currentTile.position) +class MapUnitAction( + private val location: Vector2 = Vector2.Zero, + private val id: Int = Constants.NO_ID +) : NotificationAction { + constructor(unit: MapUnit) : this(unit.currentTile.position, unit.id) override fun execute(worldScreen: WorldScreen) { - worldScreen.mapHolder.setCenterPosition(location, selectUnit = true) + val selectUnit = id == Constants.NO_ID // This is the unspecific "select any unit on that tile", specific works without this being on + val unit = if (selectUnit) null else + worldScreen.gameInfo.tileMap[location].getUnits().firstOrNull { it.id == id } + worldScreen.mapHolder.setCenterPosition(location, selectUnit = selectUnit, forceSelectUnit = unit) } companion object { // Convenience shortcut as it makes replacing LocationAction calls easier (see above) - operator fun invoke(locations: Iterable): Sequence = - locations.asSequence().map { MapUnitAction(it) } + operator fun invoke(units: Iterable): Sequence = + units.asSequence().map { MapUnitAction(it) } } } diff --git a/core/src/com/unciv/logic/civilization/managers/UnitManager.kt b/core/src/com/unciv/logic/civilization/managers/UnitManager.kt index 657fc9208b..643ddd1f39 100644 --- a/core/src/com/unciv/logic/civilization/managers/UnitManager.kt +++ b/core/src/com/unciv/logic/civilization/managers/UnitManager.kt @@ -4,6 +4,7 @@ import com.badlogic.gdx.math.Vector2 import com.unciv.UncivGame import com.unciv.logic.city.City import com.unciv.logic.civilization.Civilization +import com.unciv.logic.civilization.MapUnitAction import com.unciv.logic.civilization.NotificationCategory import com.unciv.logic.civilization.transients.CivInfoTransientCache import com.unciv.logic.map.mapunit.MapUnit @@ -62,7 +63,7 @@ class UnitManager(val civInfo: Civilization) { // silently bail if no tile to place the unit is found ?: return null if (unit.isGreatPerson) { - civInfo.addNotification("A [${unit.name}] has been born in [${cityToAddTo.name}]!", placedUnit.getTile().position, NotificationCategory.General, unit.name) + civInfo.addNotification("A [${unit.name}] has been born in [${cityToAddTo.name}]!", MapUnitAction(placedUnit), NotificationCategory.General, unit.name) } if (placedUnit.hasUnique(UniqueType.ReligiousUnit) && civInfo.gameInfo.isReligionEnabled()) { diff --git a/core/src/com/unciv/models/ruleset/unique/UniqueTriggerActivation.kt b/core/src/com/unciv/models/ruleset/unique/UniqueTriggerActivation.kt index 42d64f60d8..3289054e10 100644 --- a/core/src/com/unciv/models/ruleset/unique/UniqueTriggerActivation.kt +++ b/core/src/com/unciv/models/ruleset/unique/UniqueTriggerActivation.kt @@ -193,7 +193,7 @@ object UniqueTriggerActivation { if (actualAmount <= 0) return null fun placeUnits(): Boolean { - val tilesUnitsWerePlacedOn: MutableList = mutableListOf() + val placedUnits: MutableList = mutableListOf() repeat(actualAmount) { val placedUnit = when { // Set unit at city if there's an explict city or if there's no tile to set at @@ -207,20 +207,19 @@ object UniqueTriggerActivation { else -> null } - if (placedUnit != null) - tilesUnitsWerePlacedOn.add(placedUnit.getTile().position) + if (placedUnit != null) placedUnits += placedUnit } - if (tilesUnitsWerePlacedOn.isEmpty()) return false + if (placedUnits.isEmpty()) return false val notificationText = getNotificationText( notification, triggerNotificationText, - "Gained [${tilesUnitsWerePlacedOn.size}] [${civUnit.name}] unit(s)" + "Gained [${placedUnits.size}] [${civUnit.name}] unit(s)" ) if (notificationText != null) civInfo.addNotification( notificationText, - MapUnitAction(tilesUnitsWerePlacedOn), + MapUnitAction(placedUnits), NotificationCategory.Units, civUnit.name ) @@ -560,16 +559,17 @@ object UniqueTriggerActivation { if (unitsToPromote.isEmpty()) return null return { - val promotedUnitLocations: MutableList = mutableListOf() + val promotedUnits: MutableList = mutableListOf() for (civUnit in unitsToPromote) { + if (promotionName in civUnit.promotions.promotions) continue civUnit.promotions.addPromotion(promotionName, isFree = true) - promotedUnitLocations.add(civUnit.getTile().position) + promotedUnits += civUnit } if (notification != null) { civInfo.addNotification( notification, - MapUnitAction(promotedUnitLocations), + MapUnitAction(promotedUnits), NotificationCategory.Units, "unitPromotionIcons/$promotionName" ) @@ -955,7 +955,7 @@ object UniqueTriggerActivation { return { unit.healBy(unique.params[0].toInt()) if (notification != null) - unit.civ.addNotification(notification, unit.getTile().position, NotificationCategory.Units) // Do we have a heal icon? + unit.civ.addNotification(notification, MapUnitAction(unit), NotificationCategory.Units) // Do we have a heal icon? true } } @@ -964,7 +964,7 @@ object UniqueTriggerActivation { return { unit.takeDamage(unique.params[0].toInt()) if (notification != null) - unit.civ.addNotification(notification, unit.getTile().position, NotificationCategory.Units) // Do we have a heal icon? + unit.civ.addNotification(notification, MapUnitAction(unit), NotificationCategory.Units) // Do we have a heal icon? true } } @@ -974,7 +974,7 @@ object UniqueTriggerActivation { return { unit.promotions.XP += unique.params[0].toInt() if (notification != null) - unit.civ.addNotification(notification, unit.getTile().position, NotificationCategory.Units) + unit.civ.addNotification(notification, MapUnitAction(unit), NotificationCategory.Units) true } } @@ -985,7 +985,7 @@ object UniqueTriggerActivation { return { (upgradeAction.minBy { (it as UpgradeUnitAction).unitToUpgradeTo.cost }).action!!() if (notification != null) - unit.civ.addNotification(notification, unit.getTile().position, NotificationCategory.Units) + unit.civ.addNotification(notification, MapUnitAction(unit), NotificationCategory.Units) true } } @@ -996,7 +996,7 @@ object UniqueTriggerActivation { return { (upgradeAction.minBy { (it as UpgradeUnitAction).unitToUpgradeTo.cost }).action!!() if (notification != null) - unit.civ.addNotification(notification, unit.getTile().position, NotificationCategory.Units) + unit.civ.addNotification(notification, MapUnitAction(unit), NotificationCategory.Units) true } } @@ -1008,7 +1008,7 @@ object UniqueTriggerActivation { return { unit.promotions.addPromotion(promotion, true) if (notification != null) - unit.civ.addNotification(notification, unit.getTile().position, NotificationCategory.Units, unit.name) + unit.civ.addNotification(notification, MapUnitAction(unit), NotificationCategory.Units, unit.name) true } } diff --git a/tests/src/com/unciv/logic/SerializationTests.kt b/tests/src/com/unciv/logic/SerializationTests.kt index 45ced1441f..152af6c437 100644 --- a/tests/src/com/unciv/logic/SerializationTests.kt +++ b/tests/src/com/unciv/logic/SerializationTests.kt @@ -6,6 +6,7 @@ import com.unciv.logic.civilization.CivilopediaAction import com.unciv.logic.civilization.DiplomacyAction import com.unciv.json.LastSeenImprovement import com.unciv.logic.civilization.LocationAction +import com.unciv.logic.civilization.MapUnitAction import com.unciv.logic.civilization.Notification import com.unciv.logic.map.tile.TileHistory import com.unciv.models.Counter @@ -87,6 +88,7 @@ class SerializationTests { Notification("Oh my goddesses", arrayOf("ReligionIcons/Pray"), listOf(CivilopediaAction("Tutorial/Religion")), Notification.NotificationCategory.Religion), Notification("There's Horses", arrayOf("ResourceIcons/Horses"), LocationAction(Vector2.Zero, Vector2.X).asIterable(), Notification.NotificationCategory.General), Notification("An evil overlord has arisen", arrayOf("PersonalityIcons/Devil"), listOf(DiplomacyAction("Russia")), Notification.NotificationCategory.War), + Notification("Here's a Wizzard", arrayOf("EmojiIcons/Great Scientist"), listOf(MapUnitAction(Vector2.Y, 42)), Notification.NotificationCategory.Units), ) // Neither Notification nor NotificationAction support equality contract