You can now input distinct numbers when trading gold (#5072)

* Type amount of gold in trade requests

* Remove extraneous spaces in template.properties

* Implemented proposed changes

* Fixed tests

android/assets/jsons/translations/template.properties

@@ -146,6 +146,8 @@ Very well, we shall look for new lands to settle. =
 We shall do as we please. = 
 We noticed your new city near our borders, despite your promise. This will have....implications. = 
 
+Enter the amount of gold = 
+
 # City-States
 
 Provides [amountOfCulture] culture at 30 Influence = 
@@ -713,6 +715,10 @@ Worked by [cityName] =
 Lock = 
 Unlock = 
 Move to city = 
+Please enter a new name for your city = 
+
+# Ask for text or numbers popup UI
+
 Invalid input! Please enter a different string. = 
 Please enter some text = 
 

core/src/com/unciv/ui/pickerscreens/PromotionPickerScreen.kt

@@ -60,13 +60,10 @@ class PromotionPickerScreen(val unit: MapUnit) : PickerScreen() {
                     label = "Choose name for [${unit.baseUnit.name}]",
                     icon = ImageGetter.getUnitIcon(unit.name).surroundWithCircle(80f),
                     defaultText = unit.name,
+                    validate = { it != unit.name},
                     actionOnOk = { userInput -> 
-                        if (userInput == unit.name) 
-                            return@AskTextPopup false
-                        
                         unit.instanceName = userInput
                         this.game.setScreen(PromotionPickerScreen(unit))
-                        return@AskTextPopup true
                     }
                 ).open()
             }

core/src/com/unciv/ui/pickerscreens/ReligiousBeliefsPickerScreen.kt

@@ -117,16 +117,12 @@ class ReligiousBeliefsPickerScreen (
                 label = "Choose a name for your religion",
                 icon = ImageGetter.getCircledReligionIcon(religionName!!, 80f),
                 defaultText = religionName!!,
-                actionOnOk = { religionName ->
-                    if (religionName == Constants.noReligionName 
-                        || gameInfo.ruleSet.religions.any { it == religionName } 
-                        || gameInfo.religions.any { it.value.name == religionName }
-                    ) {
-                        return@AskTextPopup false
-                    }
-                    changeDisplayedReligionName(religionName)
-                    return@AskTextPopup true
-                }
+                validate = { religionName ->
+                    religionName != Constants.noReligionName
+                    && gameInfo.ruleSet.religions.none { it == religionName }
+                    && gameInfo.religions.none { it.value.name == religionName }
+                },
+                actionOnOk = { changeDisplayedReligionName(it) }
             ).open()
         }
         changeReligionNameButton.disable()

core/src/com/unciv/ui/trade/OfferColumnsTable.kt

@@ -10,7 +10,7 @@ import com.unciv.models.translations.tr
 import com.unciv.ui.utils.*
 
 /** This is the class that holds the 4 columns of the offers (ours/theirs/ offered/available) in trade */
-class OfferColumnsTable(private val tradeLogic: TradeLogic, val screen: DiplomacyScreen, val onChange: ()->Unit): Table(CameraStageBaseScreen.skin) {
+class OfferColumnsTable(private val tradeLogic: TradeLogic, val screen: DiplomacyScreen, val onChange: () -> Unit): Table(CameraStageBaseScreen.skin) {
 
     private fun addOffer(offer: TradeOffer, offerList: TradeOffersList, correspondingOfferList: TradeOffersList) {
         offerList.add(offer.copy())
@@ -19,20 +19,33 @@ class OfferColumnsTable(private val tradeLogic: TradeLogic, val screen: Diplomac
     }
 
     private val ourAvailableOffersTable = OffersListScroll("OurAvail") {
-        if (it.type == TradeType.Gold) openGoldSelectionPopup(it, tradeLogic.currentTrade.ourOffers, tradeLogic.ourCivilization)
-        else addOffer(it, tradeLogic.currentTrade.ourOffers, tradeLogic.currentTrade.theirOffers)
+        when (it.type) {
+            TradeType.Gold -> openGoldSelectionPopup(it, tradeLogic.currentTrade.ourOffers, tradeLogic.ourCivilization.gold)
+            TradeType.Gold_Per_Turn -> openGoldSelectionPopup(it, tradeLogic.currentTrade.ourOffers, tradeLogic.ourCivilization.statsForNextTurn.gold.toInt())
+            else -> addOffer(it, tradeLogic.currentTrade.ourOffers, tradeLogic.currentTrade.theirOffers) 
+        }
     }
+    
     private val ourOffersTable = OffersListScroll("OurTrade") {
-        if (it.type == TradeType.Gold) openGoldSelectionPopup(it, tradeLogic.currentTrade.ourOffers, tradeLogic.ourCivilization)
-        else addOffer(it.copy(amount = -it.amount), tradeLogic.currentTrade.ourOffers, tradeLogic.currentTrade.theirOffers)
+        when (it.type) {
+            TradeType.Gold -> openGoldSelectionPopup(it, tradeLogic.currentTrade.ourOffers, tradeLogic.ourCivilization.gold)
+            TradeType.Gold_Per_Turn -> openGoldSelectionPopup(it, tradeLogic.currentTrade.ourOffers, tradeLogic.ourCivilization.statsForNextTurn.gold.toInt())
+            else -> addOffer(it.copy(amount = -it.amount), tradeLogic.currentTrade.ourOffers, tradeLogic.currentTrade.theirOffers) 
+        }
     }
     private val theirOffersTable = OffersListScroll("TheirTrade") {
-        if (it.type == TradeType.Gold) openGoldSelectionPopup(it, tradeLogic.currentTrade.theirOffers, tradeLogic.otherCivilization)
-        else addOffer(it.copy(amount = -it.amount), tradeLogic.currentTrade.theirOffers, tradeLogic.currentTrade.ourOffers)
+        when (it.type) {
+            TradeType.Gold -> openGoldSelectionPopup(it, tradeLogic.currentTrade.theirOffers, tradeLogic.otherCivilization.gold)
+            TradeType.Gold_Per_Turn -> openGoldSelectionPopup(it, tradeLogic.currentTrade.theirOffers, tradeLogic.otherCivilization.statsForNextTurn.gold.toInt())
+            else -> addOffer(it.copy(amount = -it.amount), tradeLogic.currentTrade.theirOffers, tradeLogic.currentTrade.ourOffers)
+        }
     }
     private val theirAvailableOffersTable = OffersListScroll("TheirAvail") {
-        if (it.type == TradeType.Gold) openGoldSelectionPopup(it, tradeLogic.currentTrade.theirOffers, tradeLogic.otherCivilization)
-        else addOffer(it, tradeLogic.currentTrade.theirOffers, tradeLogic.currentTrade.ourOffers)
+        when (it.type) {
+            TradeType.Gold -> openGoldSelectionPopup(it, tradeLogic.currentTrade.theirOffers, tradeLogic.otherCivilization.gold)
+            TradeType.Gold_Per_Turn -> openGoldSelectionPopup(it, tradeLogic.currentTrade.theirOffers, tradeLogic.otherCivilization.statsForNextTurn.gold.toInt())
+            else -> addOffer(it, tradeLogic.currentTrade.theirOffers, tradeLogic.currentTrade.ourOffers)
+        }
     }
 
     init {
@@ -64,45 +77,27 @@ class OfferColumnsTable(private val tradeLogic: TradeLogic, val screen: Diplomac
         theirAvailableOffersTable.update(theirFilteredOffers, tradeLogic.ourAvailableOffers)
     }
 
-
-    class goldSelectionPopup(screen: DiplomacyScreen, offer: TradeOffer, ourOffers: TradeOffersList,
-                             offeringCiv: CivilizationInfo, onChange: () -> Unit):Popup(screen){
-        init {
-            val existingGoldOffer = ourOffers.firstOrNull { it.type == TradeType.Gold }
-            if (existingGoldOffer != null)
-                offer.amount = existingGoldOffer.amount
-            val amountLabel = offer.amount.toLabel()
-            val minitable = Table().apply { defaults().pad(5f) }
-
-            fun incrementAmount(delta: Int) {
-                offer.amount += delta
-                if (offer.amount < 0) offer.amount = 0
-                if (offer.amount > offeringCiv.gold) offer.amount = offeringCiv.gold
-                amountLabel.setText(offer.amount)
-            }
-
-            minitable.add("-500".toTextButton().onClick { incrementAmount(-500) })
-            minitable.add("-50".toTextButton().onClick { incrementAmount(-50) })
-            minitable.add(amountLabel)
-            minitable.add("+50".toTextButton().onClick { incrementAmount(50) })
-            minitable.add("+500".toTextButton().onClick { incrementAmount(500) })
-
-            add(minitable).row()
-
-            addCloseButton {
+    private fun openGoldSelectionPopup(offer: TradeOffer, ourOffers: TradeOffersList, maxGold: Int) {
+        val existingGoldOffer = ourOffers.firstOrNull { it.type == offer.type }
+        if (existingGoldOffer != null)
+            offer.amount = existingGoldOffer.amount
+        AskNumberPopup(
+            screen,
+            label = "Enter the amount of gold",
+            icon = ImageGetter.getStatIcon("Gold").surroundWithCircle(80f),
+            defaultText = offer.amount.toString(),
+            amountButtons = 
+                if (offer.type == TradeType.Gold) listOf(50, 500)
+                else listOf(5, 15),
+            bounds = IntRange(0, maxGold),
+            actionOnOk = { userInput ->
+                offer.amount = userInput
                 if (existingGoldOffer == null)
                     ourOffers.add(offer)
                 else existingGoldOffer.amount = offer.amount
                 if (offer.amount == 0) ourOffers.remove(offer)
                 onChange()
             }
-        }
+        ).open()
     }
-
-    private fun openGoldSelectionPopup(offer: TradeOffer, ourOffers: TradeOffersList, offeringCiv: CivilizationInfo) {
-        if (screen.stage.actors.any { it is goldSelectionPopup }) return
-        val selectionPopup = goldSelectionPopup(screen, offer, ourOffers, offeringCiv, onChange)
-        selectionPopup.open()
-    }
-
 }

core/src/com/unciv/ui/trade/TradeTable.kt

@@ -29,12 +29,12 @@ class TradeTable(val otherCivilization: CivilizationInfo, stage: DiplomacyScreen
         val lowerTable = Table().apply { defaults().pad(10f) }
 
         val existingOffer = otherCivilization.tradeRequests.firstOrNull{it.requestingCiv==currentPlayerCiv.civName}
-        if(existingOffer!=null){
+        if (existingOffer != null){
             tradeLogic.currentTrade.set(existingOffer.trade.reverse())
             offerColumnsTable.update()
         }
 
-        if(isTradeOffered()) offerButton.setText("Retract offer".tr())
+        if (isTradeOffered()) offerButton.setText("Retract offer".tr())
         else offerButton.setText("Offer trade".tr())
 
         offerButton.onClick {
@@ -59,7 +59,7 @@ class TradeTable(val otherCivilization: CivilizationInfo, stage: DiplomacyScreen
     private fun onChange(){
         offerColumnsTable.update()
         retractOffer()
-        offerButton.isEnabled = !(tradeLogic.currentTrade.theirOffers.size==0 && tradeLogic.currentTrade.ourOffers.size==0)
+        offerButton.isEnabled = !(tradeLogic.currentTrade.theirOffers.size == 0 && tradeLogic.currentTrade.ourOffers.size == 0)
     }
 
 }

core/src/com/unciv/ui/utils/AskNumberPopup.kt

@@ -0,0 +1,126 @@
+package com.unciv.ui.utils
+
+import com.badlogic.gdx.graphics.Color
+import com.badlogic.gdx.scenes.scene2d.ui.Button
+import com.badlogic.gdx.scenes.scene2d.ui.Table
+import com.badlogic.gdx.scenes.scene2d.ui.TextField
+
+/** Simple class for showing a prompt for a positive integer to the user
+ * @param screen The previous screen the user was on
+ * @param label A line of text shown to the user
+ * @param icon Icon at the top, should have size 80f
+ * @param defaultText The text that should be in the prompt at the start
+ * @param amountButtons Buttons that when clicked will add/subtract these amounts to the number
+ * @param bounds The bounds in which the number must lie. Defaults to [Int.MIN_VALUE, Int.MAX_VALUE]
+ * @param errorText Text that will be shown when an error is detected
+ * @param validate Function that should return `true` when a valid input is detected
+ * @param actionOnOk Lambda that will be executed after pressing 'OK'.
+ */
+
+class AskNumberPopup(
+    screen: CameraStageBaseScreen,
+    label: String = "Please enter a number",
+    icon: IconCircleGroup = ImageGetter.getImage("OtherIcons/Pencil").apply { this.color = Color.BLACK }.surroundWithCircle(80f),
+    defaultText: String = "",
+    amountButtons: List<Int> = listOf(),
+    bounds: IntRange = IntRange(Int.MIN_VALUE, Int.MAX_VALUE),
+    errorText: String = "Invalid input! Please enter a valid number.",
+    validate: (input: Int) -> Boolean = { true },
+    actionOnOk: (input: Int) -> Unit = { },
+): Popup(screen) {
+    /** Note for future developers: Why this class only accepts positive digits and not negative.
+     * 
+     * The problems is the minus sign. This might not seem like a large obstacle, but problems
+     * arrive quickly. First is that our clean `DigitsOnlyFilter()` must be replaced with a check
+     * that allows for adding a minus sign, but only when it is the first character. So far so good,
+     * until a user starts typing numbers before an already placed - sign --> crash. Fix that
+     * by disallowing any character being typed in front of a - sign. All is fixed right? Wrong!
+     * Because you now also disallowed writing two minus signs at the same time, copying over a
+     * new number after clamping now disallows overwriting the existing minus sign with a new minus
+     * sign, as there is already a minus sign in the number. Well, no problem, you can just remove
+     * the number before overwriting it with the clamped variant. But now you reset your cursor
+     * position every time you type a character. You might start trying to cache the cursor position
+     * as well, but at that point you're basically rewriting the setText() function, and when I
+     * reached this point I decided to stop. 
+     * 
+     * P.S., if you do decide to go on this quest of adding minus signs, don't forget that 
+     * `"-".toInt()` also crashes, so you need to exclude that before checking to clamp.
+     */
+
+    init {
+        val wrapper = Table()
+        wrapper.add(icon).padRight(10f)
+        wrapper.add(label.toLabel())
+        add(wrapper).colspan(2).row()
+
+        val nameField = TextField(defaultText, skin)
+        nameField.textFieldFilter = TextField.TextFieldFilter { _, char -> char.isDigit() || char == '-' }
+        
+        fun isValidInt(input: String): Boolean {
+            return input.toIntOrNull() != null
+        }
+        
+        
+        fun clampInBounds(input: String): String {
+            val int = input.toIntOrNull() ?: return input
+            
+            if (bounds.first > int) {
+                return bounds.first.toString()
+            }
+            if (bounds.last < int)
+                return bounds.last.toString()
+            
+            return input
+        } 
+        
+        nameField.onChange {
+            nameField.text = clampInBounds(nameField.text)
+        }
+        
+        val centerTable = Table(skin)
+        
+        fun addValueButton(value: Int) {
+            centerTable.add(
+                Button(
+                    if (value > 0) "+$value".toLabel()
+                    else value.toLabel(),
+                    skin
+                ).apply {
+                    onClick {
+                        if (isValidInt(nameField.text))
+                            nameField.text = clampInBounds((nameField.text.toInt() + value).toString())
+                    }
+                }
+            ).pad(5f)
+        }
+        
+        for (value in amountButtons.reversed()) {
+            addValueButton(-value)
+        }
+
+        centerTable.add(nameField).growX().pad(10f)
+        
+        add(centerTable).colspan(2).row()
+
+        for (value in amountButtons) {
+            addValueButton(value)
+        }
+        
+        val errorLabel = errorText.toLabel()
+        errorLabel.color = Color.RED
+
+        addOKButton(
+            validate = {
+                val errorFound = !isValidInt(nameField.text) || !validate(nameField.text.toInt())
+                if (errorFound) add(errorLabel).colspan(2).center()
+                !errorFound
+            }
+        ) {
+            actionOnOk(nameField.text.toInt())
+        }
+        addCloseButton()
+        equalizeLastTwoButtonWidths()
+        
+        keyboardFocus = nameField
+    }
+}

core/src/com/unciv/ui/utils/AskTextPopup.kt

@@ -11,6 +11,7 @@ import com.badlogic.gdx.scenes.scene2d.ui.TextField
  * @param defaultText The text that should be in the prompt at the start
  * @param errorText Text that will be shown when an error is detected
  * @param maxLength The maximal amount of characters the user may input
+ * @param validate Function that should return `true` when a valid input is entered, false otherwise
  * @param actionOnOk Lambda that will be executed after pressing 'OK'. 
  * Gets the text the user inputted as a parameter. Should return `true` if ready to close,
  * `false` if `errorText` is to be displayed
@@ -22,7 +23,8 @@ class AskTextPopup(
     defaultText: String = "",
     errorText: String = "Invalid input! Please enter a different string.",
     maxLength: Int = 32,
-    actionOnOk: (input: String) -> Boolean = { true },
+    validate: (input: String) -> Boolean = { true },
+    actionOnOk: (input: String) -> Unit = {},
 ) : Popup(screen) {
     
     val illegalChars = "[]{}\"\\<>"
@@ -41,11 +43,15 @@ class AskTextPopup(
         
         val errorLabel = errorText.toLabel()
         errorLabel.color = Color.RED
-        
-        addOKButton(automaticallyCloseOnPress = false) { 
-            if (nameField.text == "" || !actionOnOk(nameField.text)) 
-                add(errorLabel).colspan(2).center()
-            else close() 
+
+        addOKButton(
+            validate = { 
+                val errorFound = nameField.text == "" || !validate(nameField.text)
+                if (errorFound) add(errorLabel).colspan(2).center()
+                !errorFound
+            }
+        ) {
+            actionOnOk(nameField.text)
         }
         addCloseButton()
         equalizeLastTwoButtonWidths()

core/src/com/unciv/ui/utils/Popup.kt

@@ -145,20 +145,22 @@ open class Popup(val screen: CameraStageBaseScreen): Table(CameraStageBaseScreen
      * Adds a [TextButton] that can close the popup, with [RETURN][KeyCharAndCode.RETURN] already mapped.
      * @param text The button's caption, defaults to "OK".
      * @param additionalKey An additional key that should act like a click.
-     * @param automaticallyCloseOnPress Whether the popup should be closed when pressing this button.
+     * @param validate Function that should return true when the popup can be closed and `action` can be run.
+     * When this function returns false, nothing happens.
      * @param action A lambda to be executed after closing the popup when the button is clicked.
      * @return The new [Cell], NOT marked as end of row.
      */
     fun addOKButton(
         text: String = Constants.OK,
         additionalKey: KeyCharAndCode? = null,
-        automaticallyCloseOnPress: Boolean = true,
-        action: (()->Unit),
+        validate: (() -> Boolean) = { true },
+        action: (() -> Unit),
     ): Cell<TextButton> {
         val okAction = {
-            if (automaticallyCloseOnPress)
+            if (validate()) {
                 close()
-            action() 
+                action()
+            }
         }
         keyPressDispatcher[KeyCharAndCode.RETURN] = okAction
         return addButtonInRow(text, additionalKey, okAction)