mirrors/Unciv
1
0
Fork 0
mirror of https://github.com/yairm210/Unciv.git synced 2025-07-10 15:59:33 +07:00
Code Issues Packages Projects Releases Wiki Activity

UX: Notifications for map units select better when tapped (#11921)

Browse Source 
* Use MapUnitAction for more notifications

* Enable MapUnitAction to select by ID

* Forgot "[N] of your cities can bombard..."
This commit is contained in:
SomeTroglodyte
2024-07-09 16:25:38 +02:00
committed by GitHub
parent 81130a541e
commit 3b40620cd6
9 changed files with 51 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
core/src/com/unciv/logic/GameInfo.kt
View File

@ -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<City>) {
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)
}
}

7
core/src/com/unciv/logic/automation/unit/RoadToAutomation.kt
View File

@ -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
}

5
core/src/com/unciv/logic/automation/unit/UnitAutomation.kt
View File

@ -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
}

3
core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt
View File

@ -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)

3
core/src/com/unciv/logic/battle/BattleUnitCapture.kt
View File

@ -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()}]",

28
core/src/com/unciv/logic/civilization/NotificationActions.kt
View File

@ -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<Vector2>): Sequence<MapUnitAction> =
locations.asSequence().map { MapUnitAction(it) }
operator fun invoke(units: Iterable<MapUnit>): Sequence<MapUnitAction> =
units.asSequence().map { MapUnitAction(it) }
}
}

3
core/src/com/unciv/logic/civilization/managers/UnitManager.kt
View File

@ -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()) {

30
core/src/com/unciv/models/ruleset/unique/UniqueTriggerActivation.kt
View File

@ -193,7 +193,7 @@ object UniqueTriggerActivation {
if (actualAmount <= 0) return null
fun placeUnits(): Boolean {
val tilesUnitsWerePlacedOn: MutableList<Vector2> = mutableListOf()
val placedUnits: MutableList<MapUnit> = 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<Vector2> = mutableListOf()
val promotedUnits: MutableList<MapUnit> = 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
}
}

2
tests/src/com/unciv/logic/SerializationTests.kt
View File

@ -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