From 0c45ebe557a5b9dfc17c9091fcef195db2936cc3 Mon Sep 17 00:00:00 2001
From: asvitkine <alexei.svitkine@gmail.com>
Date: Thu, 18 Jul 2024 00:47:38 -0400
Subject: [PATCH] Fix infinite loop in AA casualty selection code. (#12728)

This would happen when there's more hits than units in low luck mode code in the block where there's no guaranteed groups. Adding the extra logic to findRandomTargets() also allows us to remove redundant logic at some of the callsites.
---
 .../battle/casualty/AaCasualtySelector.java   | 27 ++++++++-----------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/casualty/AaCasualtySelector.java b/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/casualty/AaCasualtySelector.java
index d12f30e86b..d07716e474 100644
--- a/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/casualty/AaCasualtySelector.java
+++ b/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/casualty/AaCasualtySelector.java
@@ -132,7 +132,6 @@ private static Collection<Unit> getLowLuckAaCasualties(
       final AaPowerStrengthAndRolls unitPowerAndRollsMap,
       final DiceRoll dice,
       final IDelegateBridge bridge) {
-
     final LowLuckTargetGroups targetGroups =
         new LowLuckTargetGroups(availableTargets, dice, unitPowerAndRollsMap);
 
@@ -143,19 +142,14 @@ private static Collection<Unit> getLowLuckAaCasualties(
     }
 
     if (dice.getHits() >= targetGroups.getGuaranteedHitGroups().size()) {
-      // there are enough hits to hit all of the guaranteed hits
+      // there are enough hits to hit all the guaranteed hits
       final List<Unit> hitUnits = targetGroups.getGuaranteedHits();
 
       // if there are more hits than groups, the extra hits come out of the remainderUnits
       final int remainderHits = dice.getHits() - hitUnits.size();
       if (remainderHits > 0) {
-        if (remainderHits == targetGroups.getRemainderUnits().size()) {
-          hitUnits.addAll(targetGroups.getRemainderUnits());
-        } else {
-          // randomly pull out units from the remainder group
-          hitUnits.addAll(
-              findRandomTargets(targetGroups.getRemainderUnits(), bridge, remainderHits));
-        }
+        // randomly pull out units from the remainder group
+        hitUnits.addAll(findRandomTargets(targetGroups.getRemainderUnits(), bridge, remainderHits));
       }
       return hitUnits;
     } else {
@@ -170,6 +164,11 @@ private static Collection<Unit> getLowLuckAaCasualties(
   /** Select a random set of targets out of availableTargets */
   private static Collection<Unit> findRandomTargets(
       final List<Unit> availableTargets, final IDelegateBridge bridge, final int hits) {
+    // No need for random if all targets will be hit. The logic below would infinite loop otherwise
+    // if hits are greater.
+    if (hits >= availableTargets.size()) {
+      return availableTargets;
+    }
     final int[] hitRandom =
         bridge.getRandom(
             availableTargets.size(),
@@ -195,19 +194,15 @@ private static Collection<Unit> calculateRolledAaCasualties(
       final AaPowerStrengthAndRolls unitPowerAndRollsMap,
       final DiceRoll dice,
       final IDelegateBridge bridge) {
-
     if (unitPowerAndRollsMap.calculateTotalRolls() == availableTargets.size()
         && dice.getHits() < availableTargets.size()) {
-      // there is a roll for every target but not enough hits to kill all of the targets
+      // there is a roll for every target but not enough hits to kill all the targets
       // so no need to get a random set of units since all units will either have a hit
       // or miss roll
       return findRolledTargets(availableTargets, dice);
-    } else if (dice.getHits() < availableTargets.size()) {
-      // there isn't a roll for every target so need to randomly pick the target for each hit
-      return findRandomTargets(availableTargets, bridge, dice.getHits());
     } else {
-      // all targets were hit so add them all
-      return availableTargets;
+      // randomly choose targets (or all targets if there's enough hits)
+      return findRandomTargets(availableTargets, bridge, dice.getHits());
     }
   }