From a5bba8dfb010cacbda346a74d92b3af0bf77e230 Mon Sep 17 00:00:00 2001 From: will-ca Date: Tue, 4 Jan 2022 12:02:32 -0800 Subject: [PATCH] Refactor pixel unit image resolving. (Includes `ImageAttempter`) (#5897) * Add ImageAttempter for finding available from candidate images. * Remove zero-arg ImageAttempter constructor. * Refactor pixel unit image resolving. --- core/src/com/unciv/ui/tilegroups/TileGroup.kt | 58 ++++++-------- core/src/com/unciv/ui/utils/ImageAttempter.kt | 75 +++++++++++++++++++ core/src/com/unciv/ui/utils/ImageGetter.kt | 8 +- 3 files changed, 103 insertions(+), 38 deletions(-) create mode 100644 core/src/com/unciv/ui/utils/ImageAttempter.kt diff --git a/core/src/com/unciv/ui/tilegroups/TileGroup.kt b/core/src/com/unciv/ui/tilegroups/TileGroup.kt index ba1f6e1116..1e86287ad8 100644 --- a/core/src/com/unciv/ui/tilegroups/TileGroup.kt +++ b/core/src/com/unciv/ui/tilegroups/TileGroup.kt @@ -11,6 +11,7 @@ import com.badlogic.gdx.utils.Align import com.unciv.UncivGame import com.unciv.logic.HexMath import com.unciv.logic.civilization.CivilizationInfo +import com.unciv.logic.map.MapUnit import com.unciv.logic.map.RoadStatus import com.unciv.logic.map.TileInfo import com.unciv.models.* @@ -651,33 +652,25 @@ open class TileGroup(var tileInfo: TileInfo, val tileSetStrings:TileSetStrings, val militaryUnit = tileInfo.militaryUnit if (militaryUnit != null && showMilitaryUnit) { - val unitType = militaryUnit.type fun TileSetStrings.getThisUnit(): String? { val specificUnitIconLocation = this.unitsLocation + militaryUnit.name - return when { - !UncivGame.Current.settings.showPixelUnits -> "" - militaryUnit.civInfo.nation.style=="" && - ImageGetter.imageExists(specificUnitIconLocation) -> specificUnitIconLocation - ImageGetter.imageExists(specificUnitIconLocation + "-" + militaryUnit.civInfo.nation.style) -> - specificUnitIconLocation + "-" + militaryUnit.civInfo.nation.style - ImageGetter.imageExists(specificUnitIconLocation) -> specificUnitIconLocation - militaryUnit.baseUnit.replaces != null && - ImageGetter.imageExists(this.unitsLocation + militaryUnit.baseUnit.replaces) -> - this.unitsLocation + militaryUnit.baseUnit.replaces - - militaryUnit.civInfo.gameInfo.ruleSet.units.values.any { - it.unitType == unitType.name && ImageGetter.imageExists(this.unitsLocation + it.name) - } -> - { - val unitWithSprite = militaryUnit.civInfo.gameInfo.ruleSet.units.values.first { - it.unitType == unitType.name && ImageGetter.imageExists(this.unitsLocation + it.name) - }.name - this.unitsLocation + unitWithSprite - } - unitType.isLandUnit() && ImageGetter.imageExists(this.landUnit) -> this.landUnit - unitType.isWaterUnit() && ImageGetter.imageExists(this.waterUnit) -> this.waterUnit - else -> null - } + return ImageAttempter(militaryUnit) + .forceImage { if (!UncivGame.Current.settings.showPixelUnits) "" else null } // For now I am just converting existing logic, but this should be made into a short-circuit at the very start. + .tryImage { if (civInfo.nation.style.isEmpty()) specificUnitIconLocation else null } + .tryImage { "$specificUnitIconLocation-${civInfo.nation.style}" } + .tryImage { specificUnitIconLocation } + .tryImage { if (baseUnit.replaces != null) "$unitsLocation${baseUnit.replaces}" else null } + .tryImages( + militaryUnit.civInfo.gameInfo.ruleSet.units.values.asSequence().map { + fun MapUnit.() = if (it.unitType == militaryUnit.type.name) + "$unitsLocation${it.name}" + else + null + } // .tryImage/.tryImages takes functions as parameters, for lazy eval. Include the check as part of the .tryImage's lazy candidate parameter, and *not* as part of the .map's transform parameter, so even the name check will be skipped by ImageAttempter if an image has already been found. + ) + .tryImage { if (type.isLandUnit()) landUnit else null } // FIXME: Based on FantasyHex's structure this also needs to be in .unitsLocation. But again I am just converting existing logic right now, will do later after this refactor has had a chance to be validated. + .tryImage { if (type.isWaterUnit()) waterUnit else null } + .getPathOrNull() } newImageLocation = tileSetStrings.getThisUnit() ?: tileSetStrings.fallback?.getThisUnit() ?: "" } @@ -705,15 +698,12 @@ open class TileGroup(var tileInfo: TileInfo, val tileSetStrings:TileSetStrings, if (civilianUnit != null && tileIsViewable) { fun TileSetStrings.getThisUnit(): String? { val specificUnitIconLocation = this.unitsLocation + civilianUnit.name - return when { - !UncivGame.Current.settings.showPixelUnits -> "" - civilianUnit.civInfo.nation.style=="" && - ImageGetter.imageExists(specificUnitIconLocation) -> specificUnitIconLocation - ImageGetter.imageExists(specificUnitIconLocation + "-" + civilianUnit.civInfo.nation.style) -> - specificUnitIconLocation + "-" + civilianUnit.civInfo.nation.style - ImageGetter.imageExists(specificUnitIconLocation) -> specificUnitIconLocation - else -> null - } + return ImageAttempter(civilianUnit) + .forceImage { if (!UncivGame.Current.settings.showPixelUnits) "" else null } // For now I am just converting existing logic, but this should be made into a short-circuit at the very start. + .tryImage { if (civInfo.nation.style.isEmpty()) specificUnitIconLocation else null } + .tryImage { "$specificUnitIconLocation-${civInfo.nation.style}" } + .tryImage { specificUnitIconLocation } // This seems redundant with the one above… But right now I'm just converting the existing code. Could remove if you can confirm they're redundant. + .getPathOrNull() } newImageLocation = tileSetStrings.getThisUnit() ?: tileSetStrings.fallback?.getThisUnit() ?: "" } diff --git a/core/src/com/unciv/ui/utils/ImageAttempter.kt b/core/src/com/unciv/ui/utils/ImageAttempter.kt new file mode 100644 index 0000000000..99dcead503 --- /dev/null +++ b/core/src/com/unciv/ui/utils/ImageAttempter.kt @@ -0,0 +1,75 @@ +package com.unciv.ui.utils + +/** + * Metaprogrammy class for short-circuitingly finding the first existing of multiple image options according to [ImageGetter.imageExists]. + * + * Has a [tryImage] method that can be chain-called with functions which return candidate image paths. The first function to return a valid image path stops subsequently chained calls from having any effect, and its result is saved to be retrieved by the [getPath] and [getImage] methods at the end of the candidate chain. Has a [forceImage] method that can be used in place of [tryImage] to force a candidate image path to be treated as valid. + * + * Binds candidate functions to a [scope] instance provided to primary constructor, for syntactic convenience. Bind to [Unit] when not needed. + * + * Non-reusable. + * + * @property scope Instance to which to bind the candidate-returning functions. For syntactic terseness when making lots of calls to, E.G., [com.unciv.ui.tilegroups.TileSetStrings]. + */ +class ImageAttempter(val scope: T) { + /** The first valid filename tried if any, or the last filename tried if none have succeeded. */ + var lastTriedFileName: String? = null + private set + /** Whether an valid image path has already been tried. Once this is true, no further calls to [tryImage] have any effect. */ + var imageFound = false + private set + + /** + * Chainable method that uses [ImageGetter.imageExists] to check whether an image exists. [getPath] and [getImage] will return either the first valid image passed here, or the last invalid image if none were valid. Calls after the first valid one are short-circuited. + * + * @see ImageAttempter + * @see forceImage + * @param fileName Function that returns the filename of the image to check. Bound to [scope]. Will not be run if a valid image has already been found. May return `null` to skip this candidate entirely. + * @return This [ImageAttempter], for chaining. + */ + fun tryImage(fileName: T.() -> String?): ImageAttempter { + if (!imageFound) { + val imagePath = scope.run(fileName) + lastTriedFileName = imagePath ?: lastTriedFileName + if (imagePath != null && ImageGetter.imageExists(imagePath)) + imageFound = true + } + return this + } + /** + * Chainable method that makes multiple invocations to [tryImage]. + * + * @see tryImage + * @param fileNames Any number of image candidate returning functions to pass to [tryImage]. + * @return This [ImageAttempter], for chaining. + */ + fun tryImages(fileNames: Sequence String?>): ImageAttempter { + for (fileName in fileNames) { + tryImage(fileName) + } // *Could* skip calls/break loop if already imageFound. But that means needing an internal guarantee/spec of tryImage being same as no-op when imageFound. + return this + } + /** + * Chainable method that forces an image candidate chain to be terminated. Passing a function here is exactly like passing a function to [tryImage] and pretending [ImageGetter.imageExists] always returns `true`. + * + * @see tryImage + * @param fileName Function that returns the filename of the image to check. Bound to [scope]. Will not be run if a valid image has already been found. When returning a string, causes the candidate chain to be terminated, all subsequent calls to [tryImage] to be short-circuited, and [getImage] and [getPath] to return the resulting image. May return `null` to skip this candidate entirely. + * @return This [ImageAttempter], for chaining. + */ + fun forceImage(fileName: T.() -> String?): ImageAttempter { + if (!imageFound) { + val imagePath = scope.run(fileName) + if (imagePath != null) { + lastTriedFileName = imagePath + imageFound = true + } + } + return this + } + /** @return The first valid image filename given to [tryImage] if any, or the last tried image filename otherwise. */ + fun getPath() = lastTriedFileName + /** @return The first valid image filename given to [tryImage] if any, or `null` if no valid image was tried. */ + fun getPathOrNull() = if (imageFound) lastTriedFileName else null + /** @return The first valid image specified to [tryImage] if any, or the last tried image otherwise. */ + fun getImage() = ImageGetter.getImage(lastTriedFileName) +} diff --git a/core/src/com/unciv/ui/utils/ImageGetter.kt b/core/src/com/unciv/ui/utils/ImageGetter.kt index 9b4f2af895..b0470acaa0 100644 --- a/core/src/com/unciv/ui/utils/ImageGetter.kt +++ b/core/src/com/unciv/ui/utils/ImageGetter.kt @@ -164,12 +164,12 @@ object ImageGetter { return Image(TextureRegion(texture)) } - fun getImage(fileName: String): Image { + fun getImage(fileName: String?): Image { return Image(getDrawable(fileName)) } - fun getDrawable(fileName: String): TextureRegionDrawable { - return if (textureRegionDrawables.containsKey(fileName)) textureRegionDrawables[fileName]!! + fun getDrawable(fileName: String?): TextureRegionDrawable { + return if (fileName != null && textureRegionDrawables.containsKey(fileName)) textureRegionDrawables[fileName]!! else textureRegionDrawables[whiteDotLocation]!! } @@ -436,4 +436,4 @@ object ImageGetter { fun getAvailableTilesets() = textureRegionDrawables.keys.asSequence().filter { it.startsWith("TileSets") } .map { it.split("/")[1] }.distinct() -} \ No newline at end of file +}