Improved options to rename units (#7676)

Co-authored-by: Brian Huckaby <bdhuckaby@gmail.com>

core/src/com/unciv/ui/cityscreen/CityScreenCityPickerTable.kt

@@ -57,6 +57,7 @@ class CityScreenCityPickerTable(private val cityScreen: CityScreen) : Table() {
                 cityScreen,
                 label = "Please enter a new name for your city",
                 defaultText = city.name,
+                validate = { it != "" },
                 actionOnOk = { text ->
                     city.name = text
                     cityScreen.game.replaceCurrentScreen(CityScreen(city))

core/src/com/unciv/ui/overviewscreen/UnitOverviewTable.kt

@@ -13,6 +13,7 @@ import com.unciv.ui.audio.SoundPlayer
 import com.unciv.ui.images.IconTextButton
 import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.pickerscreens.PromotionPickerScreen
+import com.unciv.ui.pickerscreens.UnitRenamePopup
 import com.unciv.ui.utils.ExpanderTab
 import com.unciv.ui.utils.Fonts
 import com.unciv.ui.utils.TabbedPager
@@ -22,6 +23,7 @@ import com.unciv.ui.utils.extensions.brighten
 import com.unciv.ui.utils.extensions.center
 import com.unciv.ui.utils.extensions.darken
 import com.unciv.ui.utils.extensions.onClick
+import com.unciv.ui.utils.extensions.surroundWithCircle
 import com.unciv.ui.utils.extensions.toLabel
 import com.unciv.ui.worldscreen.unit.UnitActions
 import kotlin.math.abs
@@ -131,6 +133,7 @@ class UnitOverviewTab(
     private fun Table.updateUnitHeaderTable(): Table {
         defaults().pad(5f)
         add("Name".toLabel())
+        add()
         add("Action".toLabel())
         add(Fonts.strength.toString().toLabel())
         add(Fonts.rangedStrength.toString().toLabel())
@@ -167,6 +170,15 @@ class UnitOverviewTab(
             add(button).fillX()
 
             // Columns: action, strength, ranged, moves
+            val editIcon = ImageGetter.getImage("OtherIcons/Pencil").apply { this.color = Color.WHITE }.surroundWithCircle(30f, true, Color.valueOf("000c31"))
+            editIcon.onClick {
+                UnitRenamePopup(
+                    screen = overviewScreen,
+                    unit = unit,
+                    actionOnClose = {
+                        overviewScreen.game.replaceCurrentScreen(EmpireOverviewScreen(viewingPlayer, "", "")) })
+            }
+            add(editIcon)
             if (unit.action == null) add() else add(unit.getActionLabel().toLabel())
             if (baseUnit.strength > 0) add(baseUnit.strength.toLabel()) else add()
             if (baseUnit.rangedStrength > 0) add(baseUnit.rangedStrength.toLabel()) else add()

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

@@ -42,11 +42,13 @@ class PromotionPickerScreen(val unit: MapUnit) : PickerScreen(), RecreateOnResiz
         rightSideButton.onClick(UncivSound.Promote) {
             acceptPromotion(selectedPromotion)
         }
+
         val canBePromoted = unit.promotions.canBePromoted()
         val canChangeState = game.worldScreen!!.canChangeState
         val canPromoteNow = canBePromoted && canChangeState
                 && unit.currentMovement > 0 && unit.attacksThisTurn == 0
         rightSideButton.isEnabled = canPromoteNow
+        descriptionLabel.setText(updateDescriptionLabel())
 
         val availablePromotionsGroup = Table()
         availablePromotionsGroup.defaults().pad(5f)
@@ -57,25 +59,20 @@ class PromotionPickerScreen(val unit: MapUnit) : PickerScreen(), RecreateOnResiz
         }
         val unitAvailablePromotions = unit.promotions.getAvailablePromotions()
 
-        if (canPromoteNow && unit.instanceName == null) {
+        //Always allow the user to rename the unit as many times as they like.
-            val renameButton = "Choose name for [${unit.name}]".toTextButton()
+        val renameButton = "Choose name for [${unit.name}]".toTextButton()
-            renameButton.isEnabled = true
+        renameButton.isEnabled = true
-            renameButton.onClick {
+
-                AskTextPopup(
+        renameButton.onClick {
-                    this,
+            UnitRenamePopup(
-                    label = "Choose name for [${unit.baseUnit.name}]",
+                screen = this,
-                    icon = ImageGetter.getUnitIcon(unit.name).surroundWithCircle(80f),
+                unit = unit,
-                    defaultText = unit.name,
+                actionOnClose = {
-                    validate = { it != unit.name},
+                    this.game.replaceCurrentScreen(PromotionPickerScreen(unit)) })
-                    actionOnOk = { userInput ->
-                        unit.instanceName = userInput
-                        this.game.replaceCurrentScreen(PromotionPickerScreen(unit))
-                    }
-                ).open()
-            }
-            availablePromotionsGroup.add(renameButton)
-            availablePromotionsGroup.row()
         }
+        availablePromotionsGroup.add(renameButton)
+        availablePromotionsGroup.row()
+
         for (promotion in promotionsForUnitType) {
             if (promotion.hasUnique(UniqueType.OneTimeUnitHeal) && unit.health == 100) continue
             val isPromotionAvailable = promotion in unitAvailablePromotions
@@ -89,7 +86,7 @@ class PromotionPickerScreen(val unit: MapUnit) : PickerScreen(), RecreateOnResiz
                 rightSideButton.isEnabled = enable
                 rightSideButton.setText(promotion.name.tr())
 
-                descriptionLabel.setText(promotion.getDescription(promotionsForUnitType))
+                descriptionLabel.setText(updateDescriptionLabel(promotion.getDescription(promotionsForUnitType)))
             }
 
             availablePromotionsGroup.add(selectPromotionButton)
@@ -119,6 +116,20 @@ class PromotionPickerScreen(val unit: MapUnit) : PickerScreen(), RecreateOnResiz
         scrollPane.updateVisualScroll()
     }
 
+    private fun updateDescriptionLabel(): String {
+        var newDescriptionText = unit.displayName().tr()
+
+        return newDescriptionText.toString()
+    }
+
+    private fun updateDescriptionLabel(promotionDescription: String): String {
+        var newDescriptionText = unit.displayName().tr()
+
+        newDescriptionText += "\n" + promotionDescription
+
+        return newDescriptionText.toString()
+    }
+
     override fun recreate(): BaseScreen {
         val newScreen = PromotionPickerScreen(unit)
         newScreen.setScrollY(scrollPane.scrollY)

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

@@ -123,6 +123,7 @@ class ReligiousBeliefsPickerScreen (
                     religionName != Constants.noReligionName
                     && ruleset.religions.none { it == religionName }
                     && gameInfo.religions.none { it.value.name == religionName }
+                    && religionName != ""
                 },
                 actionOnOk = { changeDisplayedReligionName(it) }
             ).open()

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

@@ -0,0 +1,41 @@
+package com.unciv.ui.pickerscreens
+
+import com.badlogic.gdx.graphics.Color
+import com.badlogic.gdx.scenes.scene2d.ui.Table
+import com.badlogic.gdx.utils.Align
+import com.unciv.UncivGame
+import com.unciv.logic.map.MapUnit
+import com.unciv.models.TutorialTrigger
+import com.unciv.models.UncivSound
+import com.unciv.models.ruleset.unique.UniqueType
+import com.unciv.models.ruleset.unit.Promotion
+import com.unciv.models.translations.tr
+import com.unciv.ui.images.ImageGetter
+import com.unciv.ui.popup.AskTextPopup
+import com.unciv.ui.utils.BaseScreen
+import com.unciv.ui.utils.KeyCharAndCode
+import com.unciv.ui.utils.RecreateOnResize
+import com.unciv.ui.utils.extensions.isEnabled
+import com.unciv.ui.utils.extensions.onClick
+import com.unciv.ui.utils.extensions.surroundWithCircle
+import com.unciv.ui.utils.extensions.toLabel
+import com.unciv.ui.utils.extensions.toTextButton
+
+class UnitRenamePopup(val screen: BaseScreen, val unit: MapUnit, val actionOnClose: ()->Unit) {
+    init {
+        AskTextPopup(
+            screen,
+            label = "Choose name for [${unit.baseUnit.name}]",
+            icon = ImageGetter.getUnitIcon(unit.name).surroundWithCircle(80f),
+            defaultText = unit.instanceName ?: unit.baseUnit.name.tr(),
+            validate = { it != unit.name },
+            actionOnOk = { userInput ->
+                //If the user inputs an empty string, clear the unit instanceName so the base name is used
+                unit.instanceName = if (userInput == "") null else userInput
+                actionOnClose()
+            }
+        ).open()
+    }
+
+}
+

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

@@ -52,8 +52,11 @@ class AskTextPopup(
         addCloseButton()
         addOKButton(
             validate = {
-                val errorFound = nameField.text == "" || !validate(nameField.text)
+                val errorFound = !validate(nameField.text)
-                if (errorFound) add(errorLabel).colspan(2).center()
+                if (errorFound) {
+                    row()
+                    add(errorLabel).colspan(2).center()
+                }
                 !errorFound
             }
         ) {

core/src/com/unciv/ui/worldscreen/WorldScreen.kt

@@ -74,6 +74,7 @@ import com.unciv.utils.concurrency.withGLContext
 import com.unciv.utils.debug
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.coroutineScope
+import com.badlogic.gdx.scenes.scene2d.ui.TextField
 
 /**
  * Do not create this screen without seriously thinking about the implications: this is the single most memory-intensive class in the application.
@@ -264,17 +265,26 @@ class WorldScreen(
                             Input.Keys.UP, Input.Keys.DOWN, Input.Keys.LEFT, Input.Keys.RIGHT)
 
 
-                    override fun keyDown(event: InputEvent?, keycode: Int): Boolean {
+                    override fun keyDown(event: InputEvent, keycode: Int): Boolean {
-                        if (keycode !in ALLOWED_KEYS) return false
+                        if (event.target !is TextField) {
-                        // Without the following Ctrl-S would leave WASD map scrolling stuck
+                            if (keycode !in ALLOWED_KEYS) return false
-                        // Might be obsolete with keyboard shortcut refactoring
+                            // Without the following Ctrl-S would leave WASD map scrolling stuck
-                        if (Gdx.input.isKeyPressed(Input.Keys.CONTROL_LEFT) || Gdx.input.isKeyPressed(Input.Keys.CONTROL_RIGHT)) return false
+                            // Might be obsolete with keyboard shortcut refactoring
+                            if (Gdx.input.isKeyPressed(Input.Keys.CONTROL_LEFT) || Gdx.input.isKeyPressed(
+                                        Input.Keys.CONTROL_RIGHT
+                                    )
+                            ) return false
 
-                        pressedKeys.add(keycode)
+                            pressedKeys.add(keycode)
-                        if (infiniteAction == null) {
+                            if (infiniteAction == null) {
-                            // create a copy of the action, because removeAction() will destroy this instance
+                                // create a copy of the action, because removeAction() will destroy this instance
-                            infiniteAction = Actions.forever(Actions.delay(0.01f, Actions.run { whileKeyPressedLoop() }))
+                                infiniteAction = Actions.forever(
-                            mapHolder.addAction(infiniteAction)
+                                    Actions.delay(
+                                        0.01f,
+                                        Actions.run { whileKeyPressedLoop() })
+                                )
+                                mapHolder.addAction(infiniteAction)
+                            }
                         }
                         return true
                     }

core/src/com/unciv/ui/worldscreen/unit/UnitTable.kt

@@ -17,6 +17,7 @@ import com.unciv.ui.civilopedia.CivilopediaCategories
 import com.unciv.ui.civilopedia.CivilopediaScreen
 import com.unciv.ui.images.ImageGetter
 import com.unciv.ui.pickerscreens.PromotionPickerScreen
+import com.unciv.ui.pickerscreens.UnitRenamePopup
 import com.unciv.ui.utils.BaseScreen
 import com.unciv.ui.utils.UnitGroup
 import com.unciv.ui.utils.extensions.addSeparator
@@ -83,7 +84,8 @@ class UnitTable(val worldScreen: WorldScreen) : Table(){
                 add(prevIdleUnitButton)
                 unitIconNameGroup.add(unitIconHolder)
                 unitIconNameGroup.add(unitNameLabel).pad(5f)
-                unitIconNameGroup.touchable = Touchable.enabled
+                unitIconHolder.touchable = Touchable.enabled
+                unitNameLabel.touchable = Touchable.enabled
                 add(unitIconNameGroup)
                 add(nextIdleUnitButton)
             }
@@ -127,15 +129,20 @@ class UnitTable(val worldScreen: WorldScreen) : Table(){
             if (selectedUnits.size == 1) { //single selected unit
                 separator.isVisible = true
                 val unit = selectedUnit!!
-                var nameLabelText = unit.displayName().tr()
+                val nameLabelText = buildNameLabelText(unit)
-                if (unit.health < 100) nameLabelText += " (" + unit.health + ")"
                 if (nameLabelText != unitNameLabel.text.toString()) {
                     unitNameLabel.setText(nameLabelText)
                     selectedUnitHasChanged = true // We need to reload the health bar of the unit in the icon - happens e.g. when picking the Heal Instantly promotion
                 }
-                unitIconNameGroup.clearListeners()
+
-                unitIconNameGroup.onClick {
+                unitNameLabel.clearListeners()
-                    worldScreen.game.pushScreen(CivilopediaScreen(worldScreen.gameInfo.ruleSet, CivilopediaCategories.Unit, unit.name))
+                unitNameLabel.onClick {
+                    UnitRenamePopup(
+                        screen = worldScreen,
+                        unit = unit,
+                        actionOnClose = {
+                            unitNameLabel.setText(buildNameLabelText(unit))
+                            selectedUnitHasChanged = true })
                 }
 
                 unitDescriptionTable.clear()
@@ -224,11 +231,15 @@ class UnitTable(val worldScreen: WorldScreen) : Table(){
                 for (promotion in selectedUnit!!.promotions.getPromotions(true))
                     promotionsTable.add(ImageGetter.getPromotionIcon(promotion.name))
 
-                // Since Clear also clears the listeners, we need to re-add it every time
+                // Since Clear also clears the listeners, we need to re-add them every time
                 promotionsTable.onClick {
                     if (selectedUnit == null || selectedUnit!!.promotions.promotions.isEmpty()) return@onClick
                     UncivGame.Current.pushScreen(PromotionPickerScreen(selectedUnit!!))
                 }
+
+                unitIconHolder.onClick {
+                    worldScreen.game.pushScreen(CivilopediaScreen(worldScreen.gameInfo.ruleSet, CivilopediaCategories.Unit, selectedUnit!!.name))
+                }
             } else { // multiple selected units
                 for (unit in selectedUnits)
                     unitIconHolder.add(UnitGroup(unit, 30f)).pad(5f)
@@ -239,6 +250,13 @@ class UnitTable(val worldScreen: WorldScreen) : Table(){
         selectedUnitHasChanged=false
     }
 
+    private fun buildNameLabelText(unit: MapUnit) : String {
+        var nameLabelText = unit.displayName().tr()
+        if (unit.health < 100) nameLabelText += " (" + unit.health + ")"
+
+        return nameLabelText
+    }
+
     fun citySelected(cityInfo: CityInfo) : Boolean {
         selectUnit()
         if (cityInfo == selectedCity) return false