Enable undo for most recent select/next turn

src/game/__tests__/dominion-lib-undo-canUndoAction.spec.ts

@@ -4,7 +4,12 @@ import { ILogEntry } from '@/game/interfaces/log-entry';
 import { GameLogActionWithCount } from '@/game/enumerations/game-log-action-with-count';
 import { faker } from '@faker-js/faker';
 import { createMockGame } from '@/__fixtures__/dominion-lib-fixtures';
-import { DefaultTurnDetails, NoPlayerActions } from '@/game/constants';
+import {
+  ActionsWithOnlyLastActionUndo,
+  DefaultTurnDetails,
+  NoPlayerActions,
+  NoUndoActions,
+} from '@/game/constants';
 import { NotEnoughSupplyError } from '../errors/not-enough-supply';
 import { NotEnoughProphecyError } from '../errors/not-enough-prophecy';
 
@@ -278,49 +283,9 @@ describe('canUndoAction', () => {
     );
   });
 
-  it.each(NoPlayerActions)('should return false when undoing a start_game', () => {
+  it.each(NoUndoActions)('should return false when undoing a no undo action', (action) => {
     const game = createMockGame(2, {
-      log: [createLogEntry(GameLogActionWithCount.START_GAME)],
-    });
-
-    reconstructGameStateSpy.mockImplementation(() => {
-      // Simulate successful reconstruction
-    });
-
-    expect(undoModule.canUndoAction(game, 0)).toBe(false);
-    expect(removeTargetAndLinkedActionsSpy).not.toHaveBeenCalled();
-    expect(reconstructGameStateSpy).not.toHaveBeenCalled();
-    expect(consoleErrorSpy).not.toHaveBeenCalled();
-  });
-
-  it.each(NoPlayerActions)(
-    'should return false when undoing a no player action',
-    (action: GameLogActionWithCount) => {
-      const game = createMockGame(2, {
-        log: [
-          createLogEntry(GameLogActionWithCount.START_GAME),
-          createLogEntry(GameLogActionWithCount.NEXT_TURN),
-          createLogEntry(action),
-        ],
-      });
-
-      reconstructGameStateSpy.mockImplementation(() => {
-        // Simulate successful reconstruction
-      });
-
-      expect(undoModule.canUndoAction(game, 2)).toBe(false);
-      expect(removeTargetAndLinkedActionsSpy).not.toHaveBeenCalled();
-      expect(reconstructGameStateSpy).not.toHaveBeenCalled();
-      expect(consoleErrorSpy).not.toHaveBeenCalled();
-    }
-  );
-
-  it.each(NoPlayerActions)('should return false when undoing a select_player', () => {
-    const game = createMockGame(2, {
-      log: [
-        createLogEntry(GameLogActionWithCount.START_GAME),
-        createLogEntry(GameLogActionWithCount.SELECT_PLAYER),
-      ],
+      log: [createLogEntry(GameLogActionWithCount.START_GAME), createLogEntry(action)],
     });
 
     reconstructGameStateSpy.mockImplementation(() => {
@@ -350,6 +315,46 @@ describe('canUndoAction', () => {
     expect(consoleErrorSpy).not.toHaveBeenCalled();
   });
 
+  it.each(ActionsWithOnlyLastActionUndo)(
+    'should return true when undoing an action allowed for the most recent action',
+    (action) => {
+      const game = createMockGame(2, {
+        log: [createLogEntry(GameLogActionWithCount.START_GAME), createLogEntry(action)],
+      });
+
+      reconstructGameStateSpy.mockImplementation(() => {
+        // Simulate successful reconstruction
+      });
+
+      expect(undoModule.canUndoAction(game, 1)).toBe(true);
+      expect(removeTargetAndLinkedActionsSpy).toHaveBeenCalledTimes(1);
+      expect(reconstructGameStateSpy).toHaveBeenCalledTimes(1);
+      expect(consoleErrorSpy).not.toHaveBeenCalled();
+    }
+  );
+
+  it.each(ActionsWithOnlyLastActionUndo)(
+    'should return false when undoing an action allowed for the most recent action when it is not the most recent',
+    (action) => {
+      const game = createMockGame(2, {
+        log: [
+          createLogEntry(GameLogActionWithCount.START_GAME),
+          createLogEntry(action),
+          createLogEntry(GameLogActionWithCount.ADD_ACTIONS, 1),
+        ],
+      });
+
+      reconstructGameStateSpy.mockImplementation(() => {
+        // Simulate successful reconstruction
+      });
+
+      expect(undoModule.canUndoAction(game, 1)).toBe(false);
+      expect(removeTargetAndLinkedActionsSpy).not.toHaveBeenCalled();
+      expect(reconstructGameStateSpy).not.toHaveBeenCalled();
+      expect(consoleErrorSpy).not.toHaveBeenCalled();
+    }
+  );
+
   it('should handle multiple linked actions correctly when undoing one of the linked actions', () => {
     const mainActionId = 'main-action';
     const game = createMockGame(2, {

src/game/constants.ts

@@ -162,13 +162,29 @@ export const AdjustmentActions = [
   GameLogActionWithCount.REMOVE_NEXT_TURN_COINS,
 ];
 
+/**
+ * Actions that have an associated player index. Others are expected to have NO_PLAYER (-1).
+ */
 export const ActionsWithPlayer = [
   ...AdjustmentActions,
   GameLogActionWithCount.START_GAME,
   GameLogActionWithCount.NEXT_TURN,
   GameLogActionWithCount.SELECT_PLAYER,
 ];
 
+/**
+ * Actions that can only be undone if they are the last action in the game log.
+ */
+export const ActionsWithOnlyLastActionUndo = [
+  GameLogActionWithCount.SELECT_PLAYER,
+  GameLogActionWithCount.NEXT_TURN,
+];
+
+/**
+ * Actions that cannot be undone.
+ */
+export const NoUndoActions = [...NoPlayerActions, GameLogActionWithCount.START_GAME];
+
 export const StepTransitions: Record<CurrentStep, CurrentStep> = {
   [CurrentStep.AddPlayerNames]: CurrentStep.SelectFirstPlayer,
   [CurrentStep.SelectFirstPlayer]: CurrentStep.SetGameOptions,

src/game/dominion-lib-undo.ts

@@ -1,4 +1,9 @@
-import { NO_PLAYER, NoPlayerActions } from '@/game/constants';
+import {
+  ActionsWithOnlyLastActionUndo,
+  NO_PLAYER,
+  NoPlayerActions,
+  NoUndoActions,
+} from '@/game/constants';
 import {
   getActionIncrementMultiplier,
   getFieldAndSubfieldFromAction,
@@ -49,17 +54,13 @@ export function canUndoAction(game: IGame, logIndex: number): boolean {
   const actionToUndo = game.log[logIndex];
 
   // none of the No Player actions can be undone (end game, etc)
-  // start game, select player, should not be allowed
-  if (
-    NoPlayerActions.includes(actionToUndo.action) ||
-    actionToUndo.action === GameLogActionWithCount.START_GAME ||
-    actionToUndo.action === GameLogActionWithCount.SELECT_PLAYER
-  ) {
+  // as well as start game
+  if (NoUndoActions.includes(actionToUndo.action)) {
     return false;
   }
 
-  // can only undo next_turn if it is the most recent action
-  if (actionToUndo.action === GameLogActionWithCount.NEXT_TURN && !isMostRecent) {
+  // can only undo next_turn or select player if it is the most recent action
+  if (ActionsWithOnlyLastActionUndo.includes(actionToUndo.action) && !isMostRecent) {
     return false;
   }