Commenting extension functions and resolve to-do (#3932)

* Commenting extension functions and resolve to-do

* Better readability in PromotionPickerScreen enable

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

@@ -1,7 +1,6 @@
 package com.unciv.ui.pickerscreens
 
 import com.badlogic.gdx.graphics.Color
-import com.badlogic.gdx.scenes.scene2d.Touchable
 import com.badlogic.gdx.scenes.scene2d.ui.Button
 import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.badlogic.gdx.utils.Align
@@ -42,7 +41,7 @@ class PromotionPickerScreen(val unit: MapUnit) : PickerScreen() {
         val canChangeState = game.worldScreen.canChangeState
         val canPromoteNow = canBePromoted && canChangeState
         if (!canPromoteNow)
-            rightSideButton.disable()
+            rightSideButton.isEnabled = false
 
         val availablePromotionsGroup = Table()
         availablePromotionsGroup.defaults().pad(5f)
@@ -55,7 +54,7 @@ class PromotionPickerScreen(val unit: MapUnit) : PickerScreen() {
 
         if(canPromoteNow && unit.instanceName == null) {
             val renameButton = "Choose name for [${unit.name}]".toTextButton()
-            renameButton.touchable = Touchable.enabled
+            renameButton.isEnabled = true
             renameButton.onClick {
                 RenameUnitPopup(unit, this).open()
             }
@@ -70,15 +69,11 @@ class PromotionPickerScreen(val unit: MapUnit) : PickerScreen() {
             val selectPromotionButton = Button(skin)
             selectPromotionButton.add(ImageGetter.getPromotionIcon(promotion.name)).size(30f).pad(10f)
             selectPromotionButton.add(promotion.name.toLabel()).pad(10f).padRight(20f)
-            selectPromotionButton.touchable = Touchable.enabled
+            selectPromotionButton.isEnabled = true
             selectPromotionButton.onClick {
-                if(canBePromoted && isPromotionAvailable && !unitHasPromotion && canChangeState) {
+                val enable = canBePromoted && isPromotionAvailable && !unitHasPromotion && canChangeState
-                    selectedPromotion = promotion
+                selectedPromotion = if (enable) promotion else null
-                    rightSideButton.enable()
+                rightSideButton.isEnabled = enable
-                } else {
-                    selectedPromotion = null
-                    rightSideButton.disable()
-                }
                 rightSideButton.setText(promotion.name.tr())
 
                 descriptionLabel.setText(promotion.getDescription(promotionsForUnitType))
@@ -104,4 +99,4 @@ class PromotionPickerScreen(val unit: MapUnit) : PickerScreen() {
 
         displayTutorial(Tutorial.Experience)
     }
-}
+}

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

@@ -59,9 +59,7 @@ class TradeTable(val otherCivilization: CivilizationInfo, stage: DiplomacyScreen
     private fun onChange(){
         offerColumnsTable.update()
         retractOffer()
-        if(tradeLogic.currentTrade.theirOffers.size==0 && tradeLogic.currentTrade.ourOffers.size==0)
+        offerButton.isEnabled = !(tradeLogic.currentTrade.theirOffers.size==0 && tradeLogic.currentTrade.ourOffers.size==0)
-            offerButton.disable()
-        else offerButton.enable()
     }
 
-}
+}

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

@@ -13,23 +13,33 @@ import com.unciv.models.translations.tr
 import kotlin.concurrent.thread
 import kotlin.random.Random
 
+/**
+ * Collection of extension functions mostly for libGdx widgets
+ */
 
+/** Disable a [Button] by setting its [touchable][Button.touchable] and [color][Button.color] properties. */
 fun Button.disable(){
     touchable= Touchable.disabled
     color= Color.GRAY
 }
+/** Enable a [Button] by setting its [touchable][Button.touchable] and [color][Button.color] properties. */
 fun Button.enable() {
     color = Color.WHITE
     touchable = Touchable.enabled
 }
+/** Enable or disable a [Button] by setting its [touchable][Button.touchable] and [color][Button.color] properties,
+ *  or returns the corresponding state. *
+ *
+ *  Do not confuse with Gdx' builtin [isDisabled][Button.isDisabled] property,
+ *  which is more appropriate to toggle On/Off buttons, while this one is good for 'click-to-do-something' buttons.
+ */
 var Button.isEnabled: Boolean
     //Todo: Use in PromotionPickerScreen, TradeTable, WorldScreen.updateNextTurnButton
     get() = touchable == Touchable.enabled
     set(value) = if (value) enable() else disable()
 
-fun colorFromRGB(r: Int, g: Int, b: Int) = Color(r/255f, g/255f, b/255f, 1f)
+fun colorFromRGB(r: Int, g: Int, b: Int) = Color(r / 255f, g / 255f, b / 255f, 1f)
-fun colorFromRGB(rgb:List<Int>) = colorFromRGB(rgb[0],rgb[1],rgb[2])
+fun colorFromRGB(rgb:List<Int>) = colorFromRGB(rgb[0], rgb[1], rgb[2])
-
 fun Actor.centerX(parent: Actor){ x = parent.width/2 - width/2 }
 fun Actor.centerY(parent: Actor){ y = parent.height/2- height/2}
 fun Actor.center(parent: Actor){ centerX(parent); centerY(parent)}
@@ -38,12 +48,11 @@ fun Actor.centerX(parent: Stage){ x = parent.width/2 - width/2 }
 fun Actor.centerY(parent: Stage){ y = parent.height/2- height/2}
 fun Actor.center(parent: Stage){ centerX(parent); centerY(parent)}
 
-
 /** same as [onClick], but sends the [InputEvent] and coordinates along */
 fun Actor.onClickEvent(sound: UncivSound = UncivSound.Click, function: (event: InputEvent?, x: Float, y: Float) -> Unit) {
     this.addListener(object : ClickListener() {
         override fun clicked(event: InputEvent?, x: Float, y: Float) {
-            thread(name="Sound") { Sounds.play(sound) }
+            thread(name = "Sound") { Sounds.play(sound) }
             function(event, x, y)
         }
     })
@@ -69,7 +78,7 @@ fun Actor.onChange(function: () -> Unit): Actor {
 }
 
 fun Actor.surroundWithCircle(size: Float, resizeActor: Boolean = true, color: Color = Color.WHITE): IconCircleGroup {
-    return IconCircleGroup(size,this,resizeActor, color)
+    return IconCircleGroup(size, this, resizeActor, color)
 }
 
 fun Actor.addBorder(size:Float, color: Color, expandCell:Boolean=false): Table {
@@ -102,7 +111,8 @@ fun <T : Actor> Table.addCell(actor: T): Table {
     return this
 }
 
-/**
+/** Gets a clone of an [ArrayList] with an additional item
+ *
  * Solves concurrent modification problems - everyone who had a reference to the previous arrayList can keep using it because it hasn't changed
  */
 fun <T> ArrayList<T>.withItem(item:T): ArrayList<T> {
@@ -111,8 +121,9 @@ fun <T> ArrayList<T>.withItem(item:T): ArrayList<T> {
     return newArrayList
 }
 
-/**
+/** Gets a clone of a [HashSet] with an additional item
- * Solves concurrent modification problems - everyone who had a reference to the previous arrayList can keep using it because it hasn't changed
+ *
+ * Solves concurrent modification problems - everyone who had a reference to the previous hashSet can keep using it because it hasn't changed
  */
 fun <T> HashSet<T>.withItem(item:T): HashSet<T> {
     val newHashSet = HashSet(this)
@@ -120,7 +131,8 @@ fun <T> HashSet<T>.withItem(item:T): HashSet<T> {
     return newHashSet
 }
 
-/**
+/** Gets a clone of an [ArrayList] without a given item
+ *
  * Solves concurrent modification problems - everyone who had a reference to the previous arrayList can keep using it because it hasn't changed
  */
 fun <T> ArrayList<T>.withoutItem(item:T): ArrayList<T> {
@@ -129,9 +141,9 @@ fun <T> ArrayList<T>.withoutItem(item:T): ArrayList<T> {
     return newArrayList
 }
 
-
+/** Gets a clone of a [HashSet] without a given item
-/**
+ *
- * Solves concurrent modification problems - everyone who had a reference to the previous arrayList can keep using it because it hasn't changed
+ * Solves concurrent modification problems - everyone who had a reference to the previous hashSet can keep using it because it hasn't changed
  */
 fun <T> HashSet<T>.withoutItem(item:T): HashSet<T> {
     val newHashSet = HashSet(this)
@@ -139,17 +151,18 @@ fun <T> HashSet<T>.withoutItem(item:T): HashSet<T> {
     return newHashSet
 }
 
+/** Translate a [String] and make a [TextButton] widget from it */
 fun String.toTextButton() = TextButton(this.tr(), CameraStageBaseScreen.skin)
 
-/** also translates */
+/** Translate a [String] and make a [Label] widget from it */
-fun String.toLabel() = Label(this.tr(),CameraStageBaseScreen.skin)
+fun String.toLabel() = Label(this.tr(), CameraStageBaseScreen.skin)
+/** Make a [Label] widget containing this [Int] as text */
 fun Int.toLabel() = this.toString().toLabel()
 
-
+/** Translate a [String] and make a [Label] widget from it with a specified font color and size */
-
-// We don't want to use setFontSize and setFontColor because they set the font,
-//  which means we need to rebuild the font cache which means more memory allocation.
 fun String.toLabel(fontColor: Color = Color.WHITE, fontSize:Int=18): Label {
+    // We don't want to use setFontSize and setFontColor because they set the font,
+    //  which means we need to rebuild the font cache which means more memory allocation.
     var labelStyle = CameraStageBaseScreen.skin.get(Label.LabelStyle::class.java)
     if(fontColor!= Color.WHITE || fontSize!=18) { // if we want the default we don't need to create another style
         labelStyle = Label.LabelStyle(labelStyle) // clone this to another
@@ -159,7 +172,6 @@ fun String.toLabel(fontColor: Color = Color.WHITE, fontSize:Int=18): Label {
     return Label(this.tr(), labelStyle).apply { setFontScale(fontSize/Fonts.ORIGINAL_FONT_SIZE) }
 }
 
-
 fun Label.setFontColor(color: Color): Label { style= Label.LabelStyle(style).apply { fontColor=color }; return this }
 
 fun Label.setFontSize(size:Int): Label {
@@ -169,7 +181,10 @@ fun Label.setFontSize(size:Int): Label {
     return this.apply { setFontScale(size/ Fonts.ORIGINAL_FONT_SIZE) } // for chaining
 }
 
-
+/** Get one random element of a given List.
+ *
+ * The probability for each element is proportional to the value of its corresponding element in the [weights] List.
+ */
 fun <T> List<T>.randomWeighted(weights: List<Float>, random: Random = Random): T {
     if (this.isEmpty()) throw NoSuchElementException("Empty list.")
     if (this.size != weights.size) throw UnsupportedOperationException("Weights size does not match this list size.")