-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Update UnitAutomation.kt #13123
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
base: master
Are you sure you want to change the base?
Update UnitAutomation.kt #13123
Conversation
val availablePromotions = if (unit.health < 60 && !(unit.baseUnit.isAirUnit() || unit.baseUnit.hasUnique(UniqueType.CanMoveAfterAttacking)) | ||
&& promotions.any { it.hasUnique(UniqueType.OneTimeUnitHeal) }) | ||
promotions.filter { it.hasUnique(UniqueType.OneTimeUnitHeal) } |
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.
???
If the unit is at less than 60 health it will ALWAYS choose the heal promotion?!
That sounds like a huge waste!
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.
Not if it's an air unit or something that move after attacking. It's in the previous code as well. The availablePromotions was not updated correctly, so the break wouldn't trigger. This is also something that should happen in basegame as well, as canBePromoted() is true when the unit has maxxed out their promotion tree with enough XP to adopt the heal promotion (in practise, no report has been made as the AI doesn't keep units alive that long).
Did I accidentally create a new bug trying to fix this one?
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.
This currently says "all non air units that can't move after attacking, if under 60hp, will always choose one time heal"
This is what existed before this PR
And it's a huge waste of promotion potential
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.
This currently says "all non air units that can't move after attacking, if under 60hp, will always choose one time heal"
Yes, that's what existed before the PR, I didn't change the logic on this right? (For reference, this was added in #12728)
This PR is to fix the problem of the game hanging when the AI gets -100% promotion XP cost.
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.
If there's a promotion that e.g. gives you +1 gold and skips promotion; And the AI as -100% promotion cost; The optimal move would be to get that promotion as many times as possible
That is, to get it forever
So the problem is actually in the recursive nature of SkipPromotion promotions, and has nothing to do with the healing
So there are 2 options here:
- If promotion cost is 0, don't allow skippromotion promotions
- Don't allow promotion cost to be 0
It seems that you're against the second option, which means we need to implement the first option
This should be its own separate if and unrelated to the super-problematic heal promotion if
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.
If there's a promotion that e.g. gives you +1 gold and skips promotion; And the AI as -100% promotion cost; The optimal move would be to get that promotion as many times as possible
Should it? If that means choosing infinitely many promotions, the game will get stuck letting this AI player adopt infinitely many promotions, or until some OS/memory limit enforced cap is reached (is this 2^63−1, or a bigger number?).
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.
The AI did categorically avoid skippromotion promotion before #12728, until that PR allowed it to situationally adopt such promotions for healing. Maybe there should be more exceptions indeed.
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.
Precisely
We need to find only the situations where we could be stuck forever
This fix only fixes a specific case of health related issues
No description provided.