Fix issue 12822 GameMap#getNeighbors IllegalArgumentException (#12831)

* Fix issue 12822 GameMap#getNeighbors IllegalArgumentException
Root cause: myCapitol was null when calling method getNeighboringLandTerritories

ProTechAi.java
- new method getMyStrength making sure method getNeighboringLandTerritories is not called with myCapitol == null
- fix use of randomness now based on common ThreadLocalRandom

* Fix randomness in LanchesterDebugAction

LanchesterDebugAction.java
- fix use of randomness now based on common ThreadLocalRandom variable for each loop run (also passed to method getRandomUnitType)

Author: frigoref

game-app/ai/src/main/java/org/triplea/ai/flowfield/odds/LanchesterDebugAction.java

@@ -24,7 +24,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
-import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
@@ -105,15 +105,16 @@ public void accept(final AiPlayerDebugAction aiPlayerDebugAction) {
             unitAttachment ->
                 unitAttachment.getDefense(defender) > 0
                     && Matches.unitTypeIsLand().test((UnitType) unitAttachment.getAttachedTo()));
+    final ThreadLocalRandom localRandom = ThreadLocalRandom.current();
     for (int i = 0; i < 4; i++) {
-      getRandomUnitType(offenseUnitTypes)
+      getRandomUnitType(localRandom, offenseUnitTypes)
           .ifPresent(
               unitType ->
-                  attackingUnits.addAll(unitType.create(new Random().nextInt(10), offender)));
-      getRandomUnitType(defenseUnitTypes)
+                  attackingUnits.addAll(unitType.create(localRandom.nextInt(10), offender)));
+      getRandomUnitType(localRandom, defenseUnitTypes)
           .ifPresent(
               unitType ->
-                  defendingUnits.addAll(unitType.create(new Random().nextInt(10), defender)));
+                  defendingUnits.addAll(unitType.create(localRandom.nextInt(10), defender)));
     }
 
     System.out.println("Attack Units: " + MyFormatter.unitsToText(attackingUnits));
@@ -192,7 +193,8 @@ private Collection<UnitType> getUnitTypes(
         .collect(Collectors.toSet());
   }
 
-  private Optional<UnitType> getRandomUnitType(final Collection<UnitType> unitTypes) {
-    return unitTypes.stream().skip((int) (unitTypes.size() * Math.random())).findFirst();
+  private Optional<UnitType> getRandomUnitType(
+      final ThreadLocalRandom localRandom, final Collection<UnitType> unitTypes) {
+    return unitTypes.stream().skip((localRandom.nextInt(unitTypes.size()))).findFirst();
   }
 }

game-app/game-core/src/main/java/games/strategy/triplea/ai/pro/ProTechAi.java

@@ -28,6 +28,7 @@
 import java.util.Objects;
 import java.util.Queue;
 import java.util.Set;
+import java.util.concurrent.ThreadLocalRandom;
 import java.util.function.Predicate;
 import javax.annotation.Nullable;
 import org.triplea.java.PredicateBuilder;
@@ -45,37 +46,60 @@ static void tech(final ITechDelegate techDelegate, final GameData data, final Ga
     final Territory myCapitol =
         TerritoryAttachment.getFirstOwnedCapitalOrFirstUnownedCapital(player, data.getMap());
     final float enemyStrength = getStrengthOfPotentialAttackers(myCapitol, data, player);
-    float myStrength =
-        (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;
-    }
+    float myStrength = getMyStrength(data, player, myCapitol);
     final boolean capDanger = myStrength < (enemyStrength * 1.25F + 3.0F);
     final Resource pus = data.getResourceList().getResource(Constants.PUS);
     final int pusRemaining = player.getResources().getQuantity(pus);
     final Resource techTokens = data.getResourceList().getResource(Constants.TECH_TOKENS);
     final int techTokensQuantity = player.getResources().getQuantity(techTokens);
+    final ThreadLocalRandom localRandom = ThreadLocalRandom.current();
     int tokensToBuy = 0;
-    if (!capDanger && techTokensQuantity < 3 && pusRemaining > Math.random() * 160) {
+    if (!capDanger && techTokensQuantity < 3 && pusRemaining > localRandom.nextInt(160)) {
       tokensToBuy = 1;
     }
     if (techTokensQuantity > 0 || tokensToBuy > 0) {
       final List<TechnologyFrontier> cats = TechAdvance.getPlayerTechCategories(player);
       // retaining 65% chance of choosing land advances using basic ww2v3 model.
       if (data.getTechnologyFrontier().isEmpty()) {
-        if (Math.random() > 0.35) {
+        if (localRandom.nextFloat() > 0.35) {
           techDelegate.rollTech(techTokensQuantity + tokensToBuy, cats.get(1), tokensToBuy, null);
         } else {
           techDelegate.rollTech(techTokensQuantity + tokensToBuy, cats.get(0), tokensToBuy, null);
         }
       } else {
-        final int rand = (int) (Math.random() * cats.size());
-        techDelegate.rollTech(techTokensQuantity + tokensToBuy, cats.get(rand), tokensToBuy, null);
+        techDelegate.rollTech(
+            techTokensQuantity + tokensToBuy,
+            cats.get(localRandom.nextInt(cats.size())),
+            tokensToBuy,
+            null);
       }
     }
   }
 
+  /**
+   * Get strength value for territory {@code myCapitol} calculated from units in the territory and
+   * neighboring land territories (latter adjusted with a factor)
+   *
+   * @param data {@code GameData}
+   * @param player current {@code GamePlayer}
+   * @param myCapitol current {@code Territory}
+   * @return strength value for territory {@code myCapitol}
+   */
+  private static float getMyStrength(
+      final GameData data, final GamePlayer player, final Territory myCapitol) {
+    if (myCapitol == null) {
+      return 0.0F;
+    }
+    final float capitolStrength = strength(myCapitol.getUnits(), false, false, false);
+    final List<Territory> neighboringLandTerritories =
+        getNeighboringLandTerritories(data, player, myCapitol);
+    final List<Unit> unitsOfNeighboringLandTerritories = new ArrayList<>();
+    neighboringLandTerritories.forEach(t -> unitsOfNeighboringLandTerritories.addAll(t.getUnits()));
+    final float neighborStrength =
+        strength(unitsOfNeighboringLandTerritories, false, false, false) * 0.75F;
+    return capitolStrength + neighborStrength;
+  }
+
   /**
    * Returns the strength of all attackers to a territory. Differentiates between sea and land
    * attack Determines all transports within range of territory Determines all air units within
@@ -282,7 +306,7 @@ private static float getStrengthOfPotentialAttackers(
     }
     for (final GamePlayer enemyPlayerCandidate : enemyPlayers) {
       if (!Objects.equals(enemyPlayer, enemyPlayerCandidate)) {
-        // give 40% of other players...this is will affect a lot of decisions by AI
+        // give 40% of other players...this will affect a lot of decisions by AI
         maxStrength += enemyPlayerAttackMap.get(enemyPlayerCandidate) * 0.40F;
       }
     }
@@ -540,8 +564,8 @@ private static Route getMaxSeaRoute(
       final Collection<Unit> units,
       final GamePlayer player,
       final int maxDistance) {
-    // note this does not care if subs are submerged or not
-    // should it? does submerging affect movement of enemies?
+    // note this does not care if subs are submerged or not, should it?
+    // does submerging affect movement of enemies?
     if (start == null || destination == null || !start.isWater() || !destination.isWater()) {
       return null;
     }
@@ -604,7 +628,7 @@ private static List<Territory> getExactNeighbors(
    * Finds list of territories at exactly distance from the start.
    *
    * @param endCondition condition that all end points must satisfy
-   * @param routeCondition condition that all traversed internal territories must satisfied
+   * @param routeCondition condition that all traversed internal territories must satisfy
    */
   private static List<Territory> findFrontier(
       final Territory start,