Great Person Points - Rounding changes, Breakdown UI (#10714)

* Change per-City GPP points math and separate breakdown (producer w/ source doc) from aggregates (consumer ignoring source)

* Change CityScreen GPP list to not do aggregation itself, but show a breakdown on click

* Change "birth" city for GPP to a ***weighted*** random - no Great Mufties from cities not generating any Mufty points!

* Nicer signature for getGreatPersonPointsBreakdown

* Minor warnings linting and template for Sweden's bonus in the breakdown Popup

android/assets/jsons/translations/template.properties

@@ -147,6 +147,7 @@ We will remember this. =
 
 [civName] has declared war on [targetCivName]! = 
 [civName] and [targetCivName] have signed a Peace Treaty! = 
+Declaration of Friendship = 
 [civName] and [targetCivName] have signed the Declaration of Friendship! = 
 [civName] has denounced [targetCivName]! = 
 Do you want to break your promise to [leaderName]? = 

core/src/com/unciv/logic/city/City.kt

@@ -34,7 +34,6 @@ enum class CityFlags {
 
 
 class City : IsPartOfGameInfoSerialization {
-    @Suppress("JoinDeclarationAndAssignment")
     @Transient
     lateinit var civ: Civilization
 
@@ -210,68 +209,100 @@ class City : IsPartOfGameInfoSerialization {
     fun containsBuildingUnique(uniqueType: UniqueType) =
         cityConstructions.builtBuildingUniqueMap.getUniques(uniqueType).any()
 
-    fun getGreatPersonPercentageBonus(): Int{
+    fun getGreatPersonPercentageBonus() = getGreatPersonPercentageBonusBreakdown().sumOf { it.second }
-        var allGppPercentageBonus = 0
+
+    @Suppress("RemoveExplicitTypeArguments")  // Clearer readability
+    /** Collects percentage boni applying to all GPP. Each returned entry is a Pair naming the source and the integer percentage. */
+    private fun getGreatPersonPercentageBonusBreakdown() = sequence<Pair<String, Int>> {
         for (unique in getMatchingUniques(UniqueType.GreatPersonPointPercentage)) {
             if (!matchesFilter(unique.params[1])) continue
-            allGppPercentageBonus += unique.params[0].toInt()
+            yield((unique.sourceObjectName ?: "Bonus") to unique.params[0].toInt())
         }
 
         // Sweden UP
         for (otherCiv in civ.getKnownCivs()) {
             if (!civ.getDiplomacyManager(otherCiv).hasFlag(DiplomacyFlags.DeclarationOfFriendship))
                 continue
-
+            val boostUniques = civ.getMatchingUniques(UniqueType.GreatPersonBoostWithFriendship) +
-            for (ourUnique in civ.getMatchingUniques(UniqueType.GreatPersonBoostWithFriendship))
+                otherCiv.getMatchingUniques(UniqueType.GreatPersonBoostWithFriendship)
-                allGppPercentageBonus += ourUnique.params[0].toInt()
+            for (unique in boostUniques)
-            for (theirUnique in otherCiv.getMatchingUniques(UniqueType.GreatPersonBoostWithFriendship))
+                yield("Declaration of Friendship" to unique.params[0].toInt())
-                allGppPercentageBonus += theirUnique.params[0].toInt()
         }
-        return allGppPercentageBonus
     }
 
-    fun getGreatPersonPointsForNextTurn(): HashMap<String, Counter<String>> {
+    data class GreatPersonPointsBreakdownEntry(val source: String, val isBonus: Boolean, val counter: Counter<String> = Counter())
-        val sourceToGPP = HashMap<String, Counter<String>>()
+    fun getGreatPersonPointsBreakdown(): Sequence<GreatPersonPointsBreakdownEntry> {
+        // First part: Base GPP points, materialized since we'll need to scan them twice
+        val basePoints = mutableListOf<GreatPersonPointsBreakdownEntry>()
 
-        val specialistsCounter = Counter<String>()
+        // ... from Specialists
+        val specialists = GreatPersonPointsBreakdownEntry("Specialists", false)
         for ((specialistName, amount) in population.getNewSpecialists())
             if (getRuleset().specialists.containsKey(specialistName)) { // To solve problems in total remake mods
                 val specialist = getRuleset().specialists[specialistName]!!
-                specialistsCounter.add(specialist.greatPersonPoints.times(amount))
+                specialists.counter.add(specialist.greatPersonPoints.times(amount))
             }
-        sourceToGPP["Specialists"] = specialistsCounter
+        basePoints.add(specialists)
 
-        val buildingsCounter = Counter<String>()
+        // ... and from buildings, allowing multiples of the same name to be aggregated since builtBuildingObjects is a List
-        for (building in cityConstructions.getBuiltBuildings())
+        basePoints.addAll(
-            buildingsCounter.add(building.greatPersonPoints)
+            cityConstructions.getBuiltBuildings()
-        sourceToGPP["Buildings"] = buildingsCounter
+                .groupBy({ it.name }, { it.greatPersonPoints })
+                .map { GreatPersonPointsBreakdownEntry(
+                    it.key,
+                    false,
+                    it.value.fold(null) { a: Counter<String>?, b -> if (a == null) b else a + b }!! // Just a sumOf<Counter>, !! because groupBy doesn't output empty lists
+                ) }
+        )
 
-        val stateForConditionals = StateForConditionals(civInfo = civ, city = this)
+        return sequence {
-        for ((_, gppCounter) in sourceToGPP) {
+            // Second part: passing the first part through first, but collecting all names with base points while doing so
+            val allGpp = mutableSetOf<String>()
+            for (element in basePoints) {
+                yield(element)
+                allGpp += element.counter.keys
+            }
+
+            // Now add boni: GreatPersonPointPercentage and GreatPersonBoostWithFriendship
+            for ((source, bonus) in getGreatPersonPercentageBonusBreakdown()) {
+                val bonusEntry = GreatPersonPointsBreakdownEntry(source, true)
+                for (gppName in allGpp)
+                    bonusEntry.counter[gppName] = bonus
+                yield(bonusEntry)
+            }
+
+            // And last, the GPP-type-specific GreatPersonEarnedFaster Unique
+            val stateForConditionals = StateForConditionals(civInfo = civ, city = this@City)
             for (unique in civ.getMatchingUniques(UniqueType.GreatPersonEarnedFaster, stateForConditionals)) {
-                val unitName = unique.params[0]
+                val bonusEntry = GreatPersonPointsBreakdownEntry(unique.sourceObjectName ?: "Bonus", true)
-                if (!gppCounter.containsKey(unitName)) continue
+                bonusEntry.counter.add(unique.params[0], unique.params[1].toInt())
-                gppCounter.add(unitName, gppCounter[unitName] * unique.params[1].toInt() / 100)
+                yield(bonusEntry)
+            }
+        }
     }
 
-            val allGppPercentageBonus = getGreatPersonPercentageBonus()
+    fun getGreatPersonPoints() = Counter<String>().apply {
-
+        // Using fixed-point(n.3) math to avoid surprises by rounding while still leveraging the Counter class
-            for (unitName in gppCounter.keys)
+        // Also accumulating boni separately - to ensure they operate additively not multiplicatively
-                gppCounter.add(unitName, gppCounter[unitName] * allGppPercentageBonus / 100)
+        val boni = Counter<String>()
+        for ((_, isBonus, counter) in getGreatPersonPointsBreakdown()) {
+            if (!isBonus) {
+                add(counter * 1000)
+            } else {
+                boni.add(counter)
             }
-
-        return sourceToGPP
         }
+        for (key in keys.filter { it in boni }) {
+            add(key, this[key] * boni[key] / 100)
+        }
+        // round fixed-point to integers
+        for (key in keys)
+            this[key] = (this[key] * 0.001).roundToInt()
 
-    fun getGreatPersonPoints(): Counter<String> {
-        val gppCounter = Counter<String>()
-        for (entry in getGreatPersonPointsForNextTurn().values)
-            gppCounter.add(entry)
         // Remove all "gpp" values that are not valid units
-        for (key in gppCounter.keys.toSet())
+        for (key in keys.toSet())
             if (key !in getRuleset().units)
-                gppCounter.remove(key)
+                remove(key)
-        return gppCounter
     }
 
     fun addStat(stat: Stat, amount: Int) {

core/src/com/unciv/logic/civilization/managers/TurnManager.kt

@@ -3,6 +3,7 @@ package com.unciv.logic.civilization.managers
 import com.unciv.UncivGame
 import com.unciv.logic.VictoryData
 import com.unciv.logic.automation.civilization.NextTurnAutomation
+import com.unciv.logic.city.City
 import com.unciv.logic.city.managers.CityTurnManager
 import com.unciv.logic.civilization.AlertType
 import com.unciv.logic.civilization.CivFlags
@@ -20,6 +21,7 @@ import com.unciv.models.ruleset.unique.UniqueType
 import com.unciv.models.ruleset.unique.endTurn
 import com.unciv.models.stats.Stats
 import com.unciv.ui.components.MayaCalendar
+import com.unciv.ui.components.extensions.randomWeighted
 import com.unciv.ui.screens.worldscreen.status.NextTurnProgress
 import com.unciv.utils.Log
 import kotlin.math.max
@@ -59,7 +61,7 @@ class TurnManager(val civInfo: Civilization) {
         if (civInfo.cities.isNotEmpty()) { //if no city available, addGreatPerson will throw exception
             val greatPerson = civInfo.greatPeople.getNewGreatPerson()
             if (greatPerson != null && civInfo.gameInfo.ruleset.units.containsKey(greatPerson))
-                civInfo.units.addUnit(greatPerson)
+                civInfo.units.addUnit(greatPerson, getRandomWeightedCity(greatPerson))
             civInfo.religionManager.startTurn()
             if (civInfo.isLongCountActive())
                 MayaCalendar.startTurnForMaya(civInfo)
@@ -94,6 +96,19 @@ class TurnManager(val civInfo: Civilization) {
         updateWinningCiv()
     }
 
+    /** Determine which city gets a new Great Person
+     *
+     *  - Choose randomly but chance proportional to the given city's contribution to [greatPerson]
+     *  - returning null will leave the decision to addUnit which will choose an unweighted random one
+     */
+    private fun getRandomWeightedCity(greatPerson: String): City? {
+        val cities = civInfo.cities.asSequence()
+            .map { it to it.getGreatPersonPoints()[greatPerson] }
+            .filter { it.second > 0 }
+            .toList()
+        if (cities.isEmpty()) return null
+        return cities.map { it.first }.randomWeighted(cities.map { it.second.toFloat() })
+    }
 
     private fun startTurnFlags() {
         for (flag in civInfo.flagsCountdown.keys.toList()) {

core/src/com/unciv/ui/screens/cityscreen/CityStatsTable.kt

@@ -2,6 +2,7 @@ package com.unciv.ui.screens.cityscreen
 
 import com.badlogic.gdx.graphics.Color
 import com.badlogic.gdx.scenes.scene2d.Actor
+import com.badlogic.gdx.scenes.scene2d.Touchable
 import com.badlogic.gdx.scenes.scene2d.ui.Cell
 import com.badlogic.gdx.scenes.scene2d.ui.Label
 import com.badlogic.gdx.scenes.scene2d.ui.Table
@@ -23,6 +24,7 @@ import com.unciv.ui.components.extensions.colorFromRGB
 import com.unciv.ui.components.extensions.surroundWithCircle
 import com.unciv.ui.components.extensions.toGroup
 import com.unciv.ui.components.extensions.toLabel
+import com.unciv.ui.components.extensions.toStringSigned
 import com.unciv.ui.components.extensions.toTextButton
 import com.unciv.ui.components.fonts.Fonts
 import com.unciv.ui.components.input.KeyboardBinding
@@ -30,6 +32,7 @@ import com.unciv.ui.components.input.onActivation
 import com.unciv.ui.components.input.onClick
 import com.unciv.ui.components.widgets.ExpanderTab
 import com.unciv.ui.images.ImageGetter
+import com.unciv.ui.popups.Popup
 import com.unciv.ui.screens.basescreen.BaseScreen
 import com.unciv.ui.screens.civilopediascreen.CivilopediaScreen
 import kotlin.math.ceil
@@ -350,31 +353,19 @@ class CityStatsTable(private val cityScreen: CityScreen) : Table() {
 
         val greatPeopleTable = Table()
 
-        val greatPersonPoints = city.getGreatPersonPointsForNextTurn()
+        val greatPersonPoints = city.getGreatPersonPoints()
-        val allGreatPersonNames = greatPersonPoints.asSequence().flatMap { it.value.keys }.distinct()
+        if (greatPersonPoints.isEmpty())
-
-        if (allGreatPersonNames.none())
             return
 
-        for (greatPersonName in allGreatPersonNames) {
+        for ((greatPersonName, gppPerTurn) in greatPersonPoints.asIterable().sortedBy { it.key }) {
-
-            var gppPerTurn = 0
-
-            for ((_, gppCounter) in greatPersonPoints) {
-                val gppPointsFromSource = gppCounter[greatPersonName]
-                if (gppPointsFromSource == 0) continue
-                gppPerTurn += gppPointsFromSource
-            }
-
             val info = Table()
 
-            info.add(ImageGetter.getUnitIcon(greatPersonName, Color.GOLD).toGroup(20f))
+            val specialistIcon = ImageGetter.getUnitIcon(greatPersonName, Color.GOLD).toGroup(20f)
-                .left().padBottom(4f).padRight(5f)
+            info.add(specialistIcon).left().padBottom(4f).padRight(5f)
             info.add("{$greatPersonName} (+$gppPerTurn)".toLabel(hideIcons = true)).left().padBottom(4f).expandX().row()
 
             val gppCurrent = city.civ.greatPeople.greatPersonPointsCounter[greatPersonName]
             val gppNeeded = city.civ.greatPeople.getPointsRequiredForGreatPerson(greatPersonName)
-
             val percent = gppCurrent / gppNeeded.toFloat()
 
             val progressBar = ImageGetter.ProgressBar(300f, 25f, false)
@@ -389,14 +380,34 @@ class CityStatsTable(private val cityScreen: CityScreen) : Table() {
                 bar.toBack()
             }
             progressBar.setLabel(Color.WHITE, "$gppCurrent/$gppNeeded", fontSize = 14)
-
             info.add(progressBar).colspan(2).left().expandX().row()
 
             greatPeopleTable.add(info).growX().top().padBottom(10f)
-            greatPeopleTable.add(ImageGetter.getConstructionPortrait(greatPersonName, 50f)).row()
+            val unitIcon = ImageGetter.getConstructionPortrait(greatPersonName, 45f) // Will be 2f bigger than ordered
+            greatPeopleTable.add(unitIcon).padLeft(10f).row()
+
+            info.touchable = Touchable.enabled
+            info.onClick { GppBreakDownPopup(greatPersonName) }
+            unitIcon.onClick { GppBreakDownPopup(greatPersonName) }
         }
 
         lowerTable.addCategory("Great People", greatPeopleTable, KeyboardBinding.GreatPeopleDetail)
     }
 
+    inner class GppBreakDownPopup(gppName: String) : Popup(cityScreen) {
+        init {
+            for ((source, isBonus, counter) in city.getGreatPersonPointsBreakdown()) {
+                val points = counter[gppName]
+                if (points == 0) continue
+                add("{$source}:".toLabel()).left().growX()
+                add((if (isBonus) points.toStringSigned() + "%" else points.toString()).toLabel(alignment = Align.right)).right().row()
+            }
+            addSeparator()
+            val total = city.getGreatPersonPoints()[gppName]
+            add("{Total}:".toLabel()).left().growX()
+            add(total.toString().toLabel(alignment = Align.right)).right().row()
+            addCloseButton()
+            open(true)
+        }
+    }
 }