yairm210
commented
6 months ago I'm not sure what it is you're actually suggesting. Also, please add documentation in unique parameters page regarding countables
Closed PLynx01 closed 1 month ago
yairm210
commented
6 months ago I'm not sure what it is you're actually suggesting. Also, please add documentation in unique parameters page regarding countables
PLynx01
commented
6 months ago I'm not sure what it is you're actually suggesting. Also, please add documentation in unique parameters page regarding countables
I am suggesting you to add more countables. Many of them are scope-bound, so they would apply to specific scope.
For example: The population in a Game-wide scope is a world population
In Civ-wide is a Civ population
In City-wide is the population of a city
Any questions?
yairm210
commented
6 months ago Yes, that's not an actual suggestion. That doesn't say what it would look like from a modders perspective at all.
PLynx01
commented
6 months ago Yes, that's not an actual suggestion. That doesn't say what it would look like from a modders perspective at all.
So in that case, it would look like this:
<when number of [scopeFilter][countable] is greater than [scopeFilter][countable]>
scopeFilter allowed values are:
And the gameWideScope (for all civilizations in game)
Examples:
<when number of [in all cities with a world wonder][number of cities] is greater than [0]>
Returns true when a civilization has at least one city with world wonder
<when number of [gameWideScope][population] is greater than [200]>
Returns true when the global population is more than 200
<when number of [{gameWideScope} {Military}][Number of units] is greater than [100]>
Returns true when the number of military units owned by all civilizations is more than 100
Now you probably know what I mean. If you have some suggestions, share them with me.
It's just a concept for now.
PLynx01
commented
6 months ago Yes, that's not an actual suggestion. That doesn't say what it would look like from a modders perspective at all.
So in that case, it would look like this:
<when number of [scopeFilter][countable] is greater than [scopeFilter][countable]>
scopeFilter allowed values are:
- civFilter
- buildingFilter
- cityFilter
- tileFilter
- mapUnitFilter
And the gameWideScope (for all civilizations in game)
Examples:
<when number of [in all cities with a world wonder][number of cities] is greater than [0]>
Returns true when a civilization has at least one city with world wonder
<when number of [gameWideScope][population] is greater than [200]>
Returns true when the global population is more than 200
<when number of [{gameWideScope} {Military}][Number of units] is greater than [100]>
Returns true when the number of military units owned by all civilizations is more than 100
Now you probably know what I mean. If you have some suggestions, share them with me.
It's just a concept for now.
@yairm210 I hope I described my idea in a way detailed enough. What do you think about it?
SomeTroglodyte
commented
6 months ago override fun getTranslationWriterStringsForOutput()<when number of [cities] is greater than [0]> <in cities with a [World Wonder]> - "cities" is civ-scope-type, while "world cities" could be the global version...~... concept:~
~... still without support for filtering through other conditionals - but nicely extensible?~
Better read: branch on my fork
PLynx01
commented
6 months ago
- "year" in the original PR is not auto-templated for translation ->
override fun getTranslationWriterStringsForOutput()- I would create a new class for this, possibly an Enum with a "value getter" member - lambda field or overridable; possibly a supported scopes flags field; or possibly a class hierarchy with an interface and implementations per scope type.
- Then... one could add them piecemeal
- Scope filters - I don't like the examples. Is it feasible to have countable identifiers unqiuely bound to a scope type each, and use all other conditionals on the same Unique as filters?
<when number of [cities] is greater than [0]> <in cities with a [World Wonder]>- "cities" is civ-scope-type, while "world cities" could be the global version...~... concept:~
```patch Index: core/src/com/unciv/models/ruleset/unique/Conditionals.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/core/src/com/unciv/models/ruleset/unique/Conditionals.kt b/core/src/com/unciv/models/ruleset/unique/Conditionals.kt --- a/core/src/com/unciv/models/ruleset/unique/Conditionals.kt (revision 7dfa6682e08ebd353e45aa5b81888be776224737) +++ b/core/src/com/unciv/models/ruleset/unique/Conditionals.kt (date 1712574009931) @@ -99,28 +99,9 @@ return compare(statReserve, lowerLimit * gameSpeedModifier, upperLimit * gameSpeedModifier) } - fun getCountableAmount(countable: String): Float? { - if (countable.toFloatOrNull() != null) return countable.toFloat() - - val relevantStat = Stat.safeValueOf(countable) - - if (relevantStat != null) { - return if (relevantCity != null) { - relevantCity!!.getStatReserve(relevantStat).toFloat() - } else if (relevantStat in Stat.statsWithCivWideField && relevantCiv != null) { - relevantCiv!!.getStatReserve(relevantStat).toFloat() - } else { - null - } - } - - if (gameInfo == null) return null - - if (countable == "year") return gameInfo!!.getYear(gameInfo!!.turns).toFloat() - if (gameInfo!!.ruleset.tileResources.containsKey(countable)) - return getResourceAmount(countable).toFloat() - - return null + fun getCountableAmount(id: String): Float? { + val countable = Countables[id, gameInfo?.ruleset] ?: return null + return countable(gameInfo, relevantCiv, relevantCity) } fun compareCountables( Index: core/src/com/unciv/models/ruleset/unique/UniqueParameterType.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/core/src/com/unciv/models/ruleset/unique/UniqueParameterType.kt b/core/src/com/unciv/models/ruleset/unique/UniqueParameterType.kt --- a/core/src/com/unciv/models/ruleset/unique/UniqueParameterType.kt (revision 7dfa6682e08ebd353e45aa5b81888be776224737) +++ b/core/src/com/unciv/models/ruleset/unique/UniqueParameterType.kt (date 1712574011246) @@ -76,25 +76,17 @@ }, Countable("countable", "1000", "This indicate a number or a numeric variable") { - // todo add more countables - private val knownValues = setOf( - "year" - ) - - override fun isKnownValue(parameterText: String, ruleset: Ruleset): Boolean { - if (parameterText in knownValues) return true - if (parameterText.toIntOrNull() != null) return true - if (parameterText.toFloatOrNull() != null) return true - if (Stat.isStat(parameterText)) return true - if (parameterText in ruleset.tileResources) return true - return false - } + override fun isKnownValue(parameterText: String, ruleset: Ruleset) = + Countables[parameterText, ruleset] != null override fun getErrorSeverity( parameterText: String, ruleset: Ruleset): UniqueType.UniqueParameterErrorSeverity? { return if (isKnownValue(parameterText, ruleset)) null else UniqueType.UniqueParameterErrorSeverity.RulesetSpecific } + + override fun getTranslationWriterStringsForOutput() = + Countables.getTranslationWriterStringsForOutput() }, // todo potentially remove if OneTimeRevealSpecificMapTiles changes Index: core/src/com/unciv/models/ruleset/unique/Countables.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/core/src/com/unciv/models/ruleset/unique/Countables.kt b/core/src/com/unciv/models/ruleset/unique/Countables.kt new file mode 100644 --- /dev/null (date 1712574009740) +++ b/core/src/com/unciv/models/ruleset/unique/Countables.kt (date 1712574009740) @@ -0,0 +1,93 @@ +package com.unciv.models.ruleset.unique + +import com.unciv.logic.GameInfo +import com.unciv.logic.city.City +import com.unciv.logic.civilization.Civilization +import com.unciv.models.ruleset.Ruleset +import com.unciv.models.ruleset.tile.TileResource +import com.unciv.models.stats.Stat + +interface Countables { + val name: String + val id: String + operator fun invoke(gameInfo: GameInfo?, civ: Civilization?, city: City?): Float? + + companion object { + operator fun get(id: String, ruleset: Ruleset?) = + GlobalCountables.idMap[id] + ?: CivCountables.idMap[id] + ?: CityCountables.idMap[id] + ?: Stat.safeValueOf(id)?.let { StatCountable(it) } + ?: ruleset?.tileResources?.get(id)?.let { ResourceCountable(it) } + ?: id.toFloatOrNull()?.let { LiteralCountable(it) } + + fun getTranslationWriterStringsForOutput() = + GlobalCountables.idMap.keys + + CivCountables.idMap.keys + + CityCountables.idMap.keys + } +} + +enum class GlobalCountables(override val id: String, val getValue: (GameInfo) -> Float?) : Countables { + Year("year", { + it.getYear(it.turns).toFloat() + }) + ; + + override fun invoke(gameInfo: GameInfo?, civ: Civilization?, city: City?) = + if (gameInfo == null) null else getValue(gameInfo) + + companion object { + val idMap = values().associateBy { it.id } + } +} + +enum class CivCountables(override val id: String, val getValue: (Civilization) -> Float?) : Countables { + Cities("cities", { + it.cities.size.toFloat() + }) + ; + override fun invoke(gameInfo: GameInfo?, civ: Civilization?, city: City?) = + if (civ == null) null else getValue(civ) + + companion object { + val idMap = GlobalCountables.values().associateBy { it.id } + } +} + +enum class CityCountables(override val id: String, val getValue: (City) -> Float?) : Countables { + Citizens("citizens", { + it.population.population.toFloat() + }) + ; + override fun invoke(gameInfo: GameInfo?, civ: Civilization?, city: City?) = + if (city == null) null else getValue(city) + + companion object { + val idMap = GlobalCountables.values().associateBy { it.id } + } +} + +class StatCountable(private val stat: Stat) : Countables { + override val name = stat.name + override val id = stat.name + override fun invoke(gameInfo: GameInfo?, civ: Civilization?, city: City?) = + city?.getStatReserve(stat)?.toFloat() + ?: civ?.takeIf { stat in Stat.statsWithCivWideField }?.getStatReserve(stat)?.toFloat() +} + +class LiteralCountable(private val value: Float) : Countables { + override val name = "" + override val id = "" + override fun invoke(gameInfo: GameInfo?, civ: Civilization?, city: City?) = value +} + +class ResourceCountable(private val tileResource: TileResource) : Countables { + override val name = tileResource.name + override val id = tileResource.name + override fun invoke(gameInfo: GameInfo?, civ: Civilization?, city: City?): Float { + if (city != null) return city.getAvailableResourceAmount(name).toFloat() + if (civ != null) return civ.getResourceAmount(name).toFloat() + return 0f + } +} ```~... still without support for filtering through other conditionals - but nicely extensible?~
Better read: branch on my fork
@SomeTroglodyte I've got a two questions:
How do we distinguish countable scope modifier from other conditionals?
What if the second and third value also have scopes?
SomeTroglodyte
commented
6 months ago In that model there are no scope modifiers (it does -quoting-: "to have countable identifiers unqiuely bound to a scope type each, and use all other conditionals on the same Unique as filters")- each ICountable.id needs to uniquely identify scope. For simplicity, that was more of an experiment to see if such a class hierarchy model could work. And it can - one interface implemented both by enums and by directly-instantiated classes.
PLynx01
commented
5 months ago In that model there are no scope modifiers (it does -quoting-: "to have countable identifiers unqiuely bound to a scope type each, and use all other conditionals on the same Unique as filters")- each ICountable.id needs to uniquely identify scope. For simplicity, that was more of an experiment to see if such a class hierarchy model could work. And it can - one interface implemented both by enums and by directly-instantiated classes.
@SomeTroglodyte You say that the countable identifiers should be bound to specific scope. But I am wondering how would we count things that met the additional filters.
Example:
Let's assume I need the count of owned units of a specific type, like number of Air units or a Wounded Land units (combined filters)
What syntax would you propose to support filters?
Let's ask @yairm210 for the advice.
SomeTroglodyte
commented
5 months ago You say that the countable identifiers should be bound to specific scope
Not "should" per se. "Could" as in, might make for simpler and more maintainable code.
But I am wondering how
Very good! Someone needs to invest brainpower, and I'm feeling a little low on that resource.
count... units... combined filters
With that experimental code, <when number of [units] is greater than [cities]> <for [Wounded Land] units> <in [Garrisoned] cities>" might work as expected (conceptually <when number of [wounded land units] is greater than [garrisoned cities]>), but an idea like "more air than land units" wouldn't be possible. As I said, that idea was to avoid new syntactic structures and work with the existing stuff as much as possible. For more control, we would need substructure within a UniqueParameterType, likely needing a good brackets parser that supports arbitrary nesting, ahem.
PLynx01
commented
5 months ago I'm doing a bump here, due to lack of activity.
So, in that case, how do we define a variable scope?
All of these examples do the same - return true when there is more than 25 cities on the entire map
Solution 1 - Additional scope parameters
<when [Game-Wide] [Number of Cities] is greater than [25]>
Solution 2 - Scope-specific names
<when [Number of All Cities] is greater than [25]>
Solution 3 - Countable limiters and modifiers
<when [Number of Cities] is greater than [25]>
My thoughts:
Solution 1, in my opinion is the best. The drawback of 4 instead of 2 parameters is not really an issue.
Solution 2 requires making a huge amount of countable filter values (possibly even hundreds which is contradictory with modding philosophy)
Solution 3 makes it difficult to distinguish between countable limiter/modifier and the next conditional (all conditionals must be true to make an unique active). There is also a problem regarding the scope of the second countable.
If you want to opt for Solution 3, you will need to add some kind of countable limiter/modifier, denoted by different character (maybe @, pronounced as "at")
@yairm210 @SomeTroglodyte
We need to decide between these three solutions. Maybe we should make a poll?
github-actions[bot]
commented
2 months ago This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 15 days.
github-actions[bot]
commented
1 month ago This issue was closed because it has been stalled for 5 days with no activity.
Before creating
Problem Description
@yairm210 I've recently introduced the countable comparison unique. However, for now it supports only a limited number of countables. There is a lot more of countables to implement. Many of proposed countables are heavily dependent on scopes, so they would apply either to:
Entire playthrough (Game-wide) Civilization(s) City/cities Unit(s) Tile(s)
Related Issue Links
11308
Desired Solution
Here is the list of the proposed countables
Game-wide:
Current year Current turn Years since beginning Number of all civilizations ever present Number of current civilizations Number of cities of all civilizations Number of tiles (with tile filter) Global population
Civ scope (supports Game-wide scope)
Number of cities (with civ filter) Number of cities (with civ and city filter) Total stats per turn (stat filter, optionally civ and city filter) Total stockpiled stats (gold, science, culture, faith) Total stats ever produced Total resources currently stockpiled Total resources ever stockpiled Number of traded resources Number of wars Number of defensive pacts Number of science agreements Number of wonders built Number of units Civ population
City-wide (supports civ and game wide)
Number of buildings Number of wonders Stat production per turn Worked tiles City population Turns until population growth/construction complete/border expansion
Unit-wide (supports previous scopes)
XP Base strength Max strength HP Number of promotions
Alternative Approaches
If you think there is a better way, share your thoughts with me.
Additional Context
No response