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

This reverts commit 64a455152a40261daf6d12351a0135390c7cd828.

android/assets/jsons/translations/template.properties

@@ -147,7 +147,6 @@ 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,6 +34,7 @@ enum class CityFlags {
 
 
 class City : IsPartOfGameInfoSerialization {
+    @Suppress("JoinDeclarationAndAssignment")
     @Transient
     lateinit var civ: Civilization
 
@@ -209,100 +210,68 @@ class City : IsPartOfGameInfoSerialization {
     fun containsBuildingUnique(uniqueType: UniqueType) =
         cityConstructions.builtBuildingUniqueMap.getUniques(uniqueType).any()
 
-    fun getGreatPersonPercentageBonus() = getGreatPersonPercentageBonusBreakdown().sumOf { it.second }
+    fun getGreatPersonPercentageBonus(): Int{
-
+        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
-            yield((unique.sourceObjectName ?: "Bonus") to unique.params[0].toInt())
+            allGppPercentageBonus += 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) +
+
-                otherCiv.getMatchingUniques(UniqueType.GreatPersonBoostWithFriendship)
+            for (ourUnique in civ.getMatchingUniques(UniqueType.GreatPersonBoostWithFriendship))
-            for (unique in boostUniques)
+                allGppPercentageBonus += ourUnique.params[0].toInt()
-                yield("Declaration of Friendship" to unique.params[0].toInt())
+            for (theirUnique in otherCiv.getMatchingUniques(UniqueType.GreatPersonBoostWithFriendship))
+                allGppPercentageBonus += theirUnique.params[0].toInt()
         }
+        return allGppPercentageBonus
     }
 
-    data class GreatPersonPointsBreakdownEntry(val source: String, val isBonus: Boolean, val counter: Counter<String> = Counter())
+    fun getGreatPersonPointsForNextTurn(): HashMap<String, Counter<String>> {
-    fun getGreatPersonPointsBreakdown(): Sequence<GreatPersonPointsBreakdownEntry> {
+        val sourceToGPP = HashMap<String, Counter<String>>()
-        // First part: Base GPP points, materialized since we'll need to scan them twice
-        val basePoints = mutableListOf<GreatPersonPointsBreakdownEntry>()
 
-        // ... from Specialists
+        val specialistsCounter = Counter<String>()
-        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]!!
-                specialists.counter.add(specialist.greatPersonPoints.times(amount))
+                specialistsCounter.add(specialist.greatPersonPoints.times(amount))
             }
-        basePoints.add(specialists)
+        sourceToGPP["Specialists"] = specialistsCounter
 
-        // ... and from buildings, allowing multiples of the same name to be aggregated since builtBuildingObjects is a List
+        val buildingsCounter = Counter<String>()
-        basePoints.addAll(
+        for (building in cityConstructions.getBuiltBuildings())
-            cityConstructions.getBuiltBuildings()
+            buildingsCounter.add(building.greatPersonPoints)
-                .groupBy({ it.name }, { it.greatPersonPoints })
+        sourceToGPP["Buildings"] = buildingsCounter
-                .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
-                ) }
-        )
 
-        return sequence {
+        val stateForConditionals = StateForConditionals(civInfo = civ, city = this)
-            // Second part: passing the first part through first, but collecting all names with base points while doing so
+        for ((_, gppCounter) in sourceToGPP) {
-            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 bonusEntry = GreatPersonPointsBreakdownEntry(unique.sourceObjectName ?: "Bonus", true)
+                val unitName = unique.params[0]
-                bonusEntry.counter.add(unique.params[0], unique.params[1].toInt())
+                if (!gppCounter.containsKey(unitName)) continue
-                yield(bonusEntry)
+                gppCounter.add(unitName, gppCounter[unitName] * unique.params[1].toInt() / 100)
-            }
-        }
             }
 
-    fun getGreatPersonPoints() = Counter<String>().apply {
+            val allGppPercentageBonus = getGreatPersonPercentageBonus()
-        // Using fixed-point(n.3) math to avoid surprises by rounding while still leveraging the Counter class
-        // Also accumulating boni separately - to ensure they operate additively not multiplicatively
-        val boni = Counter<String>()
-        for ((_, isBonus, counter) in getGreatPersonPointsBreakdown()) {
-            if (!isBonus) {
-                add(counter * 1000)
-            } else {
-                boni.add(counter)
-            }
-        }
-        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()
 
+            for (unitName in gppCounter.keys)
+                gppCounter.add(unitName, gppCounter[unitName] * allGppPercentageBonus / 100)
+        }
+
+        return sourceToGPP
+    }
+
+    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 keys.toSet())
+        for (key in gppCounter.keys.toSet())
             if (key !in getRuleset().units)
-                remove(key)
+                gppCounter.remove(key)
+        return gppCounter
     }
 
     fun addStat(stat: Stat, amount: Int) {

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

@@ -3,7 +3,6 @@ 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
@@ -23,7 +22,6 @@ 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
@@ -63,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, getRandomWeightedCity(greatPerson))
+                civInfo.units.addUnit(greatPerson)
             civInfo.religionManager.startTurn()
             if (civInfo.isLongCountActive())
                 MayaCalendar.startTurnForMaya(civInfo)
@@ -98,19 +96,6 @@ 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,7 +2,6 @@ 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
@@ -24,7 +23,6 @@ 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
@@ -32,7 +30,6 @@ 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
@@ -353,19 +350,31 @@ class CityStatsTable(private val cityScreen: CityScreen) : Table() {
 
         val greatPeopleTable = Table()
 
-        val greatPersonPoints = city.getGreatPersonPoints()
+        val greatPersonPoints = city.getGreatPersonPointsForNextTurn()
-        if (greatPersonPoints.isEmpty())
+        val allGreatPersonNames = greatPersonPoints.asSequence().flatMap { it.value.keys }.distinct()
+
+        if (allGreatPersonNames.none())
             return
 
-        for ((greatPersonName, gppPerTurn) in greatPersonPoints.asIterable().sortedBy { it.key }) {
+        for (greatPersonName in allGreatPersonNames) {
+
+            var gppPerTurn = 0
+
+            for ((_, gppCounter) in greatPersonPoints) {
+                val gppPointsFromSource = gppCounter[greatPersonName]
+                if (gppPointsFromSource == 0) continue
+                gppPerTurn += gppPointsFromSource
+            }
+
             val info = Table()
 
-            val specialistIcon = ImageGetter.getUnitIcon(greatPersonName, Color.GOLD).toGroup(20f)
+            info.add(ImageGetter.getUnitIcon(greatPersonName, Color.GOLD).toGroup(20f))
-            info.add(specialistIcon).left().padBottom(4f).padRight(5f)
+                .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)
@@ -380,34 +389,14 @@ 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)
-            val unitIcon = ImageGetter.getConstructionPortrait(greatPersonName, 45f) // Will be 2f bigger than ordered
+            greatPeopleTable.add(ImageGetter.getConstructionPortrait(greatPersonName, 50f)).row()
-            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)
-        }
-    }
 }