Fixed the visual gaps in territory borders (#5446)

* Implemented left/right-concave border segments

* Fixed ConvexConcave border image

It was flipped horizontally.

* Implemented border left/right-concave detection

* Moved border images into their own directory

They're not really icons, after all.

* Cleaned up code a bit and added some more comments

* Applied requested change and consistified some function names

* Removed the old border images

I was sure I already did this, but apparently not.

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

@@ -184,9 +184,24 @@ object HexMath {
             (abs(relativeX) + abs(relativeY)).toInt()
     }
 
+    private val clockPositionToHexVectorMap: Map<Int, Vector2> = mapOf(
+        0 to Vector2(1f, 1f), // This alias of 12 makes clock modulo logic easier
+        12 to Vector2(1f, 1f),
+        2 to Vector2(0f, 1f),
+        4 to Vector2(-1f, 0f),
+        6 to Vector2(-1f, -1f),
+        8 to Vector2(0f, -1f),
+        10 to Vector2(1f, 0f)
+    )
+
+    /** Returns the hex-space distance corresponding to [clockPosition], or a zero vector if [clockPosition] is invalid */
+    fun getClockPositionToHexVector(clockPosition: Int): Vector2 {
+        return clockPositionToHexVectorMap[clockPosition]?: Vector2.Zero
+    }
+
     // Statically allocate the Vectors (in World coordinates)
     // of the 6 clock directions for border and road drawing in TileGroup 
-    private val clockToWorldVectors: Map<Int,Vector2> = mapOf(
+    private val clockPositionToWorldVectorMap: Map<Int,Vector2> = mapOf(
         2 to hex2WorldCoords(Vector2(0f, -1f)),
         4 to hex2WorldCoords(Vector2(1f, 0f)),
         6 to hex2WorldCoords(Vector2(1f, 1f)),
@@ -194,8 +209,9 @@ object HexMath {
         10 to hex2WorldCoords(Vector2(-1f, 0f)),
         12 to hex2WorldCoords(Vector2(-1f, -1f)) )
 
-    fun getClockDirectionToWorldVector(clockDirection: Int): Vector2 =
+    /** Returns the world/screen-space distance corresponding to [clockPosition], or a zero vector if [clockPosition] is invalid */
-        clockToWorldVectors[clockDirection] ?: Vector2.Zero
+    fun getClockPositionToWorldVector(clockPosition: Int): Vector2 =
+        clockPositionToWorldVectorMap[clockPosition] ?: Vector2.Zero
 
     fun getDistanceFromEdge(vector: Vector2, mapParameters: MapParameters): Int {
         val x = vector.x.toInt()

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

@@ -157,6 +157,16 @@ open class TileInfo {
     // We have to .toList() so that the values are stored together once for caching,
     // and the toSequence so that aggregations (like neighbors.flatMap{it.units} don't take up their own space
 
+    /** Returns the left shared neighbor of [this] and [neighbor] (relative to the view direction [this]->[neighbor]), or null if there is no such tile. */
+    fun getLeftSharedNeighbor(neighbor: TileInfo): TileInfo? {
+        return tileMap.getClockPositionNeighborTile(this,(tileMap.getNeighborTileClockPosition(this, neighbor) - 2) % 12)
+    }
+
+    /** Returns the right shared neighbor [this] and [neighbor] (relative to the view direction [this]->[neighbor]), or null if there is no such tile. */
+    fun getRightSharedNeighbor(neighbor: TileInfo): TileInfo? {
+        return tileMap.getClockPositionNeighborTile(this,(tileMap.getNeighborTileClockPosition(this, neighbor) + 2) % 12)
+    }
+
     @delegate:Transient
     val height : Int by lazy {
         getAllTerrains().flatMap { it.uniqueObjects }

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

@@ -223,7 +223,7 @@ class TileMap {
     }
 
     /**
-     * Returns the clockPosition of [otherTile] seen from [tile]'s position
+     * Returns the clock position of [otherTile] seen from [tile]'s position
      * Returns -1 if not neighbors
      */
     fun getNeighborTileClockPosition(tile: TileInfo, otherTile: TileInfo): Int {
@@ -249,12 +249,24 @@ class TileMap {
         }
     }
 
+    /**
+     * Returns the neighbor tile of [tile] at [clockPosition], if it exists.
+     * Takes world wrap into account
+     * Returns null if there is no such neighbor tile or if [clockPosition] is not a valid clock position
+     */
+    fun getClockPositionNeighborTile(tile: TileInfo, clockPosition: Int): TileInfo? {
+        val difference = HexMath.getClockPositionToHexVector(clockPosition)
+        if (difference == Vector2.Zero) return null
+        val possibleNeighborPosition = tile.position.cpy().add(difference)
+        return getIfTileExistsOrNull(possibleNeighborPosition.x.toInt(), possibleNeighborPosition.y.toInt())
+    }
+
     /** Convert relative direction of [otherTile] seen from [tile]'s position into a vector
      * in world coordinates of length sqrt(3), so that it can be used to go from tile center to
      * the edge of the hex in that direction (meaning the center of the border between the hexes)
      */
     fun getNeighborTilePositionAsWorldCoords(tile: TileInfo, otherTile: TileInfo): Vector2 =
-        HexMath.getClockDirectionToWorldVector(getNeighborTileClockPosition(tile, otherTile))
+        HexMath.getClockPositionToWorldVector(getNeighborTileClockPosition(tile, otherTile))
 
     /**
      * Returns the closest position to (0, 0) outside the map which can be wrapped

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

@@ -81,8 +81,19 @@ open class TileGroup(var tileInfo: TileInfo, var tileSetStrings:TileSetStrings,
 
     var resourceImage: Actor? = null
     var resource: String? = null
+
+    class RoadImage {
+        var roadStatus: RoadStatus = RoadStatus.None
+        var image: Image? = null
+    }
+    data class BorderSegment(
+        var images: List<Image>,
+        var isLeftConcave: Boolean = false,
+        var isRightConcave: Boolean = false,
+    )
+
     private val roadImages = HashMap<TileInfo, RoadImage>()
-    private val borderImages = HashMap<TileInfo, List<Image>>() // map of neighboring tile to border images
+    private val borderSegments = HashMap<TileInfo, BorderSegment>() // map of neighboring tile to border segments
 
     @Suppress("LeakingThis")    // we trust TileGroupIcons not to use our `this` in its constructor except storing it for later
     val icons = TileGroupIcons(this)
@@ -110,11 +121,6 @@ open class TileGroup(var tileInfo: TileInfo, var tileSetStrings:TileSetStrings,
     var showEntireMap = UncivGame.Current.viewEntireMapForDebug
     var forMapEditorIcon = false
 
-    class RoadImage {
-        var roadStatus: RoadStatus = RoadStatus.None
-        var image: Image? = null
-    }
-
     init {
         this.setSize(groupSize, groupSize)
         this.addActor(baseLayerGroup)
@@ -290,7 +296,7 @@ open class TileGroup(var tileInfo: TileInfo, var tileSetStrings:TileSetStrings,
             updatePixelMilitaryUnit(false)
             updatePixelCivilianUnit(false)
 
-            if (borderImages.isNotEmpty()) clearBorders()
+            if (borderSegments.isNotEmpty()) clearBorders()
 
             icons.update(false,false ,false, false, null)
 
@@ -402,11 +408,11 @@ open class TileGroup(var tileInfo: TileInfo, var tileSetStrings:TileSetStrings,
     }
 
     private fun clearBorders() {
-        for (images in borderImages.values)
+        for (borderSegment in borderSegments.values)
-            for (image in images)
+            for (image in borderSegment.images)
                 image.remove()
 
-        borderImages.clear()
+        borderSegments.clear()
     }
 
     private var previousTileOwner: CivilizationInfo? = null
@@ -416,7 +422,6 @@ open class TileGroup(var tileInfo: TileInfo, var tileSetStrings:TileSetStrings,
         // removing all the border images and putting them back again!
         val tileOwner = tileInfo.getOwner()
 
-
         if (previousTileOwner != tileOwner) clearBorders()
 
         previousTileOwner = tileOwner
@@ -425,25 +430,65 @@ open class TileGroup(var tileInfo: TileInfo, var tileSetStrings:TileSetStrings,
         val civOuterColor = tileInfo.getOwner()!!.nation.getOuterColor()
         val civInnerColor = tileInfo.getOwner()!!.nation.getInnerColor()
         for (neighbor in tileInfo.neighbors) {
+            var shouldRemoveBorderSegment = false
+            var shouldAddBorderSegment = false
+
+            var borderSegmentShouldBeLeftConcave = false
+            var borderSegmentShouldBeRightConcave = false
+
             val neighborOwner = neighbor.getOwner()
-            if (neighborOwner == tileOwner && borderImages.containsKey(neighbor)) // the neighbor used to not belong to us, but now it's ours
+            if (neighborOwner == tileOwner && borderSegments.containsKey(neighbor)) { // the neighbor used to not belong to us, but now it's ours
-            {
+                shouldRemoveBorderSegment = true
-                for (image in borderImages[neighbor]!!)
-                    image.remove()
-                borderImages.remove(neighbor)
             }
-            if (neighborOwner != tileOwner && !borderImages.containsKey(neighbor)) { // there should be a border here but there isn't
+            else if (neighborOwner != tileOwner) {
+                val leftSharedNeighbor = tileInfo.getLeftSharedNeighbor(neighbor)
+                val rightSharedNeighbor = tileInfo.getRightSharedNeighbor(neighbor)
+
+                // If a shared neighbor doesn't exist (because it's past a map edge), we act as if it's our tile for border concave/convex-ity purposes.
+                // This is because we do not draw borders against non-existing tiles either.
+                borderSegmentShouldBeLeftConcave = leftSharedNeighbor == null || leftSharedNeighbor.getOwner() == tileOwner
+                borderSegmentShouldBeRightConcave = rightSharedNeighbor == null || rightSharedNeighbor.getOwner() == tileOwner
+
+                if (!borderSegments.containsKey(neighbor)) { // there should be a border here but there isn't
+                    shouldAddBorderSegment = true
+                }
+                else if (
+                    borderSegmentShouldBeLeftConcave != borderSegments[neighbor]!!.isLeftConcave ||
+                    borderSegmentShouldBeRightConcave != borderSegments[neighbor]!!.isRightConcave
+                ) { // the concave/convex-ity of the border here is wrong
+                    shouldRemoveBorderSegment = true
+                    shouldAddBorderSegment = true
+                }
+            }
+
+            if (shouldRemoveBorderSegment) {
+                for (image in borderSegments[neighbor]!!.images)
+                    image.remove()
+                borderSegments.remove(neighbor)
+            }
+            if (shouldAddBorderSegment) {
+                val images = mutableListOf<Image>()
+                val borderSegment = BorderSegment(images, borderSegmentShouldBeLeftConcave, borderSegmentShouldBeRightConcave)
+                borderSegments[neighbor] = borderSegment
+
+                val borderShapeString = when {
+                    borderSegment.isLeftConcave && borderSegment.isRightConcave -> "Concave"
+                    !borderSegment.isLeftConcave && !borderSegment.isRightConcave -> "Convex"
+                    else -> "ConvexConcave"
+                }
+                val isConcaveConvex = borderSegment.isLeftConcave && !borderSegment.isRightConcave
 
                 val relativeWorldPosition = tileInfo.tileMap.getNeighborTilePositionAsWorldCoords(tileInfo, neighbor)
 
                 // This is some crazy voodoo magic so I'll explain.
-                val images = mutableListOf<Image>()
-                borderImages[neighbor] = images
                 val sign = if (relativeWorldPosition.x < 0) -1 else 1
                 val angle = sign * (atan(sign * relativeWorldPosition.y / relativeWorldPosition.x) * 180 / PI - 90.0).toFloat()
                 
-                val innerBorderImage = ImageGetter.getImage("OtherIcons/Border-inner")
+                val innerBorderImage = ImageGetter.getImage("BorderImages/${borderShapeString}Inner")
-                innerBorderImage.width = 38f
+                if (isConcaveConvex) {
+                    innerBorderImage.scaleX = -innerBorderImage.scaleX
+                }
+                innerBorderImage.width = hexagonImageWidth
                 innerBorderImage.setOrigin(Align.center) // THEN the origin is correct,
                 innerBorderImage.rotateBy(angle) // and the rotation works.
                 innerBorderImage.center(this) // move to center of tile
@@ -452,8 +497,11 @@ open class TileGroup(var tileInfo: TileInfo, var tileSetStrings:TileSetStrings,
                 miscLayerGroup.addActor(innerBorderImage)
                 images.add(innerBorderImage)
 
-                val outerBorderImage = ImageGetter.getImage("OtherIcons/Border-outer")
+                val outerBorderImage = ImageGetter.getImage("BorderImages/${borderShapeString}Outer")
-                outerBorderImage.width = 38f
+                if (isConcaveConvex) {
+                    outerBorderImage.scaleX = -outerBorderImage.scaleX
+                }
+                outerBorderImage.width = hexagonImageWidth
                 outerBorderImage.setOrigin(Align.center) // THEN the origin is correct,
                 outerBorderImage.rotateBy(angle) // and the rotation works.
                 outerBorderImage.center(this) // move to center of tile