AI fix v3 subs submerging #5492
Conversation
| @@ -176,6 +186,7 @@ public ProBattleResult callBattleCalculator( | |||
| final List<Unit> attackingUnits, | |||
| final List<Unit> defendingUnits, | |||
| final Set<Unit> bombardingUnits, | |||
| final boolean checkSubmerge, | |||
There was a problem hiding this comment.
Any objection to using an enum instead of boolean parameter? The "true, true" invocation is difficult to understand, requires a redirect to look at this code, slows down reading comprehension. Alternatively a static factory method could be employed to encapsulate the boolean parameters to be class-internal.
There was a problem hiding this comment.
Moved the boolean parameters out of public methods so its just internal implementation in this class.
| final double transportedUnitValue = | ||
| TuvUtils.getTuv(defendingTransportedUnits, ProData.unitValueMap); | ||
| tuvSwing += transportedUnitValue * winPercentage / 100; | ||
| } | ||
|
|
||
| // Remove TUV and add to remaining units for defenders that can submerge before battle | ||
| if (tuvSwing > 0 | ||
| && checkSubmerge |
There was a problem hiding this comment.
are all 4 of these boolean conditions to represent "canSubmergeBeforeBattle"? If so, extracting to a variable or method could make the comment redundant and the code a bit easier to grok
| && checkSubmerge | ||
| && Properties.getSubRetreatBeforeBattle(data) | ||
| && defendingUnits.stream().anyMatch(Matches.unitCanEvade()) | ||
| && attackingUnits.stream().noneMatch(Matches.unitIsDestroyer())) { |
There was a problem hiding this comment.
side-note, it really seems like there should a rules class to do this logic. If there are other units that can prevent subs from submerging, seems this would break.
Does v2 or v1 rules allow destroyers to block sub-submerge? It might be moot if there are no destroyers on those maps, but could be worth considering that the 'isDestroyer' condition is truly universal.
There was a problem hiding this comment.
The isDestroyer is actually checking the unit property not whether the unit type itself is a destroyer. I can't remember if v1 handles it the same as the rest but at the moment all the AI logic assumes that isDestroyer property prevents submerging subs.
There was a problem hiding this comment.
I think v1 can get away with it because there are no destroyer units.
Codecov Report
@@ Coverage Diff @@
## master #5492 +/- ##
============================================
- Coverage 24.71% 24.58% -0.13%
- Complexity 7066 7120 +54
============================================
Files 1085 1089 +4
Lines 78143 79403 +1260
Branches 11553 11689 +136
============================================
+ Hits 19310 19520 +210
- Misses 56631 57676 +1045
- Partials 2202 2207 +5
Continue to review full report at Codecov.
|
Address: https://forums.triplea-game.org/topic/105/ai-development-discussion-and-feedback/181