From 82ebb01a20b1b959a0a9d1ae27d6e429e08de68e Mon Sep 17 00:00:00 2001
From: Jack Rainy <JackRainy@users.noreply.github.com>
Date: Sun, 25 Jun 2023 09:37:08 +0300
Subject: [PATCH] Zoom in/out of the history charts (#9660)

* Do not recreate the Line Chart every time

* Simplifed the Line Chart creation

* Do not create objects in draw(): VictoryScreenCivGroup table

* Do not create objects in draw(): Labels

* Create labels without negative Y

* Lift the X axis if there is an negative number

* Arbitrary number of -Y labels

* Autoscale by Y axis

* Zoom in/out by click

* Autoscale by X axis

* Unit tests for LineChart

* Rework of the line chart rendering
---
 core/src/com/unciv/ui/components/LineChart.kt | 263 ++++++++++--------
 .../victoryscreen/VictoryScreenCharts.kt      |  42 +--
 .../victoryscreen/VictoryScreenCivGroup.kt    |  62 +++--
 .../com/unciv/ui/components/LineChartTests.kt | 140 ++++++++++
 4 files changed, 338 insertions(+), 169 deletions(-)
 create mode 100644 tests/src/com/unciv/ui/components/LineChartTests.kt

diff --git a/core/src/com/unciv/ui/components/LineChart.kt b/core/src/com/unciv/ui/components/LineChart.kt
index 522145b843..4d66ad9819 100644
--- a/core/src/com/unciv/ui/components/LineChart.kt
+++ b/core/src/com/unciv/ui/components/LineChart.kt
@@ -2,35 +2,28 @@ package com.unciv.ui.components
 
 import com.badlogic.gdx.graphics.Color
 import com.badlogic.gdx.graphics.g2d.Batch
-import com.badlogic.gdx.graphics.glutils.ShapeRenderer
-import com.badlogic.gdx.math.Matrix4
-import com.badlogic.gdx.math.Vector2
-import com.badlogic.gdx.scenes.scene2d.ui.Image
+import com.badlogic.gdx.math.MathUtils.lerp
 import com.badlogic.gdx.scenes.scene2d.ui.Label
-import com.badlogic.gdx.scenes.scene2d.ui.Widget
+import com.badlogic.gdx.scenes.scene2d.ui.WidgetGroup
 import com.badlogic.gdx.utils.Align
 import com.unciv.logic.civilization.Civilization
-import com.unciv.ui.components.extensions.surroundWithCircle
+import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.screens.victoryscreen.VictoryScreenCivGroup
 import com.unciv.ui.screens.victoryscreen.VictoryScreenCivGroup.DefeatedPlayerStyle
+import kotlin.math.abs
 import kotlin.math.min
 import kotlin.math.ceil
+import kotlin.math.floor
 import kotlin.math.log10
 import kotlin.math.max
 import kotlin.math.pow
 import kotlin.math.sqrt
 
-private data class DataPoint<T>(val x: T, val y: T, val civ: Civilization)
+data class DataPoint<T>(val x: T, val y: T, val civ: Civilization)
 
 class LineChart(
-    data: Map<Int, Map<Civilization, Int>>,
-    private val viewingCiv: Civilization,
-    private val selectedCiv: Civilization,
-    private val chartWidth: Float,
-    private val chartHeight: Float
-) : Widget() {
-
-    private val shapeRenderer = ShapeRenderer()
+    private val viewingCiv: Civilization
+) : WidgetGroup() {
 
     private val axisLineWidth = 2f
     private val axisColor = Color.WHITE
@@ -45,40 +38,85 @@ class LineChart(
      * as `0` is not counted. */
     private val maxLabels = 10
 
-    private val xLabels: List<Int>
-    private val yLabels: List<Int>
+    private var xLabels = emptyList<Int>()
+    private var yLabels = emptyList<Int>()
+    private var xLabelsAsLabels = emptyList<Label>()
+    private var yLabelsAsLabels = emptyList<Label>()
 
-    private val hasNegativeYValues: Boolean
-    private val negativeYLabel: Int
+    private var dataPoints = emptyList<DataPoint<Int>>()
+    private var selectedCiv = Civilization()
 
-    private val dataPoints: List<DataPoint<Int>> = data.flatMap { turn ->
-        turn.value.map { (civ, value) ->
-            DataPoint(turn.key, value, civ)
+
+
+    fun getTurnAt(x: Float): IntRange? {
+        if (xLabels.isEmpty() || xLabelsAsLabels.isEmpty() || yLabelsAsLabels.isEmpty()) return null
+        val widestYLabelWidth = yLabelsAsLabels.maxOf { it.width }
+        val linesMinX = widestYLabelWidth + axisToLabelPadding + axisLineWidth
+        val linesMaxX = width - xLabelsAsLabels.last().width / 2
+        if (linesMinX.compareTo(linesMaxX) == 0) return (xLabels.first()..xLabels.last())
+        val ratio = (x - linesMinX) / (linesMaxX - linesMinX)
+        val turn = max(1, lerp(xLabels.first().toFloat(), xLabels.last().toFloat(), ratio).toInt())
+        return (getPrevNumberDivisibleByPowOfTen(turn-1)..getNextNumberDivisibleByPowOfTen(turn+1))
+    }
+
+    fun update(newData: List<DataPoint<Int>>, newSelectedCiv: Civilization) {
+        selectedCiv = newSelectedCiv
+
+        dataPoints = newData
+        updateLabels(dataPoints)
+        prepareForDraw()
+    }
+
+    private fun updateLabels(newData: List<DataPoint<Int>>) {
+        xLabels = generateLabels(newData, false)
+        yLabels = generateLabels(newData, true)
+
+        xLabelsAsLabels =
+            xLabels.map { Label(it.toString(), Label.LabelStyle(Fonts.font, axisLabelColor)) }
+        yLabelsAsLabels =
+            yLabels.map { Label(it.toString(), Label.LabelStyle(Fonts.font, axisLabelColor)) }
+    }
+
+    fun generateLabels(value: List<DataPoint<Int>>, yAxis: Boolean): List<Int> {
+        if (value.isEmpty()) return listOf(0)
+        val minLabelValue = getPrevNumberDivisibleByPowOfTen(value.minOf { if (yAxis) it.y else it.x })
+        val maxLabelValue = getNextNumberDivisibleByPowOfTen(value.maxOf { if (yAxis) it.y else it.x })
+        var stepSizePositive = ceil(maxLabelValue.toFloat() / maxLabels).toInt()
+
+        return when {
+            minLabelValue < 0 -> {
+                var stepSizeNegative = ceil(-minLabelValue.toFloat() / maxLabels).toInt()
+                val maxStep = max(stepSizePositive, stepSizeNegative)
+                val stepCountNegative = floor(minLabelValue / maxStep.toDouble()).toInt()
+                stepSizeNegative = if (abs(stepCountNegative) < 2) abs(minLabelValue) else maxStep
+                val stepCountPositive = ceil(maxLabelValue / maxStep.toDouble()).toInt()
+                stepSizePositive = if (abs(stepCountPositive) < 2) abs(maxLabelValue) else maxStep
+
+                (stepCountNegative until 0).map { (it * stepSizeNegative) } +
+                    if (maxLabelValue != 0)
+                    (0 until stepCountPositive + 1).map { (it * stepSizePositive) }
+                    else listOf(0)
+            }
+            maxLabelValue != 0 -> {
+                // `maxLabels + 1` because we want to end at `maxLabels * stepSize`.
+                if (minLabelValue < stepSizePositive)
+                    (0 until maxLabels + 1).map { (it * stepSizePositive) }
+                else {
+                    stepSizePositive = ceil((maxLabelValue-minLabelValue).toFloat() / maxLabels).toInt()
+                    (0 until maxLabels + 1).map { minLabelValue + (it * stepSizePositive) }
+                }
+            }
+            else -> listOf(0, 1) // line along 0
         }
     }
 
-    init {
-        hasNegativeYValues = dataPoints.any { it.y < 0 }
-        xLabels = generateLabels(dataPoints.maxOf { it.x })
-        yLabels = generateLabels(dataPoints.maxOf { it.y })
-        val lowestValue = dataPoints.minOf { it.y }
-        negativeYLabel = if (hasNegativeYValues) -getNextNumberDivisibleByPowOfTen(-lowestValue) else 0
-    }
-
-    private fun generateLabels(maxValue: Int): List<Int> {
-        val maxLabelValue = getNextNumberDivisibleByPowOfTen(maxValue)
-        val stepSize = ceil(maxLabelValue.toFloat() / maxLabels).toInt()
-        // `maxLabels + 1` because we want to end at `maxLabels * stepSize`.
-        return (0 until maxLabels + 1).map { (it * stepSize) }
-    }
-
     /**
      *  Returns the next number of power 10, with maximal step <= 100.
      *  Examples: 0 => 0, 3 => 10, 97 => 100, 567 => 600, 123321 => 123400
      */
     private fun getNextNumberDivisibleByPowOfTen(value: Int): Int {
         if (value == 0) return 0
-        val numberOfDigits = min(ceil(log10(value.toDouble())).toInt(), 3)
+        val numberOfDigits = max(2, min(ceil(log10(abs(value).toDouble())).toInt(), 3))
         val oneWithZeros = 10.0.pow(numberOfDigits - 1)
         // E.g., 3 => 10^(2-1) = 10 ; ceil(3 / 10) * 10 = 10
         //     567 => 10^(3-1) = 100 ; ceil(567 / 100) * 100 = 600
@@ -86,28 +124,39 @@ class LineChart(
         return (ceil(value / oneWithZeros) * oneWithZeros).toInt()
     }
 
+    /**
+     *  Returns the previous number of power 10, with maximal step <= 100.
+     *  Examples: 0 => 0, -3 => -10, 97 => 90, 567 => 500, 123321 => 123300
+     */
+    private fun getPrevNumberDivisibleByPowOfTen(value: Int): Int {
+        if (value == 0) return 0
+        val numberOfDigits = max(2 , min(ceil(log10(abs(value).toDouble())).toInt(), 3))
+        val oneWithZeros = 10.0.pow(numberOfDigits - 1)
+        // E.g., 3 => 10^(2-1) = 10 ; floor(3 / 10) * 10 = 0
+        //     567 => 10^(3-1) = 100 ; floor(567 / 100) * 100 = 500
+        //  123321 => 10^(3-1) = 100 ; floor(123321 / 100) * 100 = 123300
+        return (floor(value / oneWithZeros) * oneWithZeros).toInt()
+    }
+
     override fun draw(batch: Batch, parentAlpha: Float) {
         super.draw(batch, parentAlpha)
+    }
 
-        // Save the current batch transformation matrix
-        val oldTransformMatrix = batch.transformMatrix.cpy()
-        // Set the batch transformation matrix to the local coordinates of the LineChart widget
-        val stageCoords = localToStageCoordinates(Vector2(0f, 0f))
-        batch.transformMatrix = Matrix4().translate(stageCoords.x, stageCoords.y, 0f)
+    private fun prepareForDraw() {
+
+        clearChildren()
+
+        if (xLabels.isEmpty() || yLabels.isEmpty()) return
 
         val lastTurnDataPoints = getLastTurnDataPoints()
 
-        val labelHeight = Label("123", Label.LabelStyle(Fonts.font, axisLabelColor)).height
-        val yLabelsAsLabels =
-                yLabels.map { Label(it.toString(), Label.LabelStyle(Fonts.font, axisLabelColor)) }
-        val negativeYLabelAsLabel =
-                Label(negativeYLabel.toString(), Label.LabelStyle(Fonts.font, axisLabelColor))
-        val widestYLabelWidth = max(yLabelsAsLabels.maxOf { it.width }, negativeYLabelAsLabel.width)
+        val labelHeight = yLabelsAsLabels.first().height
+        val widestYLabelWidth = yLabelsAsLabels.maxOf { it.width }
         // We assume here that all labels have the same height. We need to deduct the height of
         // a label from the available height, because otherwise the label on the top would
         // overrun the height since the (x,y) coordinates define the bottom left corner of the
         // Label.
-        val yAxisLabelMaxY = chartHeight - labelHeight
+        val yAxisLabelMaxY = height - labelHeight
         // This is to account for the x axis and its labels which are below the lowest point
         val xAxisLabelsHeight = labelHeight
         val zeroYAxisLabelHeight = labelHeight
@@ -118,63 +167,60 @@ class LineChart(
         // We draw the y-axis labels first. They will take away some space on the left of the
         // widget which we need to consider when drawing the rest of the graph.
         var yAxisYPosition = 0f
-        val negativeOrientationLineYPosition = yAxisLabelMinY + labelHeight / 2
-        val yLabelsToDraw = if (hasNegativeYValues) listOf(negativeYLabelAsLabel) + yLabelsAsLabels else yLabelsAsLabels
-        yLabelsToDraw.forEachIndexed { index, label ->
-            val yPos = yAxisLabelMinY + index * (yAxisLabelYRange / (yLabelsToDraw.size - 1))
+        yLabels.forEachIndexed { index, value ->
+            val label = yLabelsAsLabels[index] // we assume yLabels.size == yLabelsAsLabels.size
+            val yPos = yAxisLabelMinY + index * (yAxisLabelYRange / (yLabels.size - 1))
             label.setPosition((widestYLabelWidth - label.width) / 2, yPos)
-            label.draw(batch, 1f)
+            addActor(label)
 
             // Draw y-axis orientation lines and x-axis
-            val zeroIndex = if (hasNegativeYValues) 1 else 0
+            val zeroIndex = value == 0
             drawLine(
-                batch,
                 widestYLabelWidth + axisToLabelPadding + axisLineWidth,
                 yPos + labelHeight / 2,
-                chartWidth,
+                width,
                 yPos + labelHeight / 2,
-                if (index != zeroIndex) orientationLineColor else axisColor,
-                if (index != zeroIndex) orientationLineWidth else axisLineWidth
+                if (zeroIndex) axisColor else orientationLineColor,
+                if (zeroIndex) axisLineWidth else orientationLineWidth
             )
-            if (index == zeroIndex) {
+            if (zeroIndex) {
                 yAxisYPosition = yPos + labelHeight / 2
             }
         }
 
         // Draw x-axis labels
-        val xLabelsAsLabels =
-                xLabels.map { Label(it.toString(), Label.LabelStyle(Fonts.font, axisLabelColor)) }
-        val lastXAxisLabelWidth = xLabelsAsLabels[xLabelsAsLabels.size - 1].width
+        val lastXAxisLabelWidth = xLabelsAsLabels.last().width
         val xAxisLabelMinX =
                 widestYLabelWidth + axisToLabelPadding + axisLineWidth / 2
-        val xAxisLabelMaxX = chartWidth - lastXAxisLabelWidth / 2
+        val xAxisLabelMaxX = width - lastXAxisLabelWidth / 2
         val xAxisLabelXRange = xAxisLabelMaxX - xAxisLabelMinX
-        xLabels.forEachIndexed { index, labelAsInt ->
-            val label = Label(labelAsInt.toString(), Label.LabelStyle(Fonts.font, axisLabelColor))
+        xLabelsAsLabels.forEachIndexed { index, label ->
             val xPos = xAxisLabelMinX + index * (xAxisLabelXRange / (xLabels.size - 1))
             label.setPosition(xPos - label.width / 2, 0f)
-            label.draw(batch, 1f)
+            addActor(label)
 
             // Draw x-axis orientation lines and y-axis
             drawLine(
-                batch,
                 xPos,
                 labelHeight + axisToLabelPadding + axisLineWidth,
                 xPos,
-                chartHeight,
+                height,
                 if (index > 0) orientationLineColor else axisColor,
-                if (index >0) orientationLineWidth else axisLineWidth
+                if (index > 0) orientationLineWidth else axisLineWidth
             )
         }
 
         // Draw line charts for each color
         val linesMinX = widestYLabelWidth + axisToLabelPadding + axisLineWidth
-        val linesMaxX = chartWidth - lastXAxisLabelWidth / 2
+        val linesMaxX = width - lastXAxisLabelWidth / 2
         val linesMinY = yAxisYPosition
-        val linesMaxY = chartHeight - labelHeight / 2
-        val scaleX = (linesMaxX - linesMinX) / xLabels.max()
-        val scaleY = (linesMaxY - linesMinY) / yLabels.max()
-        val negativeScaleY = if (hasNegativeYValues) (linesMinY - negativeOrientationLineYPosition) / -negativeYLabel else 0f
+        val linesMaxY = height - labelHeight / 2
+        val scaleX = (linesMaxX - linesMinX) / (xLabels.max() - xLabels.min())
+        val scaleY = (linesMaxY - linesMinY) / (yLabels.max() - yLabels.min())
+        val negativeOrientationLineYPosition = yAxisLabelMinY + labelHeight / 2
+        val minXLabel = xLabels.min()
+        val minYLabel = yLabels.min()
+        val negativeScaleY = (negativeOrientationLineYPosition - linesMinY) / if (minYLabel < 0) minYLabel else 1
         val sortedPoints = dataPoints.sortedBy { it.x }
         val pointsByCiv = sortedPoints.groupBy { it.civ }
         // We want the current player civ to be drawn last, so it is never overlapped by another player.
@@ -191,8 +237,10 @@ class LineChart(
         for (civ in civIterationOrder) {
             val points = pointsByCiv[civ]!!
             val scaledPoints : List<DataPoint<Float>> = points.map {
-                val yScale = if (it.y < 0f) negativeScaleY else scaleY
-                DataPoint(linesMinX + it.x * scaleX, linesMinY + it.y * yScale, it.civ)
+                if (it.y < 0f)
+                    DataPoint(linesMinX + (it.x - minXLabel) * scaleX, linesMinY + it.y * negativeScaleY, it.civ)
+                else
+                    DataPoint(linesMinX + (it.x - minXLabel) * scaleX, linesMinY + (it.y - minYLabel) * scaleY, it.civ)
             }
             // Probably nobody can tell the difference of one pixel, so that seems like a reasonable epsilon.
             val simplifiedScaledPoints = douglasPeucker(scaledPoints, 1f)
@@ -205,7 +253,7 @@ class LineChart(
                     val selectedCivBackgroundColor =
                         if (useActualColor(civ)) civ.nation.getInnerColor() else Color.LIGHT_GRAY
                     drawLine(
-                        batch, a.x, a.y, b.x, b.y,
+                        a.x, a.y, b.x, b.y,
                         selectedCivBackgroundColor, chartLineWidth * 3
                     )
                 }
@@ -214,31 +262,19 @@ class LineChart(
                 val a = simplifiedScaledPoints[i - 1]
                 val b = simplifiedScaledPoints[i]
                 val civLineColor = if (useActualColor(civ)) civ.nation.getOuterColor() else Color.DARK_GRAY
-                drawLine(batch, a.x, a.y, b.x, b.y, civLineColor, chartLineWidth)
+                drawLine(a.x, a.y, b.x, b.y, civLineColor, chartLineWidth)
 
                 // Draw the selected Civ icon on its last datapoint
                 if (i == simplifiedScaledPoints.size - 1 && selectedCiv == civ && selectedCiv in lastTurnDataPoints) {
-                    val selectedCivIcon =
-                            VictoryScreenCivGroup(
-                                selectedCiv,
-                                "",
-                                viewingCiv,
-                                DefeatedPlayerStyle.REGULAR
-                            ).children[0].run {
-                                (this as? Image)?.surroundWithCircle(30f, color = Color.LIGHT_GRAY)
-                                    ?: this
-                            }
-                    selectedCivIcon.run {
+                    val selectedCivIcon = VictoryScreenCivGroup.getCivImageAndColors(selectedCiv, viewingCiv, DefeatedPlayerStyle.REGULAR).first
+                    selectedCivIcon.apply {
                         setPosition(b.x, b.y, Align.center)
                         setSize(33f, 33f) // Dead Civs need this
-                        draw(batch, parentAlpha)
                     }
+                    addActor(selectedCivIcon)
                 }
             }
         }
-
-        // Restore the previous batch transformation matrix
-        batch.transformMatrix = oldTransformMatrix
     }
 
     private fun useActualColor(civ: Civilization) : Boolean {
@@ -260,29 +296,18 @@ class LineChart(
         return lastDataPoints
     }
 
-    private fun drawLine(
-        batch: Batch,
-        x1: Float,
-        y1: Float,
-        x2: Float,
-        y2: Float,
-        color: Color,
-        width: Float
-    ) {
-        shapeRenderer.projectionMatrix = batch.projectionMatrix
-        shapeRenderer.transformMatrix = batch.transformMatrix
-        batch.end()
+    private fun drawLine(x1: Float, y1: Float, x2: Float, y2: Float, lineColor: Color, width: Float) {
 
-        shapeRenderer.begin(ShapeRenderer.ShapeType.Filled)
-        shapeRenderer.color = color
-        shapeRenderer.rectLine(x1, y1, x2, y2, width)
-        // Draw a circle at the beginning and end points of the line to make consecutive lines
-        // (which might point in different directions) connect nicely.
-        shapeRenderer.circle(x1, y1, width / 2)
-        shapeRenderer.circle(x2, y2, width / 2)
-        shapeRenderer.end()
+        val line = ImageGetter.getLine(x1, y1, x2, y2, width)
+        line.color = lineColor
+        addActor(line)
 
-        batch.begin()
+        val edgeRounding = ImageGetter.getCircle().apply {
+            setSize(width, width)
+            color = lineColor
+            setPosition(x1 - width / 2f, y1 - width / 2f)
+        }
+        addActor(edgeRounding)
     }
 
     private fun douglasPeucker(points: List<DataPoint<Float>>, epsilon: Float): List<DataPoint<Float>> {
@@ -352,10 +377,4 @@ class LineChart(
         return sqrt((dx * dx + dy * dy).toDouble()).toFloat()
     }
 
-    override fun getMinWidth() = chartWidth
-    override fun getMinHeight() = chartHeight
-    override fun getPrefWidth() = chartWidth
-    override fun getPrefHeight() = chartHeight
-    override fun getMaxWidth() = chartWidth
-    override fun getMaxHeight() = chartHeight
 }
diff --git a/core/src/com/unciv/ui/screens/victoryscreen/VictoryScreenCharts.kt b/core/src/com/unciv/ui/screens/victoryscreen/VictoryScreenCharts.kt
index be952402a2..6a96137a3d 100644
--- a/core/src/com/unciv/ui/screens/victoryscreen/VictoryScreenCharts.kt
+++ b/core/src/com/unciv/ui/screens/victoryscreen/VictoryScreenCharts.kt
@@ -2,16 +2,16 @@ package com.unciv.ui.screens.victoryscreen
 
 import com.badlogic.gdx.graphics.Color
 import com.badlogic.gdx.scenes.scene2d.Touchable
-import com.badlogic.gdx.scenes.scene2d.ui.Container
 import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.badlogic.gdx.utils.Align
-import com.unciv.logic.civilization.Civilization
 import com.unciv.ui.components.AutoScrollPane
+import com.unciv.ui.components.DataPoint
 import com.unciv.ui.components.LineChart
 import com.unciv.ui.components.TabbedPager
 import com.unciv.ui.components.input.onChange
 import com.unciv.ui.components.input.onClick
 import com.unciv.ui.components.extensions.packIfNeeded
+import com.unciv.ui.components.input.OnClickListener
 import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.screens.basescreen.BaseScreen
 import com.unciv.ui.screens.newgamescreen.TranslatedSelectBox
@@ -36,7 +36,9 @@ class VictoryScreenCharts(
         align = Align.center
     }
 
-    private val chartHolder = Container<LineChart?>(null)
+    private var lineChart = LineChart(viewingCiv)
+    // if it is negative - no zoom, if positive - zoom at turn X
+    private var zoomAtX : IntRange? = null
 
     init {
         civButtonsScroll.setScrollingDisabled(true, false)
@@ -46,7 +48,14 @@ class VictoryScreenCharts(
         controlsColumn.add(civButtonsScroll).fillY()
         defaults().fill().pad(20f)
         add(controlsColumn)
-        add(chartHolder).growX().top().padLeft(0f)
+        updateControls()
+        add(lineChart).growX().top().padLeft(0f)
+
+        val onChartClick = OnClickListener(function = { _ , x, _ ->
+            zoomAtX = if (zoomAtX == null) lineChart.getTurnAt(x) else null
+            updateChart()
+        })
+        lineChart.addListener(onChartClick)
 
         rankingTypeSelect.onChange {
             rankingType = RankingType.values()
@@ -85,38 +94,31 @@ class VictoryScreenCharts(
     }
 
     private fun updateChart() {
-        // LineChart does not "cooperate" in Layout - the size we set here is final.
-        // These values seem to fit the cell it'll be in - we subtract padding and some extra manually
+        lineChart.update(getLineChartData(rankingType), selectedCiv)
         packIfNeeded()
-        chartHolder.actor = LineChart(
-            getLineChartData(rankingType),
-            viewingCiv,
-            selectedCiv,
-            parent.width - getColumnWidth(0) - 60f,
-            parent.height - 60f
-        )
-        chartHolder.invalidateHierarchy()
     }
 
-    private fun getLineChartData(
-        rankingType: RankingType
-    ): Map<Int, Map<Civilization, Int>> {
+    private fun getLineChartData(rankingType: RankingType): List<DataPoint<Int>> {
         return gameInfo.civilizations.asSequence()
             .filter { it.isMajorCiv() }
             .flatMap { civ ->
                 civ.statsHistory
+                    .filterKeys { zoomAtX == null || it in zoomAtX!!  }
                     .filterValues { it.containsKey(rankingType) }
                     .map { (turn, data) -> Pair(turn, Pair(civ, data.getValue(rankingType))) }
             }
             .groupBy({ it.first }, { it.second })
             .mapValues { group -> group.value.toMap() }
+            .flatMap { turn ->
+                turn.value.map { (civ, value) -> DataPoint(turn.key, value, civ) }
+            }
     }
 
     override fun activated(index: Int, caption: String, pager: TabbedPager) {
         pager.setScrollDisabled(true)
-        getCell(controlsColumn).height(parent.height)
-        getCell(chartHolder).height(parent.height)
-        if (chartHolder.actor == null) update()
+        controlsColumn.height = parent.height
+        lineChart.height = parent.height
+        update()
         civButtonsTable.invalidateHierarchy()
     }
 
diff --git a/core/src/com/unciv/ui/screens/victoryscreen/VictoryScreenCivGroup.kt b/core/src/com/unciv/ui/screens/victoryscreen/VictoryScreenCivGroup.kt
index 9d6d3f1478..e13db17b40 100644
--- a/core/src/com/unciv/ui/screens/victoryscreen/VictoryScreenCivGroup.kt
+++ b/core/src/com/unciv/ui/screens/victoryscreen/VictoryScreenCivGroup.kt
@@ -1,11 +1,13 @@
 package com.unciv.ui.screens.victoryscreen
 
 import com.badlogic.gdx.graphics.Color
+import com.badlogic.gdx.scenes.scene2d.Actor
 import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.badlogic.gdx.utils.Align
 import com.unciv.Constants
 import com.unciv.logic.civilization.Civilization
 import com.unciv.models.translations.tr
+import com.unciv.ui.components.extensions.setSize
 import com.unciv.ui.components.extensions.toLabel
 import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.screens.basescreen.BaseScreen
@@ -54,34 +56,17 @@ internal class VictoryScreenCivGroup(
             : this(civ, "\n", additionalInfo.tr(), currentPlayer, defeatedPlayerStyle)
 
     init {
-        var labelText = if (additionalInfo.isEmpty()) civ.civName
-            else "{${civ.civName}}$separator{$additionalInfo}"
-        val labelColor: Color
-        val backgroundColor: Color
+        val labelText =
+            if (currentPlayer.knows(civ) || currentPlayer == civ ||
+                    civ.isDefeated() || currentPlayer.isDefeated()) {
+                if (additionalInfo.isEmpty()) civ.civName
+                    else "{${civ.civName}}$separator{$additionalInfo}"
+            } else Constants.unknownNationName
 
-        when {
-            civ.isDefeated() && defeatedPlayerStyle == DefeatedPlayerStyle.GREYED_OUT -> {
-                add(ImageGetter.getImage("OtherIcons/DisbandUnit")).size(30f)
-                backgroundColor = Color.LIGHT_GRAY
-                labelColor = Color.BLACK
-            }
-            currentPlayer.isSpectator()
-                    || civ.isDefeated() && defeatedPlayerStyle == DefeatedPlayerStyle.REGULAR
-                    || currentPlayer == civ // || game.viewEntireMapForDebug
-                    || currentPlayer.knows(civ)
-                    || currentPlayer.isDefeated()
-                    || currentPlayer.victoryManager.hasWon() -> {
-                add(ImageGetter.getNationPortrait(civ.nation, 30f))
-                backgroundColor = civ.nation.getOuterColor()
-                labelColor = civ.nation.getInnerColor()
-            }
-            else -> {
-                add(ImageGetter.getRandomNationPortrait(30f))
-                backgroundColor = Color.DARK_GRAY
-                labelColor = Color.WHITE
-                labelText = Constants.unknownNationName
-            }
-        }
+        val civInfo = getCivImageAndColors(civ, currentPlayer, defeatedPlayerStyle)
+        add(civInfo.first).size(30f)
+        val backgroundColor = civInfo.second
+        val labelColor = civInfo.third
 
         background = BaseScreen.skinStrings.getUiBackground("VictoryScreen/CivGroup", BaseScreen.skinStrings.roundedEdgeRectangleShape, backgroundColor)
         val label = labelText.toLabel(labelColor, hideIcons = true)
@@ -89,4 +74,27 @@ internal class VictoryScreenCivGroup(
 
         add(label).padLeft(10f)
     }
+
+    companion object {
+        fun getCivImageAndColors(civ: Civilization, currentPlayer: Civilization, defeatedPlayerStyle: DefeatedPlayerStyle): Triple<Actor, Color, Color> {
+            when {
+                civ.isDefeated() && defeatedPlayerStyle == DefeatedPlayerStyle.GREYED_OUT -> {
+                    val icon = (ImageGetter.getImage("OtherIcons/DisbandUnit"))
+                    icon.setSize(30f)
+                    return Triple(icon, Color.LIGHT_GRAY, Color.BLACK)
+                }
+                currentPlayer.isSpectator()
+                    || civ.isDefeated() && defeatedPlayerStyle == DefeatedPlayerStyle.REGULAR
+                    || currentPlayer == civ // || game.viewEntireMapForDebug
+                    || currentPlayer.knows(civ)
+                    || currentPlayer.isDefeated()
+                    || currentPlayer.victoryManager.hasWon() -> {
+                    return Triple(ImageGetter.getNationPortrait(civ.nation, 30f), civ.nation.getOuterColor(), civ.nation.getInnerColor())
+                }
+                else ->
+                    return Triple((ImageGetter.getRandomNationPortrait(30f)), Color.LIGHT_GRAY, Color.BLACK)
+            }
+        }
+
+    }
 }
diff --git a/tests/src/com/unciv/ui/components/LineChartTests.kt b/tests/src/com/unciv/ui/components/LineChartTests.kt
new file mode 100644
index 0000000000..bc61d5c1bb
--- /dev/null
+++ b/tests/src/com/unciv/ui/components/LineChartTests.kt
@@ -0,0 +1,140 @@
+package com.unciv.ui.components
+
+import com.unciv.logic.civilization.Civilization
+import org.junit.Assert
+import org.junit.Test
+
+class LineChartTests {
+
+    private val civ = Civilization("My civ")
+    private val lineChart = LineChart(Civilization(civ.civName))
+
+    @Test
+    fun `labels for an empty list are just 0 label`() {
+        val data = mutableListOf <DataPoint<Int>>()
+        val result = lineChart.generateLabels(data, true)
+        Assert.assertTrue(result.size == 1 && result[0] == 0)
+    }
+
+    @Test
+    fun `chart goes along 0 line`() {
+        val data = mutableListOf <DataPoint<Int>>()
+        data.add(DataPoint(0,0, civ))
+        data.add(DataPoint(1,0, civ))
+        data.add(DataPoint(2,0, civ))
+        val result = lineChart.generateLabels(data, true)
+        Assert.assertTrue(result.size == 2 && result[0] == 0 && result[1] == 1)
+    }
+
+    @Test
+    fun `chart is from 0 to the positive value`() {
+        val data = mutableListOf <DataPoint<Int>>()
+        data.add(DataPoint(0,-100, civ))
+        data.add(DataPoint(1,200, civ))
+        data.add(DataPoint(2,1600, civ))
+        val result = lineChart.generateLabels(data, false) // testing the X axis
+        Assert.assertTrue(result.size == 11 && result.first() == 0 && result[1] == 1 && result.last() == 10)
+    }
+
+    @Test
+    fun `chart is from 0 to the negative value`() {
+        val data = mutableListOf <DataPoint<Int>>()
+        data.add(DataPoint(0,0, civ))
+        data.add(DataPoint(1,-2, civ))
+        data.add(DataPoint(2,-6, civ))
+        val result = lineChart.generateLabels(data, true) // testing the Y axis
+        Assert.assertTrue(result.size == 11 && result.first() == -10 && result[1] == -9 && result.last() == 0)
+    }
+
+    @Test
+    fun `chart goes from the negative to the positive value near 0`() {
+        val data = mutableListOf <DataPoint<Int>>()
+        data.add(DataPoint(0,-5, civ))
+        data.add(DataPoint(1,2, civ))
+        data.add(DataPoint(2,6, civ))
+        val result = lineChart.generateLabels(data, true)
+        Assert.assertTrue(result.size == 21 && result.first() == -10 && result[1] == -9 && result.last() == 10)
+    }
+
+    @Test
+    fun `chart goes from the negative to the positive far from 0`() {
+        val data = mutableListOf <DataPoint<Int>>()
+        data.add(DataPoint(0,-59, civ))
+        data.add(DataPoint(1,191, civ))
+        data.add(DataPoint(2,160, civ))
+        val result = lineChart.generateLabels(data, true)
+        Assert.assertTrue(result.size == 14 && result.first() == -60 && result[1] == -40 && result[3] == 0 && result.last() == 200)
+    }
+
+    @Test
+    fun `chart goes from the positive to the negative far from 0`() {
+        val data = mutableListOf <DataPoint<Int>>()
+        data.add(DataPoint(0,-485, civ))
+        data.add(DataPoint(1,191, civ))
+        data.add(DataPoint(2,160, civ))
+        val result = lineChart.generateLabels(data, true)
+        Assert.assertTrue(result.size == 15 && result.first() == -500 && result[1] == -450 && result[10] == 0 && result.last() == 200)
+    }
+
+    @Test
+    fun `chart is within the positive values far from 0`() {
+        val data = mutableListOf <DataPoint<Int>>()
+        data.add(DataPoint(0,290, civ))
+        data.add(DataPoint(1,1159, civ))
+        data.add(DataPoint(2,280, civ))
+        val result = lineChart.generateLabels(data, true)
+        Assert.assertTrue(result.size == 11 && result.first() == 200 && result[1] == 300 && result.last() == 1200)
+    }
+
+    @Test
+    fun `chart is within the negative values far from 0`() {
+        val data = mutableListOf <DataPoint<Int>>()
+        data.add(DataPoint(0,-290, civ))
+        data.add(DataPoint(1,-1160, civ))
+        data.add(DataPoint(2,-280, civ))
+        val result = lineChart.generateLabels(data, true)
+        Assert.assertTrue(result.size == 10 && result.first() == -1200 && result[1] == -1080 && result.last() == -120)
+    }
+
+
+    @Test
+    fun `chart is within the positive values near 0`() {
+        val data = mutableListOf <DataPoint<Int>>()
+        data.add(DataPoint(0,180, civ))
+        data.add(DataPoint(1,1101, civ))
+        data.add(DataPoint(2,980, civ))
+        val result = lineChart.generateLabels(data, true)
+        Assert.assertTrue(result.size == 11 && result.first() == 0 && result[1] == 120 && result.last() == 1200)
+    }
+
+    @Test
+    fun `chart is within the negative values near 0`() {
+        val data = mutableListOf <DataPoint<Int>>()
+        data.add(DataPoint(0,-180, civ))
+        data.add(DataPoint(1,-1101, civ))
+        data.add(DataPoint(2,-980, civ))
+        val result = lineChart.generateLabels(data, true)
+        Assert.assertTrue(result.size == 11 && result.first() == -1200 && result[1] == -1080 && result.last() == 0)
+    }
+
+    @Test
+    fun `chart is mostly in the positive values but not only`() {
+        val data = mutableListOf <DataPoint<Int>>()
+        data.add(DataPoint(0,210, civ))
+        data.add(DataPoint(1,1101, civ))
+        data.add(DataPoint(2,-89, civ))
+        val result = lineChart.generateLabels(data, true)
+        Assert.assertTrue(result.size == 12 && result.first() == -90 && result[1] == 0 && result.last() == 1200)
+    }
+
+    @Test
+    fun `chart is mostly in the negative values but not only`() {
+        val data = mutableListOf <DataPoint<Int>>()
+        data.add(DataPoint(0,111, civ))
+        data.add(DataPoint(1,-2101, civ))
+        data.add(DataPoint(2,-345, civ))
+        val result = lineChart.generateLabels(data, true)
+        Assert.assertTrue(result.size == 12 && result.first() == -2200 && result[10] == 0 && result.last() == 200)
+    }
+
+}