Skip to content
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

Use new roll dice, select casualties, and remove casualties steps #8087

Merged
merged 1 commit into from
Nov 5, 2020

Conversation

trevan
Copy link
Contributor

@trevan trevan commented Nov 3, 2020

The Fire and FireAa classes are no longer used in building the 3 fire
sub steps.

The AirVsNonSubs step is removed because that firing group is now named
and will be displayed in the battle ui when it fires, selects, and
removes.

This is the last part of #7823

This will cause the step names in the battle UI to change because the new step classes generate slightly different names. Here's an example of the old names:

BombardmentPre

Here's what it looks now:

BombardmentPost

Notice that there is a "fire", "select", and "notify" line for all of the firing groups.

Another change is how "air vs subs" is displayed in the Battle UI. Previously, a single line was shown "Air defend non subs" or "Air attack non subs". Here's an example:

AirVsSubsPre

Now, that line is no longer going to be shown and instead each of the firing groups get their own set of "fire", "select", and "remove". I've added special coding to detect the air vs subs to give it a better name. Here's what it looks like now:

AirVsNonSubs

In addition to the new names, the Battle UI will actually highlight them. So when there is different firing groups, each time the dice is rolled, it will highlight which firing group is doing it. It previously worked this way only for AA firing groups. If there were multiple other groups, it would just highlight the same " fire" line or " select casualties" until all the groups were done.

Testing

I've run a Hard AI run of WWII 1940 world for 12 rounds. No errors were found.

I ran multiple of my old saves to verify compatibility. There is potentially some issues if some of the new step names don't match the old step names. I've fixed most of them but there might be some really rare cases. If these issues do happen, the UI will just say that " wasn't found" and continue. And this will only affect the current round of the save game. So it should be very minor.

I ran AA flyover battles, strategic bombing battles, amphibious, land, sea, and sub battles.

Screens Shots

Additional Notes to Reviewer

Release Note

CHANGE|Battle step strings in Battle UI have been updated to indicate all of the possible offensive and defensive groups that are participating. Instead of one "British fire", there could be multiple "British units fire" depending on canNotTarget, canNotBeTargetedBy, subs, typeAA, etc

The Fire and FireAa classes are no longer used in building the 3 fire
sub steps.

The AirVsNonSubs step is removed because that firing group is now named
and will be displayed in the battle ui when it fires, selects, and
removes.
@codecov
Copy link

codecov bot commented Nov 4, 2020

Codecov Report

Merging #8087 into master will decrease coverage by 0.05%.
The diff coverage is 65.87%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #8087      +/-   ##
============================================
- Coverage     24.87%   24.81%   -0.06%     
+ Complexity     7313     7303      -10     
============================================
  Files          1260     1260              
  Lines         79394    79219     -175     
  Branches      11165    11132      -33     
============================================
- Hits          19747    19661      -86     
+ Misses        57550    57471      -79     
+ Partials       2097     2087      -10     
Impacted Files Coverage Δ Complexity Δ
...rategy/triplea/delegate/battle/AbstractBattle.java 54.86% <ø> (-3.14%) 36.00 <0.00> (-3.00)
...trategy/triplea/delegate/battle/BattleActions.java 33.33% <ø> (+33.33%) 1.00 <0.00> (+1.00)
.../strategy/triplea/delegate/battle/BattleState.java 86.20% <ø> (ø) 0.00 <0.00> (ø)
...a/games/strategy/triplea/delegate/battle/Fire.java 0.00% <0.00%> (-65.04%) 0.00 <0.00> (-14.00)
...games/strategy/triplea/delegate/battle/FireAa.java 0.00% <0.00%> (-89.43%) 0.00 <0.00> (-7.00)
...ategy/triplea/delegate/battle/MustFightBattle.java 63.02% <ø> (-0.55%) 117.00 <0.00> (-18.00)
...ea/delegate/battle/StrategicBombingRaidBattle.java 3.75% <0.00%> (ø) 4.00 <0.00> (ø)
...tegy/triplea/delegate/battle/steps/BattleStep.java 100.00% <ø> (ø) 1.00 <0.00> (ø)
...riplea/delegate/battle/steps/fire/FiringGroup.java 79.24% <0.00%> (-5.54%) 17.00 <0.00> (+2.00) ⬇️
.../battle/steps/fire/FiringGroupSplitterBombard.java 90.00% <ø> (+90.00%) 1.00 <0.00> (+1.00)
... and 35 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 8e49e6c...a368c50. Read the comment docs.

* units since each type of suicide on hit units need to roll separately to know which ones get
* hits.
*/
static List<Collection<Unit>> newFiringUnitGroups(final Collection<Unit> units) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method was moved from MustFightBattle without any changes.

@DanVanAtta DanVanAtta merged commit bf7a536 into triplea-game:master Nov 5, 2020
@trevan trevan deleted the use-new-fight-steps branch November 5, 2020 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants