Unified separators, CheckBox helper (#4349)

* Unified separators, CheckBox helper

* Unified separators, CheckBox helper - patch1

* Unified separators, CheckBox helper - patch2

* Unified separators, CheckBox helper - patch3

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

@@ -128,9 +128,7 @@ class ModManagementScreen: PickerScreen(disableScroll = true) {
         topTable.row()
 
         // horizontal separator looking like the SplitPane handle
-        val separator = Table(skin)
+        topTable.addSeparator(Color.CLEAR, 5, 3f)
-        separator.background = skin.get("default-vertical", SplitPane.SplitPaneStyle::class.java).handle
-        topTable.add(separator).minHeight(3f).fillX().colspan(5).row()
 
         // main row containing the three 'blocks' installed, online and information
         topTable.add()      // skip empty first column
@@ -420,11 +418,9 @@ class ModManagementScreen: PickerScreen(disableScroll = true) {
         val isVisual = visualMods.contains(mod.name)
         modStateImages[mod.name]?.isVisual = isVisual
 
-        val visualCheckBox = CheckBox("Permanent audiovisual mod", skin)
+        val visualCheckBox = "Permanent audiovisual mod".toCheckBox(isVisual) {
-        visualCheckBox.isChecked = isVisual
+            checked ->
-        modActionTable.add(visualCheckBox)
+            if (checked)
-        visualCheckBox.onChange {
-            if (visualCheckBox.isChecked)
                 visualMods.add(mod.name)
             else
                 visualMods.remove(mod.name)
@@ -432,7 +428,7 @@ class ModManagementScreen: PickerScreen(disableScroll = true) {
             ImageGetter.setNewRuleset(ImageGetter.ruleset)
             refreshModActions(mod)
         }
-        modActionTable.row()
+        modActionTable.add(visualCheckBox).row()
     }
 
     /** Rebuild the left-hand column containing all installed mods */

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

@@ -82,7 +82,7 @@ fun Actor.surroundWithCircle(size: Float, resizeActor: Boolean = true, color: Co
     return IconCircleGroup(size, this, resizeActor, color)
 }
 
-fun Actor.addBorder(size:Float, color: Color, expandCell:Boolean=false): Table {
+fun Actor.addBorder(size:Float, color: Color, expandCell:Boolean = false): Table {
     val table = Table()
     table.pad(size)
     table.background = ImageGetter.getBackground(color)
@@ -93,19 +93,35 @@ fun Actor.addBorder(size:Float, color: Color, expandCell:Boolean=false): Table {
     return table
 }
 
-fun Table.addSeparator(): Cell<Image> {
+/** get background Image for a new separator */
-    row()
+private fun getSeparatorImage(color: Color) = ImageGetter.getDot(
-    val image = ImageGetter.getWhiteDot()
+    if (color.a != 0f) color else CameraStageBaseScreen.skin.get("color", Color::class.java) //0x334d80
-    val cell = add(image).colspan(columns).height(2f).fill()
+)
+
+/**
+ * Create a horizontal separator as an empty Container with a colored background.
+ * @param colSpan Optionally override [colspan][Cell.colspan] which defaults to the current column count.
+ */
+fun Table.addSeparator(color: Color = Color.WHITE, colSpan: Int = 0, height: Float = 2f): Cell<Image> {
+    if (!cells.isEmpty && !cells.last().isEndRow) row()
+    val separator = getSeparatorImage(color)
+    val cell = add(separator)
+        .colspan(if (colSpan == 0) columns else colSpan)
+        .minHeight(height).fillX()
     row()
     return cell
 }
 
-fun Table.addSeparatorVertical(): Cell<Image> {
+/**
-    val image = ImageGetter.getWhiteDot()
+ * Create a vertical separator as an empty Container with a colored background.
-    return add(image).width(2f).fillY()
+ * 
+ * Note: Unlike the horizontal [addSeparator] this cannot automatically span several rows. Repeat the separator if needed.
+ */
+fun Table.addSeparatorVertical(color: Color = Color.WHITE, width: Float = 2f): Cell<Image> {
+    return add(getSeparatorImage(color)).width(width).fillY()
 }
 
+/** Alternative to [Table].[add][Table] that returns the Table instead of the new Cell to allow a different way of chaining */
 fun <T : Actor> Table.addCell(actor: T): Table {
     add(actor)
     return this
@@ -177,13 +193,34 @@ 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 }
+/**
+ * Translate a [String] and make a [CheckBox] widget from it.
+ * @param changeAction A callback to call on change, with a boolean lambda parameter containing the current [isChecked][CheckBox.isChecked].
+ */
+fun String.toCheckBox(startsOutChecked: Boolean = false, changeAction: ((Boolean)->Unit)? = null)
+    = CheckBox(this.tr(), CameraStageBaseScreen.skin).apply { 
+        isChecked = startsOutChecked
+        if (changeAction != null) onChange { 
+            changeAction(isChecked)
+        }
+        // Add a little distance between the icon and the text. 0 looks glued together,
+        // 5 is about half an uppercase letter, and 1 about the width of the vertical line in "P".  
+        imageCell.padRight(1f)
+    }
 
+/** Sets the [font color][Label.LabelStyle.fontColor] on a [Label] and returns it to allow chaining */
+fun Label.setFontColor(color: Color): Label {
+    style = Label.LabelStyle(style).apply { fontColor=color }
+    return this
+}
+
+/** Sets the font size on a [Label] and returns it to allow chaining */
 fun Label.setFontSize(size:Int): Label {
     style = Label.LabelStyle(style)
     style.font = Fonts.font
-    style = style // because we need it to call the SetStyle function. Yuk, I know.
+    @Suppress("UsePropertyAccessSyntax") setStyle(style)
-    return this.apply { setFontScale(size/ Fonts.ORIGINAL_FONT_SIZE) } // for chaining
+    setFontScale(size/ Fonts.ORIGINAL_FONT_SIZE)
+    return this
 }
 
 /** Get one random element of a given List.

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

@@ -21,7 +21,6 @@ import com.unciv.ui.utils.AutoScrollPane as ScrollPane
     different Notification wording for peace treaties?
     Let Notification jump to Diplomacy.trade with empty offers (accepted)
         or a clone of the rejected offer (denied) ...?
-    Unify separator implementations (here, Table.addSeparator, ModManagementScreen)
 */
 
 /**
@@ -72,9 +71,7 @@ class TradePopup(worldScreen: WorldScreen): Popup(worldScreen){
         val scrollHeight = min(tradeOffersTable.height, worldScreen.stage.height/2)
         add(ScrollPane(tradeOffersTable)).height(scrollHeight).row()
 
-        val separator = Table()
+        addSeparator(Color.DARK_GRAY, height = 1f)
-        separator.background = ImageGetter.getBackground(Color.DARK_GRAY)
-        add(separator).minHeight(1f).fillX().row()
 
         addGoodSizedLabel(nation.tradeRequest).pad(15f).row()
 

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

@@ -3,8 +3,8 @@ package com.unciv.ui.worldscreen.unit
 import com.badlogic.gdx.Gdx
 import com.badlogic.gdx.Input
 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.Image
 import com.badlogic.gdx.scenes.scene2d.ui.Table
 import com.badlogic.gdx.scenes.scene2d.ui.VerticalGroup
 import com.unciv.UncivGame
@@ -50,7 +50,7 @@ class UnitTable(val worldScreen: WorldScreen) : Table(){
     // This is so that not on every update(), we will update the unit table.
     // Most of the time it's the same unit with the same stats so why waste precious time?
     var selectedUnitHasChanged = false
-    val separator: Image
+    val separator: Actor
 
     init {
         pad(5f)
@@ -80,7 +80,7 @@ class UnitTable(val worldScreen: WorldScreen) : Table(){
             }
             add(moveBetweenUnitsTable).colspan(2).fill().row()
 
-            separator= addSeparator().actor!!
+            separator = addSeparator().actor!!
             add(promotionsTable).colspan(2).row()
             add(unitDescriptionTable)
             touchable = Touchable.enabled
@@ -169,7 +169,7 @@ class UnitTable(val worldScreen: WorldScreen) : Table(){
         }
 
         else if (selectedCity != null) {
-            separator.isVisible=true
+            separator.isVisible = true
             val city = selectedCity!!
             var nameLabelText = city.name.tr()
             if(city.health<city.getMaxHealth()) nameLabelText+=" ("+city.health+")"