Fix for the wrong placement of purchased and upgraded units (#2006)

* Place the unit at the current position firstly

* More simple solution

* Try to place unit with traceability taken into account

* Return money and notify about the problem

* Popup dialog instead of notification

* Refactoring: Split UI and logic

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

@@ -293,14 +293,17 @@ class CityConstructions {
         builtBuildings.remove(buildingName)
     }
 
-    fun purchaseConstruction(constructionName: String) {
+    fun purchaseConstruction(constructionName: String): Boolean {
+        if (!getConstruction(constructionName).postBuildEvent(this))
+            return false // nothing built - no pay
+
         cityInfo.civInfo.gold -= getConstruction(constructionName).getGoldCost(cityInfo.civInfo)
-        getConstruction(constructionName).postBuildEvent(this)
         if (currentConstruction == constructionName)
             cancelCurrentConstruction()
 
         cityInfo.cityStats.update()
         cityInfo.civInfo.updateDetailedCivResources() // this building/unit could be a resource-requiring one
+        return true
     }
 
     fun hasBuildableCultureBuilding(): Boolean {

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

@@ -10,7 +10,7 @@ interface IConstruction : INamed {
     fun getGoldCost(civInfo: CivilizationInfo): Int
     fun isBuildable(construction: CityConstructions): Boolean
     fun shouldBeDisplayed(construction: CityConstructions): Boolean
-    fun postBuildEvent(construction: CityConstructions)  // Yes I'm hilarious.
+    fun postBuildEvent(construction: CityConstructions): Boolean  // Yes I'm hilarious.
     fun canBePurchased(): Boolean
 }
 
@@ -61,7 +61,7 @@ open class SpecialConstruction(override var name: String, val description: Strin
         throw Exception("Impossible!")
     }
 
-    override fun postBuildEvent(construction: CityConstructions) {
+    override fun postBuildEvent(construction: CityConstructions): Boolean {
         throw Exception("Impossible!")
     }
 

core/src/com/unciv/logic/map/TileMap.kt

@@ -129,27 +129,30 @@ class TileMap {
     ): MapUnit? {
         val unit = gameInfo.ruleSet.units[unitName]!!.getMapUnit(gameInfo.ruleSet)
 
-        fun isTileMovePotential(tileInfo: TileInfo): Boolean =
-                when {
-                    unit.type.isAirUnit() -> true
-                    unit.type.isWaterUnit() -> tileInfo.isWater || tileInfo.isCityCenter()
-                    else -> tileInfo.isLand
-                }
+        fun getPassableNeighbours(tileInfo: TileInfo): Set<TileInfo> =
+                getTilesAtDistance(tileInfo.position, 1).filter { unit.movement.canPassThrough(it) }.toSet()
 
-        val viableTilesToPlaceUnitInAtDistance1 = getTilesInDistance(position, 1)
-                .filter { isTileMovePotential(it) }.toSet()
-        // This is so that units don't skip over non-potential tiles to go elsewhere -
-        // e.g. a city 2 tiles away from a lake could spawn water units in the lake...Or spawn beyond a mountain range...
-        val viableTilesToPlaceUnitIn = getTilesAtDistance(position, 2)
-                .filter {
-                    isTileMovePotential(it)
-                            && it.neighbors.any { n -> n in viableTilesToPlaceUnitInAtDistance1 }
-                }
-        
-        unit.assignOwner(civInfo, false)  // both the civ name and actual civ need to be in here in order to calculate the canMoveTo...Darn
-        val unitToPlaceTile = viableTilesToPlaceUnitInAtDistance1
-                .firstOrNull { unit.movement.canMoveTo(it) }
-                ?: viableTilesToPlaceUnitIn.firstOrNull { unit.movement.canMoveTo(it) }
+        // both the civ name and actual civ need to be in here in order to calculate the canMoveTo...Darn
+        unit.assignOwner(civInfo, false)
+
+        var unitToPlaceTile : TileInfo? = null
+        // try to place at the original point (this is the most probable scenario)
+        val currentTile = get(position)
+        if (unit.movement.canMoveTo(currentTile)) unitToPlaceTile = currentTile
+
+        // if it's not suitable, try to find another tile nearby
+        if (unitToPlaceTile == null) {
+            var tryCount = 0
+            var potentialCandidates = getPassableNeighbours(currentTile)
+            while (unitToPlaceTile == null && tryCount++ < 10) {
+                unitToPlaceTile = potentialCandidates.firstOrNull { unit.movement.canMoveTo(it) }
+                if (unitToPlaceTile != null) continue
+                // if it's not found yet, let's check their neighbours
+                val newPotentialCandidates = mutableSetOf<TileInfo>()
+                potentialCandidates.forEach { newPotentialCandidates.addAll(getPassableNeighbours(it)) }
+                potentialCandidates = newPotentialCandidates
+            }
+        }
 
         if (unitToPlaceTile == null) {
             civInfo.removeUnit(unit) // since we added it to the civ units in the previous assignOwner

core/src/com/unciv/models/ruleset/Building.kt

@@ -333,12 +333,12 @@ class Building : NamedStats(), IConstruction{
         return getRejectionReason(construction)==""
     }
 
-    override fun postBuildEvent(construction: CityConstructions) {
+    override fun postBuildEvent(construction: CityConstructions): Boolean {
         val civInfo = construction.cityInfo.civInfo
 
         if ("Spaceship part" in uniques) {
             civInfo.victoryManager.currentsSpaceshipParts.add(name, 1)
-            return
+            return true
         }
         construction.addBuilding(name)
 
@@ -379,6 +379,8 @@ class Building : NamedStats(), IConstruction{
             civInfo.updateHasActiveGreatWall()
 
         if("Free Technology" in uniques) civInfo.tech.freeTechs += 1
+
+        return true
     }
 
     fun isStatRelated(stat: Stat): Boolean {

core/src/com/unciv/models/ruleset/unit/BaseUnit.kt

@@ -148,9 +148,9 @@ class BaseUnit : INamed, IConstruction {
         return getRejectionReason(construction) == ""
     }
 
-    override fun postBuildEvent(construction: CityConstructions) {
+    override fun postBuildEvent(construction: CityConstructions): Boolean {
         val unit = construction.cityInfo.civInfo.placeUnitNearTile(construction.cityInfo.location, name)
-        if(unit==null) return // couldn't place the unit, so there's actually no unit =(
+        if(unit==null) return false // couldn't place the unit, so there's actually no unit =(
 
         var XP = construction.getBuiltBuildings().sumBy { it.xpForNewUnits }
         if(construction.cityInfo.civInfo.policies.isAdopted("Total War")) XP += 15
@@ -159,6 +159,8 @@ class BaseUnit : INamed, IConstruction {
         if(unit.type in listOf(UnitType.Melee,UnitType.Mounted,UnitType.Armor)
             && construction.cityInfo.containsBuildingUnique("All newly-trained melee, mounted, and armored units in this city receive the Drill I promotion"))
             unit.promotions.addPromotion("Drill I", isFree = true)
+
+        return true
     }
 
     fun getDirectUpgradeUnit(civInfo: CivilizationInfo):BaseUnit{

core/src/com/unciv/ui/cityscreen/ConstructionsTable.kt

@@ -284,15 +284,14 @@ class ConstructionsTable(val cityScreen: CityScreen) : Table(CameraStageBaseScre
             button.disable()
         } else {
 
-            val constructionGoldCost = construction.getGoldCost(city.civInfo)
-            button.setText("Buy".tr() + " " + constructionGoldCost)
-            button.add(ImageGetter.getStatIcon(Stat.Gold.name)).size(20f).padBottom(2f)
-
-            button.onClick(UncivSound.Coin) {
-                val purchasePrompt = "Currently you have [${city.civInfo.gold}] gold.".tr() + "\n\n" +
-                        "Would you like to purchase [${construction.name}] for [$constructionGoldCost] gold?".tr()
-                YesNoPopup(purchasePrompt, {
-                    cityConstructions.purchaseConstruction(construction.name)
+            fun purchaseConstruction() {
+                if (!cityConstructions.purchaseConstruction(construction.name)) {
+                    Popup(cityScreen).apply {
+                        add("No space available to place [${construction.name}] near [${city.name}]".tr()).row()
+                        addCloseButton()
+                        open()
+                    }
+                } else {
                     if (isSelectedQueueEntry()) {
                         // currentConstruction is removed from the queue by purchaseConstruction
                         // to avoid conflicts with NextTurnAutomation
@@ -303,7 +302,17 @@ class ConstructionsTable(val cityScreen: CityScreen) : Table(CameraStageBaseScre
                     }
                     if (!construction.shouldBeDisplayed(cityConstructions)) cityScreen.selectedConstruction = null
                     cityScreen.update()
-                }, cityScreen).open()
+                }
+            }
+
+            val constructionGoldCost = construction.getGoldCost(city.civInfo)
+            button.setText("Buy".tr() + " " + constructionGoldCost)
+            button.add(ImageGetter.getStatIcon(Stat.Gold.name)).size(20f).padBottom(2f)
+
+            button.onClick(UncivSound.Coin) {
+                val purchasePrompt = "Currently you have [${city.civInfo.gold}] gold.".tr() + "\n\n" +
+                        "Would you like to purchase [${construction.name}] for [$constructionGoldCost] gold?".tr()
+                YesNoPopup(purchasePrompt, { purchaseConstruction() }, cityScreen).open()
             }
 
             if (constructionGoldCost > city.civInfo.gold)