From 5b8593f3ed68236fd2f61e63a8ca99fd967a2ab6 Mon Sep 17 00:00:00 2001 From: SomeTroglodyte <63000004+SomeTroglodyte@users.noreply.github.com> Date: Thu, 23 Mar 2023 20:21:31 +0100 Subject: [PATCH] Linting and a little hardening of the UI-Skins code (#9007) --- core/src/com/unciv/models/skins/SkinCache.kt | 34 ++++++++------- core/src/com/unciv/models/skins/SkinConfig.kt | 43 ++++++++----------- .../src/com/unciv/models/skins/SkinStrings.kt | 25 +++++++---- 3 files changed, 53 insertions(+), 49 deletions(-) diff --git a/core/src/com/unciv/models/skins/SkinCache.kt b/core/src/com/unciv/models/skins/SkinCache.kt index d5ed2e32a1..3b52646c5d 100644 --- a/core/src/com/unciv/models/skins/SkinCache.kt +++ b/core/src/com/unciv/models/skins/SkinCache.kt @@ -22,38 +22,40 @@ object SkinCache : HashMap() { fun assembleSkinConfigs(ruleSetMods: Set) { // Needs to be a list and not a set, so subsequent mods override the previous ones // Otherwise you rely on hash randomness to determine override order... not good - val mods = mutableListOf("") + val mods = mutableListOf("") // Not an emptyList - placeholder for built-in skin if (UncivGame.isCurrentInitialized()) { mods.addAll(UncivGame.Current.settings.visualMods) } mods.addAll(ruleSetMods) clear() for (mod in mods.distinct()) { - for (entry in allConfigs.entries.filter { it.key.mod == mod } ) { // Built-in skins all have empty strings as their `.mod`, so loop through all of them. - val skin = entry.key.skin - if (skin in this) this[skin]!!.updateConfig(entry.value) - else this[skin] = entry.value.clone() + for ((key, config) in allConfigs.filter { it.key.mod == mod } ) { // Built-in skins all have empty strings as their `.mod`, so loop through all of them. + val skin = key.skin + if (skin in this) this[skin]!!.updateConfig(config) + else this[skin] = config.clone() } } } - fun loadSkinConfigs(consoleMode: Boolean = false){ + fun loadSkinConfigs(consoleMode: Boolean = false) { allConfigs.clear() var skinName = "" //load internal Skins val fileHandles: Sequence = if (consoleMode) FileHandle("jsons/Skins").list().asSequence() - else ImageGetter.getAvailableSkins().map { Gdx.files.internal("jsons/Skins/$it.json")}.filter { it.exists() } + else ImageGetter.getAvailableSkins() + .map { Gdx.files.internal("jsons/Skins/$it.json") } + .filter { it.exists() } - for (configFile in fileHandles){ + for (configFile in fileHandles) { skinName = configFile.nameWithoutExtension().removeSuffix("Config") try { val key = SkinAndMod(skinName, "") assert(key !in allConfigs) allConfigs[key] = json().fromJsonFile(SkinConfig::class.java, configFile) debug("SkinConfig loaded successfully: %s", configFile.name()) - } catch (ex: Exception){ + } catch (ex: Exception) { debug("Exception loading SkinConfig '%s':", configFile.path()) debug(" %s", ex.localizedMessage) debug(" (Source file %s line %s)", ex.stackTrace[0].fileName, ex.stackTrace[0].lineNumber) @@ -70,21 +72,21 @@ object SkinCache : HashMap() { if (modName.startsWith('.')) continue if (!modFolder.isDirectory) continue - try { - for (configFile in modFolder.child("jsons/Skins").list()){ + for (configFile in modFolder.child("jsons/Skins").list()) { + try { skinName = configFile.nameWithoutExtension().removeSuffix("Config") val key = SkinAndMod(skinName, modName) assert(key !in allConfigs) allConfigs[key] = json().fromJsonFile(SkinConfig::class.java, configFile) debug("Skin loaded successfully: %s", configFile.path()) + } catch (ex: Exception) { + debug("Exception loading Skin '%s/jsons/Skins/%s':", modFolder.name(), skinName) + debug(" %s", ex.localizedMessage) + debug(" (Source file %s line %s)", ex.stackTrace[0].fileName, ex.stackTrace[0].lineNumber) } - } catch (ex: Exception){ - debug("Exception loading Skin '%s/jsons/Skins/%s':", modFolder.name(), skinName) - debug(" %s", ex.localizedMessage) - debug(" (Source file %s line %s)", ex.stackTrace[0].fileName, ex.stackTrace[0].lineNumber) } } - assembleSkinConfigs(hashSetOf()) // no game is loaded, this is just the initial game setup + assembleSkinConfigs(emptySet()) // no game is loaded, this is just the initial game setup } } diff --git a/core/src/com/unciv/models/skins/SkinConfig.kt b/core/src/com/unciv/models/skins/SkinConfig.kt index aec7881dc3..9411d106eb 100644 --- a/core/src/com/unciv/models/skins/SkinConfig.kt +++ b/core/src/com/unciv/models/skins/SkinConfig.kt @@ -2,38 +2,31 @@ package com.unciv.models.skins import com.badlogic.gdx.graphics.Color -class SkinElement { - var image: String? = null - var tint: Color? = null - var alpha: Float? = null - - fun clone(): SkinElement { - val toReturn = SkinElement() - toReturn.image = image - toReturn.tint = tint?.cpy() - toReturn.alpha = alpha - return toReturn - } -} - -class SkinConfig { +class SkinConfig(initialCapacity: Int) { var baseColor: Color = Color(0x004085bf) var clearColor: Color = Color(0x000033ff) - var skinVariants: HashMap = HashMap() + var skinVariants: HashMap = HashMap(initialCapacity) - fun clone(): SkinConfig { - val toReturn = SkinConfig() - toReturn.baseColor = baseColor.cpy() - toReturn.clearColor = clearColor.cpy() - toReturn.skinVariants.putAll(skinVariants.map { Pair(it.key, it.value.clone()) }) - return toReturn + constructor() : this(16) // = HashMap.DEFAULT_INITIAL_CAPACITY which is private + + /** Skin element, read from UI SKin json + * + * **Immutable** */ + class SkinElement { + val image: String? = null + val tint: Color? = null + val alpha: Float? = null } + fun clone() = SkinConfig(skinVariants.size).also { it.updateConfig(this) } + + /** 'Merges' [other] into **`this`** + * + * [baseColor] and [clearColor] are overwritten with clones from [other]. + * [skinVariants] with the same key are copied and overwritten, new [skinVariants] are added. */ fun updateConfig(other: SkinConfig) { baseColor = other.baseColor.cpy() clearColor = other.clearColor.cpy() - for ((variantName, element) in other.skinVariants){ - skinVariants[variantName] = element.clone() - } + skinVariants.putAll(other.skinVariants) } } diff --git a/core/src/com/unciv/models/skins/SkinStrings.kt b/core/src/com/unciv/models/skins/SkinStrings.kt index 0331781e58..64edaaf277 100644 --- a/core/src/com/unciv/models/skins/SkinStrings.kt +++ b/core/src/com/unciv/models/skins/SkinStrings.kt @@ -40,18 +40,27 @@ class SkinStrings(skin: String = UncivGame.Current.settings.skin) { * @param default The path to the background which should be used if path is not available. * Should be one of the predefined ones inside SkinStrings or null to get a * solid background. + * + * @param tintColor Default tint color if the UI Skin doesn't specify one. If both not specified, + * the returned background will not be tinted. If the UI Skin specifies a + * separate alpha value, it will be applied to a clone of either color. */ fun getUiBackground(path: String, default: String? = null, tintColor: Color? = null): NinePatchDrawable { val locationByName = skinLocation + path - val locationByConfigVariant = skinLocation + skinConfig.skinVariants[path]?.image - val tint = (skinConfig.skinVariants[path]?.tint ?: tintColor)?.apply { - a = skinConfig.skinVariants[path]?.alpha ?: a + val skinVariant = skinConfig.skinVariants[path] + val locationByConfigVariant = if (skinVariant?.image != null) skinLocation + skinVariant.image else null + val tint = (skinVariant?.tint ?: tintColor)?.run { + if (skinVariant?.alpha == null) this + else cpy().apply { a = skinVariant.alpha } } - - return when { - ImageGetter.ninePatchImageExists(locationByConfigVariant) -> ImageGetter.getNinePatch(locationByConfigVariant, tint) - ImageGetter.ninePatchImageExists(locationByName) -> ImageGetter.getNinePatch(locationByName, tint) - else -> ImageGetter.getNinePatch(default, tint) + val location = when { + locationByConfigVariant != null && ImageGetter.ninePatchImageExists(locationByConfigVariant) -> + locationByConfigVariant + ImageGetter.ninePatchImageExists(locationByName) -> + locationByName + else -> + default } + return ImageGetter.getNinePatch(location, tint) } }