Fix history errors when seeking through history. (#10494)

Fix history errors when seeking through history. Unfortunately, my fix for #10429, which fixed history errors on Imperialism 1974, introduced a different bug where seeking back in time in history would cause errors.

This is because the "last change" of a Node that's not a leaf is not always consistent, since more child nodes get added to it over time, and thus seeking to such a node and then later seeking away from it would result in an inconsistent sequence of changes.

This PR reworks the History class to explicitly keep track of the change index we're past, instead of the tree node (whose last change index may change), which fixes the problem. There is no problem with serialization because the History class uses writeReplace() (i.e. serializing a different object in its place) and the properties being changed are only relevant for the History object in "seek mode", which is never serialized (it's on a copy of the game data).

Tested by playing Feudal Japan Warlords 5 players with all-AIs and moving around history nodes while it's advancing. Before this change, an error dialog about trying to remove non-existent units would be encountered.

game-app/game-core/src/main/java/games/strategy/engine/history/History.java

@@ -31,8 +31,10 @@ public class History extends DefaultTreeModel {
   private final HistoryWriter writer = new HistoryWriter(this);
   private final List<Change> changes = new ArrayList<>();
   private final GameData gameData;
-  private HistoryNode currentNode;
-  private HistoryPanel panel = null;
+  private HistoryPanel panel;
+  // Index at which point we are in history. Only valid if seekingEnabled is true.
+  private int nextChangeIndex;
+  private boolean seekingEnabled = false;
 
   public History(final GameData data) {
     super(new RootHistoryNode("Game History"));
@@ -49,8 +51,15 @@ public HistoryWriter getHistoryWriter() {
     return writer;
   }
 
-  public void setTreePanel(final HistoryPanel panel) {
+  public HistoryNode enableSeeking(final HistoryPanel panel) {
+    Preconditions.checkNotNull(panel);
+    Preconditions.checkState(!seekingEnabled);
     this.panel = panel;
+    nextChangeIndex = changes.size();
+    seekingEnabled = true;
+    HistoryNode lastNode = getLastNode();
+    gotoNode(lastNode);
+    return lastNode;
   }
 
   public void goToEnd() {
@@ -95,37 +104,25 @@ private int getLastChange(final HistoryNode node) {
     return lastChangeIndex;
   }
 
-  public Change getDelta(final HistoryNode start, final HistoryNode end) {
-    assertCorrectThread();
-    final int firstChange = getLastChange(start);
-    final int lastChange = getLastChange(end);
-    if (firstChange == lastChange) {
-      return null;
-    }
+  private Change getDeltaTo(int changeIndex) {
     final List<Change> deltaChanges =
-        changes.subList(Math.min(firstChange, lastChange), Math.max(firstChange, lastChange));
+        changes.subList(
+            Math.min(nextChangeIndex, changeIndex), Math.max(nextChangeIndex, changeIndex));
     final Change compositeChange = new CompositeChange(deltaChanges);
-    return (lastChange >= firstChange) ? compositeChange : compositeChange.invert();
+    return (changeIndex >= nextChangeIndex) ? compositeChange : compositeChange.invert();
   }
 
   /** Changes the game state to reflect the historical state at {@code node}. */
   public synchronized void gotoNode(final HistoryNode node) {
-    // Setting node to null causes problems, because we'll restore the state to the start, but then
-    // next gotoNode() call will reset currentNode to getLastNode() causing an invalid delta.
-    Preconditions.checkNotNull(node);
     assertCorrectThread();
-    gameData.acquireWriteLock();
-    try {
-      if (currentNode == null) {
-        currentNode = getLastNode();
-      }
-      final Change dataChange = getDelta(currentNode, node);
-      currentNode = node;
-      if (dataChange != null) {
-        gameData.performChange(dataChange);
+    Preconditions.checkNotNull(node);
+    Preconditions.checkState(seekingEnabled);
+    try (GameData.Unlocker ignored = gameData.acquireWriteLock()) {
+      final int nodeChangeIndex = getLastChange(node);
+      if (nodeChangeIndex != nextChangeIndex) {
+        gameData.performChange(getDeltaTo(nodeChangeIndex));
+        nextChangeIndex = nodeChangeIndex;
       }
-    } finally {
-      gameData.releaseWriteLock();
     }
   }
 
@@ -136,8 +133,7 @@ public synchronized void gotoNode(final HistoryNode node) {
   public synchronized void removeAllHistoryAfterNode(final HistoryNode removeAfterNode) {
     gotoNode(removeAfterNode);
     assertCorrectThread();
-    gameData.acquireWriteLock();
-    try {
+    try (GameData.Unlocker ignored = gameData.acquireWriteLock()) {
       final int lastChange = getLastChange(removeAfterNode) + 1;
       while (changes.size() > lastChange) {
         changes.remove(lastChange);
@@ -160,20 +156,16 @@ public synchronized void removeAllHistoryAfterNode(final HistoryNode removeAfter
         }
       }
       while (!nodesToRemove.isEmpty()) {
-        this.removeNodeFromParent(nodesToRemove.remove(0));
+        removeNodeFromParent(nodesToRemove.remove(0));
       }
-    } finally {
-      gameData.releaseWriteLock();
     }
   }
 
   synchronized void changeAdded(final Change change) {
     changes.add(change);
-    if (currentNode == null) {
-      return;
-    }
-    if (currentNode == getLastNode()) {
+    if (seekingEnabled && nextChangeIndex == changes.size()) {
       gameData.performChange(change);
+      nextChangeIndex = changes.size();
     }
   }
 

game-app/game-core/src/main/java/games/strategy/triplea/ui/history/HistoryPanel.java

@@ -80,7 +80,6 @@ public void mouseEntered(final MouseEvent e) {
     tree = new JTree(this.data.getHistory());
     // Register the tree with the tooltip manager to make the tooltips we set work.
     ToolTipManager.sharedInstance().registerComponent(tree);
-    this.data.getHistory().setTreePanel(this);
     tree.expandRow(0);
     this.popup = popup;
     tree.add(this.popup);
@@ -112,12 +111,10 @@ public void popupMenuWillBecomeVisible(final PopupMenuEvent pme) {}
     scroll.setBorder(null);
     scroll.setViewportBorder(null);
     add(scroll, BorderLayout.CENTER);
+    HistoryNode node = data.getHistory().enableSeeking(this);
     tree.setEditable(false);
-    final HistoryNode node = this.data.getHistory().getLastNode();
-    this.data.getHistory().gotoNode(node);
     tree.expandPath(new TreePath(node.getPath()));
     tree.setSelectionPath(new TreePath(node.getPath()));
-    currentPopupNode = null;
     final JButton previousButton = new JButton("<-Back");
     previousButton.addMouseListener(mouseFocusListener);
     previousButton.addActionListener(e -> previous());