diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/BattleDelegate.java b/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/BattleDelegate.java index bea0d4e2cb5..ef174f6aa12 100644 --- a/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/BattleDelegate.java +++ b/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/BattleDelegate.java @@ -309,8 +309,7 @@ public static void sortUnitsToBombard(final List units) { */ private Map> getPossibleBombardingTerritories() { final Map> possibleBombardingTerritories = new HashMap<>(); - for (final Territory t : battleTracker.getPendingBattleSites(false)) { - final IBattle battle = battleTracker.getPendingBattle(t, BattleType.NORMAL); + for (final IBattle battle : battleTracker.getPendingBattles(BattleType.NORMAL)) { // we only care about battles where we must fight // this check is really to avoid implementing getAttackingFrom() in other battle subclasses if (!(battle instanceof MustFightBattle)) { diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/BattleTracker.java b/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/BattleTracker.java index 19199721b39..db85c41c20c 100644 --- a/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/BattleTracker.java +++ b/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/BattleTracker.java @@ -271,7 +271,7 @@ private void addBombingBattle( if (!change.isEmpty()) { throw new IllegalStateException("Non empty change"); } - // dont let land battles in the same territory occur before bombing battles + // don't let land battles in the same territory occur before bombing battles final IBattle dependent = getPendingBattle(route.getEnd(), BattleType.NORMAL); if (dependent != null) { addDependency(dependent, battle); @@ -382,7 +382,7 @@ private void addAirBattle( if (!change.isEmpty()) { throw new IllegalStateException("Non empty change"); } - // dont let land battles in the same territory occur before bombing battles + // don't let land battles in the same territory occur before bombing battles if (battleType.isBombingRun()) { final IBattle dependentAirBattle = getPendingBattle(route.getEnd(), BattleType.AIR_BATTLE); if (dependentAirBattle != null) { @@ -574,7 +574,7 @@ public void takeOver( return; } } - // If it was a Convoy Route- check ownership of the associated neighboring territory and set + // If it was a Convoy Route - check ownership of the associated neighboring territory and set // message if (ta.getConvoyRoute()) { // we could be part of a convoy route for another territory @@ -652,7 +652,7 @@ public void takeOver( + ". Player should not have been able to make this attack!"); } } - // if its a capital we take the money + // if it's a capital we take the money // NOTE: this is not checking to see if it is an enemy. // instead it is relying on the fact that the capital should be owned by the person it is // attached to @@ -871,7 +871,7 @@ public static void captureOrDestroyUnits( gamePlayer.getName() + " destroys some disabled combat units", destroyed); addChange(bridge, changeTracker, ChangeFactory.removeUnits(territory, destroyed)); } - // take over non combatants + // take over non-combatants final Predicate enemyNonCom = Matches.enemyUnit(gamePlayer).and(Matches.unitIsInfrastructure()); final Predicate willBeCaptured = @@ -928,7 +928,7 @@ public static void captureOrDestroyUnits( if (!nonCom.isEmpty()) { // FYI: a dummy delegate will not do anything with this change, // meaning that the battle calculator will think this unit lived, even though it died or was - // captured, etc! + // captured, etc.! addChange(bridge, changeTracker, ChangeFactory.changeOwner(nonCom, newOwner, territory)); addChange(bridge, changeTracker, ChangeFactory.markNoMovementChange(nonCom)); final IntegerMap damageMap = new IntegerMap<>(); @@ -990,7 +990,7 @@ private Change addMustFightBattleChange( return ChangeFactory.EMPTY_CHANGE; } IBattle battle = getPendingBattle(site, BattleType.NORMAL); - // If there are no pending battles- add one for units already in the combat zone + // If there are no pending - add one for units already in the combat zone if (battle == null) { battle = new MustFightBattle(site, gamePlayer, data, this); pendingBattles.add(battle); @@ -999,7 +999,7 @@ private Change addMustFightBattleChange( // Add the units that moved into the battle final Change change = battle.addAttackChange(route, units, null); // make amphibious assaults dependent on possible naval invasions - // its only a dependency if we are unloading + // it's only a dependency if we are unloading final IBattle precede = getDependentAmphibiousAssault(route); if (precede != null && units.stream().anyMatch(Matches.unitIsLand())) { addDependency(battle, precede); @@ -1084,6 +1084,11 @@ public Collection getPendingBattleSites(final boolean bombing) { return battles; } + public Collection getPendingBattles(BattleType type) { + return CollectionUtils.getMatches( + pendingBattles, b -> !b.isEmpty() && b.getBattleType() == type); + } + public BattleListing getPendingBattleSites() { final Map> battles = new HashMap<>(); for (final IBattle battle : pendingBattles) { @@ -1199,10 +1204,9 @@ void sendBattleRecordsToGameData(final IDelegateBridge bridge) { } /** - * 'Auto-fight' all of the air battles and strategic bombing runs. Auto fight means we - * automatically begin the fight without user action. This is to avoid clicks during the air - * battle and SBR phase, and to enforce game rules that these phases are fought first before any - * other combat. + * 'Auto-fight' all the air battles and strategic bombing runs. Auto fight means we automatically + * begin the fight without user action. This is to avoid clicks during the air battle and SBR + * phase, and to enforce game rules that these phases are fought first before any other combat. */ void fightAirRaidsAndStrategicBombing(final IDelegateBridge delegateBridge) { fightAirRaidsAndStrategicBombing( @@ -1214,12 +1218,12 @@ void fightAirRaidsAndStrategicBombing( final IDelegateBridge delegateBridge, final Supplier> pendingBattleSiteSupplier, final BiFunction pendingBattleFunction) { - // First we'll fight all of the air battles (air raids) + // First we'll fight all the air battles (air raids) // Then we will have a wave of battles for the SBR. AA guns will shoot, and we'll roll for // damage. // CAUTION: air raid battles when completed will potentially spawn new bombing raids, hence // the user of a Supplier for the param. Would be good to refactor that out, in the meantime be - // aware there are mass side effects in these calls.. + // aware there are mass side effects in these calls... for (final Territory t : pendingBattleSiteSupplier.get()) { final IBattle airRaid = pendingBattleFunction.apply(t, BattleType.AIR_RAID); @@ -1228,7 +1232,7 @@ void fightAirRaidsAndStrategicBombing( } } - // now that we've done all of the air battles, do all of the SBR's as a second wave. + // now that we've done all the air battles, do all the SBR's as a second wave. for (final Territory t : pendingBattleSiteSupplier.get()) { final IBattle bombingRaid = pendingBattleFunction.apply(t, BattleType.BOMBING_RAID); if (bombingRaid != null) { @@ -1244,8 +1248,8 @@ void fightAirRaidsAndStrategicBombing( public void fightDefenselessBattles(final IDelegateBridge bridge) { // Here and below parameter "false" to getPendingBattleSites & getPendingBattle denote non-SBR // battles - for (final Territory territory : getPendingBattleSites(false)) { - final IBattle battle = getPendingBattle(territory, BattleType.NORMAL); + for (final IBattle battle : getPendingBattles(BattleType.NORMAL)) { + final Territory territory = battle.getTerritory(); final Collection defenders = battle.getDefendingUnits(); final List possibleDefenders = getPossibleDefendingUnits(territory, defenders); if (getDependentOn(battle).isEmpty() @@ -1267,8 +1271,7 @@ public void fightDefenselessBattles(final IDelegateBridge bridge) { battle.fight(bridge); } } - getPendingBattleSites(false).stream() - .map(territory -> getPendingBattle(territory, BattleType.NORMAL)) + getPendingBattles(BattleType.NORMAL).stream() .filter(NonFightingBattle.class::isInstance) .filter(battle -> getDependentOn(battle).isEmpty()) .forEach(battle -> battle.fight(bridge)); @@ -1282,11 +1285,9 @@ private static List getPossibleDefendingUnits( /** Fight battle automatically if there is only one left to pick from. */ public void fightBattleIfOnlyOne(final IDelegateBridge bridge) { - final Collection territories = getPendingBattleSites(false); - if (territories.size() == 1) { - final IBattle battle = - getPendingBattle(CollectionUtils.getAny(territories), BattleType.NORMAL); - battle.fight(bridge); + final Collection battles = getPendingBattles(BattleType.NORMAL); + if (battles.size() == 1) { + CollectionUtils.getAny(battles).fight(bridge); } } diff --git a/game-app/game-core/src/test/java/games/strategy/triplea/delegate/RevisedTest.java b/game-app/game-core/src/test/java/games/strategy/triplea/delegate/RevisedTest.java index 768c018bb5e..410cfeb5484 100644 --- a/game-app/game-core/src/test/java/games/strategy/triplea/delegate/RevisedTest.java +++ b/game-app/game-core/src/test/java/games/strategy/triplea/delegate/RevisedTest.java @@ -732,8 +732,8 @@ void testMoveSubAwayFromSubmergedSubsInBattleZone() { String error = moveDelegate.move(sz50.getUnitCollection().getMatches(Matches.unitIsAir()), sz50To45); assertNull(error); - assertEquals( - 1, AbstractMoveDelegate.getBattleTracker(gameData).getPendingBattleSites(false).size()); + final var battleTracker = AbstractMoveDelegate.getBattleTracker(gameData); + assertEquals(1, battleTracker.getPendingBattleSites(false).size()); // we should be able to move the sub out of the sz final Route sz45To50 = new Route(sz45, sz50); final List japSub = @@ -743,16 +743,14 @@ void testMoveSubAwayFromSubmergedSubsInBattleZone() { // make sure no error assertNull(error); // make sure the battle is still there - assertEquals( - 1, AbstractMoveDelegate.getBattleTracker(gameData).getPendingBattleSites(false).size()); + assertEquals(1, battleTracker.getPendingBattleSites(false).size()); // we should be able to undo the move of the sub error = moveDelegate.undoMove(1); assertNull(error); // undo the move of the fighter, should be no battles now error = moveDelegate.undoMove(0); assertNull(error); - assertEquals( - 0, AbstractMoveDelegate.getBattleTracker(gameData).getPendingBattleSites(false).size()); + assertEquals(0, battleTracker.getPendingBattleSites(false).size()); } @Test