Skip to content

Commit

Permalink
Unit handling code clean ups. (#12527)
Browse files Browse the repository at this point in the history
Introduces a helper that makes code more concise and moves some predicate logic outside of loops. No logic changes.
  • Loading branch information
asvitkine committed Apr 21, 2024
1 parent 0cb7e13 commit 58fc1d5
Show file tree
Hide file tree
Showing 32 changed files with 186 additions and 273 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package games.strategy.engine.data;

import java.util.Optional;
import java.util.function.Predicate;
import javax.annotation.Nullable;
import lombok.Getter;

Expand Down Expand Up @@ -33,11 +32,6 @@ public Territory(final String name, final boolean water, final GameData data) {
unitCollection = new UnitCollection(this, getData());
}

@Override
public final boolean anyUnitsMatch(final Predicate<Unit> matcher) {
return getUnitCollection().anyMatch(matcher);
}

public void setOwner(final @Nullable GamePlayer owner) {
this.owner = Optional.ofNullable(owner).orElse(getData().getPlayerList().getNullPlayer());
getData().notifyTerritoryOwnerChanged(this);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package games.strategy.engine.data;

import java.util.Collection;
import java.util.List;
import java.util.function.Predicate;

/** An object that contains a collection of {@link Unit}s. */
Expand All @@ -21,4 +22,8 @@ default Collection<Unit> getUnits() {
default boolean anyUnitsMatch(final Predicate<Unit> matcher) {
return getUnitCollection().anyMatch(matcher);
}

default List<Unit> getMatches(final Predicate<Unit> matcher) {
return getUnitCollection().getMatches(matcher);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -674,8 +674,7 @@ private List<Unit> moveOneDefenderToLandTerritoriesBorderingEnemy(
int minCost = Integer.MAX_VALUE;
Unit minUnit = null;
for (final Unit u :
t.getUnitCollection()
.getMatches(Matches.unitIsOwnedBy(player).and(Matches.unitIsNotInfrastructure()))) {
t.getMatches(Matches.unitIsOwnedBy(player).and(Matches.unitIsNotInfrastructure()))) {
if (proData.getUnitValue(u.getType()) < minCost) {
minCost = proData.getUnitValue(u.getType());
minUnit = u;
Expand Down Expand Up @@ -760,10 +759,7 @@ private void removeTerritoriesWhereTransportsAreExposed() {
for (final Territory unloadTerritory : territoryTransportAndBombardMap.keySet()) {
if (enemyAttackOptions.getMax(unloadTerritory) != null) {
final Set<Unit> defenders =
new HashSet<>(
unloadTerritory
.getUnitCollection()
.getMatches(ProMatches.unitIsAlliedNotOwned(player)));
new HashSet<>(unloadTerritory.getMatches(ProMatches.unitIsAlliedNotOwned(player)));
defenders.addAll(territoryTransportAndBombardMap.get(unloadTerritory));
if (defendMap.get(unloadTerritory) != null) {
defenders.addAll(defendMap.get(unloadTerritory).getMaxUnits());
Expand Down Expand Up @@ -1075,9 +1071,8 @@ private void determineUnitsToAttackWith(
int capturableUnits = 0;
if (Matches.territoryIsLand().test(t)) {
capturableUnits =
CollectionUtils.countMatches(
t.getUnitCollection(),
Matches.unitCanBeCapturedOnEnteringThisTerritory(player, t));
t.getUnitCollection()
.countMatches(Matches.unitCanBeCapturedOnEnteringThisTerritory(player, t));
}
final int isFfa = ProUtils.isFfa(data, player) ? 1 : 0;
final int production = TerritoryAttachment.getProduction(t);
Expand Down Expand Up @@ -1201,7 +1196,7 @@ private void determineBestBombingAttackForBomber(
Optional<Territory> maxBombingTerritory = Optional.empty();
int maxBombingScore = MIN_BOMBING_SCORE;
for (final Territory t : bomberTargetTerritories) {
final List<Unit> targetUnits = t.getUnitCollection().getMatches(bombingTargetMatch);
final List<Unit> targetUnits = t.getMatches(bombingTargetMatch);
if (!targetUnits.isEmpty() && canAirSafelyLandAfterAttack(bomber, t)) {
final int noAaBombingDefense =
t.anyUnitsMatch(Matches.unitIsAaForBombingThisUnitOnly()) ? 0 : 1;
Expand Down Expand Up @@ -1652,7 +1647,7 @@ private Map<Unit, Set<Territory>> tryToAttackTerritories(
attackers = enemyAttackOptions.getMax(destination).getMaxUnits();
}
final List<Unit> defenders =
destination.getUnitCollection().getMatches(Matches.isUnitAllied(player));
destination.getMatches(Matches.isUnitAllied(player));
defenders.add(transport);
final double strengthDifference =
ProBattleUtils.estimateStrengthDifference(
Expand Down Expand Up @@ -1816,13 +1811,10 @@ private void removeAttacksUntilCapitalCanBeHeld(
// Find max remaining defenders
final Set<Territory> territoriesAdjacentToCapital =
data.getMap().getNeighbors(myCapital, Matches.territoryIsLand());
final List<Unit> defenders =
myCapital.getUnitCollection().getMatches(Matches.isUnitAllied(player));
final List<Unit> defenders = myCapital.getMatches(Matches.isUnitAllied(player));
defenders.addAll(placeUnits);
for (final Territory t : territoriesAdjacentToCapital) {
defenders.addAll(
t.getUnitCollection()
.getMatches(ProMatches.unitCanBeMovedAndIsOwnedLand(player, false)));
defenders.addAll(t.getMatches(ProMatches.unitCanBeMovedAndIsOwnedLand(player, false)));
}
for (final ProTerritory t : attackMap.values()) {
defenders.removeAll(t.getUnits());
Expand Down Expand Up @@ -1905,8 +1897,7 @@ private void checkContestedSeaTerritories() {
if (!possibleMoveTerritories.isEmpty()) {
final Territory moveToTerritory = CollectionUtils.getAny(possibleMoveTerritories);
final List<Unit> mySeaUnits =
t.getUnitCollection()
.getMatches(ProMatches.unitCanBeMovedAndIsOwnedSea(player, true));
t.getMatches(ProMatches.unitCanBeMovedAndIsOwnedSea(player, true));
proData.getProTerritory(attackMap, moveToTerritory).addUnits(mySeaUnits);
ProLogger.info(t + " is a contested territory so moving subs to " + moveToTerritory);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,10 +241,9 @@ private void findUnitsThatCantMove(
final ProTerritory proTerritory = moveMap.get(t);
Preconditions.checkState(proTerritory.getCantMoveUnits().isEmpty());
final Collection<Unit> cantMoveUnits =
t.getUnitCollection()
.getMatches(
ProMatches.unitCantBeMovedAndIsAlliedDefender(player, t)
.or(proData.getUnitsToBeConsumed()::contains));
t.getMatches(
ProMatches.unitCantBeMovedAndIsAlliedDefender(player, t)
.or(proData.getUnitsToBeConsumed()::contains));
proTerritory.addCantMoveUnits(cantMoveUnits);
}

Expand Down Expand Up @@ -2170,7 +2169,7 @@ private void moveAlliedCarriedFighters(Unit u, ProTerritory to) {
if (Matches.unitIsCarrier().test(u)) {
final Territory unitTerritory = unitTerritoryMap.get(u);
final Map<Unit, Collection<Unit>> carrierMustMoveWith =
MoveValidator.carrierMustMoveWith(unitTerritory.getUnits(), unitTerritory, player);
MoveValidator.carrierMustMoveWith(unitTerritory, player);
Optional.ofNullable(carrierMustMoveWith.get(u))
.ifPresent(fighters -> to.getTempUnits().addAll(fighters));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,7 @@ private void findDefendersInPlaceTerritories(
for (final ProPurchaseTerritory ppt : purchaseTerritories.values()) {
for (final ProPlaceTerritory placeTerritory : ppt.getCanPlaceTerritories()) {
final Territory t = placeTerritory.getTerritory();
final List<Unit> units = t.getUnitCollection().getMatches(Matches.isUnitAllied(player));
final List<Unit> units = t.getMatches(Matches.isUnitAllied(player));
placeTerritory.setDefendingUnits(units);
ProLogger.debug(t + " has numDefenders=" + units.size());
}
Expand Down Expand Up @@ -741,8 +741,7 @@ private void purchaseDefenders(
+ summarizeUnits(placeTerritory.getDefendingUnits()));

// Find local owned units
final List<Unit> ownedLocalUnits =
t.getUnitCollection().getMatches(Matches.unitIsOwnedBy(player));
final List<Unit> ownedLocalUnits = t.getMatches(Matches.unitIsOwnedBy(player));
int unusedCarrierCapacity =
Math.min(0, ProTransportUtils.getUnusedCarrierCapacity(player, t, new ArrayList<>()));
int unusedLocalCarrierCapacity =
Expand Down Expand Up @@ -1057,7 +1056,7 @@ private void purchaseLandUnits(
final List<ProPlaceTerritory> prioritizedLandTerritories,
final ProPurchaseOptionMap purchaseOptions) {

final List<Unit> unplacedUnits = player.getUnitCollection().getMatches(Matches.unitIsNotSea());
final List<Unit> unplacedUnits = player.getMatches(Matches.unitIsNotSea());
if (resourceTracker.isEmpty() && unplacedUnits.isEmpty()) {
return;
}
Expand Down Expand Up @@ -1116,8 +1115,7 @@ private void purchaseLandUnits(
neighbors.add(t);
final List<Unit> ownedLocalUnits = new ArrayList<>();
for (final Territory neighbor : neighbors) {
ownedLocalUnits.addAll(
neighbor.getUnitCollection().getMatches(Matches.unitIsOwnedBy(player)));
ownedLocalUnits.addAll(neighbor.getMatches(Matches.unitIsOwnedBy(player)));
}

// Check for unplaced units
Expand Down Expand Up @@ -1275,7 +1273,7 @@ private void purchaseFactory(
} else {

// Find current battle result
final List<Unit> defenders = t.getUnitCollection().getMatches(Matches.isUnitAllied(player));
final List<Unit> defenders = t.getMatches(Matches.isUnitAllied(player));
final Set<Unit> enemyAttackingUnits =
new HashSet<>(enemyAttackOptions.getMax(t).getMaxUnits());
enemyAttackingUnits.addAll(enemyAttackOptions.getMax(t).getMaxAmphibUnits());
Expand Down Expand Up @@ -1547,8 +1545,7 @@ private boolean purchaseSeaAndAmphibUnits(
neighbors.add(t);
final List<Unit> ownedLocalUnits = new ArrayList<>();
for (final Territory neighbor : neighbors) {
ownedLocalUnits.addAll(
neighbor.getUnitCollection().getMatches(Matches.unitIsOwnedBy(player)));
ownedLocalUnits.addAll(neighbor.getMatches(Matches.unitIsOwnedBy(player)));
}
int unusedCarrierCapacity =
Math.min(0, ProTransportUtils.getUnusedCarrierCapacity(player, t, List.of()));
Expand Down Expand Up @@ -1741,13 +1738,12 @@ private boolean purchaseSeaAndAmphibUnits(
final List<Unit> enemyUnitsInLandTerritories = new ArrayList<>();
for (final Territory nearbyLandTerritory : nearbyLandTerritories) {
enemyUnitsInLandTerritories.addAll(
nearbyLandTerritory.getUnitCollection().getMatches(ProMatches.unitIsEnemyAir(player)));
nearbyLandTerritory.getMatches(ProMatches.unitIsEnemyAir(player)));
}
final Predicate<Unit> enemyNonLandUnit = ProMatches.unitIsEnemyNotLand(player);
final List<Unit> enemyUnitsInSeaTerritories = new ArrayList<>();
for (final Territory nearbySeaTerritory : nearbyEnemySeaTerritories) {
final List<Unit> enemySeaUnits =
nearbySeaTerritory.getUnitCollection().getMatches(enemyNonLandUnit);
final List<Unit> enemySeaUnits = nearbySeaTerritory.getMatches(enemyNonLandUnit);
if (enemySeaUnits.isEmpty()) {
continue;
}
Expand All @@ -1770,9 +1766,7 @@ private boolean purchaseSeaAndAmphibUnits(
final List<Unit> myUnitsInSeaTerritories = new ArrayList<>();
for (final Territory nearbySeaTerritory : nearbyAlliedSeaTerritories) {
myUnitsInSeaTerritories.addAll(
nearbySeaTerritory
.getUnitCollection()
.getMatches(ProMatches.unitIsOwnedNotLand(player)));
nearbySeaTerritory.getMatches(ProMatches.unitIsOwnedNotLand(player)));
myUnitsInSeaTerritories.addAll(
ProPurchaseUtils.getPlaceUnits(nearbySeaTerritory, purchaseTerritories));
}
Expand Down Expand Up @@ -1930,7 +1924,7 @@ private boolean purchaseSeaAndAmphibUnits(

// Find local owned units
final List<Unit> ownedLocalAmphibUnits =
landTerritory.getUnitCollection().getMatches(Matches.unitIsOwnedBy(player));
landTerritory.getMatches(Matches.unitIsOwnedBy(player));

// Determine sea and transport units that can be produced in this territory
final List<ProPurchaseOption> seaTransportPurchaseOptionsForTerritory =
Expand Down Expand Up @@ -2208,8 +2202,7 @@ private void purchaseUnitsWithRemainingProduction(
ProLogger.debug("Checking territory: " + t);

// Find local owned units
final List<Unit> ownedLocalUnits =
t.getUnitCollection().getMatches(Matches.unitIsOwnedBy(player));
final List<Unit> ownedLocalUnits = t.getMatches(Matches.unitIsOwnedBy(player));

// Determine units that can be produced in this territory
final List<ProPurchaseOption> airAndLandPurchaseOptions = new ArrayList<>(airPurchaseOptions);
Expand Down Expand Up @@ -2415,7 +2408,7 @@ private IntegerMap<ProductionRule> populateProductionRuleMap(
final ProPurchaseOptionMap purchaseOptions) {

ProLogger.info("Populate production rule map");
final List<Unit> unplacedUnits = player.getUnitCollection().getMatches(Matches.unitIsNotSea());
final List<Unit> unplacedUnits = player.getMatches(Matches.unitIsNotSea());
final IntegerMap<ProductionRule> purchaseMap = new IntegerMap<>();
for (final ProPurchaseOption ppo : purchaseOptions.getAllOptions()) {
final int numUnits =
Expand Down Expand Up @@ -2460,8 +2453,7 @@ private void placeDefenders(

// Check if any units can be placed
final PlaceableUnits placeableUnits =
placeDelegate.getPlaceableUnits(
player.getUnitCollection().getMatches(Matches.unitIsNotConstruction()), t);
placeDelegate.getPlaceableUnits(player.getMatches(Matches.unitIsNotConstruction()), t);
if (placeableUnits.isError()) {
ProLogger.trace(t + " can't place units with error: " + placeableUnits.getErrorMessage());
continue;
Expand Down Expand Up @@ -2542,7 +2534,7 @@ private void placeUnits(

// Check if any units can be placed
final PlaceableUnits placeableUnits =
placeDelegate.getPlaceableUnits(player.getUnitCollection().getMatches(unitMatch), t);
placeDelegate.getPlaceableUnits(player.getMatches(unitMatch), t);
if (placeableUnits.isError()) {
ProLogger.trace(t + " can't place units with error: " + placeableUnits.getErrorMessage());
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,7 @@ Territory retreatQuery(
}
final double strength =
ProBattleUtils.estimateStrength(
t,
t.getUnitCollection().getMatches(Matches.isUnitAllied(player)),
new ArrayList<>(),
false);
t, t.getMatches(Matches.isUnitAllied(player)), new ArrayList<>(), false);
if (strength > maxStrength) {
retreatTerritory = t;
maxStrength = strength;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@ static void tech(final ITechDelegate techDelegate, final GameData data, final Ga
TerritoryAttachment.getFirstOwnedCapitalOrFirstUnownedCapital(player, data.getMap());
final float enemyStrength = getStrengthOfPotentialAttackers(myCapitol, data, player);
float myStrength =
(myCapitol == null || myCapitol.getUnitCollection() == null)
? 0.0F
: strength(myCapitol.getUnits(), false, false, false);
(myCapitol == null) ? 0.0F : strength(myCapitol.getUnits(), false, false, false);
final List<Territory> areaStrength = getNeighboringLandTerritories(data, player, myCapitol);
for (final Territory areaTerr : areaStrength) {
myStrength += strength(areaTerr.getUnits(), false, false, false) * 0.75F;
Expand Down Expand Up @@ -131,8 +129,7 @@ private static float getStrengthOfPotentialAttackers(
// reality is 99% of time units considered will have full move.
// and likely player will have at least 1 max move plane.
for (final Territory enemyFighterTerritory : enemyFighterTerritories) {
final List<Unit> enemyFighterUnits =
enemyFighterTerritory.getUnitCollection().getMatches(enemyPlane);
final List<Unit> enemyFighterUnits = enemyFighterTerritory.getMatches(enemyPlane);
maxFighterDistance =
Math.max(
maxFighterDistance, MoveValidator.getMaxMovement(enemyFighterUnits).intValue());
Expand All @@ -146,8 +143,7 @@ private static float getStrengthOfPotentialAttackers(
final List<Territory> enemyTransportTerritories = findUnitTerr(data, transport);
int maxTransportDistance = 0;
for (final Territory enemyTransportTerritory : enemyTransportTerritories) {
final List<Unit> enemyTransportUnits =
enemyTransportTerritory.getUnitCollection().getMatches(transport);
final List<Unit> enemyTransportUnits = enemyTransportTerritory.getMatches(transport);
maxTransportDistance =
Math.max(
maxTransportDistance, MoveValidator.getMaxMovement(enemyTransportUnits).intValue());
Expand All @@ -160,8 +156,7 @@ private static float getStrengthOfPotentialAttackers(
data.getMap()
.getNeighbors(
location, onWater ? Matches.territoryIsWater() : Matches.territoryIsLand())) {
final List<Unit> enemies =
t.getUnitCollection().getMatches(Matches.unitIsOwnedBy(enemyPlayer));
final List<Unit> enemies = t.getMatches(Matches.unitIsOwnedBy(enemyPlayer));
enemyWaterUnits.addAll(enemies);
firstStrength += strength(enemies, true, onWater, transportsFirst);
checked.add(t);
Expand Down Expand Up @@ -204,7 +199,7 @@ private static float getStrengthOfPotentialAttackers(
if (enemyPlayer == null) {
continue;
}
final List<Unit> transports = t4.getUnitCollection().getMatches(enemyTransport);
final List<Unit> transports = t4.getMatches(enemyTransport);
if (transports.isEmpty()) {
continue;
}
Expand Down Expand Up @@ -238,8 +233,7 @@ private static float getStrengthOfPotentialAttackers(
final Set<Territory> transNeighbors =
data.getMap().getNeighbors(t4, Matches.isTerritoryAllied(enemyPlayer));
for (final Territory transNeighbor : transNeighbors) {
final List<Unit> transUnits =
transNeighbor.getUnitCollection().getMatches(enemyTransportable);
final List<Unit> transUnits = transNeighbor.getMatches(enemyTransportable);
transUnits.removeAll(alreadyLoaded);
final List<Unit> availTransUnits = sortTransportUnits(transUnits);
for (final Unit transUnit : availTransUnits) {
Expand Down Expand Up @@ -439,7 +433,7 @@ private static List<Unit> findAttackers(
if (ignoreDistance.contains(dist)) {
continue;
}
units.addAll(neighbor.getUnitCollection().getMatches(unitCondition));
units.addAll(neighbor.getMatches(unitCondition));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,7 @@ public List<Unit> getMaxDefenders() {
}

public List<Unit> getMaxEnemyDefenders(final GamePlayer player) {
final List<Unit> defenders =
territory.getUnitCollection().getMatches(Matches.enemyUnit(player));
final List<Unit> defenders = territory.getMatches(Matches.enemyUnit(player));
defenders.addAll(maxScrambleUnits);
return defenders;
}
Expand Down

0 comments on commit 58fc1d5

Please sign in to comment.