From c4f93140a1b00d83f05d8f5f3bc52986d0262836 Mon Sep 17 00:00:00 2001 From: asvitkine Date: Sun, 16 Jul 2023 07:45:41 -0400 Subject: [PATCH] Fix NPE with loaded save games from past versions. (#11783) 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. --- .../java/games/strategy/engine/data/GameData.java | 8 ++++++++ .../strategy/engine/framework/GameDataManager.java | 1 + .../triplea/delegate/battle/AbstractBattle.java | 13 ++++++++++++- .../triplea/delegate/battle/BattleTracker.java | 8 ++++++++ .../strategy/triplea/delegate/battle/IBattle.java | 4 ++++ 5 files changed, 33 insertions(+), 1 deletion(-) diff --git a/game-app/game-core/src/main/java/games/strategy/engine/data/GameData.java b/game-app/game-core/src/main/java/games/strategy/engine/data/GameData.java index c4c76191596..980b6c362d3 100644 --- a/game-app/game-core/src/main/java/games/strategy/engine/data/GameData.java +++ b/game-app/game-core/src/main/java/games/strategy/engine/data/GameData.java @@ -395,6 +395,14 @@ private void fixUpNullPlayers() { } } + @RemoveOnNextMajorRelease + public void fixUpNullPlayersInDelegates() { + BattleDelegate battleDelegate = (BattleDelegate) getDelegate("battle"); + if (battleDelegate != null) { + battleDelegate.getBattleTracker().fixUpNullPlayers(playerList.getNullPlayer()); + } + } + public interface Unlocker extends Closeable { @Override void close(); 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..7eb257c1f25 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,7 @@ public static Optional loadGameUncompressed(final InputStream is) { final GameData data = (GameData) input.readObject(); data.postDeSerialize(); loadDelegates(input, data); + data.fixUpNullPlayersInDelegates(); 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(); }