Multiple performance improvements - managed to lower update time of a busy map from 1 second (which is A LOT) to 0.3, still room for improvement but nowhere near as bad!

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

@@ -45,8 +45,16 @@ open class TileInfo {
     val tileImprovement: TileImprovement?
         get() = if (improvement == null) null else GameBasics.TileImprovements[improvement!!]
 
+
+    // This is for performance - since we access the neighbors of a tile ALL THE TIME,
+    // and the neighbors of a tile never change, it's much more CPU efficient to save the list once and for all!
+    @Transient private var internalNeighbors : List<TileInfo>?=null
     val neighbors: List<TileInfo>
-        get() = tileMap.getTilesAtDistance(position, 1)
+        get(){
+            if(internalNeighbors==null)
+                internalNeighbors = tileMap.getTilesAtDistance(position, 1)
+            return internalNeighbors!!
+        }
 
     val height: Int
         get() {

core/src/com/unciv/models/stats/Stats.kt

@@ -16,10 +16,13 @@ open class Stats() {
     }
 
     fun add(other: Stats) {
-        val hashMap = toHashMap()
+        // Doing this through the hashmap is nicer code but is SUPER INEFFICIENT!
-        for (stat in Stat.values())
+        production += other.production
-            hashMap[stat] = hashMap[stat]!! + other.toHashMap()[stat]!!
+        food += other.food
-        setStats(hashMap)
+        gold += other.gold
+        science += other.science
+        culture += other.culture
+        happiness += other.happiness
     }
 
     fun add(stat:Stat, value:Float): Stats {
@@ -35,12 +38,6 @@ open class Stats() {
         return stats
     }
 
-    operator fun unaryMinus(): Stats {
-        val hashMap = toHashMap()
-        for(stat in Stat.values()) hashMap[stat]= -hashMap[stat]!!
-        return Stats(hashMap)
-    }
-
     operator fun times(number: Float): Stats {
         val hashMap = toHashMap()
         for(stat in Stat.values()) hashMap[stat]= number * hashMap[stat]!!

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

@@ -21,8 +21,8 @@ open class TileGroup(var tileInfo: TileInfo) : Group() {
     protected var resourceImage: Image? = null
     protected var improvementImage: Image? =null
     var populationImage: Image? = null
-    private val roadImages = HashMap<String, Image>()
+    private val roadImages = HashMap<TileInfo, Image>()
-    private val borderImages = ArrayList<Image>()
+    private val borderImages = HashMap<TileInfo, List<Image>>() // map of neiboring tile to border images
     protected var civilianUnitImage: Group? = null
     protected var militaryUnitImage: Group? = null
     private val circleImage = ImageGetter.getImage("OtherIcons/Circle.png") // for blue and red circles on the tile
@@ -106,18 +106,36 @@ open class TileGroup(var tileInfo: TileInfo) : Group() {
     }
 
     private fun updateBorderImages() {
-        for (border in borderImages) border.remove() //clear
+        // This is longer than it could be, because of performance -
-        borderImages.clear()
+        // before fixing, about half (!) the time of update() was wasted on
+        // removing all the border images and putting them back again!
+        val tileOwner = tileInfo.getOwner()
+        if (tileOwner == null){
+            for(images in borderImages.values)
+                for(image in images)
+                    image.remove()
 
-        if (tileInfo.getOwner() != null) {
+            borderImages.clear()
-            val civColor = tileInfo.getOwner()!!.getCivilization().getColor()
+            return
-            for (neighbor in tileInfo.neighbors.filter { it.getOwner() != tileInfo.getOwner() }) {
+        }
+
+        val civColor = tileInfo.getOwner()!!.getCivilization().getColor()
+        for (neighbor in tileInfo.neighbors) {
+            val neigborOwner = neighbor.getOwner()
+            if(neigborOwner == tileOwner && borderImages.containsKey(neighbor)) // the neighbor used to not belong to us, but now it's ours
+            {
+                for(image in borderImages[neighbor]!!)
+                    image.remove()
+                borderImages.remove(neighbor)
+            }
+            if(neigborOwner!=tileOwner && !borderImages.containsKey(neighbor)){ // there should be a border here but there isn't
 
                 val relativeHexPosition = tileInfo.position.cpy().sub(neighbor.position)
                 val relativeWorldPosition = HexMath().Hex2WorldCoords(relativeHexPosition)
 
                 // This is some crazy voodoo magic so I'll explain.
-
+                val images = mutableListOf<Image>()
+                borderImages.put(neighbor,images)
                 for(i in -2..2) {
                     val image = ImageGetter.getImage("OtherIcons/Circle.png")
                     image.setSize(5f, 5f)
@@ -136,11 +154,9 @@ open class TileGroup(var tileInfo: TileInfo) : Group() {
 
                     image.color = civColor
                     addActor(image)
-                    borderImages.add(image)
+                    images.add(image)
                 }
-
             }
-
         }
     }
 
@@ -148,9 +164,9 @@ open class TileGroup(var tileInfo: TileInfo) : Group() {
         if (tileInfo.roadStatus !== RoadStatus.None) {
             for (neighbor in tileInfo.neighbors) {
                 if (neighbor.roadStatus === RoadStatus.None) continue
-                if (!roadImages.containsKey(neighbor.position.toString())) {
+                if (!roadImages.containsKey(neighbor)) {
                     val image = ImageGetter.getImage(ImageGetter.WhiteDot)
-                    roadImages[neighbor.position.toString()] = image
+                    roadImages[neighbor] = image
 
                     val relativeHexPosition = tileInfo.position.cpy().sub(neighbor.position)
                     val relativeWorldPosition = HexMath().Hex2WorldCoords(relativeHexPosition)
@@ -168,9 +184,9 @@ open class TileGroup(var tileInfo: TileInfo) : Group() {
                 }
 
                 if (tileInfo.roadStatus === RoadStatus.Railroad && neighbor.roadStatus === RoadStatus.Railroad)
-                    roadImages[neighbor.position.toString()]!!.color = Color.GRAY // railroad
+                    roadImages[neighbor]!!.color = Color.GRAY // railroad
                 else
-                    roadImages[neighbor.position.toString()]!!.color = Color.BROWN // road
+                    roadImages[neighbor]!!.color = Color.BROWN // road
             }
         }
     }
@@ -220,13 +236,15 @@ open class TileGroup(var tileInfo: TileInfo) : Group() {
     }
 
     private fun updateResourceImage(viewable: Boolean) {
-        if(resourceImage!=null){
+        val shouldDisplayResource = UnCivGame.Current.settings.showResourcesAndImprovements
+                && tileInfo.hasViewableResource(tileInfo.tileMap.gameInfo.getPlayerCivilization())
+
+        if(resourceImage!=null && !shouldDisplayResource){
             resourceImage!!.remove()
             resourceImage=null
         }
 
-        if(UnCivGame.Current.settings.showResourcesAndImprovements
+        if(resourceImage==null && shouldDisplayResource) { // Need to add the resource image!
-                && tileInfo.hasViewableResource(tileInfo.tileMap.gameInfo.getPlayerCivilization())) { // Need to add the resource image!
             val fileName = "ResourceIcons/" + tileInfo.resource + "_(Civ5).png"
                 resourceImage = ImageGetter.getImage(fileName)
                 resourceImage!!.setSize(20f, 20f)

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

@@ -42,6 +42,10 @@ class WorldScreen : CameraStageBaseScreen() {
 
         tileMapHolder.addTiles()
 
+        techButton.addClickListener {
+            game.screen = TechPickerScreen(civInfo)
+        }
+
         stage.addActor(tileMapHolder)
         stage.addActor(minimap)
         stage.addActor(topBar)
@@ -87,10 +91,6 @@ class WorldScreen : CameraStageBaseScreen() {
 
     private fun updateTechButton() {
         techButton.isVisible = civInfo.cities.isNotEmpty()
-        techButton.clearListeners()
-        techButton.addClickListener {
-            game.screen = TechPickerScreen(civInfo)
-        }
 
         if (civInfo.tech.currentTechnology() == null)
             techButton.setText("{Pick a tech}!".tr())
@@ -132,6 +132,7 @@ class WorldScreen : CameraStageBaseScreen() {
                 // but the main thread does other stuff, including showing tutorials which guess what? Changes the game data
                 // BOOM! Exception!
                 // That's why this needs to be after the game is saved.
+                bottomBar.unitTable.shouldUpdateVisually=true
                 shouldUpdate=true
 
                 nextTurnButton.setText("Next turn".tr())

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

@@ -25,6 +25,11 @@ class WorldScreenTopBar(val screen: WorldScreen) : Table() {
     private val resourceLabels = HashMap<String, Label>()
     private val resourceImages = HashMap<String, Image>()
     private val happinessImage = ImageGetter.getStatIcon("Happiness")
+    // These are all to improve performance IE recude update time (was 150 ms on my phone, which is a lot!)
+    private val malcontentColor = Color.valueOf("ef5350")
+    val happinessColor = colorFromRGB(92, 194, 77)
+    val malcontentDrawable = ImageGetter.getStatIcon("Malcontent").drawable
+    val happinessDrawable = ImageGetter.getStatIcon("Happiness").drawable
 
     init {
         background = ImageGetter.getDrawable("skin/whiteDot.png").tint(ImageGetter.getBlue().lerp(Color.BLACK, 0.5f))
@@ -123,11 +128,11 @@ class WorldScreenTopBar(val screen: WorldScreen) : Table() {
         happinessLabel.setText(getHappinessText(civInfo))
 
         if (civInfo.happiness < 0) {
-            happinessLabel.setFontColor(Color.valueOf("ef5350"))
+            happinessLabel.setFontColor(malcontentColor)
-            happinessImage.drawable = ImageGetter.getStatIcon("Malcontent").drawable
+            happinessImage.drawable = malcontentDrawable
         } else {
-            happinessLabel.setFontColor(colorFromRGB(92, 194, 77))
+            happinessLabel.setFontColor(happinessColor)
-            happinessImage.drawable = ImageGetter.getStatIcon("Happiness").drawable
+            happinessImage.drawable = happinessDrawable
         }
 
         cultureLabel.setText(getCultureText(civInfo, nextTurnStats))

core/src/com/unciv/ui/worldscreen/unit/IdleUnitButton.kt

@@ -4,8 +4,6 @@ import com.badlogic.gdx.scenes.scene2d.ui.TextButton
 import com.unciv.logic.map.TileInfo
 import com.unciv.ui.utils.CameraStageBaseScreen
 import com.unciv.ui.utils.addClickListener
-import com.unciv.ui.utils.disable
-import com.unciv.ui.utils.enable
 import com.unciv.ui.worldscreen.TileMapHolder
 
 class IdleUnitButton internal constructor(internal val unitTable: UnitTable,
@@ -32,10 +30,5 @@ class IdleUnitButton internal constructor(internal val unitTable: UnitTable,
             unitTable.worldScreen.update()
         }
     }
-
-    internal fun update() {
-        if (getTilesWithIdleUnits().isNotEmpty()) enable()
-        else disable()
-    }
 }
 

core/src/com/unciv/ui/worldscreen/unit/UnitTable.kt

@@ -5,10 +5,7 @@ import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.unciv.logic.map.MapUnit
 import com.unciv.logic.map.TileInfo
 import com.unciv.models.gamebasics.unit.UnitType
-import com.unciv.ui.utils.CameraStageBaseScreen
+import com.unciv.ui.utils.*
-import com.unciv.ui.utils.ImageGetter
-import com.unciv.ui.utils.addClickListener
-import com.unciv.ui.utils.tr
 import com.unciv.ui.worldscreen.WorldScreen
 
 class UnitTable(val worldScreen: WorldScreen) : Table(){
@@ -20,6 +17,10 @@ class UnitTable(val worldScreen: WorldScreen) : Table(){
     var selectedUnit : MapUnit? = null
     var currentlyExecutingAction : String? = null
 
+    // This is so that not on every update(), we will update the unit table.
+    // Most of the time it's the same unit with the same stats so why waste precious time?
+    var shouldUpdateVisually = false
+
     init {
         pad(5f)
 
@@ -33,16 +34,12 @@ class UnitTable(val worldScreen: WorldScreen) : Table(){
     }
 
     fun update() {
-        prevIdleUnitButton.update()
-        nextIdleUnitButton.update()
-        promotionsTable.clear()
-        unitDescriptionLabel.clearListeners()
-
         if(selectedUnit!=null)
         {
             if(selectedUnit!!.civInfo != worldScreen.civInfo) { // The unit that was selected, was captured. It exists but is no longer ours.
                 selectedUnit = null
                 currentlyExecutingAction = null
+                shouldUpdateVisually = true
             }
             else {
                 try {
@@ -50,10 +47,25 @@ class UnitTable(val worldScreen: WorldScreen) : Table(){
                 } catch (ex: Exception) { // The unit that was there no longer exists}
                     selectedUnit = null
                     currentlyExecutingAction = null
+                    shouldUpdateVisually=true
                 }
             }
         }
 
+        if(!shouldUpdateVisually) return
+
+        if(prevIdleUnitButton.getTilesWithIdleUnits().isNotEmpty()) { // more efficient to do this check once for both
+            prevIdleUnitButton.enable()
+            nextIdleUnitButton.enable()
+        }
+        else{
+            prevIdleUnitButton.disable()
+            nextIdleUnitButton.disable()
+        }
+
+        promotionsTable.clear()
+        unitDescriptionLabel.clearListeners()
+
         if(selectedUnit!=null) {
             val unit = selectedUnit!!
             var nameLabelText = unit.name
@@ -84,9 +96,11 @@ class UnitTable(val worldScreen: WorldScreen) : Table(){
         }
 
         pack()
+        shouldUpdateVisually=false
     }
 
     fun tileSelected(selectedTile: TileInfo) {
+        val previouslySelectedUnit = selectedUnit
         if(currentlyExecutingAction=="moveTo"){
             if(selectedUnit!!.movementAlgs()
                     .getShortestPath(selectedTile).isEmpty())
@@ -100,13 +114,16 @@ class UnitTable(val worldScreen: WorldScreen) : Table(){
             currentlyExecutingAction = null
         }
 
-        if(selectedTile.militaryUnit!=null && selectedTile.militaryUnit!!.civInfo == worldScreen.civInfo
+        else if(selectedTile.militaryUnit!=null && selectedTile.militaryUnit!!.civInfo == worldScreen.civInfo
                 && selectedUnit!=selectedTile.militaryUnit)
             selectedUnit = selectedTile.militaryUnit
 
         else if (selectedTile.civilianUnit!=null && selectedTile.civilianUnit!!.civInfo == worldScreen.civInfo
                         && selectedUnit!=selectedTile.civilianUnit)
                 selectedUnit = selectedTile.civilianUnit
+
+        if(selectedUnit != previouslySelectedUnit)
+            shouldUpdateVisually = true
     }
 
 }