UX: Auto rename new capital in rare cases to prevent confusing notifications later (#11507)

* Rename new capital when a civ survives losing their civ-named capital

* Linting

* Make "overflow" city founding names translatable

* Fix some unit tests relying on hardcoded Random() results
This commit is contained in:
SomeTroglodyte
2024-04-23 22:59:53 +02:00
committed by GitHub
parent e8714fb950
commit 5c6d9171c4
11 changed files with 106 additions and 45 deletions

View File

@ -73,6 +73,14 @@ Get [unitName] = Obtener [unitName]
Hydro Plant = Central Hidroeléctrica
[buildingName] obsoleted = [buildingName] obsoleto
# City naming
New [cityName] = Nueva [cityName]
Neo [cityName] = Neo [cityName]
Nova [cityName] = Nova [cityName]
Altera [cityName] = Altera [cityName]
New [civName]\n(formerly known as [cityName]) = Nueva [civName]\n(anteriormente conocida como [cityName])
# Diplomacy,Trade,Nations
Requires [buildingName] to be built in the city = Requiere un [buildingName] en la ciudad
@ -268,7 +276,6 @@ Military Rank = Rango Militar
Military near City-State = Ejército cerca de Ciudad-Estado
Sum: = Suma:
# Trades
Trade = Comercio
@ -6905,4 +6912,3 @@ Miscellaneous = Temas Varios
External links = Enlaces externos
External links support right-click or long press to copy the link to the clipboard instead of launching the browser. = Se puede dar click derecho o mantener presionado para copiar el link, en vez de abrirlo el navegador.
Example: The 'Open Github page' button on the Mod management screen. = Ejemplo: El botón de "Abrir página de Github" en el administrador de Mods

View File

@ -73,6 +73,14 @@ Get [unitName] =
Hydro Plant =
[buildingName] obsoleted =
# City naming when list exhausted
New [cityName] =
Neo [cityName] =
Nova [cityName] =
Altera [cityName] =
New [civName]\n(formerly known as [cityName]) =
# Diplomacy,Trade,Nations
Requires [buildingName] to be built in the city =
@ -268,7 +276,6 @@ Military Rank =
Military near City-State =
Sum: =
# Trades
Trade =

View File

@ -563,7 +563,7 @@ object Battle {
} else if (attackerCiv.isHuman() && !(UncivGame.Current.settings.autoPlay.fullAutoPlayAI)) {
// we're not taking our former capital
attackerCiv.popupAlerts.add(PopupAlert(AlertType.CityConquered, city.id))
} else automateCityConquer(attackerCiv, city)
} else automateCityConquer(attackerCiv, city)
if (attackerCiv.isCurrentPlayer())
UncivGame.Current.settings.addCompletedTutorialTask("Conquer a city")

View File

@ -80,7 +80,8 @@ class CityConquestFunctions(val city: City) {
*/
private fun conquerCity(conqueringCiv: Civilization, conqueredCiv: Civilization, receivingCiv: Civilization) {
city.espionage.removeAllPresentSpies(SpyFleeReason.CityCaptured)
// Gain gold for plundering city
val goldPlundered = getGoldForCapturingCity(conqueringCiv)
conqueringCiv.addGold(goldPlundered)
conqueringCiv.addNotification("Received [$goldPlundered] Gold for capturing [${city.name}]",
@ -112,9 +113,6 @@ class CityConquestFunctions(val city: City) {
/** This happens when we either puppet OR annex, basically whenever we conquer a city and don't liberate it */
fun puppetCity(conqueringCiv: Civilization) {
// Gain gold for plundering city
@Suppress("UNUSED_VARIABLE") // todo: use this val
val goldPlundered = getGoldForCapturingCity(conqueringCiv)
val oldCiv = city.civ
// must be before moving the city to the conquering civ,
@ -251,7 +249,7 @@ class CityConquestFunctions(val city: City) {
// Remove/relocate palace for old Civ - need to do this BEFORE we move the cities between
// civs so the capitalCityIndicator recognizes the unique buildings of the conquered civ
if (city.isCapital()) oldCiv.moveCapitalToNextLargest(city)
if (city.isCapital()) oldCiv.moveCapitalToNextLargest(city)
oldCiv.cities = oldCiv.cities.toMutableList().apply { remove(city) }
newCiv.cities = newCiv.cities.toMutableList().apply { add(city) }

View File

@ -24,9 +24,8 @@ class CityFounder {
city.name = generateNewCityName(
civInfo,
civInfo.gameInfo.civilizations.asSequence().filter { civ -> civ.isAlive() }.toSet(),
arrayListOf("New ", "Neo ", "Nova ", "Altera ")
) ?: "City Without A Name"
civInfo.gameInfo.civilizations.asSequence().filter { civ -> civ.isAlive() }.toSet()
) ?: NamingConstants.fallback
city.isOriginalCapital = civInfo.citiesCreated == 0
if (city.isOriginalCapital) {
@ -100,6 +99,11 @@ class CityFounder {
return city
}
private object NamingConstants {
/** Prefixes to add when every base name is taken, ordered. */
val prefixes = arrayListOf("New", "Neo", "Nova", "Altera")
const val fallback = "City Without A Name"
}
/**
* Generates and returns a new city name for the [foundingCiv].
@ -112,13 +116,11 @@ class CityFounder {
*
* @param foundingCiv The civilization that founded this city.
* @param aliveCivs Every civilization currently alive.
* @param prefixes Prefixes to add when every base name is taken, ordered.
* @return A new city name in [String]. Null if failed to generate a name.
*/
private fun generateNewCityName(
foundingCiv: Civilization,
aliveCivs: Set<Civilization>,
prefixes: List<String>
aliveCivs: Set<Civilization>
): String? {
val usedCityNames: Set<String> =
aliveCivs.asSequence().flatMap { civilization ->
@ -139,14 +141,14 @@ class CityFounder {
// If the nation doesn't have the unique above,
// return the first missing name with an increasing number of prefixes attached
// TODO: Make prefixes moddable per nation? Support suffixes?
var candidate: String?
for (number in (1..10)) {
for (prefix in prefixes) {
val currentPrefix: String = prefix.repeat(number)
candidate = foundingCiv.nation.cities.firstOrNull { cityName ->
(currentPrefix + cityName) !in usedCityNames
}
if (candidate != null) return currentPrefix + candidate
for (prefix in NamingConstants.prefixes) {
val repeatedPredix = "$prefix [".repeat(number)
val suffix = "]".repeat(number)
val candidate = foundingCiv.nation.cities.asSequence()
.map { repeatedPredix + it + suffix }
.firstOrNull { it !in usedCityNames }
if (candidate != null) return candidate
}
}

View File

@ -58,6 +58,7 @@ import com.unciv.models.stats.Stats
import com.unciv.models.translations.tr
import com.unciv.ui.components.extensions.toPercent
import com.unciv.ui.screens.victoryscreen.RankingType
import org.jetbrains.annotations.VisibleForTesting
import kotlin.math.max
import kotlin.math.min
import kotlin.math.roundToInt
@ -141,7 +142,16 @@ class Civilization : IsPartOfGameInfoSerialization {
/** The Civ's gold reserves. Public get, private set - please use [addGold] method to modify. */
var gold = 0
private set
/** The Civ's name
*
* - must always be equal to Nation.name (except in the unit test code, where only local consistency is needed)
* - used as uniquely identifying key, so no two players can used the same Nation
* - Displayed and translated as-is
*/
var civName = ""
private set
var tech = TechManager()
var policies = PolicyManager()
var civConstructions = CivConstructions()
@ -705,6 +715,11 @@ class Civilization : IsPartOfGameInfoSerialization {
//region state-changing functions
@VisibleForTesting
fun setNameForUnitTests(name: String) {
civName = name
}
/** This is separate because the REGULAR setTransients updates the viewable ties,
* 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 =\
@ -911,6 +926,7 @@ class Civilization : IsPartOfGameInfoSerialization {
oldCapital?.cityConstructions?.removeBuilding(oldCapital.capitalCityIndicator())
}
/** @param oldCapital `null` when destroying, otherwise old capital */
fun moveCapitalToNextLargest(oldCapital: City?) {
val availableCities = cities.filterNot { it.isCapital() }
if (availableCities.none()) {
@ -924,6 +940,13 @@ class Civilization : IsPartOfGameInfoSerialization {
newCapital = availableCities.maxByOrNull { it.population.population }!!
newCapital.annexCity()
}
// Slight "Easter egg": see #11486: In the rare case a City-state loses their last city but it's not their original capital, the notification names the Nation which confuses players.
// Rename the newly conquered city when the conquering Nation's first-city name is equal to the nation name (meaning Babylon too) and the civ has lost that...
val currentCapital = getCapital()
if (currentCapital != null && currentCapital.isOriginalCapital && civName == currentCapital.name)
newCapital.name = "New [${civName}]\n(formerly known as [${newCapital.name}])"
moveCapitalTo(newCapital, oldCapital)
}

View File

@ -5,6 +5,11 @@ import com.unciv.models.translations.tr
import com.unciv.ui.popups.AskTextPopup
import com.unciv.ui.screens.basescreen.BaseScreen
/** Popup to allow renaming a [city].
*
* Note - The translated name will be offered, and translation markers are removed.
* The saved name will not treat translation in any way, so possibly the user will see his text unexpectedly translated if there is a translation entry for it.
*/
class CityRenamePopup(val screen: BaseScreen, val city: City, val actionOnClose: ()->Unit) {
init {
AskTextPopup(

View File

@ -3,6 +3,8 @@ package com.unciv.logic.city
import com.badlogic.gdx.math.Vector2
import com.unciv.logic.city.managers.CityConquestFunctions
import com.unciv.logic.civilization.Civilization
import com.unciv.models.ruleset.unique.UniqueType
import com.unciv.models.translations.fillPlaceholders
import com.unciv.testing.GdxTestRunner
import com.unciv.testing.TestGame
import org.junit.Assert.assertEquals
@ -23,6 +25,8 @@ class CityConquestFunctionsTest {
private val testGame = TestGame()
private val gainFreeBuildingTestingUniqueText = UniqueType.GainFreeBuildings.text.fillPlaceholders("Monument", "in all cities")
@Before
fun setUp() {
testGame.makeHexagonalMap(3)
@ -51,18 +55,34 @@ class CityConquestFunctionsTest {
@Test
fun `should plunder gold upon capture`() {
// given
testGame.gameInfo.turns = 50
fun resetPlunderState() {
testGame.gameInfo.turns = 50
attackerCiv.addGold(-attackerCiv.gold)
if (defenderCity.civ == defenderCiv) return
cityConquestFunctions.puppetCity(defenderCiv)
defenderCity.population.setPopulation(10)
defenderCity.turnAcquired = 0
}
// when
cityConquestFunctions.puppetCity(attackerCiv)
// Do not assume because TileBasedRandom ensures deterministic results to hardcode those
// Ranges are from hardcoded function in getGoldForCapturingCity - all modifiers are == 1
// Repeat test without reinitializing the CityConquestFunctions.tileBasedRandom to get coverage
repeat(20) {
// given
resetPlunderState()
// then
assertEquals(159, attackerCiv.gold)
// when
cityConquestFunctions.puppetCity(attackerCiv)
// then
assertTrue(attackerCiv.gold in 120 until 160) // Range for city size 10 and all modifiers 1
}
}
@Test
fun `should plunder more gold the more people live in the captured city`() {
// No repeat like `should plunder gold upon capture` - mor work and one range check might be enough
// given
val largeCity = testGame.addCity(defenderCiv, testGame.getTile(Vector2.X), initialPopulation = 20)
val smallCity = testGame.addCity(defenderCiv, testGame.getTile(Vector2.Y), initialPopulation = 5)
@ -75,14 +95,15 @@ class CityConquestFunctionsTest {
// then
assertTrue(attackerCiv.gold > secondAttackerCiv.gold)
assertEquals(230, attackerCiv.gold)
assertEquals(87, secondAttackerCiv.gold)
assertTrue(attackerCiv.gold in 220 until 260) // population 20
assertTrue(secondAttackerCiv.gold in 70 until 110) // population 5
}
@Test
fun `should plunder more gold with building uniques upone capture`() {
fun `should plunder more gold with building uniques upon capture`() {
// given
val building = testGame.createBuilding("Doubles Gold given to enemy if city is captured")
val building = testGame.createBuilding(UniqueType.DoublesGoldFromCapturingCity.text)
val city = testGame.addCity(defenderCiv, testGame.getTile(Vector2.X), initialPopulation = 10)
city.cityConstructions.addBuilding(building.name)
@ -95,9 +116,8 @@ class CityConquestFunctionsTest {
// then
assertTrue(attackerCiv.gold < secondAttackerCiv.gold)
// note: not exactly double because there is some randomness
assertEquals(159, attackerCiv.gold)
assertEquals(260, secondAttackerCiv.gold)
assertTrue(attackerCiv.gold in 120 until 160) // Range for city size 10 and all modifiers 1
assertTrue(secondAttackerCiv.gold in 240 until 320) // Doubled by Unique
}
@Test
@ -106,7 +126,7 @@ class CityConquestFunctionsTest {
val wonders = mutableListOf<String>()
// remove randomness
repeat(20) {
val wonder = testGame.createWonder("Science gained from research agreements [+${it}]%")
val wonder = testGame.createWonder(UniqueType.ScienceFromResearchAgreements.text.fillPlaceholders("$it"))
defenderCity.cityConstructions.addBuilding(wonder.name)
wonders.add(wonder.name)
}
@ -192,7 +212,7 @@ class CityConquestFunctionsTest {
@Test
fun `should remove free buildings upon city moving`() {
// given
val freeBuildingCiv = testGame.addCiv("Gain a free [Monument] [in all cities]")
val freeBuildingCiv = testGame.addCiv(gainFreeBuildingTestingUniqueText)
val city = testGame.addCity(freeBuildingCiv, testGame.getTile(Vector2.Y), initialPopulation = 10)
// when
@ -205,7 +225,7 @@ class CityConquestFunctionsTest {
@Test
fun `should give free buildings upon city moving`() {
// given
val freeBuildingCiv = testGame.addCiv("Gain a free [Monument] [in all cities]")
val freeBuildingCiv = testGame.addCiv(gainFreeBuildingTestingUniqueText)
// when
cityConquestFunctions.moveToCiv(freeBuildingCiv)

View File

@ -56,7 +56,7 @@ class TileImprovementConstructionTests {
tile.setTransients()
if (improvement.uniqueTo != null) {
civInfo.civName = improvement.uniqueTo!!
civInfo.setNameForUnitTests(improvement.uniqueTo!!)
}
val canBeBuilt = tile.improvementFunctions.canBuildImprovement(improvement, civInfo)
@ -92,7 +92,7 @@ class TileImprovementConstructionTests {
for (improvement in testGame.ruleset.tileImprovements.values) {
if (!improvement.uniques.contains("Can only be built on [Coastal] tiles")) continue
civInfo.civName = improvement.uniqueTo ?: "OtherCiv"
civInfo.setNameForUnitTests(improvement.uniqueTo ?: "OtherCiv")
val canBeBuilt = coastalTile.improvementFunctions.canBuildImprovement(improvement, civInfo)
Assert.assertTrue(improvement.name, canBeBuilt)
}
@ -104,7 +104,7 @@ class TileImprovementConstructionTests {
for (improvement in testGame.ruleset.tileImprovements.values) {
if (!improvement.uniques.contains("Can only be built on [Coastal] tiles")) continue
civInfo.civName = improvement.uniqueTo ?: "OtherCiv"
civInfo.setNameForUnitTests(improvement.uniqueTo ?: "OtherCiv")
val canBeBuilt = tile.improvementFunctions.canBuildImprovement(improvement, civInfo)
Assert.assertFalse(improvement.name, canBeBuilt)
}
@ -114,7 +114,7 @@ class TileImprovementConstructionTests {
fun uniqueToOtherImprovementsCanNOTBeBuilt() {
for (improvement in testGame.ruleset.tileImprovements.values) {
if (improvement.uniqueTo == null) continue
civInfo.civName = "OtherCiv"
civInfo.setNameForUnitTests("OtherCiv")
val tile = tileMap[1,1]
val canBeBuilt = tile.improvementFunctions.canBuildImprovement(improvement, civInfo)
Assert.assertFalse(improvement.name, canBeBuilt)
@ -165,7 +165,7 @@ class TileImprovementConstructionTests {
tile.resource = "Sheep"
tile.setTransients()
tile.addTerrainFeature("Hill")
civInfo.civName = "Inca"
civInfo.setNameForUnitTests("Inca")
for (improvement in testGame.ruleset.tileImprovements.values) {
if (!improvement.uniques.contains("Cannot be built on [Bonus resource] tiles")) continue

View File

@ -176,7 +176,7 @@ class UnitMovementTests {
fun canNOTPassThroughTileWithEnemyUnits() {
val barbCiv = Civilization()
barbCiv.gameInfo = testGame.gameInfo
barbCiv.civName = Constants.barbarians // they are always enemies
barbCiv.setNameForUnitTests(Constants.barbarians) // they are always enemies
barbCiv.nation = Nation().apply { name = Constants.barbarians }
testGame.gameInfo.civilizations.add(barbCiv)

View File

@ -133,7 +133,7 @@ class TestGame {
val civInfo = Civilization()
civInfo.nation = nation
civInfo.gameInfo = gameInfo
civInfo.civName = nation.name
civInfo.setNameForUnitTests(nation.name)
if (isPlayer) civInfo.playerType = PlayerType.Human
civInfo.setTransients()