diff --git a/android/assets/jsons/translations/Spanish.properties b/android/assets/jsons/translations/Spanish.properties index c5d4db747c..7c2d73f360 100644 --- a/android/assets/jsons/translations/Spanish.properties +++ b/android/assets/jsons/translations/Spanish.properties @@ -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 - diff --git a/android/assets/jsons/translations/template.properties b/android/assets/jsons/translations/template.properties index 6a9dd51ef5..6a71f7a62c 100644 --- a/android/assets/jsons/translations/template.properties +++ b/android/assets/jsons/translations/template.properties @@ -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 = diff --git a/core/src/com/unciv/logic/battle/Battle.kt b/core/src/com/unciv/logic/battle/Battle.kt index ab4aef8cb8..9c625bdd22 100644 --- a/core/src/com/unciv/logic/battle/Battle.kt +++ b/core/src/com/unciv/logic/battle/Battle.kt @@ -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") diff --git a/core/src/com/unciv/logic/city/managers/CityConquestFunctions.kt b/core/src/com/unciv/logic/city/managers/CityConquestFunctions.kt index d6a34937f6..330dc5cfd5 100644 --- a/core/src/com/unciv/logic/city/managers/CityConquestFunctions.kt +++ b/core/src/com/unciv/logic/city/managers/CityConquestFunctions.kt @@ -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) } diff --git a/core/src/com/unciv/logic/city/managers/CityFounder.kt b/core/src/com/unciv/logic/city/managers/CityFounder.kt index e50be71a80..5c5e1575a8 100644 --- a/core/src/com/unciv/logic/city/managers/CityFounder.kt +++ b/core/src/com/unciv/logic/city/managers/CityFounder.kt @@ -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, - prefixes: List + aliveCivs: Set ): String? { val usedCityNames: Set = 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 } } diff --git a/core/src/com/unciv/logic/civilization/Civilization.kt b/core/src/com/unciv/logic/civilization/Civilization.kt index 0753a6b763..504a058657 100644 --- a/core/src/com/unciv/logic/civilization/Civilization.kt +++ b/core/src/com/unciv/logic/civilization/Civilization.kt @@ -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) } diff --git a/core/src/com/unciv/ui/screens/pickerscreens/CityRenamePopup.kt b/core/src/com/unciv/ui/screens/pickerscreens/CityRenamePopup.kt index 6eea41c74a..5f4f9b24df 100644 --- a/core/src/com/unciv/ui/screens/pickerscreens/CityRenamePopup.kt +++ b/core/src/com/unciv/ui/screens/pickerscreens/CityRenamePopup.kt @@ -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( diff --git a/tests/src/com/unciv/logic/city/CityConquestFunctionsTest.kt b/tests/src/com/unciv/logic/city/CityConquestFunctionsTest.kt index 63f72460ff..ddb42a172a 100644 --- a/tests/src/com/unciv/logic/city/CityConquestFunctionsTest.kt +++ b/tests/src/com/unciv/logic/city/CityConquestFunctionsTest.kt @@ -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() // 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) diff --git a/tests/src/com/unciv/logic/map/TileImprovementConstructionTests.kt b/tests/src/com/unciv/logic/map/TileImprovementConstructionTests.kt index bc70125189..89618d57e3 100644 --- a/tests/src/com/unciv/logic/map/TileImprovementConstructionTests.kt +++ b/tests/src/com/unciv/logic/map/TileImprovementConstructionTests.kt @@ -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 diff --git a/tests/src/com/unciv/logic/map/UnitMovementTests.kt b/tests/src/com/unciv/logic/map/UnitMovementTests.kt index 2831e76aa9..2b2be5ed26 100644 --- a/tests/src/com/unciv/logic/map/UnitMovementTests.kt +++ b/tests/src/com/unciv/logic/map/UnitMovementTests.kt @@ -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) diff --git a/tests/src/com/unciv/testing/TestGame.kt b/tests/src/com/unciv/testing/TestGame.kt index 339d3f00c9..d50092f4d6 100644 --- a/tests/src/com/unciv/testing/TestGame.kt +++ b/tests/src/com/unciv/testing/TestGame.kt @@ -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()