Skip to content

Commit

Permalink
Clean up some battle code. No logic changes. (#12008)
Browse files Browse the repository at this point in the history 
* Clean up some battle code. No logic changes.

* Missing brace.

* Revert a logic change.
  • Loading branch information
@asvitkine
asvitkine authored Oct 2, 2023
1 parent 0f16441 commit 22e7c47
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 76 deletions.
124 changes: 54 additions & 70 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 @@ -86,7 +86,7 @@ public void start() {
doInitialize(battleTracker, bridge);
needToInitialize = false;
}
// do pre-combat stuff, like scrambling, after we have setup all battles, but before we have
// do pre-combat stuff, like scrambling, after we have set up all battles, but before we have
// bombardment, etc.
// the order of all of this stuff matters quite a bit.
if (needToScramble) {
Expand Down Expand Up @@ -270,7 +270,6 @@ public void addBombardmentSources() {
for (final Territory t : adjBombardment.keySet()) {
if (!battleTracker.hasPendingNonBombingBattle(t)) {
Collection<IBattle> battles = adjBombardment.get(t);
battles = CollectionUtils.getMatches(battles, Matches.battleIsAmphibious());
if (!battles.isEmpty()) {
final Collection<Unit> bombardUnits =
t.getUnitCollection().getMatches(ownedAndCanBombard);
Expand Down Expand Up @@ -317,17 +316,21 @@ private Map<Territory, Collection<IBattle>> getPossibleBombardingTerritories() {
if (!(battle instanceof MustFightBattle)) {
continue;
}
// Bombarding only allowed in amphibious battles.
if (!battle.isAmphibious()) {
continue;
}
// bombarding can only occur in territories from which at least 1 land unit attacked
final Map<Territory, Collection<Unit>> attackingFromMap =
((MustFightBattle) battle).getAttackingFromMap();
for (final Territory neighbor : ((MustFightBattle) battle).getAttackingFrom()) {
// we do not allow bombarding from certain sea zones (like if there was a kamikaze suicide
// attack there, etc)
// attack there, etc.)
if (battleTracker.noBombardAllowedFromHere(neighbor)) {
continue;
}
final Collection<Unit> neighbourUnits = attackingFromMap.get(neighbor);
// If all units from a territory are air- no bombard
// If all units from a territory are air - no bombard
if (!neighbourUnits.isEmpty() && neighbourUnits.stream().allMatch(Matches.unitIsAir())) {
continue;
}
Expand Down Expand Up @@ -399,10 +402,10 @@ private static void landParatroopers(
}

/**
* Setup the battles where the battle occurs because units are in the same territory. This happens
* when subs emerge (after being submerged), and when naval units are placed in enemy occupied sea
* zones, and also when political relationships change and potentially leave units in now-hostile
* territories.
* Set up the battles where the battle occurs because units are in the same territory. This
* happens when subs emerge (after being submerged), and when naval units are placed in enemy
* occupied sea zones, and also when political relationships change and potentially leave units in
* now-hostile territories.
*/
private static void setupUnitsInSameTerritoryBattles(
final BattleTracker battleTracker, final IDelegateBridge bridge) {
Expand All @@ -411,7 +414,7 @@ private static void setupUnitsInSameTerritoryBattles(
final boolean ignoreTransports = Properties.getIgnoreTransportInMovement(data.getProperties());
final Predicate<Unit> seaTransports =
Matches.unitIsSeaTransportButNotCombatSeaTransport().and(Matches.unitIsSea());
final Predicate<Unit> seaTranportsOrSubs = seaTransports.or(Matches.unitCanEvade());
final Predicate<Unit> seaTransportsOrSubs = seaTransports.or(Matches.unitCanEvade());
// we want to match all sea zones with our units and enemy units
final Predicate<Territory> anyTerritoryWithOwnAndEnemy =
Matches.territoryHasUnitsOwnedBy(player).and(Matches.territoryHasEnemyUnits(player));
Expand Down Expand Up @@ -585,7 +588,7 @@ private static void setupUnitsInSameTerritoryBattles(
// if only enemy transports and subs... attack them?
if (ignoreTransports
&& !enemyUnits.isEmpty()
&& enemyUnits.stream().allMatch(seaTranportsOrSubs)
&& enemyUnits.stream().allMatch(seaTransportsOrSubs)
&& !remotePlayer.selectAttackUnits(territory)) {
final BattleResults results = new BattleResults(battle, WhoWon.NOT_FINISHED, data);
battleTracker
Expand All @@ -606,7 +609,7 @@ private static void setupUnitsInSameTerritoryBattles(
}

/**
* Setup the battles where we have abandoned a contested territory during combat move to the
* Set up the battles where we have abandoned a contested territory during combat move to the
* enemy. The enemy then takes over the territory in question.
*/
private static void setupTerritoriesAbandonedToTheEnemy(
Expand Down Expand Up @@ -672,15 +675,12 @@ private static void setupTerritoriesAbandonedToTheEnemy(
+ territory.getName()
+ " and have abandoned or only bombing there too.");
}
bridge
.getHistoryWriter()
.startEvent(
player.getName()
+ " has abandoned "
+ territory.getName()
+ " to "
+ abandonedToPlayer.getName(),
abandonedToUnits);
final var historyWriter = bridge.getHistoryWriter();
historyWriter.startEvent(
String.format(
"%s has abandoned %s to %s",
player.getName(), territory.getName(), abandonedToPlayer.getName()),
abandonedToUnits);
battleTracker.takeOver(territory, abandonedToPlayer, bridge, null, abandonedToUnits);
// TODO: if there are multiple defending unit owners, allow picking which one takes over the
// territory
Expand Down Expand Up @@ -749,14 +749,14 @@ private void doScrambling() {
}
// verify max allowed
for (final Territory t : scramblers.keySet()) {
if (toScramble.get(t) == null) {
Collection<Unit> units = toScramble.get(t);
if (units == null) {
continue;
}
if (toScramble.get(t).size()
> ScrambleLogic.getMaxScrambleCount(scramblers.get(t).getFirst())) {
if (units.size() > ScrambleLogic.getMaxScrambleCount(scramblers.get(t).getFirst())) {
throw new IllegalStateException(
"Trying to scramble "
+ toScramble.get(t).size()
+ units.size()
+ " out of "
+ t.getName()
+ ", but max allowed is "
Expand Down Expand Up @@ -829,17 +829,12 @@ private void doScrambling() {
}
// should we mark combat, or call setupUnitsInSameTerritoryBattles again?
change.add(ChangeFactory.moveUnits(t, to, scrambling));
bridge
.getHistoryWriter()
.startEvent(
defender.getName()
+ " scrambles "
+ scrambling.size()
+ " units out of "
+ t.getName()
+ " to defend against the attack in "
+ to.getName(),
scrambling);
final var historyWriter = bridge.getHistoryWriter();
historyWriter.startEvent(
String.format(
"%s scrambles %d units out of %s to defend against the attack in %s",
defender.getName(), scrambling.size(), t.getName(), to.getName()),
scrambling);
scrambledHere = true;
}
if (!change.isEmpty()) {
Expand All @@ -863,10 +858,9 @@ private void doScrambling() {
if (attackingUnits.isEmpty()) {
continue;
}
bridge
.getHistoryWriter()
.startEvent(
defender.getName() + " scrambles to create a battle in territory " + to.getName());
final var historyWriter = bridge.getHistoryWriter();
historyWriter.startEvent(
defender.getName() + " scrambles to create a battle in territory " + to.getName());
// TODO: the attacking sea units do not remember where they came from, so they cannot
// retreat anywhere. Need to fix.
battleTracker.addBattle(new RouteScripted(to), attackingUnits, player, bridge, null, null);
Expand All @@ -886,13 +880,9 @@ private void doScrambling() {
// TODO: for now, we will hack and say that the attackers came from Everywhere, and hope
// the user will choose the correct place to retreat to! (TODO: Fix this)
final Map<Territory, Collection<Unit>> attackingFromMap = new HashMap<>();
final Collection<Territory> neighbors =
data.getMap()
.getNeighbors(
to,
(Matches.territoryIsLand().test(to)
? Matches.territoryIsLand()
: Matches.territoryIsWater()));
final Predicate<Territory> predicate =
to.isWater() ? Matches.territoryIsWater() : Matches.territoryIsLand();
final Collection<Territory> neighbors = data.getMap().getNeighbors(to, predicate);
// neighbors.removeAll(territoriesWithBattles);
// neighbors.removeAll(Matches.getMatches(neighbors,
// Matches.territoryHasEnemyUnits(player, data)));
Expand Down Expand Up @@ -937,23 +927,20 @@ private void scramblingCleanup() {
final Collection<Unit> wasScrambled =
t.getUnitCollection().getMatches(Matches.unitWasScrambled());
for (final Unit u : wasScrambled) {
final GamePlayer owner = u.getOwner();
final CompositeChange change = new CompositeChange();
Territory landingTerr = null;
final String historyText;
if (!mustReturnToBase
|| !Matches.isTerritoryAllied(u.getOwner()).test(u.getOriginatedFrom())) {
final Collection<Territory> possible =
whereCanAirLand(u, t, u.getOwner(), data, battleTracker, carrierCostOfCurrentTerr);
whereCanAirLand(u, t, owner, data, battleTracker, carrierCostOfCurrentTerr);
if (possible.size() > 1) {
landingTerr =
getRemotePlayer(u.getOwner())
.selectTerritoryForAirToLand(
possible,
t,
"Select territory for air units to land. (Current territory is "
+ t.getName()
+ "): "
+ MyFormatter.unitsToText(List.of(u)));
String text =
String.format(
"Select territory for air units to land. (Current territory is %s): %s",
t.getName(), MyFormatter.unitsToText(List.of(u)));
landingTerr = getRemotePlayer(owner).selectTerritoryForAirToLand(possible, t, text);
} else if (possible.size() == 1) {
landingTerr = CollectionUtils.getAny(possible);
}
Expand All @@ -967,15 +954,13 @@ private void scramblingCleanup() {
} else {
landingTerr = u.getOriginatedFrom();
historyText =
"Moving scrambled unit from "
+ t.getName()
+ " back to originating territory: "
+ landingTerr.getName();
String.format(
"Moving scrambled unit from %s back to originating territory: %s",
t.getName(), landingTerr.getName());
}
if (landingTerr != null && !landingTerr.equals(t)) {
change.add(ChangeFactory.moveUnits(t, landingTerr, List.of(u)));
change.add(
Route.getFuelChanges(Set.of(u), new Route(t, landingTerr), u.getOwner(), data));
change.add(Route.getFuelChanges(Set.of(u), new Route(t, landingTerr), owner, data));
}
change.add(ChangeFactory.unitPropertyChange(u, null, Unit.ORIGINATED_FROM));
change.add(ChangeFactory.unitPropertyChange(u, false, Unit.WAS_SCRAMBLED));
Expand Down Expand Up @@ -1132,22 +1117,21 @@ private void checkDefendingPlanesCanLand() {
}
}
} else if (!canLandHere.isEmpty()) {
// now defending air has what cant stay, is there a place we can go?
// now defending air has what can't stay, is there a place we can go?
// check for an island in this sea zone
for (final Territory currentTerritory : canLandHere) {
// only one neighbor, its an island.
// only one neighbor, it's an island.
if (data.getMap().getNeighbors(currentTerritory).size() == 1) {
moveAirAndLand(bridge, defendingAir, defendingAir, currentTerritory, battleSite);
}
}
}
if (!defendingAir.isEmpty()) {
// no where to go, they must die
bridge
.getHistoryWriter()
.addChildToEvent(
MyFormatter.unitsToText(defendingAir) + " could not land and were killed",
new ArrayList<>(defendingAir));
// nowhere to go, they must die
final var historyWriter = bridge.getHistoryWriter();
historyWriter.addChildToEvent(
MyFormatter.unitsToText(defendingAir) + " could not land and were killed",
new ArrayList<>(defendingAir));
final Change change = ChangeFactory.removeUnits(battleSite, defendingAir);
bridge.addChange(change);
}
Expand Down
2 changes: 1 addition & 1 deletion game-app/game-headed/src/main/java/games/strategy/triplea/TripleAPlayer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ public Territory selectBombardingTerritory(
final Territory unitTerritory,
final Collection<Territory> territories,
final boolean noneAvailable) {
return ui.getBattlePanel().getBombardment(unit, unitTerritory, territories, noneAvailable);
return ui.getBattlePanel().getBombardment(unit, unitTerritory, territories);
}

@Override
Expand Down
7 changes: 2 additions & 5 deletions game-app/game-headed/src/main/java/games/strategy/triplea/ui/BattlePanel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,10 +288,7 @@ FightBattleDetails waitForBattleSelection() {

/** Ask user which territory to bombard with a given unit. */
public @Nullable Territory getBombardment(
final Unit unit,
final Territory unitTerritory,
final Collection<Territory> territories,
final boolean noneAvailable) {
final Unit unit, final Territory unitTerritory, final Collection<Territory> territories) {
final Supplier<SelectTerritoryComponent> action =
() -> {
final var panel = new SelectTerritoryComponent(unitTerritory, territories, map);
Expand All @@ -311,7 +308,7 @@ FightBattleDetails waitForBattleSelection() {
JOptionPane.OK_CANCEL_OPTION,
JOptionPane.PLAIN_MESSAGE,
null,
noneAvailable ? new String[] {"OK", "None"} : new String[] {"OK"},
new String[] {"OK", "None"},
null,
getMap().getUiContext().getCountDownLatchHandler());
if (choice != JOptionPane.OK_OPTION) {
Expand Down

0 comments on commit 22e7c47

Please sign in to comment.