Skip to content
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

Merged
merged 8 commits into from
May 27, 2024

Conversation

tuvus
Copy link
Collaborator

@tuvus tuvus commented May 16, 2024

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?

Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link

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) {
Copy link
Owner

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

Copy link
Collaborator

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.

Copy link
Collaborator Author

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.

Copy link
Owner

@yairm210 yairm210 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine

Comment on lines 256 to 263
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
}
Copy link
Collaborator

@SeventhM SeventhM May 27, 2024

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)

Copy link
Collaborator Author

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.

@yairm210 yairm210 merged commit dd8072c into yairm210:master May 27, 2024
4 checks passed
@tuvus tuvus deleted the ConstructionAutomationRework branch June 1, 2024 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants