Add isSuicideOnHit Unit Attribute

[ron-murhammer]

Addresses: https://forums.triplea-game.org/topic/323/unit-option-which-suicides-only-when-it-registers-a-hit-mines

Recommend reviewing without whitespace changes (?w=1).

Functional Changes

• Add new isSuicideOnHit unit attribute
• Implement logic to roll isSuicideOnHit unit types separately so the engine knows if they roll hits
• Implement logic to remove any isSuicideOnHit units that roll hits

Testing

• Added unit test to test basic functionality of "mine" unit using isSuicideOnHit attribute
• Did some actual manual gameplay testing with an edited version of TWW
• Save game compatibility seems fine

Example XML:

<attachment name="unitAttachment" attachTo="germanNavalMine" javaClass="games.strategy.triplea.attachments.UnitAttachment" type="unitType">
<option name="attack" value="0"/>
<option name="defense" value="0"/>
<option name="isSea" value="true"/>
<option name="isSub" value="true"/>
<option name="isAAforCombatOnly" value="true"/>
<option name="mayOverStackAA" value="true"/>
<option name="typeAA" value="navalmine"/>
<option name="targetsAA" value="Battleship:Cruiser:Destroyer:Submarine:Transport"/>
<option name="maxAAattacks" value="1"/>
<option name="maxRoundsAA" value="1"/>	
<option name="movement" value="0"/>
<option name="attackAA" value="2"/>
<option name="damageableAA" value="true"/>
<option name="canInvadeOnlyFrom" value="none"/>
<option name="isSuicideOnHit" value="true"/>
</attachment>

[codecov-io]

Codecov Report

Merging #2543 into master will increase coverage by 1.1%.
The diff coverage is 76.99%.

@@             Coverage Diff             @@
##             master    #2543     +/-   ##
===========================================
+ Coverage     20.37%   21.47%   +1.1%     
- Complexity     5809     6098    +289     
===========================================
  Files           831      831             
  Lines         73631    74228    +597     
  Branches      12400    12636    +236     
===========================================
+ Hits          14999    15942    +943     
+ Misses        56578    56067    -511     
- Partials       2054     2219    +165

Impacted Files	Coverage Δ	Complexity Δ
...gy/triplea/ai/proAI/data/ProPurchaseOptionMap.java	0% <0%> (ø)	0 <0> (ø)	⬇️
.../java/games/strategy/triplea/delegate/Matches.java	43.93% <100%> (+0.77%)	356 <1> (+8)	⬆️
...ain/java/games/strategy/triplea/delegate/Fire.java	87.8% <100%> (+0.09%)	19 <0> (ø)	⬇️
...s/strategy/triplea/attachments/UnitAttachment.java	43.81% <54.54%> (+1.14%)	329 <3> (+11)	⬆️
...mes/strategy/triplea/delegate/MustFightBattle.java	72.89% <80.61%> (+10.5%)	475 <8> (+134)	⬆️
...games/strategy/triplea/image/TileImageFactory.java	0% <0%> (ø)	0% <0%> (ø)	⬇️
...in/java/games/strategy/engine/data/GameParser.java	69.91% <0%> (+0.12%)	169% <0%> (+1%)	⬆️
...games/strategy/triplea/delegate/BattleTracker.java	51.82% <0%> (+0.15%)	118% <0%> (+1%)	⬆️
...c/main/java/games/strategy/triplea/Properties.java	69.4% <0%> (+0.74%)	92% <0%> (+1%)	⬆️
... and 21 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 61219cc...7854cc5. Read the comment docs.

[Heppisorus]

I need a bigger thumbs up!

[ssoloff]

No code issues.

I'll try to do some compatibility testing tonight. I should be able to use any map, right? We're just trying to ensure the new field in UnitAttachment doesn't affect older clients?

[ron-murhammer]

@ssoloff Yeah any map should work for testing. UnitAttachment should be fine (almost 100% sure on that since I just added a new boolean property). I'm more concerned on the MustFightBattle and Fire changes as they are part of delegates and game data (also just very core to the engine).

[ron-murhammer]

@ssoloff I ran some manual testing and made one fix to make sure the battle display actually remove any 'isSuicideOnHit' units that do get a hit. Everything else seems to work well.

[ssoloff]

@ron-murhammer I tested the following scenarios for compatibility:

• Create a save game from this branch and load in 3635.
• Create a save game from 3635 and load in this branch.
• Network game using a server from this branch and a 3635 client.
• Network game using a 3635 server and a client from this branch.

I observed no issues during play.

The MustFightBattle and Fire changes were just logic, right? I didn't see any new fields or anything that would be persisted, so I think this PR is good from a compatibility standpoint.

[ron-murhammer]

@ssoloff Yeah, should be just logic. I purposely avoided adding/changing any member fields.