Reorder TileGroup layers to draw borders under Pixel Units (#6980)

* Reorder TileGroup layers to draw borders under Pixel Units

* Reorder TileGroup layers to draw borders under Pixel Units - reviews

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

@@ -15,7 +15,6 @@ import com.unciv.models.ruleset.Building
 import com.unciv.models.ruleset.tile.TileImprovement
 import com.unciv.models.ruleset.unique.UniqueType
 import com.unciv.models.stats.Stat
-import com.unciv.models.translations.tr
 import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.map.TileGroupMap
 import com.unciv.ui.popup.ToastPopup
@@ -298,7 +297,7 @@ class CityScreen(
             tileGroups.add(tileGroup)
         }
 
-        val tilesToUnwrap = ArrayList<CityTileGroup>()
+        val tilesToUnwrap = mutableSetOf<CityTileGroup>()
         for (tileGroup in tileGroups) {
             val xDifference = cityInfo.getCenterTile().position.x - tileGroup.tileInfo.position.x
             val yDifference = cityInfo.getCenterTile().position.y - tileGroup.tileInfo.position.y

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

@@ -5,17 +5,27 @@ import com.badlogic.gdx.math.Vector2
 import com.badlogic.gdx.scenes.scene2d.Group
 import com.unciv.logic.HexMath
 import com.unciv.logic.map.TileInfo
+import com.unciv.logic.map.TileMap
+import com.unciv.ui.cityscreen.CityTileGroup
 import com.unciv.ui.tilegroups.ActionlessGroup
 import com.unciv.ui.tilegroups.TileGroup
+import com.unciv.ui.tilegroups.WorldTileGroup
 import kotlin.math.max
 import kotlin.math.min
 
+
+/**
+ * A (potentially partial) map view
+ * @param T [TileGroup] or a subclass ([WorldTileGroup], [CityTileGroup])
+ * @param tileGroups Source of [TileGroup]s to include, will be **iterated several times**.
+ * @param tileGroupsToUnwrap For these, coordinates will be unwrapped using [TileMap.getUnWrappedPosition]
+ */
 class TileGroupMap<T: TileGroup>(
-    tileGroups: Collection<T>,
+    tileGroups: Iterable<T>,
     private val leftAndRightPadding: Float,
     private val topAndBottomPadding: Float,
     worldWrap: Boolean = false,
-    tileGroupsToUnwrap: Collection<T>? = null
+    tileGroupsToUnwrap: Set<T>? = null
 ): Group() {
     private var topX = -Float.MAX_VALUE
     private var topY = -Float.MAX_VALUE
@@ -33,7 +43,7 @@ class TileGroupMap<T: TileGroup>(
         }
 
         for (tileGroup in tileGroups) {
-            val positionalVector = if (tileGroupsToUnwrap != null && tileGroupsToUnwrap.contains(tileGroup)) {
+            val positionalVector = if (tileGroupsToUnwrap?.contains(tileGroup) == true) {
                 HexMath.hex2WorldCoords(
                     tileGroup.tileInfo.tileMap.getUnWrappedPosition(tileGroup.tileInfo.position)
                 )
@@ -82,10 +92,11 @@ class TileGroupMap<T: TileGroup>(
         val baseLayers = ArrayList<ActionlessGroup>()
         val featureLayers = ArrayList<ActionlessGroup>()
         val miscLayers = ArrayList<ActionlessGroup>()
+        val pixelUnitLayers = ArrayList<ActionlessGroup>()
+        val circleFogCrosshairLayers = ArrayList<ActionlessGroup>()
         val unitLayers = ArrayList<Group>()
         val unitImageLayers = ArrayList<ActionlessGroup>()
         val cityButtonLayers = ArrayList<Group>()
-        val circleCrosshairFogLayers = ArrayList<ActionlessGroup>()
 
         // Apparently the sortedByDescending is kinda memory-intensive because it needs to sort ALL the tiles
         for (group in tileGroups.sortedByDescending { it.tileInfo.position.x + it.tileInfo.position.y }) {
@@ -93,27 +104,32 @@ class TileGroupMap<T: TileGroup>(
             baseLayers.add(group.baseLayerGroup.apply { setPosition(group.x,group.y) })
             featureLayers.add(group.terrainFeatureLayerGroup.apply { setPosition(group.x,group.y) })
             miscLayers.add(group.miscLayerGroup.apply { setPosition(group.x,group.y) })
+            pixelUnitLayers.add(group.pixelMilitaryUnitGroup.apply { setPosition(group.x,group.y) })
+            pixelUnitLayers.add(group.pixelCivilianUnitGroup.apply { setPosition(group.x,group.y) })
+            circleFogCrosshairLayers.add(group.highlightFogCrosshairLayerGroup.apply { setPosition(group.x,group.y) })
             unitLayers.add(group.unitLayerGroup.apply { setPosition(group.x,group.y) })
             unitImageLayers.add(group.unitImageLayerGroup.apply { setPosition(group.x,group.y) })
             cityButtonLayers.add(group.cityButtonLayerGroup.apply { setPosition(group.x,group.y) })
-            circleCrosshairFogLayers.add(group.highlightCrosshairFogLayerGroup.apply { setPosition(group.x,group.y) })
 
             if (worldWrap) {
                 for (mirrorTile in mirrorTileGroups[group.tileInfo]!!.toList()) {
                     baseLayers.add(mirrorTile.baseLayerGroup.apply { setPosition(mirrorTile.x,mirrorTile.y) })
                     featureLayers.add(mirrorTile.terrainFeatureLayerGroup.apply { setPosition(mirrorTile.x,mirrorTile.y) })
                     miscLayers.add(mirrorTile.miscLayerGroup.apply { setPosition(mirrorTile.x,mirrorTile.y) })
+                    pixelUnitLayers.add(mirrorTile.pixelMilitaryUnitGroup.apply { setPosition(mirrorTile.x,mirrorTile.y) })
+                    pixelUnitLayers.add(mirrorTile.pixelCivilianUnitGroup.apply { setPosition(mirrorTile.x,mirrorTile.y) })
+                    circleFogCrosshairLayers.add(mirrorTile.highlightFogCrosshairLayerGroup.apply { setPosition(mirrorTile.x,mirrorTile.y) })
                     unitLayers.add(mirrorTile.unitLayerGroup.apply { setPosition(mirrorTile.x,mirrorTile.y) })
                     unitImageLayers.add(mirrorTile.unitImageLayerGroup.apply { setPosition(mirrorTile.x,mirrorTile.y) })
                     cityButtonLayers.add(mirrorTile.cityButtonLayerGroup.apply { setPosition(mirrorTile.x,mirrorTile.y) })
-                    circleCrosshairFogLayers.add(mirrorTile.highlightCrosshairFogLayerGroup.apply { setPosition(mirrorTile.x,mirrorTile.y) })
                 }
             }
         }
         for (group in baseLayers) addActor(group)
         for (group in featureLayers) addActor(group)
         for (group in miscLayers) addActor(group)
-        for (group in circleCrosshairFogLayers) addActor(group)
+        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) {
@@ -125,7 +141,6 @@ class TileGroupMap<T: TileGroup>(
         for (group in unitImageLayers) addActor(group) // This is so the individual textures for the units are rendered together
         for (group in cityButtonLayers) addActor(group) // city buttons + clickability
 
-
         // there are tiles "below the zero",
         // so we zero out the starting position of the whole board so they will be displayed as well
         // Map's width is reduced by groupSize if it is wrapped, because wrapped map will miss a tile on the right.
@@ -150,5 +165,6 @@ class TileGroupMap<T: TileGroup>(
 
     // For debugging purposes
      override fun draw(batch: Batch?, parentAlpha: Float) { super.draw(batch, parentAlpha) }
+     @Suppress("RedundantOverride")
      override fun act(delta: Float) { super.act(delta) }
 }

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

@@ -0,0 +1,24 @@
+package com.unciv.ui.tilegroups
+
+import com.badlogic.gdx.scenes.scene2d.Actor
+import com.badlogic.gdx.scenes.scene2d.Group
+
+/** A lot of the render time was spent on snapshot arrays of the TileGroupMap's groups, in the act() function.
+ * These classes are to avoid the overhead of useless act() calls. */
+
+/** A [Group] with [actions] effectively disabled. */
+abstract class ActionlessGroupWithHit : Group() {
+    override fun act(delta: Float) {}
+}
+
+/** A [Group] with [actions] and [hit] effectively disabled. */
+open class ActionlessGroup() : ActionlessGroupWithHit() {
+    /** A [Group] with [actions], [hit] and scaling effectively disabled, pre-sized.
+     *  @param groupSize [Sets size][setSize] initially */
+    constructor(groupSize: Float) : this() {
+        isTransform = false
+        @Suppress("LeakingThis")  // works by setting fields only
+        setSize(groupSize, groupSize)
+    }
+    override fun hit(x: Float, y: Float, touchable: Boolean): Actor? = null
+}

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

@@ -26,37 +26,36 @@ import java.lang.IllegalStateException
 import kotlin.math.*
 import kotlin.random.Random
 
-/** A lot of the render time was spent on snapshot arrays of the TileGroupMap's groups, in the act() function.
+open class TileGroup(
- * This class is to avoid the overhead of useless act() calls. */
+    var tileInfo: TileInfo,
-open class ActionlessGroup(val checkHit:Boolean=false):Group() {
+    val tileSetStrings: TileSetStrings,
-    override fun act(delta: Float) {}
+    groupSize: Float = 54f
-    override fun hit(x: Float, y: Float, touchable: Boolean): Actor? {
+) : ActionlessGroupWithHit() {
-        if (checkHit)
-            return super.hit(x, y, touchable)
-        return null
-    }
-}
-
-open class TileGroup(var tileInfo: TileInfo, val tileSetStrings:TileSetStrings, private val groupSize: Float = 54f) : ActionlessGroup(true) {
     /*
-    Layers:
+        Layers (reordered in TileGroupMap):
         Base image (+ overlay)
-    Feature overlay / city overlay
+        Terrain Feature overlay (including roads and pixel units)
-    Misc: Units, improvements, resources, border, arrows
+        Misc: improvements, resources, yields, worked, resources, border, arrows, and starting locations in editor
-    Highlight, Crosshair, Fog layer
+        Pixel Units
+        Highlight, Fog, Crosshair layer (in that order)
+        Units
+        City button
         City name
     */
 
     /** Cache simple but frequent calculations. */
     private val hexagonImageWidth = groupSize * 1.5f
     /** Cache simple but frequent calculations. */
-    private val hexagonImageOrigin = Pair(hexagonImageWidth/2f, sqrt((hexagonImageWidth/2f).pow(2) - (hexagonImageWidth/4f).pow(2))) // Pair, not Vector2, for immutability. Second number is triangle height for hex center.
+    private val hexagonImageOrigin = Pair(hexagonImageWidth / 2f, sqrt((hexagonImageWidth / 2f).pow(2) - (hexagonImageWidth / 4f).pow(2)))
+    // Pair, not Vector2, for immutability. Second number is triangle height for hex center.
     /** Cache simple but frequent calculations. */
-    private val hexagonImagePosition = Pair(-hexagonImageOrigin.first/3f, -hexagonImageOrigin.second/4f) // Honestly, I got these numbers empirically by printing `.x` and `.y` after `.center()`, and I'm not totally clear on the stack of transformations that makes them work. But they are still exact ratios, AFAICT.
+    private val hexagonImagePosition = Pair(-hexagonImageOrigin.first / 3f, -hexagonImageOrigin.second / 4f)
+    // Honestly, I got these numbers empirically by printing `.x` and `.y` after `.center()`, and I'm not totally
+    // clear on the stack of transformations that makes them work. But they are still exact ratios, AFAICT.
 
     // For recognizing the group in the profiler
-    class BaseLayerGroupClass:ActionlessGroup()
+    class BaseLayerGroupClass(groupSize: Float) : ActionlessGroup(groupSize)
-    val baseLayerGroup = BaseLayerGroupClass().apply { isTransform = false; setSize(groupSize, groupSize)  }
+    val baseLayerGroup = BaseLayerGroupClass(groupSize)
 
     val tileBaseImages: ArrayList<Image> = ArrayList()
     /** List of image locations comprising the layers so we don't need to change images all the time */
@@ -66,9 +65,8 @@ open class TileGroup(var tileInfo: TileInfo, val tileSetStrings:TileSetStrings,
     private  var baseTerrainOverlayImage: Image? = null
     private  var baseTerrain: String = ""
 
-    class TerrainFeatureLayerGroupClass:ActionlessGroup()
+    class TerrainFeatureLayerGroupClass(groupSize: Float) : ActionlessGroup(groupSize)
-    val terrainFeatureLayerGroup = TerrainFeatureLayerGroupClass()
+    val terrainFeatureLayerGroup = TerrainFeatureLayerGroupClass(groupSize)
-            .apply { isTransform = false; setSize(groupSize, groupSize) }
 
     // These are for OLD tiles - for instance the "forest" symbol on the forest
     private var terrainFeatureOverlayImage: Image? = null
@@ -77,14 +75,14 @@ open class TileGroup(var tileInfo: TileInfo, val tileSetStrings:TileSetStrings,
     private var naturalWonderImage: Image? = null
 
     private var pixelMilitaryUnitImageLocation = ""
-    var pixelMilitaryUnitGroup = ActionlessGroup().apply { isTransform = false; setSize(groupSize, groupSize) }
+    var pixelMilitaryUnitGroup = ActionlessGroup(groupSize)
     private var pixelCivilianUnitImageLocation = ""
-    var pixelCivilianUnitGroup = ActionlessGroup().apply { isTransform = false; setSize(groupSize, groupSize) }
+    var pixelCivilianUnitGroup = ActionlessGroup(groupSize)
 
-    class MiscLayerGroupClass:ActionlessGroup(){
+    class MiscLayerGroupClass(groupSize: Float) : ActionlessGroup(groupSize) {
         override fun draw(batch: Batch?, parentAlpha: Float) = super.draw(batch, parentAlpha)
     }
-    val miscLayerGroup = MiscLayerGroupClass().apply { isTransform = false; setSize(groupSize, groupSize) }
+    val miscLayerGroup = MiscLayerGroupClass(groupSize)
 
     var tileYieldGroupInitialized = false
     val tileYieldGroup: YieldGroup by lazy { YieldGroup() }
@@ -110,22 +108,31 @@ open class TileGroup(var tileInfo: TileInfo, val tileSetStrings:TileSetStrings,
     @Suppress("LeakingThis")    // we trust TileGroupIcons not to use our `this` in its constructor except storing it for later
     val icons = TileGroupIcons(this)
 
-    class UnitLayerGroupClass:Group(){
+    class UnitLayerGroupClass(groupSize: Float) : Group() {
+        init {
+            isTransform = false
+            touchable = Touchable.disabled
+            setSize(groupSize, groupSize)
+        }
+
         override fun draw(batch: Batch?, parentAlpha: Float) = super.draw(batch, parentAlpha)
-        override fun act(delta: Float) { // No 'snapshotting' since we trust it wil remain the same
+        override fun act(delta: Float) { // No 'snapshotting' since we trust it will remain the same
             for (child in children)
                 child.act(delta)
         }
     }
 
-    class UnitImageLayerGroupClass:ActionlessGroup(){
+    class UnitImageLayerGroupClass(groupSize: Float) : ActionlessGroup(groupSize) {
         override fun draw(batch: Batch?, parentAlpha: Float) = super.draw(batch, parentAlpha)
+        init {
+            touchable = Touchable.disabled
+        }
     }
     // We separate the units from the units' backgrounds, because all the background elements are in the same texture, and the units' aren't
-    val unitLayerGroup = UnitLayerGroupClass().apply { isTransform = false; setSize(groupSize, groupSize);touchable = Touchable.disabled }
+    val unitLayerGroup = UnitLayerGroupClass(groupSize)
-    val unitImageLayerGroup = UnitImageLayerGroupClass().apply { isTransform = false; setSize(groupSize, groupSize);touchable = Touchable.disabled }
+    val unitImageLayerGroup = UnitImageLayerGroupClass(groupSize)
 
-    class CityButtonLayerGroupClass(val tileInfo: TileInfo) :Group() {
+    class CityButtonLayerGroupClass(val tileInfo: TileInfo, groupSize: Float) : Group() {
         override fun draw(batch: Batch?, parentAlpha: Float) {
             if (!tileInfo.isCityCenter()) return
             super.draw(batch, parentAlpha)
@@ -138,12 +145,17 @@ open class TileGroup(var tileInfo: TileInfo, val tileSetStrings:TileSetStrings,
             if (!tileInfo.isCityCenter()) return null
             return super.hit(x, y, touchable)
         }
+        init {
+            isTransform = false
+            setSize(groupSize, groupSize)
+            touchable = Touchable.childrenOnly
+            setOrigin(Align.center)
+        }
     }
 
-    val cityButtonLayerGroup = CityButtonLayerGroupClass(tileInfo).apply { isTransform = false; setSize(groupSize, groupSize)
+    val cityButtonLayerGroup = CityButtonLayerGroupClass(tileInfo, groupSize)
-        touchable = Touchable.childrenOnly; setOrigin(Align.center);  }
 
-    val highlightCrosshairFogLayerGroup = ActionlessGroup().apply { isTransform = false; setSize(groupSize, groupSize) }
+    val highlightFogCrosshairLayerGroup = ActionlessGroup(groupSize)
     val highlightImage = ImageGetter.getImage(tileSetStrings.highlight) // for blue and red circles/emphasis on the tile
     private val crosshairImage = ImageGetter.getImage(tileSetStrings.crosshair) // for when a unit is targeted
     private val fogImage = ImageGetter.getImage(tileSetStrings.crosshatchHexagon )
@@ -181,12 +193,11 @@ open class TileGroup(var tileInfo: TileInfo, val tileSetStrings:TileSetStrings,
         this.addActor(baseLayerGroup)
         this.addActor(terrainFeatureLayerGroup)
         this.addActor(miscLayerGroup)
+        this.addActor(pixelMilitaryUnitGroup)
+        this.addActor(pixelCivilianUnitGroup)
         this.addActor(unitLayerGroup)
         this.addActor(cityButtonLayerGroup)
-        this.addActor(highlightCrosshairFogLayerGroup)
+        this.addActor(highlightFogCrosshairLayerGroup)
-
-        terrainFeatureLayerGroup.addActor(pixelMilitaryUnitGroup)
-        terrainFeatureLayerGroup.addActor(pixelCivilianUnitGroup)
 
         updateTileImage(null)
 
@@ -196,25 +207,25 @@ open class TileGroup(var tileInfo: TileInfo, val tileSetStrings:TileSetStrings,
         isTransform = false // performance helper - nothing here is rotated or scaled
     }
 
-    open fun clone(): TileGroup = TileGroup(tileInfo, tileSetStrings)
+    open fun clone() = TileGroup(tileInfo, tileSetStrings)
 
 
     //region init functions
     private fun addHighlightImage() {
-        highlightCrosshairFogLayerGroup.addActor(highlightImage)
+        highlightFogCrosshairLayerGroup.addActor(highlightImage)
         setHexagonImageSize(highlightImage)
         highlightImage.isVisible = false
     }
 
     private fun addFogImage() {
         fogImage.color = Color.WHITE.cpy().apply { a = 0.2f }
-        highlightCrosshairFogLayerGroup.addActor(fogImage)
+        highlightFogCrosshairLayerGroup.addActor(fogImage)
         setHexagonImageSize(fogImage)
     }
 
     private fun addCrosshairImage() {
         crosshairImage.isVisible = false
-        highlightCrosshairFogLayerGroup.addActor(crosshairImage)
+        highlightFogCrosshairLayerGroup.addActor(crosshairImage)
         setHexagonImageSize(crosshairImage)
     }
     //endregion