AI - Fix battle results when checking for submerging

[ron-murhammer]

Overview

• Address subs sometimes not submerging reported here: https://forums.triplea-game.org/topic/1181/total-world-war-december-1941-3-0-0-3/97

Functional Changes

• When determining whether to submerge, ensure battle calc results don't already take it into account causing the subs to then not choose to submerge

Manual Testing Performed

• Tested TWW and revised

[codecov-io]

Codecov Report

Merging #4739 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #4739      +/-   ##
============================================
- Coverage     23.16%   23.15%   -0.01%     
- Complexity     6185     6200      +15     
============================================
  Files           871      873       +2     
  Lines         70452    70776     +324     
  Branches      11256    11338      +82     
============================================
+ Hits          16321    16389      +68     
- Misses        52097    52348     +251     
- Partials       2034     2039       +5

Impacted Files	Coverage Δ	Complexity Δ
...va/games/strategy/triplea/ai/pro/ProRetreatAi.java	6.12% <0%> (-0.13%)	1 <0> (ø)
...trategy/triplea/ai/pro/util/ProOddsCalculator.java	4.59% <0%> (-0.11%)	1 <0> (ø)
...rategy/triplea/attachments/UnitTypeComparator.java	46.15% <0%> (-7.7%)	12% <0%> (-1%)
.../strategy/triplea/odds/calculator/DummyPlayer.java	34.83% <0%> (-1.13%)	9% <0%> (-1%)
...plea/lobby/server/db/BannedUsernameController.java	0% <0%> (ø)	0% <0%> (ø)	⬇️
.../org/triplea/lobby/server/ModeratorController.java	0% <0%> (ø)	0% <0%> (ø)	⬇️
...org/triplea/lobby/server/LobbyServerMessenger.java	0% <0%> (ø)	0% <0%> (ø)	⬇️
...e/lobby/client/login/CreateUpdateAccountPanel.java	0% <0%> (ø)	0% <0%> (ø)	⬇️
...org/triplea/lobby/server/db/BadWordController.java	0% <0%> (ø)	0% <0%> (ø)	⬇️
...g/triplea/lobby/server/db/AccessLogController.java	0% <0%> (ø)	0% <0%> (ø)	⬇️
... and 15 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 416bd54...bc977a1. Read the comment docs.

[DanVanAtta]

The core fix looks good. The public API with a boolean parameter I think can become problematic, left some commentary, please let me know what you think.

[DanVanAtta]

Nitpicking the API, any thoughts to removing the boolean parameter?

Looking at this diff, it looks like if calculateBattleResults with the boolean parameter were made private, the boolean parameter could be made class-internal by adding something like:

  public ProBattleResult calculateBattleResultsNoSubmerge(final Territory t, final List<Unit> attackingUnits,
      final List<Unit> attackingUnits, final List<Unit> defendingUnits, final Set<Unit> bombardingUnits) {
      final List<Unit> defendingUnits, final Set<Unit> bombardingUnits) {
    return calculateBattleResults(t, attackingUnits, defendingUnits, bombardingUnits, false);
  }

[ron-murhammer]

I think you can argue it either way in this case given that the method is only used in one place. But I went ahead and added a method to avoid exposing the boolean.

[DanVanAtta]

From my perspective it is the class boundary that makes the difference. Regardless thanks for the update.