From b90014e0cc1569d4632f596fd825e77c099f963f Mon Sep 17 00:00:00 2001 From: asvitkine Date: Wed, 3 Jul 2024 12:11:31 -0400 Subject: [PATCH] Some code clean ups. No intended logic changes. --- .../games/strategy/triplea/UnitUtils.java | 20 +--- .../triplea/ai/pro/ProPurchaseAi.java | 2 - .../triplea/ai/pro/util/ProPurchaseUtils.java | 8 +- .../strategy/triplea/ai/weak/WeakAi.java | 64 ++++------- .../delegate/AbstractPlaceDelegate.java | 2 +- .../calculator/BattleCalculatorPanel.java | 103 +++++++++--------- .../strategy/triplea/ui/PurchasePanel.java | 2 +- 7 files changed, 78 insertions(+), 123 deletions(-) diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/UnitUtils.java b/game-app/game-core/src/main/java/games/strategy/triplea/UnitUtils.java index ee0b7e7aeb2..7afd1719608 100644 --- a/game-app/game-core/src/main/java/games/strategy/triplea/UnitUtils.java +++ b/game-app/game-core/src/main/java/games/strategy/triplea/UnitUtils.java @@ -40,20 +40,11 @@ public static int getProductionPotentialOfTerritory( final Collection unitsAtStartOfStepInTerritory, final Territory producer, final GamePlayer player, - final GameProperties properties, final boolean accountForDamage, final boolean mathMaxZero) { return getHowMuchCanUnitProduce( - getBiggestProducer( - unitsAtStartOfStepInTerritory, - producer, - player, - player.getData().getTechTracker(), - properties, - accountForDamage), + getBiggestProducer(unitsAtStartOfStepInTerritory, producer, player, accountForDamage), producer, - player.getData().getTechTracker(), - properties, accountForDamage, mathMaxZero); } @@ -69,8 +60,6 @@ public static int getProductionPotentialOfTerritory( final Collection units, final Territory producer, final GamePlayer player, - final TechTracker techTracker, - final GameProperties properties, final boolean accountForDamage) { final Predicate factoryMatch = Matches.unitIsOwnedAndIsFactoryOrCanProduceUnits(player) @@ -84,8 +73,7 @@ public static int getProductionPotentialOfTerritory( Unit highestUnit = CollectionUtils.getAny(factories); int highestCapacity = Integer.MIN_VALUE; for (final Unit u : factories) { - final int capacity = - getHowMuchCanUnitProduce(u, producer, techTracker, properties, accountForDamage, false); + final int capacity = getHowMuchCanUnitProduce(u, producer, accountForDamage, false); productionPotential.put(u, capacity); if (capacity > highestCapacity) { highestCapacity = capacity; @@ -106,8 +94,6 @@ public static int getProductionPotentialOfTerritory( public static int getHowMuchCanUnitProduce( final @Nullable Unit unit, final Territory producer, - final TechTracker techTracker, - final GameProperties properties, final boolean accountForDamage, final boolean mathMaxZero) { if (unit == null) { @@ -125,6 +111,7 @@ public static int getHowMuchCanUnitProduce( territoryUnitProduction = ta.getUnitProduction(); } int productionCapacity; + final GameProperties properties = producer.getData().getProperties(); if (accountForDamage) { if (Properties.getDamageFromBombingDoneToUnitsInsteadOfTerritories(properties)) { if (ua.getCanProduceXUnits() < 0) { @@ -158,6 +145,7 @@ public static int getHowMuchCanUnitProduce( (Properties.getWW2V2(properties) || Properties.getWW2V3(properties)) ? 0 : 1; } } + final TechTracker techTracker = producer.getData().getTechTracker(); // Increase production if we have industrial technology if (territoryProduction >= techTracker.getMinimumTerritoryValueForProductionBonus(unit.getOwner())) { diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/ProPurchaseAi.java b/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/ProPurchaseAi.java index 703aacb5634..680b0538727 100644 --- a/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/ProPurchaseAi.java +++ b/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/ProPurchaseAi.java @@ -104,8 +104,6 @@ void repair( CollectionUtils.getMatches(fixTerr.getUnits(), ourFactories), fixTerr, player, - data.getTechTracker(), - data.getProperties(), false); if (Matches.unitHasTakenSomeBombingUnitDamage().test(possibleFactoryNeedingRepair)) { unitsThatCanProduceNeedingRepair.put(possibleFactoryNeedingRepair, fixTerr); diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/util/ProPurchaseUtils.java b/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/util/ProPurchaseUtils.java index 052a2dc9c26..cd3041df2bd 100644 --- a/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/util/ProPurchaseUtils.java +++ b/game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/util/ProPurchaseUtils.java @@ -108,7 +108,7 @@ public static List findMaxPurchaseDefenders( final List placeUnits = new ArrayList<>(); if (bestDefenseOption != null) { ProLogger.debug("Best defense option: " + bestDefenseOption.getUnitType().getName()); - int remainingUnitProduction = getUnitProduction(t, data, player); + int remainingUnitProduction = getUnitProduction(t, player); int pusSpent = 0; while (bestDefenseOption.getCost() <= (pusRemaining - pusSpent) && remainingUnitProduction >= bestDefenseOption.getQuantity()) { @@ -190,7 +190,7 @@ public static Map findPurchaseTerritories( // Create purchase territory holder for each factory territory final Map purchaseTerritories = new HashMap<>(); for (final Territory t : ownedAndNotConqueredFactoryTerritories) { - final int unitProduction = getUnitProduction(t, data, player); + final int unitProduction = getUnitProduction(t, player); final ProPurchaseTerritory ppt = new ProPurchaseTerritory(t, data, player, unitProduction); purchaseTerritories.put(t, ppt); ProLogger.debug(ppt.toString()); @@ -199,7 +199,7 @@ public static Map findPurchaseTerritories( } private static int getUnitProduction( - final Territory territory, final GameState data, final GamePlayer player) { + final Territory territory, final GamePlayer player) { final Predicate factoryMatch = Matches.unitIsOwnedAndIsFactoryOrCanProduceUnits(player) .and(Matches.unitIsBeingTransported().negate()) @@ -220,7 +220,7 @@ private static int getUnitProduction( return Integer.MAX_VALUE; } return UnitUtils.getProductionPotentialOfTerritory( - territory.getUnits(), territory, player, data.getProperties(), true, true); + territory.getUnits(), territory, player, true, true); } /** diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/ai/weak/WeakAi.java b/game-app/game-core/src/main/java/games/strategy/triplea/ai/weak/WeakAi.java index 829d623e6cd..4a7f3b2cf25 100644 --- a/game-app/game-core/src/main/java/games/strategy/triplea/ai/weak/WeakAi.java +++ b/game-app/game-core/src/main/java/games/strategy/triplea/ai/weak/WeakAi.java @@ -72,8 +72,8 @@ private static Route getAmphibRoute(final GamePlayer player, final GameState dat } final Predicate endMatch = o -> { - final boolean impassable = - TerritoryAttachment.get(o) != null && TerritoryAttachment.get(o).getIsImpassable(); + final var ta = TerritoryAttachment.get(o); + final boolean impassable = ta != null && ta.getIsImpassable(); return !impassable && !o.isWater() && Utils.hasLandRouteToEnemyOwnedCapitol(o, player, data); @@ -435,7 +435,8 @@ private List calculateNonCombat(final GameData data, final Game if (t.isWater()) { continue; } - if (TerritoryAttachment.get(t) != null && TerritoryAttachment.get(t).isCapital()) { + final var ta = TerritoryAttachment.get(t); + if (ta != null && ta.isCapital()) { // if they are a threat to take our capitol, dont move // compare the strength of units we can place final float ourStrength = AiUtils.strength(player.getUnits(), false, false); @@ -651,8 +652,9 @@ private List calculateCombatMove(final GameState data, final Ga final Collection attackFrom = data.getMap().getNeighbors(enemy, Matches.territoryHasLandUnitsOwnedBy(player)); for (final Territory owned : attackFrom) { - if (TerritoryAttachment.get(owned) != null - && TerritoryAttachment.get(owned).isCapital() + final var ta = TerritoryAttachment.get(owned); + if (ta != null + && ta.isCapital() && (Utils.getStrengthOfPotentialAttackers(owned, data) > AiUtils.strength(owned.getUnits(), false, false))) { dontMoveFrom.add(owned); @@ -833,65 +835,43 @@ public void purchase( CollectionUtils.getMatches(fixTerr.getUnits(), ourFactories), fixTerr, player, - data.getTechTracker(), - data.getProperties(), false); if (Matches.unitHasTakenSomeBombingUnitDamage().test(possibleFactoryNeedingRepair)) { unitsThatCanProduceNeedingRepair.put(possibleFactoryNeedingRepair, fixTerr); } if (fixTerr.equals(capitol)) { capProduction = - UnitUtils.getHowMuchCanUnitProduce( - possibleFactoryNeedingRepair, - fixTerr, - data.getTechTracker(), - data.getProperties(), - true, - true); + UnitUtils.getHowMuchCanUnitProduce(possibleFactoryNeedingRepair, fixTerr, true, true); capUnit = possibleFactoryNeedingRepair; capUnitTerritory = fixTerr; } currentProduction += - UnitUtils.getHowMuchCanUnitProduce( - possibleFactoryNeedingRepair, - fixTerr, - data.getTechTracker(), - data.getProperties(), - true, - true); + UnitUtils.getHowMuchCanUnitProduce(possibleFactoryNeedingRepair, fixTerr, true, true); } repairFactories.remove(capitol); unitsThatCanProduceNeedingRepair.remove(capUnit); + final var territoryIsOwnedAndHasOwnedUnitMatching = + Matches.territoryIsOwnedAndHasOwnedUnitMatching( + player, Matches.unitCanProduceUnitsAndCanBeDamaged()); // assume minimum unit price is 3, and that we are buying only that... if we over repair, oh - // well, that is better - // than under-repairing + // well, that is better than under-repairing // goal is to be able to produce all our units, and at least half of that production in the // capitol // // if capitol is super safe, we don't have to do this. and if capitol is under siege, we - // should repair enough to - // place all our units here + // should repair enough to place all our units here int maxUnits = (totalPu - 1) / minimumUnitPrice; if ((capProduction <= maxUnits / 2 || repairFactories.isEmpty()) && capUnit != null) { for (final RepairRule rrule : repairRules) { if (!capUnit.getType().equals(rrule.getAnyResultKey())) { continue; } - if (!Matches.territoryIsOwnedAndHasOwnedUnitMatching( - player, Matches.unitCanProduceUnitsAndCanBeDamaged()) - .test(capitol)) { + if (!territoryIsOwnedAndHasOwnedUnitMatching.test(capitol)) { continue; } diff = capUnit.getUnitDamage(); final int unitProductionAllowNegative = - UnitUtils.getHowMuchCanUnitProduce( - capUnit, - capUnitTerritory, - data.getTechTracker(), - data.getProperties(), - false, - true) - - diff; + UnitUtils.getHowMuchCanUnitProduce(capUnit, capUnitTerritory, false, true) - diff; if (!repairFactories.isEmpty()) { diff = Math.min(diff, (maxUnits / 2 - unitProductionAllowNegative) + 1); } else { @@ -922,9 +902,8 @@ public void purchase( if (fixUnit == null || !fixUnit.getType().equals(rrule.getAnyResultKey())) { continue; } - if (!Matches.territoryIsOwnedAndHasOwnedUnitMatching( - player, Matches.unitCanProduceUnitsAndCanBeDamaged()) - .test(unitsThatCanProduceNeedingRepair.get(fixUnit))) { + if (!territoryIsOwnedAndHasOwnedUnitMatching.test( + unitsThatCanProduceNeedingRepair.get(fixUnit))) { continue; } // we will repair the first territories in the list as much as we can, until we fulfill @@ -936,12 +915,7 @@ public void purchase( diff = fixUnit.getUnitDamage(); final int unitProductionAllowNegative = UnitUtils.getHowMuchCanUnitProduce( - fixUnit, - unitsThatCanProduceNeedingRepair.get(fixUnit), - data.getTechTracker(), - data.getProperties(), - false, - true) + fixUnit, unitsThatCanProduceNeedingRepair.get(fixUnit), false, true) - diff; if (i == 0) { if (unitProductionAllowNegative < 0) { 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 af6ea49d00e..b58f65f4871 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 @@ -1194,7 +1194,7 @@ protected int getMaxUnitsToBePlacedFrom( // maxConstructions int production = UnitUtils.getProductionPotentialOfTerritory( - unitsAtStartOfStepInTerritory(producer), producer, player, properties, true, true); + unitsAtStartOfStepInTerritory(producer), producer, player, true, true); // increase the production by the number of constructions allowed if (maxConstructions > 0) { production += maxConstructions; diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/odds/calculator/BattleCalculatorPanel.java b/game-app/game-core/src/main/java/games/strategy/triplea/odds/calculator/BattleCalculatorPanel.java index 4832e437b5e..ff3c238602c 100644 --- a/game-app/game-core/src/main/java/games/strategy/triplea/odds/calculator/BattleCalculatorPanel.java +++ b/game-app/game-core/src/main/java/games/strategy/triplea/odds/calculator/BattleCalculatorPanel.java @@ -547,11 +547,13 @@ private Collection getTerritoryEffects() { private void updateStats() { Util.ensureOnEventDispatchThread(); - final AtomicReference results = new AtomicReference<>(); + final AtomicReference resultsRef = new AtomicReference<>(); final WaitDialog dialog = new WaitDialog(this, "Calculating Odds... (this may take a while)", calculator::cancel); final AtomicReference> defenders = new AtomicReference<>(); final AtomicReference> attackers = new AtomicReference<>(); + final GamePlayer attacker = getAttacker(); + final GamePlayer defender = getDefender(); ThreadRunner.runInNewThread( () -> { try { @@ -585,10 +587,10 @@ private void updateStats() { final Collection territoryEffects = getTerritoryEffects(); defenders.set(defending); attackers.set(attacking); - results.set( + resultsRef.set( calculator.calculate( - getAttacker(), - getDefender(), + attacker, + defender, location, attacking, defending, @@ -605,60 +607,53 @@ private void updateStats() { } }); // the runnable setting the dialog visible must run after this code executes, since this code is - // running on the - // swing event thread + // running on the swing event thread dialog.setVisible(true); // results.get() could be null if we cancelled to quickly or something weird like that. - if (results.get() == null) { + final var results = resultsRef.get(); + if (results == null) { setResultsToBlank(); - } else { - // All AggregateResults method return NaN if there are no battle results to aggregate over. - // For "unrestricted" average methods, this cannot happen as we ensure that at least 1 round - // is simulated. However, the ...IfAbcWon() methods restrict that set of results which might - // become empty. In this case we display N/A (not applicable) instead of NaN (not a number). - attackerWin.setText(formatPercentage(results.get().getAttackerWinPercent())); - defenderWin.setText(formatPercentage(results.get().getDefenderWinPercent())); - draw.setText(formatPercentage(results.get().getDrawPercent())); - final boolean isLand = isLand(); - final List mainCombatAttackers = - CollectionUtils.getMatches( - attackers.get(), Matches.unitCanBeInBattle(true, isLand, 1, true)); - final List mainCombatDefenders = - CollectionUtils.getMatches( - defenders.get(), Matches.unitCanBeInBattle(false, isLand, 1, true)); - final int attackersTotal = mainCombatAttackers.size(); - final int defendersTotal = mainCombatDefenders.size(); - defenderLeft.setText( - formatValue(results.get().getAverageDefendingUnitsLeft()) + " / " + defendersTotal); - attackerLeft.setText( - formatValue(results.get().getAverageAttackingUnitsLeft()) + " / " + attackersTotal); - final double avgDefIfDefWon = results.get().getAverageDefendingUnitsLeftWhenDefenderWon(); - defenderLeftWhenDefenderWon.setText( - Double.isNaN(avgDefIfDefWon) - ? "N/A" - : formatValue(avgDefIfDefWon) + " / " + defendersTotal); - final double avgAttIfAttWon = results.get().getAverageAttackingUnitsLeftWhenAttackerWon(); - attackerLeftWhenAttackerWon.setText( - Double.isNaN(avgAttIfAttWon) - ? "N/A" - : formatValue(avgAttIfAttWon) + " / " + attackersTotal); - roundsAverage.setText("" + formatValue(results.get().getAverageBattleRoundsFought())); - try (GameData.Unlocker ignored = data.acquireReadLock()) { - averageChangeInTuv.setText( - "" - + formatValue( - results - .get() - .getAverageTuvSwing( - getAttacker(), - mainCombatAttackers, - getDefender(), - mainCombatDefenders, - data))); - } - count.setText(results.get().getRollCount() + ""); - time.setText(formatValue(results.get().getTime() / 1000.0) + " s"); + return; + } + // All AggregateResults method return NaN if there are no battle results to aggregate over. + // For "unrestricted" average methods, this cannot happen as we ensure that at least 1 round + // is simulated. However, the ...IfAbcWon() methods restrict that set of results which might + // become empty. In this case we display N/A (not applicable) instead of NaN (not a number). + attackerWin.setText(formatPercentage(results.getAttackerWinPercent())); + defenderWin.setText(formatPercentage(results.getDefenderWinPercent())); + draw.setText(formatPercentage(results.getDrawPercent())); + final boolean isLand = isLand(); + final List mainCombatAttackers = + CollectionUtils.getMatches( + attackers.get(), Matches.unitCanBeInBattle(true, isLand, 1, true)); + final List mainCombatDefenders = + CollectionUtils.getMatches( + defenders.get(), Matches.unitCanBeInBattle(false, isLand, 1, true)); + final int attackersTotal = mainCombatAttackers.size(); + final int defendersTotal = mainCombatDefenders.size(); + defenderLeft.setText( + formatValue(results.getAverageDefendingUnitsLeft()) + " / " + defendersTotal); + attackerLeft.setText( + formatValue(results.getAverageAttackingUnitsLeft()) + " / " + attackersTotal); + final double avgDefIfDefWon = results.getAverageDefendingUnitsLeftWhenDefenderWon(); + defenderLeftWhenDefenderWon.setText( + Double.isNaN(avgDefIfDefWon) + ? "N/A" + : formatValue(avgDefIfDefWon) + " / " + defendersTotal); + final double avgAttIfAttWon = results.getAverageAttackingUnitsLeftWhenAttackerWon(); + attackerLeftWhenAttackerWon.setText( + Double.isNaN(avgAttIfAttWon) + ? "N/A" + : formatValue(avgAttIfAttWon) + " / " + attackersTotal); + roundsAverage.setText(formatValue(results.getAverageBattleRoundsFought())); + try (GameData.Unlocker ignored = data.acquireReadLock()) { + double tuvSwing = + results.getAverageTuvSwing( + attacker, mainCombatAttackers, defender, mainCombatDefenders, data); + averageChangeInTuv.setText(formatValue(tuvSwing)); } + count.setText(results.getRollCount() + ""); + time.setText(formatValue(results.getTime() / 1000.0) + " s"); } private Territory findPotentialBattleSite() { diff --git a/game-app/game-headed/src/main/java/games/strategy/triplea/ui/PurchasePanel.java b/game-app/game-headed/src/main/java/games/strategy/triplea/ui/PurchasePanel.java index 937f8075dbe..4174b27d7f7 100644 --- a/game-app/game-headed/src/main/java/games/strategy/triplea/ui/PurchasePanel.java +++ b/game-app/game-headed/src/main/java/games/strategy/triplea/ui/PurchasePanel.java @@ -164,9 +164,9 @@ public void performDone() { // give a warning if the // player tries to produce too much final GameData data = getData(); - final GamePlayer player = getCurrentPlayer(); final GameProperties properties = data.getProperties(); if (Properties.getWW2V2(properties) || Properties.getPlacementRestrictedByFactory(properties)) { + final GamePlayer player = getCurrentPlayer(); int totalProd = 0; try (GameData.Unlocker ignored = data.acquireReadLock()) { final var predicate = Matches.territoryHasOwnedIsFactoryOrCanProduceUnits(player);