Fix "Not in a step, but trying to add event". (#11844)

This was caused by a race condition where the code that was filling adding history events could grab a temp empty history that was being put on the game data during serialization for the battle calculator.
triplea-game · Aug 2, 2023 · dc9416e · dc9416e
1 parent 65278de
commit dc9416e
Showing 1 changed file with 10 additions and 11 deletions.
diff --git a/game-app/game-core/src/main/java/games/strategy/engine/framework/ServerGame.java b/game-app/game-core/src/main/java/games/strategy/engine/framework/ServerGame.java
@@ -25,6 +25,7 @@
 import games.strategy.engine.history.Event;
 import games.strategy.engine.history.EventChild;
 import games.strategy.engine.history.HistoryNode;
+import games.strategy.engine.history.HistoryWriter;
 import games.strategy.engine.history.Step;
 import games.strategy.engine.message.ConnectionLostException;
 import games.strategy.engine.message.IRemote;
@@ -117,13 +118,17 @@ public ServerGame(
     this.clientNetworkBridge = clientNetworkBridge;
     this.launchAction = launchAction;
     this.inGameLobbyWatcher = inGameLobbyWatcher;
+    // Keep a ref to the history writer. This not only makes the calls below more concise, but also
+    // prevents a need to grab the lock on gameData (as its history object can get reset temporarily
+    // during game cloning operations for the battle calculator, e.g. by AIs).
+    final HistoryWriter historyWriter = gameData.getHistory().getHistoryWriter();
     gameModifiedChannel =
         new IGameModifiedChannel() {
           @Override
           public void gameDataChanged(final Change change) {
             assertCorrectCaller();
             gameData.performChange(change);
-            gameData.getHistory().getHistoryWriter().addChange(change);
+            historyWriter.addChange(change);
           }
 
           private void assertCorrectCaller() {
@@ -143,21 +148,18 @@ public void startHistoryEvent(final String event, final Object renderingData) {
           @Override
           public void startHistoryEvent(final String event) {
             assertCorrectCaller();
-            gameData.getHistory().getHistoryWriter().startEvent(event);
+            historyWriter.startEvent(event);
           }
 
           @Override
           public void addChildToEvent(final String text, final Object renderingData) {
             assertCorrectCaller();
-            gameData
-                .getHistory()
-                .getHistoryWriter()
-                .addChildToEvent(new EventChild(text, renderingData));
+            historyWriter.addChildToEvent(new EventChild(text, renderingData));
           }
 
           void setRenderingData(final Object renderingData) {
             assertCorrectCaller();
-            gameData.getHistory().getHistoryWriter().setRenderingData(renderingData);
+            historyWriter.setRenderingData(renderingData);
           }
 
           @Override
@@ -172,10 +174,7 @@ public void stepChanged(
             if (loadedFromSavedGame) {
               return;
             }
-            gameData
-                .getHistory()
-                .getHistoryWriter()
-                .startNextStep(stepName, delegateName, player, displayName);
+            historyWriter.startNextStep(stepName, delegateName, player, displayName);
           }
 
           // nothing to do, we call this