Changes to Puppet - Annex - Liberate:

- From Conquer you can move either to Liberate, or to Puppet -> Annex -> Raze
- All unit actions at battle are completely independant of your decision what you do with the city, so they're not deffered until you decide

core/src/com/unciv/logic/battle/Battle.kt

@@ -94,15 +94,6 @@ class Battle(val gameInfo:GameInfo) {
             }
         }
 
-        if(defender.isDefeated()
-                && defender is CityCombatant
-                && attacker.isMelee()){
-            conquerCity(defender.city, attacker)
-        } else
-            postBattleActionsPart2(attacker, defender, attackedTile)
-    }
-
-    fun postBattleActionsPart2(attacker: ICombatant, defender: ICombatant, attackedTile:TileInfo) {
 
         // German unique - needs to be checked before we try to move to the enemy tile, since the encampment disappears after we move in
         if(defender.isDefeated() && defender.getCivInfo().isBarbarian()
@@ -152,6 +143,11 @@ class Battle(val gameInfo:GameInfo) {
         }
 
 
+        if(defender.isDefeated()
+                && defender is CityCombatant
+                && attacker.isMelee())
+            conquerCity(defender.city, attacker)
+
 
         if(attacker.isMelee()){
             if(!defender.getUnitType().isCivilian()) // unit was not captured but actually attacked
@@ -208,12 +204,18 @@ class Battle(val gameInfo:GameInfo) {
         val attackerCiv = attacker.getCivInfo()
 
         attackerCiv.addNotification("We have conquered the city of [${city.name}]!", city.location, Color.RED)
-        city.conquerer = attacker
+
+        city.getCenterTile().apply {
+            if(militaryUnit!=null) militaryUnit!!.destroy()
+            if(civilianUnit!=null) captureCivilianUnit(attacker, MapUnitCombatant(civilianUnit!!))
+            for(airUnit in airUnits.toList()) airUnit.destroy()
+        }
+
 
         if (attacker.getCivInfo().isPlayerCivilization()) {
             attackerCiv.popupAlerts.add(PopupAlert(AlertType.CityConquered, city.name))
         } else {
-            city.annexCity()
+            city.annexCity(attacker.getCivInfo())
         }
     }
 

core/src/com/unciv/logic/city/CityInfo.kt

@@ -4,10 +4,6 @@ import com.badlogic.gdx.graphics.Color
 import com.badlogic.gdx.math.Vector2
 import com.unciv.Constants
 import com.unciv.UnCivGame
-import com.unciv.logic.battle.Battle
-import com.unciv.logic.battle.CityCombatant
-import com.unciv.logic.battle.ICombatant
-import com.unciv.logic.battle.MapUnitCombatant
 import com.unciv.logic.civilization.AlertType
 import com.unciv.logic.civilization.CivilizationInfo
 import com.unciv.logic.civilization.PopupAlert
@@ -47,7 +43,6 @@ class CityInfo {
     var isBeingRazed = false
     var attackedThisTurn = false
     var hasSoldBuildingThisTurn = false
-    var conquerer : ICombatant? = null
     var isPuppet = false
 
     constructor()   // for json parsing, we need to have a default constructor
@@ -111,7 +106,6 @@ class CityInfo {
         toReturn.attackedThisTurn = attackedThisTurn
         toReturn.resistanceCounter = resistanceCounter
         toReturn.foundingCiv = foundingCiv
-        toReturn.conquerer = conquerer
         toReturn.isPuppet = isPuppet
         return toReturn
     }
@@ -225,9 +219,7 @@ class CityInfo {
         attackedThisTurn = false
         if (resistanceCounter > 0) resistanceCounter--
 
-        if (isPuppet) {
+        if (isPuppet) reassignWorkers()
-            reassignWorkers()
-        }
     }
 
     fun reassignWorkers() {
@@ -235,6 +227,7 @@ class CityInfo {
         population.specialists.clear()
         for (i in 0..population.population)
             population.autoAssignPopulation()
+        cityStats.update()
     }
 
     fun endTurn() {
@@ -279,124 +272,93 @@ class CityInfo {
         getCenterTile().improvement="City ruins"
     }
 
-    fun annexCity() {
+    fun annexCity(conqueringCiv: CivilizationInfo) {
-        if (conquerer == null) return
+        puppetCity(conqueringCiv)
-        val attackerCiv = conquerer!!.getCivInfo()
-        val defenderCiv = civInfo
 
-        getCenterTile().apply {
+        if(!conqueringCiv.policies.isAdopted("Police State")) {
-            if(militaryUnit!=null) militaryUnit!!.destroy()
-            if(civilianUnit!=null) Battle(civInfo.gameInfo).captureCivilianUnit(conquerer!!, MapUnitCombatant(civilianUnit!!))
-            for(airUnit in airUnits.toList()) airUnit.destroy()
-        }
-
-        if (!attackerCiv.isMajorCiv()){
-            destroyCity()
-        }
-        else {
-            val currentPopulation = population.population
-
-            val percentageOfCivPopulationInThatCity = currentPopulation*100f / civInfo.cities.sumBy { it.population.population }
-            val aggroGenerated = 10f+percentageOfCivPopulationInThatCity.roundToInt()
-            civInfo.getDiplomacyManager(attackerCiv)
-                    .addModifier(DiplomaticModifiers.CapturedOurCities, -aggroGenerated)
-
-            for(thirdPartyCiv in attackerCiv.getKnownCivs().filter { it.isMajorCiv() }){
-                val aggroGeneratedForOtherCivs = (aggroGenerated/10).roundToInt().toFloat()
-                if(thirdPartyCiv.isAtWarWith(civInfo)) // You annoyed our enemy?
-                    thirdPartyCiv.getDiplomacyManager(attackerCiv)
-                            .addModifier(DiplomaticModifiers.SharedEnemy, aggroGeneratedForOtherCivs) // Cool, keep at at! =D
-                else thirdPartyCiv.getDiplomacyManager(attackerCiv)
-                        .addModifier(DiplomaticModifiers.WarMongerer, -aggroGeneratedForOtherCivs) // Uncool bro.
-            }
-
-            if(currentPopulation>1) population.population -= 1 + currentPopulation/4 // so from 2-4 population, remove 1, from 5-8, remove 2, etc.
-            population.unassignExtraPopulation()
-
-            health = getMaxHealth() / 2 // I think that cities recover to half health when conquered?
-
-            if(!attackerCiv.policies.isAdopted("Police State")) {
             expansion.cultureStored = 0
             expansion.reset()
         }
 
-            moveToCiv(attackerCiv)
+        isPuppet=false
-            resistanceCounter = population.population
-            workedTiles = hashSetOf() //reassign 1st working tile
-            population.specialists.clear()
-            for (i in 0..population.population)
-                population.autoAssignPopulation()
-            cityStats.update()
-        }
 
-        if(cityConstructions.isBuilt("Palace")){
-            cityConstructions.removeBuilding("Palace")
-            if(defenderCiv.isDefeated()) {
-                defenderCiv.destroy()
-                attackerCiv.popupAlerts.add(PopupAlert(AlertType.Defeated,defenderCiv.civName))
-            }
-            else if(defenderCiv.cities.isNotEmpty()){
-                defenderCiv.cities.first().cityConstructions.addBuilding("Palace") // relocate palace
-            }
-        }
-
-        Battle(civInfo.gameInfo).postBattleActionsPart2(conquerer as MapUnitCombatant, CityCombatant(this), ccenterTile)
-        conquerer = null
         UnCivGame.Current.worldScreen.shouldUpdate=true
     }
 
-
+    /** This happens when we either puppet OR annex, basically whenever we conquer a city and don't liberate it */
-    fun puppetCity() {
+    fun puppetCity(conqueringCiv: CivilizationInfo) {
-        annexCity()
+        if (!conqueringCiv.isMajorCiv()){
-        isPuppet = true
+            destroyCity()
-        resistanceCounter = 0
         }
 
-    fun liberateCity() {
+        val oldCiv = civInfo
-        if (foundingCiv == "") {
+        moveToCiv(conqueringCiv)
-            annexCity()
+        if(oldCiv.isDefeated()) {
+            oldCiv.destroy()
+            conqueringCiv.popupAlerts.add(PopupAlert(AlertType.Defeated,oldCiv.civName))
+        }
+
+        health = getMaxHealth() / 2 // I think that cities recover to half health when conquered?
+
+        diplomaticReprecussionsForConqueringCity(oldCiv, conqueringCiv)
+
+        if(population.population>1) population.population -= 1 + population.population/4 // so from 2-4 population, remove 1, from 5-8, remove 2, etc.
+
+        resistanceCounter = population.population  // I checked, and even if you puppet there's resistance for conquering
+        reassignWorkers()
+
+        isPuppet = true
+    }
+
+    private fun diplomaticReprecussionsForConqueringCity(oldCiv: CivilizationInfo, conqueringCiv: CivilizationInfo) {
+        val currentPopulation = population.population
+        val percentageOfCivPopulationInThatCity = currentPopulation * 100f / civInfo.cities.sumBy { it.population.population }
+        val aggroGenerated = 10f + percentageOfCivPopulationInThatCity.roundToInt()
+        oldCiv.getDiplomacyManager(conqueringCiv)
+                .addModifier(DiplomaticModifiers.CapturedOurCities, -aggroGenerated)
+
+        for (thirdPartyCiv in conqueringCiv.getKnownCivs().filter { it.isMajorCiv() }) {
+            val aggroGeneratedForOtherCivs = (aggroGenerated / 10).roundToInt().toFloat()
+            if (thirdPartyCiv.isAtWarWith(civInfo)) // You annoyed our enemy?
+                thirdPartyCiv.getDiplomacyManager(conqueringCiv)
+                        .addModifier(DiplomaticModifiers.SharedEnemy, aggroGeneratedForOtherCivs) // Cool, keep at at! =D
+            else thirdPartyCiv.getDiplomacyManager(conqueringCiv)
+                    .addModifier(DiplomaticModifiers.WarMongerer, -aggroGeneratedForOtherCivs) // Uncool bro.
+        }
+    }
+
+    /* Liberating is returning a city to its founder - makes you LOSE warmongering points **/
+    fun liberateCity(conqueringCiv: CivilizationInfo) {
+        if (foundingCiv == "") { // this should never happen but just in case...
+            annexCity(conqueringCiv)
             return
         }
 
-        val attackerCiv = conquerer!!.getCivInfo()
+        val oldOwningCiv = civInfo
-        val defenderCiv = civInfo
         val foundingCiv = civInfo.gameInfo.civilizations.first { it.civName == foundingCiv }
 
-        getCenterTile().apply {
-            if(militaryUnit!=null) militaryUnit!!.destroy()
-            if(civilianUnit!=null) civilianUnit!!.destroy()
-            for(airUnit in airUnits.toList()) airUnit.destroy()
-        }
-
         val currentPopulation = population.population
 
         val percentageOfCivPopulationInThatCity = currentPopulation*100f / (foundingCiv.cities.sumBy { it.population.population } + currentPopulation)
-        val aggroGenerated = 10f+percentageOfCivPopulationInThatCity.roundToInt()
+        val respecForLiberatingOurCity = 10f+percentageOfCivPopulationInThatCity.roundToInt()
-        foundingCiv.getDiplomacyManager(attackerCiv)
+        foundingCiv.getDiplomacyManager(conqueringCiv)
-                .addModifier(DiplomaticModifiers.CapturedOurCities, aggroGenerated)
+                .addModifier(DiplomaticModifiers.CapturedOurCities, respecForLiberatingOurCity)
 
-        for(thirdPartyCiv in attackerCiv.getKnownCivs().filter { it.isMajorCiv() }){
+        val otherCivsRespecForLiberating = (respecForLiberatingOurCity/10).roundToInt().toFloat()
-            val aggroGeneratedForOtherCivs = (aggroGenerated/10).roundToInt().toFloat()
+        for(thirdPartyCiv in conqueringCiv.getKnownCivs().filter { it.isMajorCiv() && it != oldOwningCiv }){
-            thirdPartyCiv.getDiplomacyManager(attackerCiv)
+            thirdPartyCiv.getDiplomacyManager(conqueringCiv)
-                        .addModifier(DiplomaticModifiers.WarMongerer, aggroGeneratedForOtherCivs) // Cool, keep at at! =D
+                        .addModifier(DiplomaticModifiers.WarMongerer, otherCivsRespecForLiberating) // Cool, keep at at! =D
         }
 
-        population.unassignExtraPopulation()
-        health = getMaxHealth() // I think that cities recover to half health when conquered?
-
         moveToCiv(foundingCiv)
-        workedTiles = hashSetOf() //reassign 1st working tile
+        health = getMaxHealth() / 2 // I think that cities recover to half health when conquered?
-        population.specialists.clear()
-        for (i in 0..population.population)
-            population.autoAssignPopulation()
-        cityStats.update()
 
-        if(foundingCiv.cities.size == 1) {
+        reassignWorkers()
+
+        if(foundingCiv.cities.size == 1) { // Resurrection!
             cityConstructions.addBuilding("Palace")
         }
 
-        Battle(civInfo.gameInfo).postBattleActionsPart2(conquerer as MapUnitCombatant, CityCombatant(this), ccenterTile)
-        conquerer = null
         UnCivGame.Current.worldScreen.shouldUpdate=true
     }
 
@@ -407,6 +369,7 @@ class CityInfo {
     }
 
     fun moveToCiv(newCivInfo: CivilizationInfo){
+        val oldCiv = civInfo
         civInfo.cities = civInfo.cities.toMutableList().apply { remove(this@CityInfo) }
         newCivInfo.cities = newCivInfo.cities.toMutableList().apply { add(this@CityInfo) }
         civInfo = newCivInfo
@@ -418,6 +381,15 @@ class CityInfo {
         // Remove all national wonders
         for(building in cityConstructions.getBuiltBuildings().filter { it.requiredBuildingInAllCities!=null })
             cityConstructions.removeBuilding(building.name)
+
+        // Remove/relocate palace
+        if(cityConstructions.isBuilt("Palace")){
+            cityConstructions.removeBuilding("Palace")
+            if(oldCiv.cities.isNotEmpty()){
+                oldCiv.cities.first().cityConstructions.addBuilding("Palace") // relocate palace
+            }
+        }
+
         isBeingRazed=false
 
         // Transfer unique buildings
@@ -429,6 +401,8 @@ class CityInfo {
             }
         }
 
+
+
         tryUpdateRoadStatus()
     }
 

core/src/com/unciv/ui/worldscreen/AlertPopup.kt

@@ -59,27 +59,32 @@ class AlertPopup(val worldScreen: WorldScreen, val popupAlert: PopupAlert): Popu
                 }
             }
             AlertType.CityConquered -> {
-                val city = worldScreen.gameInfo.civilizations.flatMap { it->it.cities }.first { it.name == popupAlert.value}
+                val city = worldScreen.gameInfo.civilizations.flatMap { it.cities }.first { it.name == popupAlert.value}
                 addGoodSizedLabel("What would you like to do with the city?").row()
+                val conqueringCiv = worldScreen.gameInfo.currentPlayerCiv
                 if (city.foundingCiv != ""
-                        && city.civInfo.civName != city.foundingCiv
+                        && city.civInfo.civName != city.foundingCiv // can't liberate if the city actually belongs to those guys
-                        && city.conquerer!!.getCivInfo().civName != city.foundingCiv) {
+                        && conqueringCiv.civName != city.foundingCiv) { // or belongs originally to us
                     add(TextButton("Liberate".tr(), skin).onClick {
-                        city.liberateCity()
+                        city.liberateCity(conqueringCiv)
+                        worldScreen.shouldUpdate=true
                         close()
                     }).row()
                 }
                 add(TextButton("Annex".tr(), skin).onClick {
-                    city.annexCity()
+                    city.annexCity(conqueringCiv)
+                    worldScreen.shouldUpdate=true
                     close()
                 }).row()
                 add(TextButton("Puppet City".tr(), skin).onClick {
-                    city.puppetCity()
+                    city.puppetCity(conqueringCiv)
+                    worldScreen.shouldUpdate=true
                     close()
                 }).row()
                 add(TextButton("Raze".tr(), skin).onClick {
-                    city.annexCity()
+                    city.annexCity(conqueringCiv)
                     city.isBeingRazed = true
+                    worldScreen.shouldUpdate=true
                     close()
                 })
             }