Fix up and reinstate #1646 to not remove infrastructure units

[simon33-2]

This seems to resolve the issue in Greyhawk_Wars from #1952 #1953.

Works in WW2 v5 & Classic so I'm not sure of any other cases (Global has the same rules as v5 for these purposes).

[codecov-io]

Codecov Report

Merging #2080 into master will increase coverage by 0.38%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master    #2080      +/-   ##
============================================
+ Coverage      18.6%   18.98%   +0.38%     
- Complexity     5378     5522     +144     
============================================
  Files           777      777              
  Lines         77713    78222     +509     
  Branches      12906    13135     +229     
============================================
+ Hits          14456    14852     +396     
- Misses        61201    61288      +87     
- Partials       2056     2082      +26

Impacted Files	Coverage Δ	Complexity Δ
...mes/strategy/triplea/delegate/MustFightBattle.java	66.41% <100%> (+3.83%)	480 <3> (+144)	⬆️
...rc/main/java/games/strategy/net/nio/NIOReader.java	67.96% <0%> (-3.89%)	18% <0%> (-1%)
...strategy/net/nio/ServerQuarantineConversation.java	67.79% <0%> (-1.7%)	11% <0%> (-1%)
...java/games/strategy/triplea/delegate/DiceRoll.java	54.22% <0%> (+0.16%)	101% <0%> (+1%)	⬆️
...tegy/triplea/oddsCalculator/ta/OddsCalculator.java	44.02% <0%> (+0.31%)	13% <0%> (ø)	⬇️
...trategy/triplea/delegate/UnitBattleComparator.java	35.35% <0%> (+4.04%)	22% <0%> (+1%)	⬆️

---

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 26e4f89...dabfd9c. Read the comment docs.

[DanVanAtta]

@simon33-2 can you walk us through the testing for this?

[simon33-2]

There's 4 cases I'm aware of.

• Attack at sea with only defenseless transports remaining
• Attack on land with destroyable AA Guns
• Attack on land with convertible AA Guns
• The Greyhawk Wars case described in Can't recapture infrastructure #1952.

[ron-murhammer]

@simon33-2 So what about the following case:

• have a group of attacking units with total attack power > 0
• have a group of defending units (say 10) with total defense power = 0
• want to attack for 1 round and retreat so I can kill of some of them but don't want to take the territory due to potential counter attack

I believe that is possible to have a map like that today and this change would break that. Is that a valid case?

[simon33-2]

In that case you should not be able to retreat, at least in any of the Axis and Allies games.

That is the bug referred to in #1270.

[simon33-2]

Does it need a map property, if there are maps where it is valid to not take the territory?

[DanVanAtta]

@simon33-2 sorry to say, though this is incredibly hard to follow and nobody is shepherding this. I reviewed the description over the last 10 minutes, it's daunting to see text from: #1952 and that the updates in #1953 references more updates.

I do not wish for PR's to be sitting for upwards of a month!

I think more info is needed before this can really be considered in its due merit. At this point is where it becomes apparent that TripleA is a pain in the ass project. It seems simple because all the code is so straight forward and procedural, but then when it comes time to design and testing you start seeing all the technical debt and extra time you have to pay.

For this PR:

• what was broken exactly?
• are we adding something back in that was removed? If so, why was it removed? Is there any trade off we are making by adding these updates?
• can you go through all test scenarios and note the results. Again, here is where TripleA is a pain in the ass, it's up to you here @simon33-2 to do a ton of testing and to present the results here. It may be worth while to bite the bullet, extract this code to a micro class and test it there. Then the manual testing is cut down drastically at least.

[ron-murhammer]

@simon33-2 Never mind. I reviewed the call it makes to remove units and includes dependents. I'm gonna merge this now.

@DanVanAtta This has gone back and forth a few times. Its primarily to address #1270. Previous attempts caused other issues but this appears to work properly by checking total attack/defense and remove infra units from being killed off.

[simon33-2]

Thanks @ron-murhammer

@DanVanAtta . The problem with #1646 was that there was a case I wasn't aware of.