-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Construction automation rework #11601
Conversation
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
# Conflicts: # core/src/com/unciv/logic/automation/city/ConstructionAutomation.kt
Conflicts have been resolved. |
@@ -348,7 +312,7 @@ class ConstructionAutomation(val cityConstructions: CityConstructions) { | |||
buildingStats.food *= 3 | |||
} | |||
|
|||
if (buildingStats.gold < 0 && civInfo.gold < 0) { | |||
if (buildingStats.gold < 0 && civInfo.stats.statsForNextTurn.gold < 10) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stats for next turn is per turn gold
You can be in deficit even if you're at -9 gpt
Maybe this can have 2 stages: *1.5 if current deficit and gpt < 0, and another *1.5 if gpt < 10
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In other words, you changed "if our bank account is piss-poor no matter the cash flow" to "if we're losing money every day no matter the millions in the bank"... Just food for thought.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the reasoning behind this is because buildings generally take multiple turns to build.
If our income is low, then we should pay more attention to the gold production/maintenance cost of the building.
Instead of: If we are temporarily in debt then we should focus on building a gold production building.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine
fun getValueOfBuilding(building: Building): Float { | ||
var value = 0f | ||
value = applyBuildingStats(building, value) | ||
value = applyMilitaryBuildingValue(building, value) | ||
value = applyVictoryBuildingValue(building, value) | ||
value = applyOnetimeUniqueBonuses(building, value) | ||
return value | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Sorry, been taking a break from stuff to work on separate projects and generally take a break, Finally back to looking over stuff)
Why are we doing value = function(building, value)
. None of these functions actually use the value that's passed in. We could instead do value += function(building)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, been taking a break from stuff
No problem!
None of these functions actually use the value that's passed in.
Good point, I think that came from an intermediate idea I had and was left over. I'll clean it up.
This is part of #11264 and is a prototype of a different and probably better style of choosing constructions.
Previously we filtered the constructions into categories and picked the best construction in that category from the values that category has. This has many problems, for example if a building doesn't just fit into one category.
This new design is cleaner, dynamic and more stat, unique and personality oriented. Whereas the old design just made an educated guess.
To find the value of each building we simply sum up the value of all the stats and uniques that are present and value these based on the state of the civ and city.
I am not sure if we should do the same to the units. However I do know that we need to count up the military units and make sure that the AI has a balanced assortment of units.
This is just the first draft since it would take more time to balance and add the other uniques. Does this approach seem viable and should we continue on with it?