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 index 65db420477e..0172752d5a0 100644 --- 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 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 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 index 81f93f6e295..4977e834a9d 100644 --- 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 @@ -394,17 +394,17 @@ private void determineIfMoveTerritoriesCanBeHeld() { final ProTerritory patd = moveMap.get(t); // Check if no enemy attackers - if (enemyAttackOptions.getMax(t) == null) { + final ProTerritory enemyAttackMax = enemyAttackOptions.getMax(t); + if (enemyAttackMax == null) { ProLogger.debug("Territory=" + t.getName() + ", CanHold=true since has no enemy attackers"); continue; } // Check if min defenders can hold it (not considering AA) - final Set 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 Set enemyAttackingUnits = new HashSet<>(enemyAttackMax.getMaxUnits()); + enemyAttackingUnits.addAll(enemyAttackMax.getMaxAmphibUnits()); + patd.setMaxEnemyUnits(enemyAttackingUnits); + patd.setMaxEnemyBombardUnits(enemyAttackMax.getMaxBombardUnits()); final List 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 index 672569c1370..321bc35deef 100644 --- 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 units) { this.cantMoveUnits.addAll(units); } - public void setMaxEnemyUnits(final List maxEnemyUnits) { - this.maxEnemyUnits = maxEnemyUnits; + public void setMaxEnemyUnits(final Collection maxEnemyUnits) { + this.maxEnemyUnits = new ArrayList<>(maxEnemyUnits); } public List 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 index cfb4ccdff44..1b6f8703bcd 100644 --- 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 unitIsCarrierOwnedByCombinedPlayers(GamePlayer player) { } private boolean canWeConsumeUnits( - final Collection units, - final Territory to, - final boolean actuallyDoIt, - final CompositeChange change) { - boolean weCanConsume = true; + final Collection units, final Territory to, final CompositeChange change) { final Collection unitsAtStartOfTurnInTo = unitsAtStartOfStepInTerritory(to); final Collection removedUnits = new ArrayList<>(); final Collection 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 requiredUnitsMap = ua.getConsumesUnits(); - final Collection requiredUnits = requiredUnitsMap.keySet(); - for (final UnitType ut : requiredUnits) { + final IntegerMap requiredUnitsMap = unit.getUnitAttachment().getConsumesUnits(); + for (final UnitType ut : requiredUnitsMap.keySet()) { final int requiredNumber = requiredUnitsMap.getInt(ut); final Predicate 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 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