-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wrong city-state shown in notification log after being destroyed #11486
Comments
Wow! Can you please provide the autosave of the turn before, so we can debug how this happened? |
Ah crap, I do not have access to those autosaves anymore... I already started a new game, I guess they have been overwritten... I did not know providing a save more turns prior to a bug occuring is useful for debugging. I will remember this for the next time I submit a bug report! I hope this save and bug report is still somewhat useful for now. |
The notification is correct. Load your save, go to diplomacy though the "Belgrade" city's menu -> Belgrade the City belongs to Kathmandu the Nation. How you managed to do that is another matter... Look at the replay: Border changes colour from turn 212 to turn 213 - Kathmandu the Nation conquered Belgrade then lost Kathmandu the City to Babylon on turn 253. In other words: Not city destroyed but city-state destroyed - key wording. Should we rename City-States that happen to survive losing their Capital to maybe "Ex-Kathmandu"?1 Footnotes
|
Wow, great find! I did not know city-states could do that, haha. |
They can, though it needs several unlikely happenstances. You seen them conquering another city from time to time, but they always raze and go back to their one-loneliness. In this case, they couldn't - either because it's forbidden to raze an original capital, or because they lost their own before they could raze, or both. I'll leave this open - somebody decide |
I'm definitely fine with it as-is, but having the city-state AI change the name to "new X" would be a cool Easter egg for this edge case |
Playing to Temptation, eh? Well, changing Nation.name or civName is out of the question. Regrettably - if we had that open feature where a Nation could be played by more than one Civilization, possibly with different Leaders - then we'd probably already have the flexibility... So, a displayName... Uh, and civName should definitely have a Kdoc - and be Like this:Subject: [PATCH] civName linting
---
Index: tests/src/com/unciv/logic/map/TileImprovementConstructionTests.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/tests/src/com/unciv/logic/map/TileImprovementConstructionTests.kt b/tests/src/com/unciv/logic/map/TileImprovementConstructionTests.kt
--- a/tests/src/com/unciv/logic/map/TileImprovementConstructionTests.kt (revision 008fbea81788b1b39caffeda5b35e5762e662704)
+++ b/tests/src/com/unciv/logic/map/TileImprovementConstructionTests.kt (date 1713709184698)
@@ -56,7 +56,7 @@
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 @@
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 @@
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 @@
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 @@
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
Index: tests/src/com/unciv/testing/TestGame.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/tests/src/com/unciv/testing/TestGame.kt b/tests/src/com/unciv/testing/TestGame.kt
--- a/tests/src/com/unciv/testing/TestGame.kt (revision 008fbea81788b1b39caffeda5b35e5762e662704)
+++ b/tests/src/com/unciv/testing/TestGame.kt (date 1713709184746)
@@ -133,7 +133,7 @@
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()
Index: core/src/com/unciv/logic/civilization/Civilization.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/core/src/com/unciv/logic/civilization/Civilization.kt b/core/src/com/unciv/logic/civilization/Civilization.kt
--- a/core/src/com/unciv/logic/civilization/Civilization.kt (revision 008fbea81788b1b39caffeda5b35e5762e662704)
+++ b/core/src/com/unciv/logic/civilization/Civilization.kt (date 1713709504227)
@@ -58,6 +58,7 @@
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 @@
/** 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()
@@ -701,6 +711,11 @@
//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 =\
Index: tests/src/com/unciv/logic/map/UnitMovementTests.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/tests/src/com/unciv/logic/map/UnitMovementTests.kt b/tests/src/com/unciv/logic/map/UnitMovementTests.kt
--- a/tests/src/com/unciv/logic/map/UnitMovementTests.kt (revision 008fbea81788b1b39caffeda5b35e5762e662704)
+++ b/tests/src/com/unciv/logic/map/UnitMovementTests.kt (date 1713709184754)
@@ -176,7 +176,7 @@
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) |
Nonono change the city name!! |
you mean in this case, rename the City "Belgrade" which was formerly owned and founded by Belgrade the Nation, and is now the last bastion of the Nation Kathmandu - to - "New Kathmandu (formerly known as Belgrade)" (...or... to "Belgrade (last bastion of Kathmandu, the wussies that managed to lose their name-giving capital)" ... 🤔 )... Won't change the observation of this issue - the notification doesn't display any city name, it just has a city-pointing LocationAction. Becomes clearer only after clicking it. Also, I don't think CityButton is able to wrap...? Seems to me some more effort is involved anyway. |
... and translatability of that easter-egg name, and the "city names exhausted" feature and its non-translatability, ... save
(Thank Yair for that console, for without, it would have been a nightmare to stage such a save) |
Is there an existing issue for this?
Game Version
4.11.4
Describe the bug
In one of my games Babylon attacked and destroyed the city-state of Belgrade. When Belgrade was destoryed, a notification appears saying the city-state of Kathmandu has been destroyed. Clicking on this notification points towards Belgrade.
The city-state of Kathmandu was already destroyed and puppeted earlier in the game by Babylon.
Steps to Reproduce
Screenshots
No response
Link to save file
https://hastebin.com/share/mafawewapu.bash
Operating System
Android
Additional Information
Had to use a paste website because the save file is longer than 65536 characters.
The text was updated successfully, but these errors were encountered: