From f57c9b003870d0e7c98dc8facff34b0c52ea050c Mon Sep 17 00:00:00 2001
From: asvitkine <alexei.svitkine@gmail.com>
Date: Sat, 15 Jul 2023 14:32:16 -0400
Subject: [PATCH] Fix NPE with loaded save games from past versions.

The issue is that old saved games may save references to the GamePlayer on various objects, including pending battles.

Since 2.6 has an invariant that all GamePlayer objects have a non-null getData(), which was not true in 2.5 and earlier, we need to update these after loading the game. In particular, we need to fix up all references to the null player with the instance from the game data.

This was already being done for units and territories via GameData::fixUpNullPlayers(). This adds it also for pending battles.
---
 .../strategy/engine/framework/GameDataManager.java  |  3 +++
 .../triplea/delegate/battle/AbstractBattle.java     | 13 ++++++++++++-
 .../triplea/delegate/battle/BattleTracker.java      |  8 ++++++++
 .../strategy/triplea/delegate/battle/IBattle.java   |  4 ++++
 4 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/game-app/game-core/src/main/java/games/strategy/engine/framework/GameDataManager.java b/game-app/game-core/src/main/java/games/strategy/engine/framework/GameDataManager.java
index 0b8a0280d66..9a03b8a44e8 100644
--- a/game-app/game-core/src/main/java/games/strategy/engine/framework/GameDataManager.java
+++ b/game-app/game-core/src/main/java/games/strategy/engine/framework/GameDataManager.java
@@ -74,6 +74,9 @@ public static Optional<GameData> loadGameUncompressed(final InputStream is) {
       final GameData data = (GameData) input.readObject();
       data.postDeSerialize();
       loadDelegates(input, data);
+      data.getBattleDelegate()
+          .getBattleTracker()
+          .fixUpNullPlayers(data.getPlayerList().getNullPlayer());
       return Optional.of(data);
     } catch (final Throwable e) {
       log.warn(
diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/AbstractBattle.java b/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/AbstractBattle.java
index 7ba87a569d5..7608023adbb 100644
--- a/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/AbstractBattle.java
+++ b/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/AbstractBattle.java
@@ -27,6 +27,7 @@
 import java.util.UUID;
 import java.util.stream.Collectors;
 import lombok.Getter;
+import org.triplea.java.ObjectUtils;
 import org.triplea.java.RemoveOnNextMajorRelease;
 import org.triplea.java.collections.CollectionUtils;
 import org.triplea.java.collections.IntegerMap;
@@ -42,7 +43,7 @@ abstract class AbstractBattle implements IBattle {
   boolean headless = false;
 
   @Getter final Territory battleSite;
-  final GamePlayer attacker;
+  GamePlayer attacker;
   GamePlayer defender;
   final BattleTracker battleTracker;
   int round = 1;
@@ -227,6 +228,16 @@ public GamePlayer getDefender() {
     return defender;
   }
 
+  @Override
+  public void fixUpNullPlayer(GamePlayer nullPlayer) {
+    if (attacker.isNull() && !ObjectUtils.referenceEquals(attacker, nullPlayer)) {
+      attacker = nullPlayer;
+    }
+    if (defender.isNull() && !ObjectUtils.referenceEquals(defender, nullPlayer)) {
+      defender = nullPlayer;
+    }
+  }
+
   public void setHeadless(final boolean headless) {
     this.headless = headless;
   }
diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/BattleTracker.java b/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/BattleTracker.java
index bdf9ee6af9e..6863179a857 100644
--- a/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/BattleTracker.java
+++ b/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/BattleTracker.java
@@ -51,6 +51,7 @@
 import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 import lombok.extern.slf4j.Slf4j;
+import org.triplea.java.RemoveOnNextMajorRelease;
 import org.triplea.java.collections.CollectionUtils;
 import org.triplea.java.collections.IntegerMap;
 import org.triplea.sound.ISound;
@@ -176,6 +177,13 @@ private boolean didThesePlayersJustGoToWarThisTurn(final GamePlayer p1, final Ga
     return false;
   }
 
+  @RemoveOnNextMajorRelease
+  public void fixUpNullPlayers(GamePlayer nullPlayer) {
+    for (var b : pendingBattles) {
+      b.fixUpNullPlayer(nullPlayer);
+    }
+  }
+
   void clearFinishedBattles(final IDelegateBridge bridge) {
     for (final IBattle battle : List.copyOf(pendingBattles)) {
       if (FinishedBattle.class.isAssignableFrom(battle.getClass())) {
diff --git a/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/IBattle.java b/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/IBattle.java
index a2bda249acf..cb12ae417f8 100644
--- a/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/IBattle.java
+++ b/game-app/game-core/src/main/java/games/strategy/triplea/delegate/battle/IBattle.java
@@ -17,6 +17,7 @@
 import lombok.AllArgsConstructor;
 import lombok.Getter;
 import lombok.ToString;
+import org.triplea.java.RemoveOnNextMajorRelease;
 
 /** Represents a battle. */
 public interface IBattle extends Serializable {
@@ -165,5 +166,8 @@ void unitsLostInPrecedingBattle(
 
   GamePlayer getDefender();
 
+  @RemoveOnNextMajorRelease
+  void fixUpNullPlayer(GamePlayer nullPlayer);
+
   UUID getBattleId();
 }