Automated Workers replacing Great Improvements #11082
Comments
|
I have noticed it for Customs House, Holy site, but I thought that was just the way the stat valuing system works. I'll look into a more complete solution that isn't |
|
Oh my - Still haven't found a good place to set a conditional breakpoint to let autoplay generate that missing save for us... But there's some linting to be had: Index: core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt b/core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt
--- a/core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt (revision 371460038446e2e9efd6fc3b39c9bbecf5374711)
+++ b/core/src/com/unciv/logic/automation/unit/WorkerAutomation.kt (date 1707298117918)
@@ -57,9 +57,12 @@
* The improvementPriority and bestImprovement are by default not set.
* Once improvementPriority is set we have already checked for the best improvement, repairImprovement.
*/
- data class TileImprovementRank(val tilePriority: Float, var improvementPriority: Float? = null,
- var bestImprovement: TileImprovement? = null,
- var repairImprovment: Boolean? = null)
+ data class TileImprovementRank(
+ val tilePriority: Float,
+ var improvementPriority: Float? = null,
+ var bestImprovement: TileImprovement? = null,
+ var repairImprovement: Boolean? = null
+ )
private val tileRankings = HashMap<Tile, TileImprovementRank>()
@@ -114,7 +117,7 @@
if (tileHasWorkToDo(currentTile, unit)) {
val tileRankings = tileRankings[currentTile]!!
- if (tileRankings.repairImprovment!!) {
+ if (tileRankings.repairImprovement!!) {
debug("WorkerAutomation: $unit -> repairs $currentTile")
UnitActionsFromUniques.getRepairAction(unit)?.action?.invoke()
return
@@ -206,7 +209,7 @@
}
/**
- * Calculate a priority for the tile without accounting for the improvement it'self
+ * Calculate a priority for the tile without accounting for the improvement itself
* This is a cheap guess on how helpful it might be to do work on this tile
*/
fun getBasePriority(tile: Tile, unit: MapUnit): Float {
@@ -249,11 +252,11 @@
private fun getImprovementPriority(tile: Tile, unit: MapUnit): Float {
getBasePriority(tile, unit)
val rank = tileRankings[tile]
- if(rank!!.improvementPriority == null) {
+ if (rank!!.improvementPriority == null) {
// All values of rank have to be initialized
rank.improvementPriority = -100f
rank.bestImprovement = null
- rank.repairImprovment = false
+ rank.repairImprovement = false
val bestImprovement = chooseImprovement(unit, tile)
if (bestImprovement != null) {
@@ -270,16 +273,17 @@
var repairBonusPriority = tile.getImprovementToRepair()!!.getTurnsToBuild(unit.civ,unit) - UnitActionsFromUniques.getRepairTurns(unit)
if (tile.improvementInProgress == Constants.repair) repairBonusPriority += UnitActionsFromUniques.getRepairTurns(unit) - tile.turnsToImprovement
- val repairPriority = repairBonusPriority + Automation.rankStatsValue(TileStatFunctions(tile).getStatDiffForImprovement(tile.getTileImprovement()!!, unit.civ, tile.owningCity), unit.civ)
+ val repairPriority = repairBonusPriority +
+ Automation.rankStatsValue(TileStatFunctions(tile).getStatDiffForImprovement(tile.getTileImprovement()!!, unit.civ, tile.owningCity), unit.civ)
if (repairPriority > rank.improvementPriority!!) {
rank.improvementPriority = repairPriority
rank.bestImprovement = null
- rank.repairImprovment = true
+ rank.repairImprovement = true
}
}
}
// A better tile than this unit can build might have been stored in the cache
- if (!rank.repairImprovment!! && (rank.bestImprovement == null ||
+ if (!rank.repairImprovement!! && (rank.bestImprovement == null ||
!unit.canBuildImprovement(rank.bestImprovement!!, tile))) return -100f
return rank.improvementPriority!!
}
@@ -296,7 +300,7 @@
*/
private fun tileHasWorkToDo(tile: Tile, unit: MapUnit): Boolean {
if (getImprovementPriority(tile, unit) <= 0) return false
- if (!(tileRankings[tile]!!.bestImprovement != null || tileRankings[tile]!!.repairImprovment!!))
+ if (!(tileRankings[tile]!!.bestImprovement != null || tileRankings[tile]!!.repairImprovement!!))
throw IllegalStateException("There was an improvementPriority > 0 and nothing to do")
return true
} |
Here's a cheated exampleNext turn, that Tyre Worker will try to replace the Holy Site with a Farm. Caught with a breakpoint on WorkerAutomation line 127 ( ... is the problem here that getImprovementPriority is positive? I don't think I have the big picture anymore... OK, when getStatDiffForImprovement runs for that it correctly says +2Food,-6Faith. Automation.rankStatsValue then converts that to +2.4 value, which is still in the ranking instance when startWorkingOnImprovement is reached. Why + for a net -? Faith has no value at all... Could we find a non-faith, non-gold such example? |
|
(...uh, shouldn't we remove the Holy site from the Vanilla ruleset?) |
|
So far no luck. Ran several games now, (annoying due to this bug still unfixed), and the breakpoint with condition So - if the issue claim is true for all Great Improvements, we need proof in form of a save just before such a replace happens. Holy site is explained by the tile ranking function valuing faith at zero, and Customs house -probably- is explained by the ranking function devaluating gold yield by 67% (imho a slight contradiction with the comment text there - the comment would justify 33%). |
I did notice that the AI workers don't like the base holy site and customs house. But since it was a stat rating problem I deemed it out of the scope of what I wanted to change. Interestingly, it is only causing a problem now since the automated workers do work. |
Other way round: The player decided a Holy sh - ahem, site - was worth spending a Prophet on. How exactly should we respect that? I'd say simply change the Automation.rankStatsValue function to place less widely varying weights. Maybe aim for max-weight/min-weight <= 5 and not ♾️ . That would mean no stat gets < 0.75f weight if the happiness one stays as-is. And with faith worth 0.75, a Farm overridung a Holy outh... Merde! ... a Farm overriding a Holy site would no longer be considered a win. As for a Grinchhouse - with gold up to 0.75 from 0.33 - 3 to 2.4 at most - also not a win. Replacing with something other than a Farm, if in synergy with a bonus resource maybe - they could still be built over, but then maybe rightfully so? |
|
And/Or - make all those weight factors moddable. And I can't even do division properly. |
|
Tile ranking for Faith should definitely not be zero, good catch! e6893ef |
|
Summary:
|
|
High chance I have misunderstood this but please see attached my save of an automated working replacing a citadel. |
|
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. |
|
I think the correct action would be to add "is not replaced by automated workers" unique |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Is there an existing issue for this?
Game Version
4.10.6 (Build 965)
Describe the bug
Automated Workers now replace Great Improvements (Customs house, Holy site, Landmark, Manufactory) with regular tile improvements. I'm pretty sure they didn't do this in earlier versions, and I hope this is not a new feature.
Steps to Reproduce
Screenshots
No response
Link to save file
No response
Operating System
Android
Additional Information
No response
The text was updated successfully, but these errors were encountered: