From 5f523cb548fdd5dd3f62ffb50a606530feabf294 Mon Sep 17 00:00:00 2001 From: SomeTroglodyte <63000004+SomeTroglodyte@users.noreply.github.com> Date: Wed, 26 May 2021 22:31:00 +0200 Subject: [PATCH] Improved location notifications (#3992) * Improved location notifications * Improved location notifications - patch * Improved location notifications - patch2 * Improved location notifications - patch3 --- .../jsons/translations/template.properties | 1 + core/src/com/unciv/logic/battle/Battle.kt | 60 +++++++++++-------- .../unciv/logic/city/CityExpansionManager.kt | 11 ++-- .../logic/civilization/CivilizationInfo.kt | 59 ++++++++++-------- core/src/com/unciv/logic/map/MapUnit.kt | 35 ++++++----- 5 files changed, 99 insertions(+), 67 deletions(-) diff --git a/android/assets/jsons/translations/template.properties b/android/assets/jsons/translations/template.properties index 3f3ae257da..8ee74c2c57 100644 --- a/android/assets/jsons/translations/template.properties +++ b/android/assets/jsons/translations/template.properties @@ -402,6 +402,7 @@ Enemy city [cityName] has attacked our [ourUnit] = An enemy [unit] has captured [cityName] = An enemy [unit] has captured our [ourUnit] = An enemy [unit] has destroyed our [ourUnit] = +Your [ourUnit] has destroyed an enemy [unit] = An enemy [RangedUnit] has destroyed the defence of [cityName] = Enemy city [cityName] has destroyed our [ourUnit] = An enemy [unit] was destroyed while attacking [cityName] = diff --git a/core/src/com/unciv/logic/battle/Battle.kt b/core/src/com/unciv/logic/battle/Battle.kt index 6d0678eb7f..6a90b2f5a4 100644 --- a/core/src/com/unciv/logic/battle/Battle.kt +++ b/core/src/com/unciv/logic/battle/Battle.kt @@ -3,10 +3,7 @@ package com.unciv.logic.battle import com.unciv.Constants import com.unciv.UncivGame import com.unciv.logic.city.CityInfo -import com.unciv.logic.civilization.AlertType -import com.unciv.logic.civilization.CivilizationInfo -import com.unciv.logic.civilization.NotificationIcon -import com.unciv.logic.civilization.PopupAlert +import com.unciv.logic.civilization.* import com.unciv.logic.civilization.diplomacy.DiplomaticModifiers import com.unciv.logic.map.RoadStatus import com.unciv.logic.map.TileInfo @@ -50,7 +47,8 @@ object Battle { // Withdraw from melee ability if (attacker is MapUnitCombatant && attacker.isMelee() && defender is MapUnitCombatant) { - val withdraw = defender.unit.getMatchingUniques("May withdraw before melee ([]%)").firstOrNull() + val withdraw = defender.unit.getMatchingUniques("May withdraw before melee ([]%)") + .maxByOrNull{ it.params[0] } // If a mod allows multiple withdraw properties, ensure the best is used if (withdraw != null && doWithdrawFromMeleeAbility(attacker, defender, withdraw)) return } @@ -58,7 +56,7 @@ object Battle { takeDamage(attacker, defender) - postBattleNotifications(attacker, defender, attackedTile) + postBattleNotifications(attacker, defender, attackedTile, attacker.getTile()) postBattleNationUniques(defender, attackedTile, attacker) @@ -154,8 +152,13 @@ object Battle { } - private fun postBattleNotifications(attacker: ICombatant, defender: ICombatant, attackedTile: TileInfo) { - if (attacker.getCivInfo() != defender.getCivInfo()) { // If what happened was that a civilian unit was captures, that's dealt with in the CaptureCilvilianUnit function + private fun postBattleNotifications( + attacker: ICombatant, + defender: ICombatant, + attackedTile: TileInfo, + attackerTile: TileInfo? = null + ) { + if (attacker.getCivInfo() != defender.getCivInfo()) { // If what happened was that a civilian unit was captures, that's dealt with in the captureCivilianUnit function val whatHappenedString = if (attacker !is CityCombatant && attacker.isDefeated()) " was destroyed while attacking" else " has " + ( @@ -176,7 +179,12 @@ object Battle { val cityIcon = "ImprovementIcons/Citadel" val attackerIcon = if (attacker is CityCombatant) cityIcon else attacker.getName() val defenderIcon = if (defender is CityCombatant) cityIcon else defender.getName() - defender.getCivInfo().addNotification(notificationString, attackedTile.position, attackerIcon, NotificationIcon.War, defenderIcon) + val locations = LocationAction ( + if (attackerTile != null && attackerTile.position != attackedTile.position) + listOf(attackedTile.position, attackerTile.position) + else listOf(attackedTile.position) + ) + defender.getCivInfo().addNotification(notificationString, locations, attackerIcon, NotificationIcon.War, defenderIcon) } } @@ -260,16 +268,16 @@ object Battle { && otherCombatant.getCivInfo().isBarbarian()) return - var XPModifier = 1f + var xpModifier = 1f for (unique in thisCombatant.getCivInfo().getMatchingUniques("[] units gain []% more Experience from combat")) { if (thisCombatant.unit.matchesFilter(unique.params[0])) - XPModifier += unique.params[1].toFloat() / 100 + xpModifier += unique.params[1].toFloat() / 100 } for (unique in thisCombatant.unit.getMatchingUniques("[]% Bonus XP gain")) - XPModifier += unique.params[0].toFloat() / 100 + xpModifier += unique.params[0].toFloat() / 100 - val XPGained = (amount * XPModifier).toInt() - thisCombatant.unit.promotions.XP += XPGained + val xpGained = (amount * xpModifier).toInt() + thisCombatant.unit.promotions.XP += xpGained if (thisCombatant.getCivInfo().isMajorCiv()) { @@ -283,7 +291,7 @@ object Battle { greatGeneralPointsModifier += unique.params[1].toFloat() / 100 } - val greatGeneralPointsGained = (XPGained * greatGeneralPointsModifier).toInt() + val greatGeneralPointsGained = (xpGained * greatGeneralPointsModifier).toInt() thisCombatant.getCivInfo().greatPeople.greatGeneralPoints += greatGeneralPointsGained } } @@ -343,7 +351,7 @@ object Battle { if (capturedUnit.name == Constants.settler) { val tile = capturedUnit.getTile() capturedUnit.destroy() - // This is so that future checks which check if a unit has been caatured are caught give the right answer + // This is so that future checks which check if a unit has been captured are caught give the right answer // For example, in postBattleMoveToAttackedTile capturedUnit.civInfo = attacker.getCivInfo() attacker.getCivInfo().placeUnitNearTile(tile.position, Constants.worker) @@ -416,7 +424,7 @@ object Battle { // Instead of postBattleAction() just destroy the missile, all other functions are not relevant if ((attacker as MapUnitCombatant).unit.hasUnique("Self-destructs when attacking")) { - (attacker as MapUnitCombatant).unit.destroy() + attacker.unit.destroy() } } @@ -434,21 +442,22 @@ object Battle { val attackerName = attacker.getName() val interceptorName = interceptor.name + val locations = LocationAction(listOf(interceptor.currentTile.position, attacker.unit.currentTile.position)) if (attacker.isDefeated()) { attacker.getCivInfo() .addNotification("Our [$attackerName] was destroyed by an intercepting [$interceptorName]", - attackerName, NotificationIcon.War, interceptorName) + interceptor.currentTile.position, attackerName, NotificationIcon.War, interceptorName) defender.getCivInfo() .addNotification("Our [$interceptorName] intercepted and destroyed an enemy [$attackerName]", - interceptor.currentTile.position, interceptorName, NotificationIcon.War, attackerName) + locations, interceptorName, NotificationIcon.War, attackerName) } else { attacker.getCivInfo() .addNotification("Our [$attackerName] was attacked by an intercepting [$interceptorName]", - attackerName, NotificationIcon.War, interceptorName) + interceptor.currentTile.position, attackerName, NotificationIcon.War, interceptorName) defender.getCivInfo() .addNotification("Our [$interceptorName] intercepted and attacked an enemy [$attackerName]", - interceptor.currentTile.position, interceptorName, NotificationIcon.War, attackerName) + locations, interceptorName, NotificationIcon.War, attackerName) } return } @@ -456,8 +465,8 @@ object Battle { private fun doWithdrawFromMeleeAbility(attacker: ICombatant, defender: ICombatant, withdrawUnique: Unique): Boolean { // Some notes... - // unit.getUniques() is a union of baseunit uniques and promotion effects. - // according to some strategy guide the slinger's withdraw ability is inherited on upgrade, + // unit.getUniques() is a union of BaseUnit uniques and Promotion effects. + // according to some strategy guide the Slinger's withdraw ability is inherited on upgrade, // according to the Ironclad entry of the wiki the Caravel's is lost on upgrade. // therefore: Implement the flag as unique for the Caravel and Destroyer, as promotion for the Slinger. if (attacker !is MapUnitCombatant) return false // allow simple access to unit property @@ -505,8 +514,9 @@ object Battle { val attackingUnit = attackBaseUnit.name; val defendingUnit = defendBaseUnit.name val notificationString = "[$defendingUnit] withdrew from a [$attackingUnit]" - defender.getCivInfo().addNotification(notificationString, toTile.position, defendingUnit, NotificationIcon.War, attackingUnit) - attacker.getCivInfo().addNotification(notificationString, toTile.position, defendingUnit, NotificationIcon.War, attackingUnit) + val locations = LocationAction(listOf(toTile.position, attacker.getTile().position)) + defender.getCivInfo().addNotification(notificationString, locations, defendingUnit, NotificationIcon.War, attackingUnit) + attacker.getCivInfo().addNotification(notificationString, locations, defendingUnit, NotificationIcon.War, attackingUnit) return true } diff --git a/core/src/com/unciv/logic/city/CityExpansionManager.kt b/core/src/com/unciv/logic/city/CityExpansionManager.kt index eb7d6e4106..a09cb50f18 100644 --- a/core/src/com/unciv/logic/city/CityExpansionManager.kt +++ b/core/src/com/unciv/logic/city/CityExpansionManager.kt @@ -2,6 +2,7 @@ package com.unciv.logic.city import com.badlogic.gdx.math.Vector2 import com.unciv.logic.automation.Automation +import com.unciv.logic.civilization.LocationAction import com.unciv.logic.civilization.NotificationIcon import com.unciv.logic.map.TileInfo import com.unciv.ui.utils.withItem @@ -74,7 +75,7 @@ class CityExpansionManager { it.getOwner() == null && it.neighbors.any { tile -> tile.getCity() == cityInfo } } - val chosenTile = tiles.maxBy { Automation.rankTile(it, cityInfo.civInfo) } + val chosenTile = tiles.maxByOrNull { Automation.rankTile(it, cityInfo.civInfo) } if (chosenTile != null) return chosenTile } @@ -134,7 +135,7 @@ class CityExpansionManager { cityInfo.civInfo.updateDetailedCivResources() cityInfo.cityStats.update() - for (unit in tileInfo.getUnits().toList()) // tolisted because we're modifying + for (unit in tileInfo.getUnits().toList()) // toListed because we're modifying if (!unit.civInfo.canEnterTiles(cityInfo.civInfo)) unit.movement.teleportToClosestMoveableTile() @@ -145,8 +146,10 @@ class CityExpansionManager { cultureStored += culture.toInt() if (cultureStored >= getCultureToNextTile()) { val location = addNewTileWithCulture() - if (location != null) - cityInfo.civInfo.addNotification("[" + cityInfo.name + "] has expanded its borders!", location, NotificationIcon.Culture) + if (location != null) { + val locations = LocationAction(listOf(location, cityInfo.location)) + cityInfo.civInfo.addNotification("[" + cityInfo.name + "] has expanded its borders!", locations, NotificationIcon.Culture) + } } } diff --git a/core/src/com/unciv/logic/civilization/CivilizationInfo.kt b/core/src/com/unciv/logic/civilization/CivilizationInfo.kt index f38a94db5c..6bbdbf1640 100644 --- a/core/src/com/unciv/logic/civilization/CivilizationInfo.kt +++ b/core/src/com/unciv/logic/civilization/CivilizationInfo.kt @@ -123,8 +123,8 @@ class CivilizationInfo { toReturn.cities = cities.map { it.clone() } // This is the only thing that is NOT switched out, which makes it a source of ConcurrentModification errors. - // Cloning it by-pointer is a horrific move, since the serialization would go over it ANYWAY and still led to concurrency prolems. - // Cloning it by iterating on the tilemap values may seem ridiculous, but it's a perfectly thread-safe way to go about it, unlike the other solutions. + // Cloning it by-pointer is a horrific move, since the serialization would go over it ANYWAY and still lead to concurrency problems. + // Cloning it by iterating on the tilemap values may seem ridiculous, but it's a perfectly thread-safe way to go about it, unlike the other solutions. toReturn.exploredTiles.addAll(gameInfo.tileMap.values.asSequence().map { it.position }.filter { it in exploredTiles }) toReturn.notifications.addAll(notifications) toReturn.citiesCreated = citiesCreated @@ -169,11 +169,12 @@ class CivilizationInfo { private fun getCivTerritory() = cities.asSequence().flatMap { it.tiles.asSequence() } fun victoryType(): VictoryType { - if (gameInfo.gameParameters.victoryTypes.size == 1) - return gameInfo.gameParameters.victoryTypes.first() // That is the most relevant one + val victoryTypes = gameInfo.gameParameters.victoryTypes + if (victoryTypes.size == 1) + return victoryTypes.first() // That is the most relevant one val victoryType = nation.preferredVictoryType - if (gameInfo.gameParameters.victoryTypes.contains(victoryType)) return victoryType - else return VictoryType.Neutral + return if (victoryType in victoryTypes) victoryType + else VictoryType.Neutral } fun stats() = CivInfoStats(this) @@ -223,7 +224,7 @@ class CivilizationInfo { fun hasResource(resourceName: String): Boolean = getCivResourcesByName()[resourceName]!! > 0 fun getCivWideBuildingUniques(): Sequence = cities.asSequence().flatMap { - it.cityConstructions.builtBuildingUniqueMap.getAllUniques() + city -> city.cityConstructions.builtBuildingUniqueMap.getAllUniques() .filter { it.params.isEmpty() || it.params.last() != "in this city" } } @@ -233,7 +234,7 @@ class CivilizationInfo { fun getMatchingUniques(uniqueTemplate: String): Sequence { return nation.uniqueObjects.asSequence().filter { it.placeholderText == uniqueTemplate } + cities.asSequence().flatMap { - it.cityConstructions.builtBuildingUniqueMap.getUniques(uniqueTemplate).asSequence() + city -> city.cityConstructions.builtBuildingUniqueMap.getUniques(uniqueTemplate).asSequence() .filter { it.params.isEmpty() || it.params.last() != "in this city" } } + policies.policyUniques.getUniques(uniqueTemplate) + @@ -251,7 +252,7 @@ class CivilizationInfo { units = newList if (updateCivInfo) { - // Not relevant when updating tileinfo transients, since some info of the civ itself isn't yet available, + // Not relevant when updating TileInfo transients, since some info of the civ itself isn't yet available, // and in any case it'll be updated once civ info transients are updateStatsForNextTurn() // unit upkeep updateDetailedCivResources() @@ -301,6 +302,7 @@ class CivilizationInfo { fun getEquivalentUnit(baseUnitName: String): BaseUnit { val baseUnit = gameInfo.ruleSet.units[baseUnitName] + @Suppress("FoldInitializerAndIfToElvis") if (baseUnit == null) throw UncivShowableException("Unit $baseUnitName doesn't seem to exist!") if (baseUnit.replaces != null) return getEquivalentUnit(baseUnit.replaces!!) // Equivalent of unique unit is the equivalent of the replaced unit @@ -364,13 +366,15 @@ class CivilizationInfo { fun isAtWar() = diplomacy.values.any { it.diplomaticStatus == DiplomaticStatus.War && !it.otherCiv().isDefeated() } fun getLeaderDisplayName(): String { - var leaderName = nation.getLeaderDisplayName().tr() - if (playerType == PlayerType.AI) - leaderName += " (" + "AI".tr() + ")" - else if (gameInfo.civilizations.count { it.playerType == PlayerType.Human } > 1) - leaderName += " (" + "Human".tr() + " - " + "Hotseat".tr() + ")" - else leaderName += " (" + "Human".tr() + " - " + "Multiplayer".tr() + ")" - return leaderName + return nation.getLeaderDisplayName().tr() + + when { + playerType == PlayerType.AI -> + " (" + "AI".tr() + ")" + gameInfo.civilizations.count { it.playerType == PlayerType.Human } > 1 -> + " (" + "Human".tr() + " - " + "Hotseat".tr() + ")" + else -> + " (" + "Human".tr() + " - " + "Multiplayer".tr() + ")" + } } fun canSignResearchAgreement(): Boolean { @@ -414,8 +418,8 @@ class CivilizationInfo { //region state-changing functions /** This is separate because the REGULAR setTransients updates the viewable ties, - * and the updateVisibleTiles tries to meet civs... - * And if they civs on't yet know who they are then they don;t know if they're barbarians =\ + * and updateVisibleTiles tries to meet civs... + * And if the civs don't yet know who they are then they don't know if they're barbarians =\ * */ fun setNationTransient() { nation = gameInfo.ruleSet.nations[civName] @@ -573,9 +577,11 @@ class CivilizationInfo { val cityToAddTo = city ?: cities.random() if (!gameInfo.ruleSet.units.containsKey(unitName)) return val unit = getEquivalentUnit(unitName) - placeUnitNearTile(cityToAddTo.location, unit.name) - if (unit.isGreatPerson()) - addNotification("A [${unit.name}] has been born in [${cityToAddTo.name}]!", cityToAddTo.location, unit.name) + // silently bail if no tile to place the unit is found + val placedUnit = placeUnitNearTile(cityToAddTo.location, unit.name) + if (placedUnit != null && unit.isGreatPerson()) { + addNotification("A [${unit.name}] has been born in [${cityToAddTo.name}]!", placedUnit.getTile().position, unit.name) + } } fun placeUnitNearTile(location: Vector2, unitName: String): MapUnit? { @@ -632,12 +638,17 @@ class CivilizationInfo { } fun giftMilitaryUnitTo(otherCiv: CivilizationInfo) { - val city = NextTurnAutomation.getClosestCities(this, otherCiv).city1 + val cities = NextTurnAutomation.getClosestCities(this, otherCiv) + val city = cities.city1 val militaryUnit = city.cityConstructions.getConstructableUnits() .filter { !it.unitType.isCivilian() && it.unitType.isLandUnit() } .toList().random() - placeUnitNearTile(city.location, militaryUnit.name) - addNotification("[${otherCiv.civName}] gave us a [${militaryUnit.name}] as gift near [${city.name}]!", city.location, otherCiv.civName, militaryUnit.name) + // placing the unit may fail - in that case stay quiet + val placedUnit = placeUnitNearTile(city.location, militaryUnit.name) ?: return + // Point to the places mentioned in the message _in that order_ (debatable) + val placedLocation = placedUnit.getTile().position + val locations = LocationAction(listOf(placedLocation, cities.city2.location, city.location)) + addNotification("[${otherCiv.civName}] gave us a [${militaryUnit.name}] as gift near [${city.name}]!", locations, otherCiv.civName, militaryUnit.name) } fun getAllyCiv() = allyCivName diff --git a/core/src/com/unciv/logic/map/MapUnit.kt b/core/src/com/unciv/logic/map/MapUnit.kt index 3d28e173d3..1edf566b4d 100644 --- a/core/src/com/unciv/logic/map/MapUnit.kt +++ b/core/src/com/unciv/logic/map/MapUnit.kt @@ -6,6 +6,7 @@ import com.unciv.UncivGame import com.unciv.logic.automation.UnitAutomation import com.unciv.logic.automation.WorkerAutomation import com.unciv.logic.civilization.CivilizationInfo +import com.unciv.logic.civilization.LocationAction import com.unciv.logic.civilization.NotificationIcon import com.unciv.models.ruleset.Ruleset import com.unciv.models.ruleset.Unique @@ -439,19 +440,21 @@ class MapUnit { tile.improvementInProgress = null } + private fun tryProvideProductionToClosestCity(removedTerrainFeature: String) { val tile = getTile() val closestCity = civInfo.cities.minByOrNull { it.getCenterTile().aerialDistanceTo(tile) } + @Suppress("FoldInitializerAndIfToElvis") if (closestCity == null) return val distance = closestCity.getCenterTile().aerialDistanceTo(tile) var productionPointsToAdd = if (distance == 1) 20 else 20 - (distance - 2) * 5 if (tile.owningCity == null || tile.owningCity!!.civInfo != civInfo) productionPointsToAdd = productionPointsToAdd * 2 / 3 if (productionPointsToAdd > 0) { closestCity.cityConstructions.addProductionPoints(productionPointsToAdd) + val locations = LocationAction(listOf(tile.position, closestCity.location)) civInfo.addNotification("Clearing a [$removedTerrainFeature] has created [$productionPointsToAdd] Production for [${closestCity.name}]", - closestCity.location, NotificationIcon.Construction) + locations, NotificationIcon.Construction) } - } private fun heal() { @@ -630,7 +633,8 @@ class MapUnit { val city = civInfo.cities.random(tileBasedRandom) city.population.population++ city.population.autoAssignPopulation() - civInfo.addNotification("We have found survivors in the ruins - population added to [" + city.name + "]", tile.position, NotificationIcon.Growth) + val locations = LocationAction(listOf(tile.position, city.location)) + civInfo.addNotification("We have found survivors in the ruins - population added to [" + city.name + "]", locations, NotificationIcon.Growth) } val researchableAncientEraTechs = tile.tileMap.gameInfo.ruleSet.technologies.values .filter { @@ -752,21 +756,24 @@ class MapUnit { } private fun getCitadelDamage() { - // Check for Citadel damage - val applyCitadelDamage = currentTile.neighbors - .filter { it.getOwner() != null && civInfo.isAtWarWith(it.getOwner()!!) } - .map { it.getTileImprovement() } - .filter { it != null && it.hasUnique("Deal 30 damage to adjacent enemy units") } - .any() + // Check for Citadel damage - note: 'Damage does not stack with other Citadels' + val citadelTile = currentTile.neighbors + .filter { + it.getOwner() != null && civInfo.isAtWarWith(it.getOwner()!!) && + with (it.getTileImprovement()) { + this != null && this.hasUnique("Deal 30 damage to adjacent enemy units") + } + } + .firstOrNull() - if (applyCitadelDamage) { + if (citadelTile != null) { health -= 30 - + val locations = LocationAction(listOf(citadelTile.position,currentTile.position)) if (health <= 0) { - civInfo.addNotification("An enemy [Citadel] has destroyed our [$name]", currentTile.position, name, NotificationIcon.Death) - // todo - add notification for attacking civ + civInfo.addNotification("An enemy [Citadel] has destroyed our [$name]", locations, name, NotificationIcon.Death) + citadelTile.getOwner()?.addNotification("Your [Citadel] has destroyed an enemy [$name]", locations, name, NotificationIcon.Death) destroy() - } else civInfo.addNotification("An enemy [Citadel] has attacked our [$name]", currentTile.position, name) + } else civInfo.addNotification("An enemy [Citadel] has attacked our [$name]", locations, name, NotificationIcon.War) } }