Skip to content

Commit

Permalink
Fix duplicate territory listed for amphib bombardment
Browse files Browse the repository at this point in the history
  • Loading branch information
@asvitkine
asvitkine committed Oct 2, 2023
1 parent 22e7c47 commit e4d3fd3
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 32 deletions.
3 changes: 1 addition & 2 deletions game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/BattleDelegate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,7 @@ public static void sortUnitsToBombard(final List<Unit> units) {
*/
private Map<Territory, Collection<IBattle>> getPossibleBombardingTerritories() {
final Map<Territory, Collection<IBattle>> 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)) {
Expand Down
49 changes: 25 additions & 24 deletions game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/BattleTracker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<Unit> enemyNonCom =
Matches.enemyUnit(gamePlayer).and(Matches.unitIsInfrastructure());
final Predicate<Unit> willBeCaptured =
Expand Down Expand Up @@ -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<Unit> damageMap = new IntegerMap<>();
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -1084,6 +1084,11 @@ public Collection<Territory> getPendingBattleSites(final boolean bombing) {
return battles;
}

public Collection<IBattle> getPendingBattles(BattleType type) {
return CollectionUtils.getMatches(
pendingBattles, b -> !b.isEmpty() && b.getBattleType() == type);
}

public BattleListing getPendingBattleSites() {
final Map<BattleType, Collection<Territory>> battles = new HashMap<>();
for (final IBattle battle : pendingBattles) {
Expand Down Expand Up @@ -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(
Expand All @@ -1214,12 +1218,12 @@ void fightAirRaidsAndStrategicBombing(
final IDelegateBridge delegateBridge,
final Supplier<Collection<Territory>> pendingBattleSiteSupplier,
final BiFunction<Territory, BattleType, IBattle> 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);
Expand All @@ -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) {
Expand All @@ -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<Unit> defenders = battle.getDefendingUnits();
final List<Unit> possibleDefenders = getPossibleDefendingUnits(territory, defenders);
if (getDependentOn(battle).isEmpty()
Expand All @@ -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));
Expand All @@ -1282,11 +1285,9 @@ private static List<Unit> getPossibleDefendingUnits(

/** Fight battle automatically if there is only one left to pick from. */
public void fightBattleIfOnlyOne(final IDelegateBridge bridge) {
final Collection<Territory> territories = getPendingBattleSites(false);
if (territories.size() == 1) {
final IBattle battle =
getPendingBattle(CollectionUtils.getAny(territories), BattleType.NORMAL);
battle.fight(bridge);
final Collection<IBattle> battles = getPendingBattles(BattleType.NORMAL);
if (battles.size() == 1) {
CollectionUtils.getAny(battles).fight(bridge);
}
}

Expand Down
10 changes: 4 additions & 6 deletions game-app/game-core/src/test/java/games/strategy/triplea/delegate/RevisedTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Unit> japSub =
Expand All @@ -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
Expand Down

0 comments on commit e4d3fd3

Please sign in to comment.