StartingLocation-Improvements-be-gone phase 2 (#4975)

core/src/com/unciv/logic/GameStarter.kt

@@ -21,8 +21,8 @@ import kotlin.math.max
 
 object GameStarter {
     // temporary instrumentation while tuning/debugging
-    private const val consoleOutput = true
+    private const val consoleOutput = false
-    private const val consoleTimings = true
+    private const val consoleTimings = false
 
     fun startNewGame(gameSetupInfo: GameSetupInfo): GameInfo {
         if (consoleOutput || consoleTimings)

core/src/com/unciv/logic/MapSaver.kt

@@ -10,7 +10,7 @@ object MapSaver {
     fun json() = GameSaver.json()
 
     private const val mapsFolder = "maps"
-    private const val saveZipped = false
+    private const val saveZipped = true
 
     private fun getMap(mapName:String) = Gdx.files.local("$mapsFolder/$mapName")
 
@@ -38,4 +38,4 @@ object MapSaver {
     fun getMaps(): Array<FileHandle> = Gdx.files.local(mapsFolder).list()
 
     private fun mapFromJson(json:String): TileMap = json().fromJson(TileMap::class.java, json)
-}
+}

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

@@ -158,6 +158,14 @@ class MapParameters {
     }
 
     // For debugging and MapGenerator console output
-    override fun toString() = if (name.isNotEmpty()) "\"$name\""
+    override fun toString() = sequence {
-        else "($mapSize ${if (worldWrap)"wrapped " else ""}$shape $type, Seed $seed, $elevationExponent/$temperatureExtremeness/$resourceRichness/$vegetationRichness/$rareFeaturesRichness/$maxCoastExtension/$tilesPerBiomeArea/$waterThreshold)"
+        if (name.isNotEmpty()) yield("\"$name\" ")
+        yield("($mapSize ")
+        if (worldWrap) yield("wrapped ")
+        yield(shape)
+        if (name.isEmpty()) return@sequence
+        yield(" $type, Seed $seed, ")
+        yield("$elevationExponent/$temperatureExtremeness/$resourceRichness/$vegetationRichness/")
+        yield("$rareFeaturesRichness/$maxCoastExtension/$tilesPerBiomeArea/$waterThreshold")
+    }.joinToString("", postfix = ")")
 }

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

@@ -757,11 +757,6 @@ open class TileInfo {
     }
 
     private fun normalizeTileImprovement(ruleset: Ruleset) {
-        // This runs from map editor too, so the Pseudo-improvements for starting locations need to stay.
-        if (improvement!!.startsWith(TileMap.startingLocationPrefix)) {
-            if (!isLand || getLastTerrain().impassable) improvement = null
-            return
-        }
         val improvementObject = ruleset.tileImprovements[improvement]
         if (improvementObject == null) {
             improvement = null

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

@@ -17,12 +17,13 @@ import kotlin.math.abs
  */
 class TileMap {
     companion object {
+        /** Legacy way to store starting locations - now this is used only in [translateStartingLocationsFromMap] */
         const val startingLocationPrefix = "StartingLocation "
 
         /**
          * To be backwards compatible, a json without a startingLocations element will be recognized by an entry with this marker
          * New saved maps will never have this marker and will always have a serialized startingLocations list even if empty.
-         * New saved maps will also never have "StartingLocation" improvements, these _must_ be converted before use anywhere outside map editor.
+         * New saved maps will also never have "StartingLocation" improvements, these are converted on load in [setTransients].
          */
         private const val legacyMarker = " Legacy "
     }
@@ -452,32 +453,41 @@ class TileMap {
 
     /** Strips all units and starting locations from [TileMap] for specified [Player]
      * Operation in place
+     * 
+     * Currently unreachable code
+     * 
      * @param player units of this player will be removed
      */
     fun stripPlayer(player: Player) {
         tileList.forEach {
-            if (it.improvement == startingLocationPrefix + player.chosenCiv) {
-                it.improvement = null
-            }
             for (unit in it.getUnits()) if (unit.owner == player.chosenCiv) unit.removeFromTile()
         }
+        startingLocations.removeAll(startingLocations.filter { it.nation == player.chosenCiv }) // filter creates a copy, no concurrent modification
+        startingLocationsByNation.remove(player.chosenCiv)
     }
 
     /** Finds all units and starting location of [Player] and changes their [Nation]
      * Operation in place
+     * 
+     * Currently unreachable code
+     * 
      * @param player player whose all units will be changed
      * @param newNation new nation to be set up
      */
     fun switchPlayersNation(player: Player, newNation: Nation) {
+        val newCiv = CivilizationInfo(newNation.name).apply { nation = newNation }
         tileList.forEach {
-            if (it.improvement == startingLocationPrefix + player.chosenCiv) {
-                it.improvement = startingLocationPrefix + newNation.name
-            }
             for (unit in it.getUnits()) if (unit.owner == player.chosenCiv) {
                 unit.owner = newNation.name
-                unit.civInfo = CivilizationInfo(newNation.name).apply { nation = newNation }
+                unit.civInfo = newCiv
             }
         }
+        for (element in startingLocations.filter { it.nation != player.chosenCiv }) {
+            startingLocations.remove(element)
+            if (startingLocations.none { it.nation == newNation.name && it.position == element.position })
+                startingLocations.add(StartingLocation(element.position, newNation.name))
+        }
+        setStartingLocationsTransients()
     }
 
     /**
@@ -496,7 +506,7 @@ class TileMap {
     /**
      *  Scan and remove placeholder improvements from map and build startingLocations from them
      */
-    fun translateStartingLocationsFromMap() {
+    private fun translateStartingLocationsFromMap() {
         startingLocations.clear()
         tileList.asSequence()
             .filter { it.improvement?.startsWith(startingLocationPrefix) == true }
@@ -509,25 +519,22 @@ class TileMap {
         setStartingLocationsTransients()
     }
 
-    /**
+    /** Adds a starting position, maintaining the transients 
-     *  Place placeholder improvements on the map for the startingLocations entries.
+     * @return true if the starting position was not already stored as per [Collection]'s add */
-     *  
+    fun addStartingLocation(nationName: String, tile: TileInfo): Boolean {
-     *  **For use by the map editor only**
+        if (startingLocationsByNation[nationName]?.contains(tile) == true) return false
-     *  
-     *  This is a copy, the startingLocations array and transients are untouched.
-     *  Any actual improvements on the tiles will be overwritten.
-     */
-    fun translateStartingLocationsToMap() {
-        for ((position, nationName) in startingLocations) {
-            get(position).improvement = startingLocationPrefix + nationName
-        }
-    }
-
-    /** Adds a starting position, maintaining the transients */
-    fun addStartingLocation(nationName: String, tile: TileInfo) {
         startingLocations.add(StartingLocation(tile.position, nationName))
         val nationSet = startingLocationsByNation[nationName] ?: hashSetOf<TileInfo>().also { startingLocationsByNation[nationName] = it }
-        nationSet.add(tile)
+        return nationSet.add(tile)
+    }
+
+    /** Removes a starting position, maintaining the transients
+     * @return true if the starting position was removed as per [Collection]'s remove */
+    fun removeStartingLocation(nationName: String, tile: TileInfo): Boolean {
+        if (startingLocationsByNation[nationName]?.contains(tile) != true) return false
+        startingLocations.remove(StartingLocation(tile.position, nationName))
+        return startingLocationsByNation[nationName]!!.remove(tile)
+        // we do not clean up an empty startingLocationsByNation[nationName] set - not worth it
     }
 
     /** Clears starting positions, e.g. after GameStarter is done with them. Does not clear the pseudo-improvements. */

core/src/com/unciv/models/metadata/GameParameters.kt

@@ -49,23 +49,21 @@ class GameParameters { // Default values are the default new game
         return parameters
     }
 
-    // For debugging and MapGenerator console output
+    // For debugging and GameStarter console output
-    override fun toString() = "($difficulty $gameSpeed $startingEra, " +
+    override fun toString() = sequence<String> {
-            "${players.count { it.playerType == PlayerType.Human }} ${PlayerType.Human} " +
+            yield("$difficulty $gameSpeed $startingEra")
-            "${players.count { it.playerType == PlayerType.AI }} ${PlayerType.AI} " +
+            yield("${players.count { it.playerType == PlayerType.Human }} ${PlayerType.Human}")
-            "$numberOfCityStates CS, " +
+            yield("${players.count { it.playerType == PlayerType.AI }} ${PlayerType.AI}")
-            sequence<String> {
+            yield("$numberOfCityStates CS")
-                if (isOnlineMultiplayer) yield("Online Multiplayer")
+            if (isOnlineMultiplayer) yield("Online Multiplayer")
-                if (noBarbarians) yield("No barbs")
+            if (noBarbarians) yield("No barbs")
-                if (oneCityChallenge) yield("OCC")
+            if (oneCityChallenge) yield("OCC")
-                if (!nuclearWeaponsEnabled) yield("No nukes")
+            if (!nuclearWeaponsEnabled) yield("No nukes")
-                if (religionEnabled) yield("Religion")
+            if (religionEnabled) yield("Religion")
-                if (godMode) yield("God mode")
+            if (godMode) yield("God mode")
-                if (VictoryType.Cultural !in victoryTypes) yield("No ${VictoryType.Cultural} Victory")
+            for (victoryType in VictoryType.values()) {
-                if (VictoryType.Diplomatic in victoryTypes) yield("${VictoryType.Diplomatic} Victory")
+                if (victoryType !in victoryTypes) yield("No $victoryType Victory")
-                if (VictoryType.Domination !in victoryTypes) yield("No ${VictoryType.Domination} Victory")
+            }
-                if (VictoryType.Scientific !in victoryTypes) yield("No ${VictoryType.Scientific} Victory")
+            yield(if (mods.isEmpty()) "no mods" else mods.joinToString(",", "mods=(", ")", 6) )
-            }.joinToString() +
+        }.joinToString(prefix = "(", postfix = ")")
-            (if (mods.isEmpty()) ", no mods" else mods.joinToString(",", ", mods=(", ")", 6) ) +
-            ")"
 }

core/src/com/unciv/ui/mapeditor/MapEditorOptionsTable.kt

@@ -61,11 +61,9 @@ class MapEditorOptionsTable(val mapEditorScreen: MapEditorScreen): Table(CameraS
 
         val sliderTab = Table()
 
-        val slider = Slider(1f, 5f, 1f, false, skin)
         val sliderLabel = "{Brush Size} $brushSize".toLabel()
-
+        val slider = UncivSlider(1f, 5f, 1f, initial = brushSize.toFloat()) {
-        slider.onChange {
+            brushSize = it.toInt()
-            brushSize = slider.value.toInt()
             sliderLabel.setText("{Brush Size} $brushSize".tr())
         }
 
@@ -153,23 +151,20 @@ class MapEditorOptionsTable(val mapEditorScreen: MapEditorScreen): Table(CameraS
         }
         editorPickTable.add(AutoScrollPane(improvementsTable).apply { setScrollingDisabled(true, false) }).height(scrollPanelHeight)
 
+        // Menu for the Starting Locations
         val nationTable = Table()
 
-        /** old way improvements for all civs
-         * */
         for (nation in ruleset.nations.values) {
-            if (nation.isSpectator()) continue  // no improvements for spectator
+            if (nation.isSpectator() || nation.isBarbarian()) continue  // no improvements for spectator
 
             val nationImage = getHex(ImageGetter.getNationIndicator(nation, 40f))
             nationImage.onClick {
-                val improvementName = TileMap.startingLocationPrefix + nation.name
                 tileAction = {
-                    it.improvement = improvementName
+                    mapEditorScreen.tileMap.apply {
-                    for ((tileInfo, tileGroups) in mapEditorScreen.mapHolder.tileGroups) {
+                        // toggle the starting location here, note this allows
-                        if (tileInfo.improvement == improvementName && tileInfo != it)
+                        // both multiple locations per nation and multiple nations per tile
-                            tileInfo.improvement = null
+                        if (!addStartingLocation(nation.name, it))
-                        tileInfo.setTerrainTransients()
+                            removeStartingLocation(nation.name, it)
-                        tileGroups.forEach { it.update() }
                     }
                 }
 
@@ -182,6 +177,7 @@ class MapEditorOptionsTable(val mapEditorScreen: MapEditorScreen): Table(CameraS
         editorPickTable.add(AutoScrollPane(nationTable).apply { setScrollingDisabled(true, false) }).height(scrollPanelHeight)
     }
 
+    /** currently unused */
     fun setUnits() {
         editorPickTable.clear()
 

core/src/com/unciv/ui/mapeditor/MapEditorScreen.kt

@@ -36,7 +36,6 @@ class MapEditorScreen(): CameraStageBaseScreen() {
         ImageGetter.setNewRuleset(ruleset)
         tileMap.setTransients(ruleset,false)
         tileMap.setStartingLocationsTransients()
-        tileMap.translateStartingLocationsToMap()
         UncivGame.Current.translations.translationActiveMods = ruleset.mods
 
         mapHolder = EditorMapHolder(this, tileMap)

core/src/com/unciv/ui/mapeditor/SaveAndLoadMapScreen.kt

@@ -182,8 +182,8 @@ class SaveAndLoadMapScreen(mapToSave: TileMap?, save:Boolean = false, previousSc
         }
     }
 
-    fun getMapCloneForSave(mapToSave: TileMap) = mapToSave!!.clone().also {
+    private fun getMapCloneForSave(mapToSave: TileMap) =
-            it.setTransients(setUnitCivTransients = false)
+        mapToSave.clone().apply {
-            it.translateStartingLocationsFromMap()
+            setTransients(setUnitCivTransients = false)
         }
 }

core/src/com/unciv/ui/tilegroups/TileGroup.kt

@@ -332,15 +332,6 @@ open class TileGroup(var tileInfo: TileInfo, var tileSetStrings:TileSetStrings,
     }
 
     private fun removeMissingModReferences() {
-        // This runs from map editor too, so the Pseudo-improvements for starting locations need to stay.
-        // The nations can be checked.
-        val improvementName = tileInfo.improvement
-        if (improvementName != null && improvementName.startsWith(TileMap.startingLocationPrefix)) {
-            val nationName = improvementName.removePrefix(TileMap.startingLocationPrefix)
-            if (!tileInfo.ruleset.nations.containsKey(nationName))
-                tileInfo.improvement = null
-        }
-
         for (unit in tileInfo.getUnits())
             if (!tileInfo.ruleset.nations.containsKey(unit.owner)) unit.removeFromTile()
     }

core/src/com/unciv/ui/tilegroups/TileGroupIcons.kt

@@ -15,6 +15,7 @@ class TileGroupIcons(val tileGroup: TileGroup) {
 
     var improvementIcon: Actor? = null
     var populationIcon: Image? = null //reuse for acquire icon
+    val startingLocationIcons = mutableListOf<Actor>()
 
     var civilianUnitIcon: UnitGroup? = null
     var militaryUnitIcon: UnitGroup? = null
@@ -22,6 +23,7 @@ class TileGroupIcons(val tileGroup: TileGroup) {
     fun update(showResourcesAndImprovements: Boolean, showTileYields: Boolean, tileIsViewable: Boolean, showMilitaryUnit: Boolean, viewingCiv: CivilizationInfo?) {
         updateResourceIcon(showResourcesAndImprovements)
         updateImprovementIcon(showResourcesAndImprovements)
+        updateStartingLocationIcon(showResourcesAndImprovements)
 
         if (viewingCiv != null) updateYieldIcon(showTileYields, viewingCiv)
 
@@ -49,7 +51,7 @@ class TileGroupIcons(val tileGroup: TileGroup) {
     }
 
 
-    fun newUnitIcon(unit: MapUnit?, oldUnitGroup: UnitGroup?, isViewable: Boolean, yFromCenter: Float, viewingCiv: CivilizationInfo?): UnitGroup? {
+    private fun newUnitIcon(unit: MapUnit?, oldUnitGroup: UnitGroup?, isViewable: Boolean, yFromCenter: Float, viewingCiv: CivilizationInfo?): UnitGroup? {
         var newImage: UnitGroup? = null
         // The unit can change within one update - for instance, when attacking, the attacker replaces the defender!
         oldUnitGroup?.unitBaseImage?.remove()
@@ -101,24 +103,21 @@ class TileGroupIcons(val tileGroup: TileGroup) {
     }
 
 
-    fun updateImprovementIcon(showResourcesAndImprovements: Boolean) {
+    private fun updateImprovementIcon(showResourcesAndImprovements: Boolean) {
         improvementIcon?.remove()
         improvementIcon = null
+        if (tileGroup.tileInfo.improvement == null || !showResourcesAndImprovements) return
 
-        if (tileGroup.tileInfo.improvement != null && showResourcesAndImprovements) {
+        val newImprovementImage = ImageGetter.getImprovementIcon(tileGroup.tileInfo.improvement!!)
-            val newImprovementImage = ImageGetter.getImprovementIcon(tileGroup.tileInfo.improvement!!)
+        tileGroup.miscLayerGroup.addActor(newImprovementImage)
-            tileGroup.miscLayerGroup.addActor(newImprovementImage)
+        newImprovementImage.run {
-            newImprovementImage.run {
+            setSize(20f, 20f)
-                setSize(20f, 20f)
+            center(tileGroup)
-                center(tileGroup)
+            this.x -= 22 // left
-                this.x -= 22 // left
+            this.y -= 10 // bottom
-                this.y -= 10 // bottom
+            color = Color.WHITE.cpy().apply { a = 0.7f }
-            }
-            improvementIcon = newImprovementImage
-        }
-        if (improvementIcon != null) {
-            improvementIcon!!.color = Color.WHITE.cpy().apply { a = 0.7f }
         }
+        improvementIcon = newImprovementImage
     }
 
     // JN updating display of tile yields
@@ -144,7 +143,7 @@ class TileGroupIcons(val tileGroup: TileGroup) {
     }
 
 
-    fun updateResourceIcon(showResourcesAndImprovements: Boolean) {
+    private fun updateResourceIcon(showResourcesAndImprovements: Boolean) {
         if (tileGroup.resource != tileGroup.tileInfo.resource) {
             tileGroup.resource = tileGroup.tileInfo.resource
             tileGroup.resourceImage?.remove()
@@ -169,4 +168,39 @@ class TileGroupIcons(val tileGroup: TileGroup) {
     }
 
 
-}
+    private fun updateStartingLocationIcon(showResourcesAndImprovements: Boolean) {
+        // these are visible in map editor only, but making that bit available here seems overkill
+
+        startingLocationIcons.forEach { it.remove() }
+        startingLocationIcons.clear()
+        if (!showResourcesAndImprovements) return
+        if (tileGroup.forMapEditorIcon) return  // the editor options for terrain do not bother to fully initialize, so tileInfo.tileMap would be an uninitialized lateinit
+
+        // Allow display of up to three nations starting locations on the same tile, ignore rest
+        // The sort is just so it shows _some_ deterministic behaviour, otherwise you could get
+        // different stacking order of the same nations in the same editing session
+        val tileInfo = tileGroup.tileInfo
+        val nations = tileInfo.tileMap.startingLocationsByNation.asSequence()
+            .filter { tileInfo in it.value }.map { it.key }.take(3)
+            .sorted().toList()
+        if (nations.isEmpty()) return
+
+        var offsetX = (nations.size - 1) * 4f
+        var offsetY = (nations.size - 1) * 2f
+        for (nation in nations) {
+            val newNationIcon =
+                ImageGetter.getNationIndicator(ImageGetter.ruleset.nations[nation]!!, 20f)
+            tileGroup.miscLayerGroup.addActor(newNationIcon)
+            newNationIcon.run {
+                setSize(20f, 20f)
+                center(tileGroup)
+                x += offsetX
+                y += offsetY
+                color = Color.WHITE.cpy().apply { a = 0.6f }
+            }
+            startingLocationIcons.add(newNationIcon)
+            offsetX -= 8f
+            offsetY -= 4f
+        }
+    } 
+}

core/src/com/unciv/ui/utils/ImageGetter.kt

@@ -254,11 +254,6 @@ object ImageGetter {
     fun getImprovementIcon(improvementName: String, size: Float = 20f): Actor {
         if (improvementName.startsWith("Remove") || improvementName == Constants.cancelImprovementOrder)
             return Table().apply { add(getImage("OtherIcons/Stop")).size(size) }
-        if (improvementName.startsWith(TileMap.startingLocationPrefix)) {
-            val nationName = improvementName.removePrefix(TileMap.startingLocationPrefix)
-            val nation = ruleset.nations[nationName]!!
-            return getNationIndicator(nation, size)
-        }
 
         val iconGroup = getImage("ImprovementIcons/$improvementName").surroundWithCircle(size)