-
Notifications
You must be signed in to change notification settings - Fork 340
Add allied damage consideration to Princess AI #7721
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
Conversation
Fixes #7109 Root cause: Princess was being overly cautious when she had numerical superiority because the bravery formula summed ALL enemy damage against her while only considering the MAX damage she could do to ONE enemy. This asymmetry caused her to overestimate threats and underestimate collective firepower. Fix: Implement "Allied Discount" - when calculating expected enemy damage, divide each enemy's threat by the number of friendly units who can engage that enemy (plus self). This makes Princess more aggressive when she has allies who can also shoot back at the same targets. Example: If an enemy does 50 damage but 3 allies can engage them, the perceived threat becomes 50/4 = 12.5 damage. Changes: - Add considerAlliedDamage setting to BehaviorSettings (default: ON) - Add countAlliesWhoCanEngage() helper to BasicPathRanker - Apply allied discount in rankPath() enemy damage loop - Add UI toggle to BotConfigDialog - Add debug logging for decision visibility
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.
Pull request overview
This PR implements an "Allied Discount" feature for the Princess AI bot to address issue #7109, where Princess was overly cautious despite having numerical superiority. The fix divides enemy damage threats by the number of allied units who can engage each enemy, making Princess more aggressive when supported by allies.
Key Changes:
- Added
considerAlliedDamageboolean setting (default: true) to BehaviorSettings with full serialization support - Implemented
countAlliesWhoCanEngage()helper method and allied damage discount calculation in BasicPathRanker - Added UI toggle in BotConfigDialog with appropriate tooltips and preset management
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| BehaviorSettings.java | Adds new considerAlliedDamage field with getters/setters, XML serialization/deserialization, and updates to equals()/hashCode()/toLog()/getCopy() methods |
| BasicPathRanker.java | Implements allied damage discount logic in rankPath() and adds countAlliesWhoCanEngage() helper method to count allies in weapon range of enemies |
| BotConfigDialog.java | Adds UI toggle for the new setting with tooltip, integrates it into preset comparison, and ensures proper save/load functionality |
| messages.properties | Adds label and tooltip text for the new UI toggle explaining the feature's behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Issue | Status | Fix
|
|--------------------------------|---------|----------------------------------------------------------------------
--------------------------|
| Double-counting moving unit | ✅ Fixed | Changed countAlliesWhoCanEngage() to accept movingUnit parameter and
exclude it from the count |
| Unused myFinalCoords parameter | ✅ Fixed | Removed the unused parameter, replaced with movingUnit
|
| Missing unit tests | ✅ Fixed | Added 7 test cases in AlliedDamageDiscountTests nested class
|
Test cases added:
1. testCountAlliesWhoCanEngage_NoAllies - Empty friends list
2. testCountAlliesWhoCanEngage_OneAllyInRange - Single ally in weapon range
3. testCountAlliesWhoCanEngage_AllyOutOfRange - Ally beyond weapon range
4. testCountAlliesWhoCanEngage_ExcludesMovingUnit - Verifies moving unit is excluded
5. testCountAlliesWhoCanEngage_SkipsOffBoardAllies - Off-board entities ignored
6. testCountAlliesWhoCanEngage_SkipsNullPositionAllies - Null position handled
7. testCountAlliesWhoCanEngage_EnemyNullPosition - Enemy null position returns 0
| // Apply allied damage discount if enabled - when allies can also engage this enemy, | ||
| // the threat is reduced proportionally | ||
| double enemyDamage = eval.getEstimatedEnemyDamage(); | ||
| if (getOwner().getBehaviorSettings().isConsiderAlliedDamage()) { | ||
| int alliesEngaging = countAlliesWhoCanEngage(enemy, movingUnit); | ||
| if (alliesEngaging > 0) { | ||
| double allyFactor = alliesEngaging + 1.0; // +1 for self | ||
| double originalDamage = enemyDamage; | ||
| enemyDamage = enemyDamage / allyFactor; | ||
| logger.debug("Allied discount for {}: {} allies engaging, damage {} -> {} (factor {})", | ||
| enemy.getDisplayName(), alliesEngaging, originalDamage, enemyDamage, allyFactor); | ||
| } | ||
| } | ||
| expectedDamageTaken += enemyDamage; |
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 specifically needs unit test coverage. We need to have some insight into what the expected behavior should be in different situations.
|
Closing as I've incorporated this code into #7736 |
Fixes #7109
Now part of #7736
Root cause: Princess was being overly cautious when she had numerical
superiority because the bravery formula summed ALL enemy damage against
her while only considering the MAX damage she could do to ONE enemy.
This asymmetry caused her to overestimate threats and underestimate
collective firepower.
Fix: Implement "Allied Discount" - when calculating expected enemy
damage, divide each enemy's threat by the number of friendly units
who can engage that enemy (plus self). This makes Princess more
aggressive when she has allies who can also shoot back at the same
targets.
Example: If an enemy does 50 damage but 3 allies can engage them,
the perceived threat becomes 50/4 = 12.5 damage.
Changes:
See the issue itself for all the specifcs