Fix "Could not find step name: withdraw?" error. (#11860)

This can happen when the withdraw step becomes available only when they are planes left. Since the UI doesn't get updated step names, it means switching to this step would produce the error. Since switching to the step isn't really needed, since the action is via a dialog, omit the message to do so in this case. Also, cleans up a bunch of related code.
triplea-game · Aug 6, 2023 · 27ee244 · 27ee244
1 parent 7c1a019
commit 27ee244
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 66 deletions.
diff --git a/game-app/game-core/src/main/java/games/strategy/engine/player/Player.java b/game-app/game-core/src/main/java/games/strategy/engine/player/Player.java
@@ -233,6 +233,7 @@ Territory selectTerritoryForAirToLand(
    * @param battleId - the battle
    * @param submerge - is submerging possible (means the retreat territory CAN be the current battle
    *     territory)
+   * @param battleTerritory - where the battle is taking place
    * @param possibleTerritories - where the player can retreat to
    * @param message - user displayable message
    * @return the territory to retreat to, or null if the player doesnt wish to retreat

diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/AirBattle.java b/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/AirBattle.java
@@ -251,7 +251,9 @@ public void execute(final ExecutionStack stack, final IDelegateBridge bridge) {
           @Override
           public void execute(final ExecutionStack stack, final IDelegateBridge bridge) {
             if (!isOver && canAttackerRetreat()) {
-              attackerRetreat(bridge);
+              // planes retreat to the same square the battle is in, and then should move during
+              // non combat to their landing site, or be scrapped if they can't find one.
+              queryRetreat(false, bridge, battleSite);
             }
           }
         });
@@ -262,7 +264,9 @@ public void execute(final ExecutionStack stack, final IDelegateBridge bridge) {
           @Override
           public void execute(final ExecutionStack stack, final IDelegateBridge bridge) {
             if (!isOver && canDefenderRetreat()) {
-              defenderRetreat(bridge);
+              // planes retreat to the same square the battle is in, and then should move during
+              // non combat to their landing site, or be scrapped if they can't find one
+              queryRetreat(true, bridge, battleSite);
             }
           }
         });
@@ -473,79 +477,53 @@ void finishBattleAndRemoveFromTrackerHeadless(final IDelegateBridge bridge) {
     battleTracker.removeBattle(AirBattle.this, bridge.getData());
   }
 
-  private void attackerRetreat(final IDelegateBridge bridge) {
-    // planes retreat to the same square the battle is in, and then should
-    // move during non combat to their landing site, or be scrapped if they can't find one.
-    // retreat planes
-    if (!attackingUnits.isEmpty()) {
-      queryRetreat(false, bridge, Set.of(battleSite));
-    }
-  }
-
-  private void defenderRetreat(final IDelegateBridge bridge) {
-    // planes retreat to the same square the battle is in, and then should
-    // move during non combat to their landing site, or be scrapped if they can't find one.
-    // retreat planes
-    if (!defendingUnits.isEmpty()) {
-      queryRetreat(true, bridge, Set.of(battleSite));
-    }
-  }
-
   private void queryRetreat(
-      final boolean defender,
-      final IDelegateBridge bridge,
-      final Collection<Territory> availableTerritories) {
-    if (availableTerritories.isEmpty()) {
-      return;
-    }
+      final boolean defender, final IDelegateBridge bridge, final Territory battleSite) {
     final Collection<Unit> units =
         defender ? new ArrayList<>(defendingUnits) : new ArrayList<>(attackingUnits);
     if (units.isEmpty()) {
       return;
     }
-    final GamePlayer retreatingPlayer = defender ? this.defender : attacker;
-    final String text = retreatingPlayer.getName() + " retreat?";
     final String step = defender ? DEFENDERS_WITHDRAW : ATTACKERS_WITHDRAW;
-
     if (ClientSetting.useWebsocketNetwork.getValue().orElse(false)) {
       bridge.sendMessage(new IDisplay.GoToBattleStepMessage(battleId.toString(), step));
     } else {
       bridge.getDisplayChannelBroadcaster().gotoBattleStep(battleId, step);
     }
+
+    final GamePlayer retreatingPlayer = defender ? this.defender : attacker;
+    final String text = retreatingPlayer.getName() + " retreat?";
     final Territory retreatTo =
         getRemote(retreatingPlayer, bridge)
-            .retreatQuery(battleId, false, battleSite, availableTerritories, text);
-    if (retreatTo != null && !availableTerritories.contains(retreatTo)) {
-      log.error(
-          "Invalid retreat selection :"
-              + retreatTo
-              + " not in "
-              + MyFormatter.defaultNamedToTextList(availableTerritories));
+            .retreatQuery(battleId, false, battleSite, List.of(battleSite), text);
+    if (retreatTo == null) {
       return;
     }
-    if (retreatTo != null) {
-      if (!headless) {
-        bridge
-            .getSoundChannelBroadcaster()
-            .playSoundForAll(SoundPath.CLIP_BATTLE_RETREAT_AIR, attacker);
-      }
-      retreat(units, defender, bridge);
-      final String messageShort = retreatingPlayer.getName() + " retreats";
-      final String messageLong =
-          retreatingPlayer.getName() + " retreats all units to " + retreatTo.getName();
-      if (ClientSetting.useWebsocketNetwork.getValue().orElse(false)) {
-        bridge.sendMessage(
-            IDisplay.NotifyRetreatMessage.builder()
-                .shortMessage(messageShort)
-                .message(messageLong)
-                .step(step)
-                .retreatingPlayerName(retreatingPlayer.getName())
-                .build());
-      } else {
-        bridge
-            .getDisplayChannelBroadcaster()
-            .notifyRetreat(messageShort, messageLong, step, retreatingPlayer);
-      }
+    if (!retreatTo.equals(battleSite)) {
+      log.error("Invalid retreat selection : {} does not equal {}", retreatTo, battleSite);
+      return;
+    }
+    if (!headless) {
+      bridge
+          .getSoundChannelBroadcaster()
+          .playSoundForAll(SoundPath.CLIP_BATTLE_RETREAT_AIR, attacker);
+    }
+    retreat(units, defender, bridge);
+    final String messageShort = retreatingPlayer.getName() + " retreats";
+    final String messageLong =
+        retreatingPlayer.getName() + " retreats all units to " + retreatTo.getName();
+    if (ClientSetting.useWebsocketNetwork.getValue().orElse(false)) {
+      bridge.sendMessage(
+          IDisplay.NotifyRetreatMessage.builder()
+              .shortMessage(messageShort)
+              .message(messageLong)
+              .step(step)
+              .retreatingPlayerName(retreatingPlayer.getName())
+              .build());
+    } else {
+      bridge
+          .getDisplayChannelBroadcaster()
+          .notifyRetreat(messageShort, messageLong, step, retreatingPlayer);
     }
   }
 

diff --git a/...in/java/games/strategy/triplea/delegate/battle/steps/retreat/OffensiveGeneralRetreat.java b/...in/java/games/strategy/triplea/delegate/battle/steps/retreat/OffensiveGeneralRetreat.java
@@ -22,6 +22,7 @@
 import games.strategy.triplea.settings.ClientSetting;
 import java.util.Collection;
 import java.util.List;
+import java.util.Optional;
 import lombok.AllArgsConstructor;
 import org.triplea.java.RemoveOnNextMajorRelease;
 import org.triplea.sound.SoundUtils;
@@ -107,16 +108,22 @@ public void retreatUnits(final IDelegateBridge bridge) {
     }
 
     if (retreater != null) {
+      final String stepName = getName();
+      // Only send the gotoBattleStep message if the step exists in the UI. It will not exist in the
+      // case where normally retreat is not possible but only becomes possible when there are only
+      // planes left.
+      if (Optional.ofNullable(battleState.getStepStrings()).orElse(List.of()).contains(stepName)) {
+        final var battleId = battleState.getBattleId();
+        if (ClientSetting.useWebsocketNetwork.getValue().orElse(false)) {
+          bridge.sendMessage(new IDisplay.GoToBattleStepMessage(battleId.toString(), stepName));
+        } else {
+          bridge.getDisplayChannelBroadcaster().gotoBattleStep(battleId, stepName);
+        }
+      }
+
       final Collection<Unit> retreatUnits = retreater.getRetreatUnits();
       final Collection<Territory> possibleRetreatSites =
           retreater.getPossibleRetreatSites(retreatUnits);
-
-      if (ClientSetting.useWebsocketNetwork.getValue().orElse(false)) {
-        bridge.sendMessage(
-            new IDisplay.GoToBattleStepMessage(battleState.getBattleId().toString(), getName()));
-      } else {
-        bridge.getDisplayChannelBroadcaster().gotoBattleStep(battleState.getBattleId(), getName());
-      }
       final Territory retreatTo =
           battleActions.queryRetreatTerritory(
               battleState,