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.
This commit is contained in:
will-ca 2022-01-04 12:02:32 -08:00 committed by GitHub
parent 96beed4b3e
commit a5bba8dfb0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 103 additions and 38 deletions

View File

@ -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() ?: ""
}

View File

@ -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<out T: Any>(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<T> {
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<T.() -> String?>): ImageAttempter<T> {
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<T> {
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)
}

View File

@ -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()
}
}