Some small code clean ups.

triplea-game · Aug 27, 2023 · 65ac957 · 65ac957
1 parent ebe1d4c
commit 65ac957
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 56 deletions.
diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/ProCombatMoveAi.java b/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/ProCombatMoveAi.java
@@ -434,12 +434,12 @@ private void determineTerritoriesThatCanBeHeld(
       }
 
       // Set max enemy attackers
-      if (enemyAttackOptions.getMax(t) != null) {
-        final Set<Unit> enemyAttackingUnits =
-            new HashSet<>(enemyAttackOptions.getMax(t).getMaxUnits());
-        enemyAttackingUnits.addAll(enemyAttackOptions.getMax(t).getMaxAmphibUnits());
-        patd.setMaxEnemyUnits(new ArrayList<>(enemyAttackingUnits));
-        patd.setMaxEnemyBombardUnits(enemyAttackOptions.getMax(t).getMaxBombardUnits());
+      final ProTerritory enemyAttackMax = enemyAttackOptions.getMax(t);
+      if (enemyAttackMax != null) {
+        final Set<Unit> enemyAttackingUnits = new HashSet<>(enemyAttackMax.getMaxUnits());
+        enemyAttackingUnits.addAll(enemyAttackMax.getMaxAmphibUnits());
+        patd.setMaxEnemyUnits(enemyAttackingUnits);
+        patd.setMaxEnemyBombardUnits(enemyAttackMax.getMaxBombardUnits());
       }
 
       // Add strategic value for factories

diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/ProNonCombatMoveAi.java b/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/ProNonCombatMoveAi.java
@@ -400,11 +400,11 @@ private void determineIfMoveTerritoriesCanBeHeld() {
       }
 
       // Check if min defenders can hold it (not considering AA)
-      final Set<Unit> enemyAttackingUnits =
-          new HashSet<>(enemyAttackOptions.getMax(t).getMaxUnits());
-      enemyAttackingUnits.addAll(enemyAttackOptions.getMax(t).getMaxAmphibUnits());
-      patd.setMaxEnemyUnits(new ArrayList<>(enemyAttackingUnits));
-      patd.setMaxEnemyBombardUnits(enemyAttackOptions.getMax(t).getMaxBombardUnits());
+      final ProTerritory enemyAttackMax = enemyAttackOptions.getMax(t);
+      final Set<Unit> enemyAttackingUnits = new HashSet<>(enemyAttackMax.getMaxUnits());
+      enemyAttackingUnits.addAll(enemyAttackMax.getMaxAmphibUnits());
+      patd.setMaxEnemyUnits(enemyAttackingUnits);
+      patd.setMaxEnemyBombardUnits(enemyAttackMax.getMaxBombardUnits());
       final List<Unit> minDefendingUnitsAndNotAa =
           CollectionUtils.getMatches(
               patd.getCantMoveUnits(), Matches.unitIsAaForAnything().negate());
@@ -414,7 +414,7 @@ private void determineIfMoveTerritoriesCanBeHeld() {
               t,
               enemyAttackingUnits,
               minDefendingUnitsAndNotAa,
-              enemyAttackOptions.getMax(t).getMaxBombardUnits());
+              enemyAttackMax.getMaxBombardUnits());
       patd.setMinBattleResult(minResult);
       if (minResult.getTuvSwing() <= 0 && !minDefendingUnitsAndNotAa.isEmpty()) {
         ProLogger.debug(

diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/data/ProTerritory.java b/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/data/ProTerritory.java
@@ -344,8 +344,8 @@ public void addCantMoveUnits(final Collection<Unit> units) {
     this.cantMoveUnits.addAll(units);
   }
 
-  public void setMaxEnemyUnits(final List<Unit> maxEnemyUnits) {
-    this.maxEnemyUnits = maxEnemyUnits;
+  public void setMaxEnemyUnits(final Collection<Unit> maxEnemyUnits) {
+    this.maxEnemyUnits = new ArrayList<>(maxEnemyUnits);
   }
 
   public List<Unit> getMaxEnemyUnits() {

diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/delegate/AbstractPlaceDelegate.java b/game-app/game-core/src/main/java/games/strategy/triplea/delegate/AbstractPlaceDelegate.java
@@ -313,7 +313,7 @@ private void performPlaceFrom(
       final GamePlayer player) {
     final CompositeChange change = new CompositeChange();
     // make sure we can place consuming units
-    final boolean didIt = canWeConsumeUnits(placeableUnits, at, true, change);
+    final boolean didIt = canWeConsumeUnits(placeableUnits, at, change);
     if (!didIt) {
       throw new IllegalStateException("Something wrong with consuming/upgrading units");
     }
@@ -844,7 +844,7 @@ public String canUnitsBePlaced(
       }
     }
     // make sure we can place consuming units
-    if (!canWeConsumeUnits(units, to, false, null)) {
+    if (!canWeConsumeUnits(units, to, null)) {
       return "Not Enough Units To Upgrade or Be Consumed";
     }
     // now return null (valid placement) if we have placement restrictions disabled in game options
@@ -992,30 +992,19 @@ private Predicate<Unit> unitIsCarrierOwnedByCombinedPlayers(GamePlayer player) {
   }
 
   private boolean canWeConsumeUnits(
-      final Collection<Unit> units,
-      final Territory to,
-      final boolean actuallyDoIt,
-      final CompositeChange change) {
-    boolean weCanConsume = true;
+      final Collection<Unit> units, final Territory to, final CompositeChange change) {
     final Collection<Unit> unitsAtStartOfTurnInTo = unitsAtStartOfStepInTerritory(to);
     final Collection<Unit> removedUnits = new ArrayList<>();
     final Collection<Unit> unitsWhichConsume =
         CollectionUtils.getMatches(units, Matches.unitConsumesUnitsOnCreation());
     for (final Unit unit : unitsWhichConsume) {
-      if (Matches.unitWhichConsumesUnitsHasRequiredUnits(unitsAtStartOfTurnInTo)
-          .negate()
-          .test(unit)) {
-        weCanConsume = false;
-      }
-      if (!weCanConsume) {
-        break;
+      if (!Matches.unitWhichConsumesUnitsHasRequiredUnits(unitsAtStartOfTurnInTo).test(unit)) {
+        return false;
       }
       // remove units which are now consumed, then test the rest of the consuming units on the
       // diminishing pile of units which were in the territory at start of turn
-      final UnitAttachment ua = unit.getUnitAttachment();
-      final IntegerMap<UnitType> requiredUnitsMap = ua.getConsumesUnits();
-      final Collection<UnitType> requiredUnits = requiredUnitsMap.keySet();
-      for (final UnitType ut : requiredUnits) {
+      final IntegerMap<UnitType> requiredUnitsMap = unit.getUnitAttachment().getConsumesUnits();
+      for (final UnitType ut : requiredUnitsMap.keySet()) {
         final int requiredNumber = requiredUnitsMap.getInt(ut);
         final Predicate<Unit> unitIsOwnedByAndOfTypeAndNotDamaged =
             Matches.unitIsOwnedBy(unit.getOwner())
@@ -1028,23 +1017,20 @@ private boolean canWeConsumeUnits(
                 unitsAtStartOfTurnInTo, requiredNumber, unitIsOwnedByAndOfTypeAndNotDamaged);
         unitsAtStartOfTurnInTo.removeAll(unitsBeingRemoved);
         // if we should actually do it, not just test, then add to bridge
-        if (actuallyDoIt && change != null) {
-          final Change remove = ChangeFactory.removeUnits(to, unitsBeingRemoved);
-          change.add(remove);
+        if (change != null) {
+          change.add(ChangeFactory.removeUnits(to, unitsBeingRemoved));
           removedUnits.addAll(unitsBeingRemoved);
         }
       }
     }
-    if (weCanConsume && actuallyDoIt && change != null && !change.isEmpty()) {
-      bridge
-          .getHistoryWriter()
-          .startEvent(
-              String.format(
-                  "Units in %s being upgraded or consumed: %s",
-                  to.getName(), MyFormatter.unitsToTextNoOwner(removedUnits)),
-              removedUnits);
+    if (change != null && !change.isEmpty()) {
+      String message =
+          String.format(
+              "Units in %s being upgraded or consumed: %s",
+              to.getName(), MyFormatter.unitsToTextNoOwner(removedUnits));
+      bridge.getHistoryWriter().startEvent(message, removedUnits);
     }
-    return weCanConsume;
+    return true;
   }
 
   /** Returns -1 if we can place unlimited units. */
@@ -1275,11 +1261,9 @@ units, unitWhichRequiresUnitsHasRequiredUnits(producer, true))
           Math.max(0, unitCountAlreadyProduced - productionThatCanBeTakenOver);
     }
     if (ra != null && ra.getMaxPlacePerTerritory() > 0) {
-      return Math.max(
-          0,
-          Math.min(
-              production - unitCountHaveToAndHaveBeenBeProducedHere,
-              ra.getMaxPlacePerTerritory() - unitCountHaveToAndHaveBeenBeProducedHere));
+      int currentValue = unitCountHaveToAndHaveBeenBeProducedHere;
+      int value = Math.min(production - currentValue, ra.getMaxPlacePerTerritory() - currentValue);
+      return Math.max(0, value);
     }
     return Math.max(0, production - unitCountHaveToAndHaveBeenBeProducedHere);
   }
@@ -1390,13 +1374,10 @@ IntegerMap<String> howManyOfEachConstructionCanPlace(
                   Math.max(unitMax, (moreWithoutFactory ? toProduction : 0)),
                   (unlimitedConstructions ? 10000 : 0));
         }
-        unitMapHeld.put(
-            constructionType,
-            Math.max(
-                0,
-                Math.min(
-                    unitMax - unitMapTo.getInt(constructionType),
-                    unitMapHeld.getInt(constructionType))));
+        int value =
+            Math.min(
+                unitMax - unitMapTo.getInt(constructionType), unitMapHeld.getInt(constructionType));
+        unitMapHeld.put(constructionType, Math.max(0, value));
       }
     }
     // deal with already placed units