Better World-Wrap (#8494)

* Better World-Wrap

* Remove redundant list

* Fix flickering on sides + cleanup of ZoomableScrollPane

* Resolved #8529 - fixed 5hex image issues

---------

Co-authored-by: tunerzinc@gmail.com <vfylfhby>
Co-authored-by: Yair Morgenstern <yairm210@hotmail.com>

core/src/com/unciv/logic/automation/ai/TacticalAI.kt

@@ -38,7 +38,7 @@ class TacticalAI : IsPartOfGameInfoSerialization {
 
             val otherZoneId = tacticalAnalysisMap.plotPositionToZoneId[otherTile.position]
             if (otherZoneId == zoneId) {
-                mapHolder.tileGroups[otherTile]?.forEach {
+                mapHolder.tileGroups[otherTile]?.let {
                     mapHolder.addOverlayOnTileGroup(it, ImageGetter.getCircle().apply {
                         color = when (zone?.territoryType) {
                             TacticalTerritoryType.FRIENDLY -> Color.GREEN
@@ -49,7 +49,7 @@ class TacticalAI : IsPartOfGameInfoSerialization {
             }
 
             if (zone?.neighboringZones?.contains(otherZoneId) == true) {
-                mapHolder.tileGroups[otherTile]?.forEach {
+                mapHolder.tileGroups[otherTile]?.let {
                     mapHolder.addOverlayOnTileGroup(it, ImageGetter.getCircle().apply { color = Color.GRAY }.toGroup(20f)) }
             }
         }

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

@@ -319,7 +319,7 @@ class CityScreen(
             }
         }
 
-        val tileMapGroup = TileGroupMap(tileGroups, tileGroupsToUnwrap = tilesToUnwrap)
+        val tileMapGroup = TileGroupMap(mapScrollPane, tileGroups, tileGroupsToUnwrap = tilesToUnwrap)
         mapScrollPane.actor = tileMapGroup
         mapScrollPane.setSize(stage.width, stage.height)
         stage.addActor(mapScrollPane)

core/src/com/unciv/ui/map/TileGroupMap.kt

@@ -4,7 +4,6 @@ import com.badlogic.gdx.graphics.g2d.Batch
 import com.badlogic.gdx.math.Vector2
 import com.badlogic.gdx.scenes.scene2d.Group
 import com.unciv.logic.map.HexMath
-import com.unciv.logic.map.tile.Tile
 import com.unciv.logic.map.TileMap
 import com.unciv.ui.tilegroups.CityTileGroup
 import com.unciv.ui.tilegroups.TileGroup
@@ -17,6 +16,7 @@ import com.unciv.ui.tilegroups.layers.TileLayerOverlay
 import com.unciv.ui.tilegroups.layers.TileLayerTerrain
 import com.unciv.ui.tilegroups.layers.TileLayerUnitArt
 import com.unciv.ui.tilegroups.layers.TileLayerUnitFlag
+import com.unciv.ui.utils.ZoomableScrollPane
 import kotlin.math.max
 import kotlin.math.min
 
@@ -28,8 +28,9 @@ import kotlin.math.min
  * @param tileGroupsToUnwrap For these, coordinates will be unwrapped using [TileMap.getUnWrappedPosition]
  */
 class TileGroupMap<T: TileGroup>(
+    val mapHolder: ZoomableScrollPane,
     tileGroups: Iterable<T>,
-    worldWrap: Boolean = false,
+    val worldWrap: Boolean = false,
     tileGroupsToUnwrap: Set<T>? = null
 ): Group() {
     companion object {
@@ -56,15 +57,8 @@ class TileGroupMap<T: TileGroup>(
     private var topY = -Float.MAX_VALUE
     private var bottomX = Float.MAX_VALUE
     private var bottomY = Float.MAX_VALUE
-    private val mirrorTileGroups = HashMap<Tile, Pair<T, T>>()
 
     init {
-        if (worldWrap) {
-            for (tileGroup in tileGroups) {
-                @Suppress("UNCHECKED_CAST")  // T is constrained such that casting these TileGroup clones to T should be OK
-                mirrorTileGroups[tileGroup.tile] = Pair(tileGroup.clone() as T, tileGroup.clone() as T)
-            }
-        }
 
         for (tileGroup in tileGroups) {
             val positionalVector = if (tileGroupsToUnwrap?.contains(tileGroup) == true) {
@@ -99,20 +93,6 @@ class TileGroupMap<T: TileGroup>(
             group.moveBy(-bottomX, -bottomY)
         }
 
-        if (worldWrap) {
-            for (mirrorTiles in mirrorTileGroups.values){
-                val positionalVector = HexMath.hex2WorldCoords(mirrorTiles.first.tile.position)
-
-                mirrorTiles.first.setPosition(positionalVector.x * 0.8f * groupSize,
-                    positionalVector.y * 0.8f * groupSize)
-                mirrorTiles.first.moveBy(-bottomX - bottomX * 2, -bottomY )
-
-                mirrorTiles.second.setPosition(positionalVector.x * 0.8f * groupSize,
-                    positionalVector.y * 0.8f * groupSize)
-                mirrorTiles.second.moveBy(-bottomX + bottomX * 2, -bottomY)
-            }
-        }
-
         val baseLayers = ArrayList<TileLayerTerrain>()
         val featureLayers = ArrayList<TileLayerFeatures>()
         val borderLayers = ArrayList<TileLayerBorders>()
@@ -133,20 +113,8 @@ class TileGroupMap<T: TileGroup>(
             circleFogCrosshairLayers.add(group.layerOverlay.apply { setPosition(group.x,group.y) })
             unitLayers.add(group.layerUnitFlag.apply { setPosition(group.x,group.y) })
             cityButtonLayers.add(group.layerCityButton.apply { setPosition(group.x,group.y) })
-
-            if (worldWrap) {
-                for (mirrorTile in mirrorTileGroups[group.tile]!!.toList()) {
-                    baseLayers.add(mirrorTile.layerTerrain.apply { setPosition(mirrorTile.x,mirrorTile.y) })
-                    featureLayers.add(mirrorTile.layerFeatures.apply { setPosition(mirrorTile.x,mirrorTile.y) })
-                    borderLayers.add(mirrorTile.layerBorders.apply { setPosition(mirrorTile.x,mirrorTile.y) })
-                    miscLayers.add(mirrorTile.layerMisc.apply { setPosition(mirrorTile.x,mirrorTile.y) })
-                    pixelUnitLayers.add(mirrorTile.layerUnitArt.apply { setPosition(mirrorTile.x,mirrorTile.y) })
-                    circleFogCrosshairLayers.add(mirrorTile.layerOverlay.apply { setPosition(mirrorTile.x,mirrorTile.y) })
-                    unitLayers.add(mirrorTile.layerUnitFlag.apply { setPosition(mirrorTile.x,mirrorTile.y) })
-                    cityButtonLayers.add(mirrorTile.layerCityButton.apply { setPosition(mirrorTile.x,mirrorTile.y) })
-                }
-            }
         }
+
         for (group in baseLayers) addActor(group)
         for (group in featureLayers) addActor(group)
         for (group in borderLayers) addActor(group)
@@ -154,12 +122,6 @@ class TileGroupMap<T: TileGroup>(
         for (group in pixelUnitLayers) addActor(group)
         for (group in circleFogCrosshairLayers) addActor(group)
         for (group in tileGroups) addActor(group) // The above layers are for the visual layers, this is for the clickability of the tile
-        if (worldWrap) {
-            for (mirrorTiles in mirrorTileGroups.values) {
-                addActor(mirrorTiles.first)
-                addActor(mirrorTiles.second)
-            }
-        }
         for (group in unitLayers) addActor(group) // Aaand units above everything else.
         for (group in cityButtonLayers) addActor(group) // city buttons + clickability
 
@@ -183,15 +145,56 @@ class TileGroupMap<T: TileGroup>(
                 .scl(1f / trueGroupSize)
     }
 
-    fun getMirrorTiles(): HashMap<Tile, Pair<T, T>> = mirrorTileGroups
-
     override fun act(delta: Float) {
         if(shouldAct) {
             super.act(delta)
         }
     }
 
-    // For debugging purposes
+    override fun draw(batch: Batch?, parentAlpha: Float) {
-    override fun draw(batch: Batch?, parentAlpha: Float) { super.draw(batch, parentAlpha) }
+
+        if (worldWrap) {
+
+            // Where is viewport's boundaries
+            val rightSide = mapHolder.scrollX + mapHolder.width/2f
+            val leftSide = mapHolder.scrollX - mapHolder.width/2f
+
+            // Have we looked beyond map?
+            val diffRight = rightSide - topX
+            val diffLeft = leftSide - bottomX
+
+            val beyondRight = diffRight >= 0f
+            val beyondLeft = diffLeft <= 0f
+
+            if (beyondRight || beyondLeft) {
+
+                // If we looked beyond - reposition needed tiles from the other side
+                // and update topX and bottomX accordingly.
+
+                var newBottomX = Float.MAX_VALUE
+                var newTopX = -Float.MAX_VALUE
+
+                children.forEach {
+                    if (beyondRight) {
+                        // Move from left to right
+                        if (it.x - bottomX <= diffRight)
+                            it.x += width
+                    } else if (beyondLeft) {
+                        // Move from right to left
+                        if (it.x + groupSize >= topX + diffLeft)
+                            it.x -= width
+                    }
+                    newBottomX = min(newBottomX, it.x)
+                    newTopX = max(newTopX, it.x + groupSize)
+                }
+
+                bottomX = newBottomX
+                topX = newTopX
+            }
+
+        }
+
+        super.draw(batch, parentAlpha)
+    }
 
 }

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

@@ -31,7 +31,7 @@ class EditorMapHolder(
 ): ZoomableScrollPane(20f, 20f) {
     val editorScreen = parentScreen as? MapEditorScreen
 
-    val tileGroups = HashMap<Tile, List<TileGroup>>()
+    val tileGroups = HashMap<Tile, TileGroup>()
     private lateinit var tileGroupMap: TileGroupMap<TileGroup>
     private val allTileGroups = ArrayList<TileGroup>()
 
@@ -48,37 +48,17 @@ class EditorMapHolder(
         reloadMaxZoom()
     }
 
-    internal fun reloadMaxZoom() {
-        maxZoom = UncivGame.Current.settings.maxWorldZoomOut
-        minZoom = 1f / maxZoom
-        if (scaleX < minZoom) zoom(1f)   // since normally min isn't reached exactly, only powers of 0.8
-    }
-
     internal fun addTiles(stage: Stage) {
 
         val tileSetStrings = TileSetStrings()
         val daTileGroups = tileMap.values.map { TileGroup(it, tileSetStrings) }
 
-        tileGroupMap = TileGroupMap(
+        tileGroupMap = TileGroupMap(this, daTileGroups, continuousScrollingX)
-            daTileGroups,
-            continuousScrollingX)
         actor = tileGroupMap
-        val mirrorTileGroups = tileGroupMap.getMirrorTiles()
 
         for (tileGroup in daTileGroups) {
-            if (continuousScrollingX){
+            allTileGroups.add(tileGroup)
-                val mirrorTileGroupLeft = mirrorTileGroups[tileGroup.tile]!!.first
+            tileGroups[tileGroup.tile] = tileGroup
-                val mirrorTileGroupRight = mirrorTileGroups[tileGroup.tile]!!.second
-
-                allTileGroups.add(tileGroup)
-                allTileGroups.add(mirrorTileGroupLeft)
-                allTileGroups.add(mirrorTileGroupRight)
-
-                tileGroups[tileGroup.tile] = listOf(tileGroup, mirrorTileGroupLeft, mirrorTileGroupRight)
-            } else {
-                tileGroups[tileGroup.tile] = listOf(tileGroup)
-                allTileGroups.add(tileGroup)
-            }
         }
 
         for (tileGroup in allTileGroups) {

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

@@ -196,15 +196,12 @@ class MapEditorScreen(map: TileMap? = null): BaseScreen(), RecreateOnResize {
         highlightedTileGroups.clear()
     }
     fun highlightTile(tile: Tile, color: Color = Color.WHITE) {
-        for (group in mapHolder.tileGroups[tile] ?: return) {
+        val group = mapHolder.tileGroups[tile] ?: return
-            group.layerOverlay.showHighlight(color)
+        group.layerOverlay.showHighlight(color)
-            highlightedTileGroups.add(group)
+        highlightedTileGroups.add(group)
-        }
     }
     fun updateTile(tile: Tile) {
-        mapHolder.tileGroups[tile]!!.forEach {
+        mapHolder.tileGroups[tile]!!.update()
-            it.update()
-        }
     }
     fun updateAndHighlight(tile: Tile, color: Color = Color.WHITE) {
         updateTile(tile)

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

@@ -12,49 +12,54 @@ import com.badlogic.gdx.scenes.scene2d.actions.FloatAction
 import com.badlogic.gdx.scenes.scene2d.ui.ScrollPane
 import com.badlogic.gdx.scenes.scene2d.utils.ActorGestureListener
 import com.badlogic.gdx.scenes.scene2d.utils.Cullable
+import com.unciv.UncivGame
+import java.lang.Float.max
+import java.lang.Float.min
 import kotlin.math.sqrt
 
 
 open class ZoomableScrollPane(
-    val extraCullingX: Float = 0f,
+    private val extraCullingX: Float = 0f,
-    val extraCullingY: Float = 0f,
+    private val extraCullingY: Float = 0f,
     var minZoom: Float = 0.5f,
     var maxZoom: Float = 1 / minZoom // if we can halve the size, then by default also allow to double it
-) : ScrollPane(null) {
+) : ScrollPane(Group()) {
     var continuousScrollingX = false
 
     var onViewportChangedListener: ((width: Float, height: Float, viewport: Rectangle) -> Unit)? = null
     var onPanStopListener: (() -> Unit)? = null
     var onPanStartListener: (() -> Unit)? = null
 
-    /**
-     * Exists so that we are always able to set the center to the edge of the contained actor.
-     * Otherwise, the [ScrollPane] would always stop at the actor's edge, keeping the center always ([width or height]/2) away from the edge.
-     * This is lateinit because unfortunately [ScrollPane] uses [setActor] in its constructor, and we override [setActor], so paddingGroup has not been
-     * constructed at that moment, throwing a NPE.
-     */
-    @Suppress("UNNECESSARY_LATEINIT")
-    private lateinit var paddingGroup: Group
-
     private val horizontalPadding get() = width / 2
     private val verticalPadding get() = height / 2
 
     init {
-        paddingGroup = Group()
-        super.setActor(paddingGroup)
-
         addZoomListeners()
     }
 
-    override fun setActor(actor: Actor?) {
+    fun reloadMaxZoom() {
-        if (!this::paddingGroup.isInitialized) return
+
-        paddingGroup.clearChildren()
+        maxZoom = UncivGame.Current.settings.maxWorldZoomOut
-        paddingGroup.addActor(actor)
+        minZoom = 1f / maxZoom
+
+        // Since normally min isn't reached exactly, only powers of 0.8
+        if (scaleX < minZoom)
+            zoom(1f)
     }
 
-    override fun getActor(): Actor? {
+    override fun getActor() : Actor? {
-        if (!this::paddingGroup.isInitialized || !paddingGroup.hasChildren()) return null
+        val group: Group = super.getActor() as Group
-        return paddingGroup.children[0]
+        return if (group.hasChildren()) group.children[0] else null
+    }
+
+    override fun setActor(content: Actor?) {
+        val group: Group? = super.getActor() as Group?
+        if (group != null) {
+            group.clearChildren()
+            group.addActor(content)
+        } else {
+            super.setActor(content)
+        }
     }
 
     override fun scrollX(pixelsX: Float) {
@@ -73,16 +78,26 @@ open class ZoomableScrollPane(
         updatePadding()
         super.sizeChanged()
         updateCulling()
+
+        if (continuousScrollingX) {
+            // For world-wrap we do not allow viewport to become bigger than the map size,
+            // because we don't want to render the same tiles multiple times (they will be
+            // flickering because of movement).
+            // Hence we limit minimal possible zoom to content width + some extra offset.
+            val content = actor
+            if (content != null)
+                minZoom = max((width + 80f) * scaleX / content.width, 1f / UncivGame.Current.settings.maxWorldZoomOut)// add some extra padding offset
+        }
+
     }
 
     private fun updatePadding() {
-        val content = actor
+        val content = actor ?: return
-        if (content == null) return
         // Padding is always [dimension / 2] because we want to be able to have the center of the scrollPane at the very edge of the content
         content.x = horizontalPadding
-        paddingGroup.width = content.width + horizontalPadding * 2
         content.y = verticalPadding
-        paddingGroup.height = content.height + verticalPadding * 2
+        super.getActor().width = content.width + horizontalPadding * 2
+        super.getActor().height = content.height + verticalPadding * 2
     }
 
     fun updateCulling() {
@@ -103,17 +118,19 @@ open class ZoomableScrollPane(
     }
 
     open fun zoom(zoomScale: Float) {
-        if (zoomScale < minZoom || zoomScale > maxZoom) return
+        val newZoom = min(max(zoomScale, minZoom), maxZoom)
+        val oldZoomX = scaleX
+        val oldZoomY = scaleY
 
-        val previousScaleX = scaleX
+        if (newZoom == oldZoomX)
-        val previousScaleY = scaleY
+            return
 
-        setScale(zoomScale)
+        val newWidth = width * oldZoomX / newZoom
+        val newHeight = height * oldZoomY / newZoom
 
         // When we scale, the width & height values stay the same. However, after scaling up/down, the width will be rendered wider/narrower than before.
         // But we want to keep the size of the pane the same, so we do need to adjust the width & height: smaller if the scale increased, larger if it decreased.
-        val newWidth = width * previousScaleX / zoomScale
+        setScale(newZoom)
-        val newHeight = height * previousScaleY / zoomScale
         setSize(newWidth, newHeight)
 
         onViewportChanged()
@@ -129,7 +146,7 @@ open class ZoomableScrollPane(
         zoom(scaleX * 0.8f)
     }
 
-    class ScrollZoomListener(val zoomableScrollPane: ZoomableScrollPane):InputListener(){
+    class ScrollZoomListener(private val zoomableScrollPane: ZoomableScrollPane):InputListener(){
         override fun scrolled(event: InputEvent?, x: Float, y: Float, amountX: Float, amountY: Float): Boolean {
             if (amountX > 0 || amountY > 0) zoomableScrollPane.zoomOut()
             else zoomableScrollPane.zoomIn()
@@ -137,7 +154,7 @@ open class ZoomableScrollPane(
         }
     }
 
-    class ZoomListener(val zoomableScrollPane: ZoomableScrollPane):ActorGestureListener(){
+    class ZoomListener(private val zoomableScrollPane: ZoomableScrollPane):ActorGestureListener(){
         var lastScale = 1f
         var lastInitialDistance = 0f
 
@@ -161,7 +178,7 @@ open class ZoomableScrollPane(
         addListener(ZoomListener(this))
     }
 
-    class FlickScrollListener(val zoomableScrollPane: ZoomableScrollPane): ActorGestureListener(){
+    class FlickScrollListener(private val zoomableScrollPane: ZoomableScrollPane): ActorGestureListener(){
         private var wasPanning = false
         override fun pan(event: InputEvent, x: Float, y: Float, deltaX: Float, deltaY: Float) {
             if (!wasPanning) {
@@ -204,16 +221,14 @@ open class ZoomableScrollPane(
 
     /** Get the scrolling destination if currently scrolling, else the current scroll position. */
     fun scrollingDestination(): Vector2 {
-        if (isScrolling())
+        return if (isScrolling()) scrollingTo!! else Vector2(scrollX, scrollY)
-            return scrollingTo!!
-        else
-            return Vector2(scrollX, scrollY)
     }
 
-    class ScrollToAction(val zoomableScrollPane: ZoomableScrollPane):FloatAction(0f, 1f, 0.4f) {
+    class ScrollToAction(private val zoomableScrollPane: ZoomableScrollPane):FloatAction(0f, 1f, 0.4f) {
+
+        private val originalScrollX = zoomableScrollPane.scrollX
+        private val originalScrollY = zoomableScrollPane.scrollY
 
-        val originalScrollX = zoomableScrollPane.scrollX
-        val originalScrollY = zoomableScrollPane.scrollY
         override fun update(percent: Float) {
             zoomableScrollPane.scrollX = zoomableScrollPane.scrollingTo!!.x * percent + originalScrollX * (1 - percent)
             zoomableScrollPane.scrollY = zoomableScrollPane.scrollingTo!!.y * percent + originalScrollY * (1 - percent)
@@ -246,7 +261,7 @@ open class ZoomableScrollPane(
     }
 
     /** @return the currently scrolled-to viewport of the whole scrollable area */
-    fun getViewport(): Rectangle {
+    private fun getViewport(): Rectangle {
         val viewportFromLeft = scrollX
         /** In the default coordinate system, the y origin is at the bottom, but scrollY is from the top, so we need to invert. */
         val viewportFromBottom = maxY - scrollY

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

@@ -60,12 +60,7 @@ class WorldMapHolder(
     internal val tileMap: TileMap
 ) : ZoomableScrollPane(20f, 20f) {
     internal var selectedTile: Tile? = null
-    val tileGroups = HashMap<Tile, List<WorldTileGroup>>()
+    val tileGroups = HashMap<Tile, WorldTileGroup>()
-
-    //allWorldTileGroups exists to easily access all WordTileGroups
-    //since tileGroup is a HashMap of Lists and getting all WordTileGroups
-    //would need a double for loop
-    private val allWorldTileGroups = ArrayList<WorldTileGroup>()
 
     private val unitActionOverlays: ArrayList<Actor> = ArrayList()
 
@@ -101,12 +96,6 @@ class WorldMapHolder(
         }
     }
 
-    internal fun reloadMaxZoom() {
-        maxZoom = UncivGame.Current.settings.maxWorldZoomOut
-        minZoom = 1f / maxZoom
-        if (scaleX < minZoom) zoom(1f)   // since normally min isn't reached exactly, only powers of 0.8
-    }
-
     // Interface for classes that contain the data required to draw a button
     interface ButtonDto
     // Contains the data required to draw a "move here" button
@@ -116,36 +105,19 @@ class WorldMapHolder(
 
     internal fun addTiles() {
         val tileSetStrings = TileSetStrings()
-        val daTileGroups = tileMap.values.map { WorldTileGroup(worldScreen, it, tileSetStrings) }
+        val tileGroupsNew = tileMap.values.map { WorldTileGroup(worldScreen, it, tileSetStrings) }
-        tileGroupMap = TileGroupMap(
+        tileGroupMap = TileGroupMap(this, tileGroupsNew, continuousScrollingX)
-            daTileGroups,
-            continuousScrollingX)
-        val mirrorTileGroups = tileGroupMap.getMirrorTiles()
 
-        for (tileGroup in daTileGroups) {
+        for (tileGroup in tileGroupsNew) {
-            if (continuousScrollingX) {
+            tileGroups[tileGroup.tile] = tileGroup
-                val mirrorTileGroupLeft = mirrorTileGroups[tileGroup.tile]!!.first
-                val mirrorTileGroupRight = mirrorTileGroups[tileGroup.tile]!!.second
-
-                allWorldTileGroups.add(tileGroup)
-                allWorldTileGroups.add(mirrorTileGroupLeft)
-                allWorldTileGroups.add(mirrorTileGroupRight)
-
-                tileGroups[tileGroup.tile] = listOf(tileGroup, mirrorTileGroupLeft, mirrorTileGroupRight)
-            } else {
-                tileGroups[tileGroup.tile] = listOf(tileGroup)
-                allWorldTileGroups.add(tileGroup)
-            }
-        }
-
-        for (tileGroup in allWorldTileGroups) {
             tileGroup.layerCityButton.onClick(UncivSound.Silent) {
                 onTileClicked(tileGroup.tile)
             }
             tileGroup.onClick { onTileClicked(tileGroup.tile) }
 
             // On 'droid two-finger tap is mapped to right click and dissent has been expressed
-            if (Gdx.app.type == Application.ApplicationType.Android) continue
+            if (Gdx.app.type == Application.ApplicationType.Android)
+                continue
 
             // Right mouse click listener
             tileGroup.addListener(object : ClickListener() {
@@ -162,11 +134,8 @@ class WorldMapHolder(
                 }
             })
         }
-
         actor = tileGroupMap
-
         setSize(worldScreen.stage.width, worldScreen.stage.height)
-
         layout() // Fit the scroll pane to the contents - otherwise, setScroll won't work!
     }
 
@@ -187,11 +156,8 @@ class WorldMapHolder(
         unitTable.tileSelected(tile)
         val newSelectedUnit = unitTable.selectedUnit
 
-        if (previousSelectedCity != null && tile != previousSelectedCity.getCenterTile()) {
+        if (previousSelectedCity != null && tile != previousSelectedCity.getCenterTile())
-            tileGroups[previousSelectedCity.getCenterTile()]?.forEach {
+            tileGroups[previousSelectedCity.getCenterTile()]!!.layerCityButton.moveUp()
-                it.layerCityButton.moveUp()
-            }
-        }
 
         if (previousSelectedUnits.isNotEmpty() && previousSelectedUnits.any { it.getTile() != tile }
                 && worldScreen.isPlayersTurn
@@ -410,42 +376,41 @@ class WorldMapHolder(
     }
 
     private fun addTileOverlays(tile: Tile, buttonDto: ButtonDto? = null) {
-        for (group in tileGroups[tile]!!) {
+        val table = Table().apply { defaults().pad(10f) }
-            val table = Table().apply { defaults().pad(10f) }
+        if (buttonDto != null && worldScreen.canChangeState)
-            if (buttonDto != null && worldScreen.canChangeState)
+            table.add(
-                table.add(
+                when (buttonDto) {
-                    when (buttonDto) {
+                    is MoveHereButtonDto -> getMoveHereButton(buttonDto)
-                        is MoveHereButtonDto -> getMoveHereButton(buttonDto)
+                    is SwapWithButtonDto -> getSwapWithButton(buttonDto)
-                        is SwapWithButtonDto -> getSwapWithButton(buttonDto)
+                    else -> null
-                        else -> null
-                    }
-                )
-
-            val unitList = ArrayList<MapUnit>()
-            if (tile.isCityCenter()
-                    && (tile.getOwner() == worldScreen.viewingCiv || worldScreen.viewingCiv.isSpectator())) {
-                unitList.addAll(tile.getCity()!!.getCenterTile().getUnits())
-            } else if (tile.airUnits.isNotEmpty()
-                    && (tile.airUnits.first().civInfo == worldScreen.viewingCiv || worldScreen.viewingCiv.isSpectator())) {
-                unitList.addAll(tile.getUnits())
-            }
-
-            for (unit in unitList) {
-                val unitGroup = UnitGroup(unit, 60f).surroundWithCircle(85f, resizeActor = false)
-                unitGroup.circle.color = Color.GRAY.cpy().apply { a = 0.5f }
-                if (unit.currentMovement == 0f) unitGroup.color.a = 0.5f
-                unitGroup.touchable = Touchable.enabled
-                unitGroup.onClick {
-                    worldScreen.bottomUnitTable.selectUnit(unit, Gdx.input.isKeyPressed(Input.Keys.SHIFT_LEFT))
-                    worldScreen.shouldUpdate = true
-                    removeUnitActionOverlay()
                 }
-                table.add(unitGroup)
+            )
-            }
 
-            addOverlayOnTileGroup(group, table)
+        val unitList = ArrayList<MapUnit>()
-            table.moveBy(0f, 60f)
+        if (tile.isCityCenter()
+                && (tile.getOwner() == worldScreen.viewingCiv || worldScreen.viewingCiv.isSpectator())) {
+            unitList.addAll(tile.getCity()!!.getCenterTile().getUnits())
+        } else if (tile.airUnits.isNotEmpty()
+                && (tile.airUnits.first().civInfo == worldScreen.viewingCiv || worldScreen.viewingCiv.isSpectator())) {
+            unitList.addAll(tile.getUnits())
         }
+
+        for (unit in unitList) {
+            val unitGroup = UnitGroup(unit, 60f).surroundWithCircle(85f, resizeActor = false)
+            unitGroup.circle.color = Color.GRAY.cpy().apply { a = 0.5f }
+            if (unit.currentMovement == 0f) unitGroup.color.a = 0.5f
+            unitGroup.touchable = Touchable.enabled
+            unitGroup.onClick {
+                worldScreen.bottomUnitTable.selectUnit(unit, Gdx.input.isKeyPressed(Input.Keys.SHIFT_LEFT))
+                worldScreen.shouldUpdate = true
+                removeUnitActionOverlay()
+            }
+            table.add(unitGroup)
+        }
+
+        addOverlayOnTileGroup(tileGroups[tile]!!, table)
+        table.moveBy(0f, 60f)
+
     }
 
     val buttonSize = 60f
@@ -529,19 +494,13 @@ class WorldMapHolder(
 
     /** Clear all arrows to be drawn on the next update. */
     fun resetArrows() {
-        for (tile in tileGroups.values) {
+        for (tile in tileGroups.asSequence())
-            for (group in tile) {
+            tile.value.layerMisc.resetArrows()
-                group.layerMisc.resetArrows()
-            }
-        } // Inefficient?
     }
 
     /** Add an arrow to draw on the next update. */
     fun addArrow(fromTile: Tile, toTile: Tile, arrowType: MapArrowType) {
-        val tile = tileGroups[fromTile]
+        tileGroups[fromTile]?.layerMisc?.addArrow(toTile, arrowType)
-        if (tile != null) for (group in tile) {
-            group.layerMisc.addArrow(toTile, arrowType)
-        }
     }
 
     /**
@@ -578,13 +537,13 @@ class WorldMapHolder(
 
         if (isMapRevealEnabled(viewingCiv)) {
             // Only needs to be done once - this is so the minimap will also be revealed
-            allWorldTileGroups.forEach {
+            tileGroups.values.forEach {
                 it.tile.setExplored(viewingCiv, true)
                 it.isForceVisible = true } // So we can see all resources, regardless of tech
         }
 
         // General update of all tiles
-        for (tileGroup in allWorldTileGroups)
+        for (tileGroup in tileGroups.values)
             tileGroup.update(viewingCiv)
 
         // Update tiles according to selected unit/city
@@ -605,10 +564,7 @@ class WorldMapHolder(
         }
 
         // Same as below - randomly, tileGroups doesn't seem to contain the selected tile, and this doesn't seem reproducible
-        val worldTileGroupsForSelectedTile = tileGroups[selectedTile]
+        tileGroups[selectedTile]?.layerOverlay?.showHighlight(Color.WHITE)
-        if (worldTileGroupsForSelectedTile != null)
-            for (group in worldTileGroupsForSelectedTile)
-                group.layerOverlay.showHighlight(Color.WHITE)
 
         zoom(scaleX) // zoom to current scale, to set the size of the city buttons after "next turn"
     }
@@ -619,13 +575,12 @@ class WorldMapHolder(
 
         // Update flags for units which have them
         if (!unit.baseUnit.movesLikeAirUnits()) {
-            for (group in tileGroup)
+            tileGroup.layerUnitFlag.selectFlag(unit)
-                group.layerUnitFlag.selectFlag(unit)
         }
 
         // Fade out less relevant images if a military unit is selected
         if (unit.isMilitary()) {
-            for (group in allWorldTileGroups) {
+            for (group in tileGroups.values) {
 
                 // Fade out population icons
                 group.layerMisc.dimPopulation(true)
@@ -644,10 +599,8 @@ class WorldMapHolder(
             val unitSwappableTiles = unit.movement.getUnitSwappableTiles()
             val swapUnitsTileOverlayColor = Color.PURPLE
             for (tile in unitSwappableTiles)  {
-                for (tileToColor in tileGroups[tile]!!) {
+                tileGroups[tile]!!.layerOverlay.showHighlight(swapUnitsTileOverlayColor,
-                    tileToColor.layerOverlay.showHighlight(swapUnitsTileOverlayColor,
+                    if (UncivGame.Current.settings.singleTapMove) 0.7f else 0.3f)
-                        if (UncivGame.Current.settings.singleTapMove) 0.7f else 0.3f)
-                }
             }
             // In swapping-mode don't want to show other overlays
             return
@@ -659,52 +612,47 @@ class WorldMapHolder(
 
         // Highlight tiles within movement range
         for (tile in tilesInMoveRange) {
-            for (tileToColor in tileGroups[tile]!!) {
+            val group = tileGroups[tile]!!
 
-                // Air-units have additional highlights
+            // Air-units have additional highlights
-                if (isAirUnit && !unit.isPreparingAirSweep()) {
+            if (isAirUnit && !unit.isPreparingAirSweep()) {
-                    if (tile.aerialDistanceTo(unit.getTile()) <= unit.getRange()) {
+                if (tile.aerialDistanceTo(unit.getTile()) <= unit.getRange()) {
-                        // The tile is within attack range
+                    // The tile is within attack range
-                        tileToColor.layerOverlay.showHighlight(Color.RED, 0.3f)
+                    group.layerOverlay.showHighlight(Color.RED, 0.3f)
-                    } else {
+                } else {
-                        // The tile is within move range
+                    // The tile is within move range
-                        tileToColor.layerOverlay.showHighlight(Color.BLUE, 0.3f)
+                    group.layerOverlay.showHighlight(Color.BLUE, 0.3f)
-                    }
-                }
-
-                // Highlight tile unit can move to
-                if (unit.movement.canMoveTo(tile) ||
-                        unit.movement.isUnknownTileWeShouldAssumeToBePassable(tile) && !unit.baseUnit.movesLikeAirUnits()) {
-                    val alpha = if (UncivGame.Current.settings.singleTapMove || isAirUnit) 0.7f else 0.3f
-                    tileToColor.layerOverlay.showHighlight(moveTileOverlayColor, alpha)
                 }
             }
+
+            // Highlight tile unit can move to
+            if (unit.movement.canMoveTo(tile) ||
+                    unit.movement.isUnknownTileWeShouldAssumeToBePassable(tile) && !unit.baseUnit.movesLikeAirUnits()) {
+                val alpha = if (UncivGame.Current.settings.singleTapMove || isAirUnit) 0.7f else 0.3f
+                group.layerOverlay.showHighlight(moveTileOverlayColor, alpha)
+            }
+
         }
 
         // Add back in the red markers for Air Unit Attack range since they can't move, but can still attack
         if (unit.hasUnique(UniqueType.CannotMove) && isAirUnit && !unit.isPreparingAirSweep()) {
             val tilesInAttackRange = unit.getTile().getTilesInDistanceRange(IntRange(1, unit.getRange()))
             for (tile in tilesInAttackRange) {
-                for (tileToColor in tileGroups[tile]!!) {
+                // The tile is within attack range
-                    // The tile is within attack range
+                tileGroups[tile]!!.layerOverlay.showHighlight(Color.RED, 0.3f)
-                    tileToColor.layerOverlay.showHighlight(Color.RED, 0.3f)
-                }
             }
         }
 
         // Movement paths
         if (unitMovementPaths.containsKey(unit)) {
             for (tile in unitMovementPaths[unit]!!) {
-                for (tileToColor in tileGroups[tile]!!)
+                tileGroups[tile]!!.layerOverlay.showHighlight(Color.SKY, 0.8f)
-                    tileToColor.layerOverlay.showHighlight(Color.SKY, 0.8f)
             }
         }
 
         // Highlight movement destination tile
         if (unit.isMoving()) {
-            val destinationTileGroups = tileGroups[unit.getMovementDestination()]!!
+            tileGroups[unit.getMovementDestination()]!!.layerOverlay.showHighlight(Color.WHITE, 0.7f)
-            for (destinationTileGroup in destinationTileGroups)
-                destinationTileGroup.layerOverlay.showHighlight(Color.WHITE, 0.7f)
         }
 
         // Highlight attackable tiles
@@ -716,15 +664,14 @@ class WorldMapHolder(
                     .distinctBy { it.tileToAttack }
 
             for (attackableTile in attackableTiles) {
-                for (tileGroupToAttack in tileGroups[attackableTile.tileToAttack]!!) {
+                val tileGroupToAttack = tileGroups[attackableTile.tileToAttack]!!
-                    tileGroupToAttack.layerOverlay.showHighlight(colorFromRGB(237, 41, 57))
+                tileGroupToAttack.layerOverlay.showHighlight(colorFromRGB(237, 41, 57))
-                    tileGroupToAttack.layerOverlay.showCrosshair(
+                tileGroupToAttack.layerOverlay.showCrosshair(
-                        // the targets which cannot be attacked without movements shown as orange-ish
+                    // the targets which cannot be attacked without movements shown as orange-ish
-                        if (attackableTile.tileToAttackFrom != unit.currentTile)
+                    if (attackableTile.tileToAttackFrom != unit.currentTile)
-                            0.5f
+                        0.5f
-                        else 1f
+                    else 1f
-                    )
+                )
-                }
             }
         }
     }
@@ -732,10 +679,9 @@ class WorldMapHolder(
     private fun updateBombardableTilesForSelectedCity(city: City) {
         if (!city.canBombard()) return
         for (attackableTile in UnitAutomation.getBombardableTiles(city)) {
-            for (group in tileGroups[attackableTile]!!) {
+            val group = tileGroups[attackableTile]!!
-                group.layerOverlay.showHighlight(colorFromRGB(237, 41, 57))
+            group.layerOverlay.showHighlight(colorFromRGB(237, 41, 57))
-                group.layerOverlay.showCrosshair()
+            group.layerOverlay.showCrosshair()
-            }
         }
     }
 
@@ -748,7 +694,7 @@ class WorldMapHolder(
      * @return `true` if scroll position was changed, `false` otherwise
      */
     fun setCenterPosition(vector: Vector2, immediately: Boolean = false, selectUnit: Boolean = true, forceSelectUnit: MapUnit? = null): Boolean {
-        val tileGroup = allWorldTileGroups.firstOrNull { it.tile.position == vector } ?: return false
+        val tileGroup = tileGroups.values.firstOrNull { it.tile.position == vector } ?: return false
         selectedTile = tileGroup.tile
         if (selectUnit || forceSelectUnit != null)
             worldScreen.bottomUnitTable.tileSelected(selectedTile!!, forceSelectUnit)
@@ -781,12 +727,12 @@ class WorldMapHolder(
         // use scaleX instead of zoomScale itself, because zoomScale might have been outside minZoom..maxZoom and thus not applied
         val clampedCityButtonZoom = 1 / scaleX
         if (clampedCityButtonZoom >= 1) {
-            for (tileGroup in allWorldTileGroups) {
+            for (tileGroup in tileGroups.values) {
                 tileGroup.layerCityButton.isTransform = false // to save on rendering time to improve framerate
             }
         }
         if (clampedCityButtonZoom < 1 && clampedCityButtonZoom >= minZoom) {
-            for (tileGroup in allWorldTileGroups) {
+            for (tileGroup in tileGroups.values) {
                 // ONLY set those groups that have active city buttons as transformable!
                 // This is massively framerate-improving!
                 if (tileGroup.layerCityButton.hasChildren())

core/src/com/unciv/ui/worldscreen/bottombar/BattleTableHelpers.kt

@@ -22,14 +22,12 @@ object BattleTableHelpers {
     ) {
         fun getMapActorsForCombatant(combatant: ICombatant):Sequence<Actor> =
                 sequence {
-                    val tilegroups = mapHolder.tileGroups[combatant.getTile()]!!
+                    val tileGroup = mapHolder.tileGroups[combatant.getTile()]!!
                     when {
-                        combatant.isCity() -> yieldAll(tilegroups.mapNotNull { it.layerMisc.improvementIcon })
+                        combatant.isCity() -> yield(tileGroup.layerMisc.improvementIcon!!)
                         else -> {
                             val slot = if (combatant.isCivilian()) 0 else 1
-                            for (tileGroup in tilegroups) {
+                            yieldAll((tileGroup.layerUnitArt.getChild(slot) as Group).children)
-                                yieldAll((tileGroup.layerUnitArt.getChild(slot) as Group).children)
-                            }
                         }
                     }
                 }