Removed HandleImprovementCompletion function, added test for citadel tile takeover

core/src/com/unciv/logic/map/mapunit/UnitTurnManager.kt

@@ -206,8 +206,7 @@ class UnitTurnManager(val unit: MapUnit) {
             tile.improvementInProgress == RoadStatus.Railroad.name -> tile.addRoad(RoadStatus.Railroad, unit.civ)
             tile.improvementInProgress == Constants.repair -> tile.setRepaired()
             else -> {
-                tile.changeImprovement(tile.improvementInProgress)
-                tile.getTileImprovement()!!.handleImprovementCompletion(unit)
+                tile.changeImprovement(tile.improvementInProgress, unit.civ)
             }
         }
 

core/src/com/unciv/logic/map/tile/Tile.kt

@@ -25,6 +25,7 @@ import com.unciv.models.ruleset.unique.UniqueMap
 import com.unciv.models.ruleset.unique.UniqueType
 import com.unciv.ui.components.extensions.withItem
 import com.unciv.ui.components.extensions.withoutItem
+import com.unciv.ui.screens.worldscreen.unit.actions.UnitActions
 import com.unciv.utils.DebugUtils
 import kotlin.math.abs
 import kotlin.math.min
@@ -311,7 +312,7 @@ open class Tile : IsPartOfGameInfoSerialization {
     /** Does not remove roads */
     fun removeImprovement() = changeImprovement(null)
 
-    fun changeImprovement(improvementStr: String?) {
+    fun changeImprovement(improvementStr: String?, civToHandleCompletion:Civilization? = null) {
         improvementIsPillaged = false
         improvement = improvementStr
 
@@ -329,9 +330,16 @@ open class Tile : IsPartOfGameInfoSerialization {
             setTerrainFeatures(terrainFeatures.filterNot { it in removableTerrainFeatures })
         }
 
+        if (civToHandleCompletion != null && improvementObject != null
+            && improvementObject.hasUnique(UniqueType.TakesOverAdjacentTiles)
+        )
+            UnitActions.takeOverTilesAround(civToHandleCompletion, this)
+
         if (owningCity != null) {
             owningCity!!.civ.cache.updateCivResources()
+            owningCity!!.reassignPopulationDeferred()
         }
+
     }
 
     // function handling when adding a road to the tile

core/src/com/unciv/models/ruleset/tile/TileImprovement.kt

@@ -16,7 +16,6 @@ import com.unciv.models.translations.tr
 import com.unciv.ui.components.extensions.toPercent
 import com.unciv.ui.screens.civilopediascreen.CivilopediaScreen.Companion.showReligionInCivilopedia
 import com.unciv.ui.screens.civilopediascreen.FormattedLine
-import com.unciv.ui.screens.worldscreen.unit.actions.UnitActions
 import kotlin.math.roundToInt
 
 class TileImprovement : RulesetStatsObject() {
@@ -73,15 +72,6 @@ class TileImprovement : RulesetStatsObject() {
         return terrain in terrainsCanBeBuiltOn
     }
 
-    /** ONLY to be called if the improvement was ACTUALLY built, not for simulating builds! */
-    fun handleImprovementCompletion(builder: MapUnit) {
-        val tile = builder.getTile()
-        if (hasUnique(UniqueType.TakesOverAdjacentTiles))
-            UnitActions.takeOverTilesAround(builder.civ, builder.currentTile)
-
-        tile.owningCity?.reassignPopulationDeferred()
-    }
-
     /**
      * Check: Is this improvement allowed on a [given][name] terrain feature?
      *

core/src/com/unciv/ui/screens/worldscreen/unit/actions/UnitActions.kt

@@ -158,8 +158,7 @@ object UnitActions {
 
         return UnitAction(UnitActionType.Create, "Create [$improvementName]",
             action = {
-                tile.changeImprovement(improvementName)
-                tile.getTileImprovement()!!.handleImprovementCompletion(unit)
+                tile.changeImprovement(improvementName, unit.civ)
                 unit.destroy()  // Modders may wish for a nondestructive way, but that should be another Unique
             }.takeIf { unit.currentMovement > 0 })
     }
@@ -493,9 +492,8 @@ object UnitActions {
                 action = {
                     val unitTile = unit.getTile()
                     unitTile.improvementFunctions.removeCreatesOneImprovementMarker()
-                    unitTile.changeImprovement(improvementName)
+                    unitTile.changeImprovement(improvementName, unit.civ)
                     unitTile.stopWorkingOnImprovement()
-                    improvement.handleImprovementCompletion(unit)
 
                     // without this the world screen won't show the improvement because it isn't the 'last seen improvement'
                     unit.civ.cache.updateViewableTiles()

tests/src/com/unciv/logic/map/TileImprovementConstructionTests.kt

@@ -1,6 +1,7 @@
 //  Taken from https://github.com/TomGrill/gdx-testing
 package com.unciv.logic.map
 
+import com.unciv.logic.city.City
 import com.unciv.logic.civilization.Civilization
 import com.unciv.models.ruleset.tile.TerrainType
 import com.unciv.models.ruleset.unique.StateForConditionals
@@ -17,18 +18,19 @@ class TileImprovementConstructionTests {
 
     private lateinit var civInfo: Civilization
     private lateinit var tileMap: TileMap
+    private lateinit var city: City
 
     val testGame = TestGame()
 
 
     @Before
     fun initTheWorld() {
-        testGame.makeHexagonalMap(3)
+        testGame.makeHexagonalMap(4)
         tileMap = testGame.tileMap
         civInfo = testGame.addCiv()
         civInfo.tech.researchedTechnologies.addAll(testGame.ruleset.technologies.values)
         civInfo.tech.techsResearched.addAll(testGame.ruleset.technologies.keys)
-        testGame.addCity(civInfo, tileMap[0,0])
+        city = testGame.addCity(civInfo, tileMap[0,0])
     }
 
 
@@ -148,6 +150,14 @@ class TileImprovementConstructionTests {
         Assert.assertEquals(tile.terrainFeatures, listOf("Hill"))
     }
 
+    @Test
+    fun citadelTakesOverAdjacentTiles() {
+        val tile = tileMap[1,1]
+        Assert.assertFalse(tile.neighbors.all { it.owningCity == city })
+        tile.changeImprovement("Citadel", civInfo)
+        Assert.assertTrue(tile.neighbors.all { it.owningCity == city })
+    }
+
     @Test
     fun terraceFarmCanNOTBeBuiltOnBonus() {
         val tile = tileMap[1,1]