From 412f6c421d2294b606f523d0e93fe61c97acc4b5 Mon Sep 17 00:00:00 2001 From: Jessica Mulein Date: Tue, 22 Oct 2024 22:30:14 +0000 Subject: [PATCH] rework log/start/next --- src/__fixtures__/dominion-lib-fixtures.ts | 2 +- src/components/DominionAssistant.tsx | 4 +- src/components/GameLogEntry.tsx | 81 ++++++++++-------- src/components/GameScreen.tsx | 10 ++- src/components/Player.tsx | 4 +- src/components/Scoreboard.tsx | 3 +- .../dominion-lib-NewGameState.spec.ts | 2 +- .../dominion-lib-load-save-loadGame.spec.ts | 4 +- ...inion-lib-load-save-loadGameAddLog.spec.ts | 10 +-- ...ion-lib-load-save-restoreSavedGame.spec.ts | 1 - .../dominion-lib-log-addLogEntry.spec.ts | 40 ++------- ...n-lib-log-getTimeSpanFromStartGame.spec.ts | 22 ++--- .../dominion-lib-log-logEntryToString.spec.ts | 21 ++--- .../dominion-lib-undo-applyLogAction.spec.ts | 19 ++--- .../dominion-lib-undo-canUndoAction.spec.ts | 83 +++++++++++++++++-- ...nion-lib-undo-reconstructGameState.spec.ts | 14 ++-- src/game/constants.ts | 10 ++- src/game/dominion-lib-log.ts | 7 +- src/game/dominion-lib-undo-helpers.ts | 3 +- src/game/dominion-lib-undo.ts | 26 ++++-- src/game/dominion-lib.ts | 17 ++-- src/game/interfaces/log-entry-raw.ts | 6 -- src/game/interfaces/log-entry.ts | 6 -- 23 files changed, 220 insertions(+), 175 deletions(-) diff --git a/src/__fixtures__/dominion-lib-fixtures.ts b/src/__fixtures__/dominion-lib-fixtures.ts index 9886280..78cd22a 100644 --- a/src/__fixtures__/dominion-lib-fixtures.ts +++ b/src/__fixtures__/dominion-lib-fixtures.ts @@ -47,7 +47,7 @@ export function createMockGame(playerCount: number, overrides?: Partial): { id: faker.string.uuid(), timestamp: new Date(), - playerIndex: NO_PLAYER, + playerIndex: 0, action: GameLogActionWithCount.START_GAME, }, ], diff --git a/src/components/DominionAssistant.tsx b/src/components/DominionAssistant.tsx index 9eb9363..45a87c3 100644 --- a/src/components/DominionAssistant.tsx +++ b/src/components/DominionAssistant.tsx @@ -67,10 +67,9 @@ const DominionAssistant: React.FC = ({ route, navigation const nextTurn = () => { setGameState((prevGame) => { const nextPlayerIndex = getNextPlayerIndex(prevGame); - addLogEntry(prevGame, NO_PLAYER, GameLogActionWithCount.NEXT_TURN, { + addLogEntry(prevGame, nextPlayerIndex, GameLogActionWithCount.NEXT_TURN, { playerTurnDetails: gameState.players.map((player) => player.turn), prevPlayerIndex: gameState.currentPlayerIndex, - newPlayerIndex: nextPlayerIndex, }); const updatedGame = incrementTurnCountersAndPlayerIndices(prevGame); return resetPlayerTurnCounters(updatedGame); @@ -81,7 +80,6 @@ const DominionAssistant: React.FC = ({ route, navigation setGameState((prevState) => { addLogEntry(prevState, NO_PLAYER, GameLogActionWithCount.END_GAME, { prevPlayerIndex: gameState.currentPlayerIndex, - newPlayerIndex: NO_PLAYER, }); return { diff --git a/src/components/GameLogEntry.tsx b/src/components/GameLogEntry.tsx index ca723c8..8f1e5cb 100644 --- a/src/components/GameLogEntry.tsx +++ b/src/components/GameLogEntry.tsx @@ -14,7 +14,6 @@ import { Button, Box, } from '@mui/material'; -import TurnIcon from '@mui/icons-material/TurnedIn'; // Example icon for new turn import EditIcon from '@mui/icons-material/Edit'; // Icon for corrections import LinkIcon from '@mui/icons-material/Link'; import UndoIcon from '@mui/icons-material/Undo'; @@ -61,16 +60,14 @@ const GameLogEntry: React.FC = ({ logIndex, entry, isCurrentP }; const actionText = logEntryToString(entry); - const playerName = - entry.playerIndex !== undefined && gameState.players[entry.playerIndex] - ? gameState.players[entry.playerIndex].name - : ''; + + const relevantPlayer = entry.playerIndex > -1 ? gameState.players[entry.playerIndex] : undefined; const isActivePlayer = entry.playerIndex === gameState.currentPlayerIndex; const isNewTurn = entry.action === GameLogActionWithCount.NEXT_TURN; const isAttributeChange = AdjustmentActions.includes(entry.action); const isAttributeChangeOutOfTurn = isAttributeChange && !isActivePlayer; - if (entry.playerIndex !== undefined && !gameState.players[entry.playerIndex]) { + if (entry.playerIndex > -1 && !gameState.players[entry.playerIndex]) { console.warn(`Player not found for index ${entry.playerIndex}`, { entry, gamePlayersLength: gameState.players.length, @@ -95,16 +92,12 @@ const GameLogEntry: React.FC = ({ logIndex, entry, isCurrentP - {playerName && ( + {relevantPlayer && ( : undefined} style={{ - backgroundColor: - entry.playerIndex !== undefined - ? gameState.players[entry.playerIndex].color - : 'gray', + backgroundColor: relevantPlayer !== undefined ? relevantPlayer.color : 'gray', color: 'white', marginRight: '8px', fontWeight: isActivePlayer ? 'bold' : 'normal', @@ -112,35 +105,51 @@ const GameLogEntry: React.FC = ({ logIndex, entry, isCurrentP }} /> )} - - {actionText} - - {isAttributeChangeOutOfTurn && ( - - )} - {entry.correction && ( - - - - )} + + {relevantPlayer !== undefined && ( + + <{relevantPlayer.name}>: + + )} + + {actionText} + + {isAttributeChangeOutOfTurn && ( + + )} + {entry.correction && ( + + + + )} + {entry.linkedActionId && } {canUndoAction(gameState, logIndex) && ( - + + + )} diff --git a/src/components/GameScreen.tsx b/src/components/GameScreen.tsx index f38fd2b..cfb3f44 100644 --- a/src/components/GameScreen.tsx +++ b/src/components/GameScreen.tsx @@ -1,5 +1,5 @@ import React, { useEffect, useState } from 'react'; -import { Box, Button, Dialog, DialogContent, Fab, styled } from '@mui/material'; +import { Box, Button, Dialog, DialogContent, Fab, styled, Tooltip } from '@mui/material'; import UndoIcon from '@mui/icons-material/Undo'; import InventoryIcon from '@mui/icons-material/Inventory'; import Scoreboard from '@/components/Scoreboard'; @@ -72,10 +72,14 @@ const GameScreen: React.FC = ({ nextTurn, endGame, undoLastActi - + + + - + + + diff --git a/src/components/Player.tsx b/src/components/Player.tsx index 6c44bf7..210d145 100644 --- a/src/components/Player.tsx +++ b/src/components/Player.tsx @@ -216,7 +216,9 @@ const Player: React.FC = () => { onChange={handleCorrectionChange} inputProps={{ 'aria-label': 'Correction Checkbox' }} /> - Correction + + Correction + {player && ( diff --git a/src/components/Scoreboard.tsx b/src/components/Scoreboard.tsx index bd8dee0..ba50de7 100644 --- a/src/components/Scoreboard.tsx +++ b/src/components/Scoreboard.tsx @@ -43,9 +43,8 @@ const Scoreboard: React.FC = () => { const handlePlayerSelect = (index: number) => { setGameState((prevState) => { - addLogEntry(prevState, NO_PLAYER, GameLogActionWithCount.SELECT_PLAYER, { + addLogEntry(prevState, index, GameLogActionWithCount.SELECT_PLAYER, { prevPlayerIndex: prevState.selectedPlayerIndex, - newPlayerIndex: index, }); return { ...prevState, selectedPlayerIndex: index }; }); diff --git a/src/game/__tests__/dominion-lib-NewGameState.spec.ts b/src/game/__tests__/dominion-lib-NewGameState.spec.ts index 9035960..6f354e2 100644 --- a/src/game/__tests__/dominion-lib-NewGameState.spec.ts +++ b/src/game/__tests__/dominion-lib-NewGameState.spec.ts @@ -38,7 +38,7 @@ describe('NewGameState', () => { id: expect.any(String), timestamp: expect.any(Date), action: GameLogActionWithCount.START_GAME, - playerIndex: -1, + playerIndex: initialGameState.firstPlayerIndex, } as ILogEntry, ]); expect(result.supply).toBeDefined(); diff --git a/src/game/__tests__/dominion-lib-load-save-loadGame.spec.ts b/src/game/__tests__/dominion-lib-load-save-loadGame.spec.ts index 6759780..4aa00ef 100644 --- a/src/game/__tests__/dominion-lib-load-save-loadGame.spec.ts +++ b/src/game/__tests__/dominion-lib-load-save-loadGame.spec.ts @@ -65,7 +65,7 @@ describe('loadGame', () => { id: id, action: GameLogActionWithCount.START_GAME, timestamp: new Date(), - playerIndex: NO_PLAYER, + playerIndex: 0, }, ], }); @@ -87,7 +87,7 @@ describe('loadGame', () => { id: faker.string.uuid(), action: GameLogActionWithCount.START_GAME, timestamp: new Date(), - playerIndex: NO_PLAYER, + playerIndex: 0, }, { id: saveGameLogId, diff --git a/src/game/__tests__/dominion-lib-load-save-loadGameAddLog.spec.ts b/src/game/__tests__/dominion-lib-load-save-loadGameAddLog.spec.ts index b51c3bc..8c38a7a 100644 --- a/src/game/__tests__/dominion-lib-load-save-loadGameAddLog.spec.ts +++ b/src/game/__tests__/dominion-lib-load-save-loadGameAddLog.spec.ts @@ -12,7 +12,7 @@ describe('loadGameAddLog', () => { log: [ { id: 'start', - playerIndex: NO_PLAYER, + playerIndex: 0, action: GameLogActionWithCount.START_GAME, timestamp: new Date(), } as ILogEntry, @@ -46,7 +46,7 @@ describe('loadGameAddLog', () => { log: [ { id: 'start', - playerIndex: NO_PLAYER, + playerIndex: 0, action: GameLogActionWithCount.START_GAME, timestamp: new Date(), } as ILogEntry, @@ -67,7 +67,7 @@ describe('loadGameAddLog', () => { log: [ { id: 'start', - playerIndex: NO_PLAYER, + playerIndex: 0, action: GameLogActionWithCount.START_GAME, timestamp: new Date(), } as ILogEntry, @@ -107,7 +107,7 @@ describe('loadGameAddLog', () => { log: [ { id: 'start', - playerIndex: NO_PLAYER, + playerIndex: 0, action: GameLogActionWithCount.START_GAME, timestamp: new Date(), } as ILogEntry, @@ -139,7 +139,7 @@ describe('loadGameAddLog', () => { log: [ { id: 'start', - playerIndex: NO_PLAYER, + playerIndex: 0, action: GameLogActionWithCount.START_GAME, timestamp: new Date(), } as ILogEntry, diff --git a/src/game/__tests__/dominion-lib-load-save-restoreSavedGame.spec.ts b/src/game/__tests__/dominion-lib-load-save-restoreSavedGame.spec.ts index 050ce8b..c6bed60 100644 --- a/src/game/__tests__/dominion-lib-load-save-restoreSavedGame.spec.ts +++ b/src/game/__tests__/dominion-lib-load-save-restoreSavedGame.spec.ts @@ -26,7 +26,6 @@ describe('restoreSavedGame', () => { action: GameLogActionWithCount.SAVE_GAME, timestamp: saveGameTime.toISOString(), playerIndex: NO_PLAYER, - playerName: 'player1', }; // Assuming `createMockGameRaw` returns an IGameRaw object with string timestamps diff --git a/src/game/__tests__/dominion-lib-log-addLogEntry.spec.ts b/src/game/__tests__/dominion-lib-log-addLogEntry.spec.ts index 1fc2cb4..4cf73c3 100644 --- a/src/game/__tests__/dominion-lib-log-addLogEntry.spec.ts +++ b/src/game/__tests__/dominion-lib-log-addLogEntry.spec.ts @@ -2,7 +2,6 @@ import { addLogEntry } from '@/game/dominion-lib-log'; import { GameLogActionWithCount } from '@/game/enumerations/game-log-action-with-count'; import { createMockGame } from '@/__fixtures__/dominion-lib-fixtures'; import { IGame } from '@/game/interfaces/game'; -import { NO_PLAYER } from '@/game/constants'; describe('addLogEntry', () => { let mockGame: IGame; @@ -12,10 +11,10 @@ describe('addLogEntry', () => { }); it('should add a log entry with minimal fields', () => { - const newEntry = addLogEntry(mockGame, NO_PLAYER, GameLogActionWithCount.START_GAME); + const newEntry = addLogEntry(mockGame, 0, GameLogActionWithCount.START_GAME); expect(mockGame.log).toContainEqual( expect.objectContaining({ - playerIndex: NO_PLAYER, + playerIndex: 0, action: GameLogActionWithCount.START_GAME, }) ); @@ -31,7 +30,6 @@ describe('addLogEntry', () => { expect(mockGame.log).toContainEqual( expect.objectContaining({ playerIndex: 0, - playerName: mockGame.players[0].name, action: GameLogActionWithCount.ADD_COINS, count: 5, correction: false, @@ -41,25 +39,11 @@ describe('addLogEntry', () => { ); }); - it('should add a log entry with special characters in player name', () => { - mockGame.players[0].name = 'Al!ce@#'; - const newEntry = addLogEntry(mockGame, 0, GameLogActionWithCount.ADD_COINS, { count: 5 }); - expect(mockGame.log).toContainEqual( - expect.objectContaining({ - playerIndex: 0, - playerName: 'Al!ce@#', - action: GameLogActionWithCount.ADD_COINS, - count: 5, - }) - ); - }); - it('should add a log entry with only some fields overridden', () => { const newEntry = addLogEntry(mockGame, 0, GameLogActionWithCount.ADD_COINS, { count: 5 }); expect(mockGame.log).toContainEqual( expect.objectContaining({ playerIndex: 0, - playerName: mockGame.players[0].name, action: GameLogActionWithCount.ADD_COINS, count: 5, }) @@ -80,7 +64,6 @@ describe('addLogEntry', () => { expect(mockGame.log).toContainEqual( expect.objectContaining({ playerIndex: 0, - playerName: mockGame.players[0].name, action: GameLogActionWithCount.ADD_COINS, count: 5, correction: true, @@ -97,7 +80,6 @@ describe('addLogEntry', () => { expect(mockGame.log).toContainEqual( expect.objectContaining({ playerIndex: 0, - playerName: mockGame.players[0].name, action: GameLogActionWithCount.ADD_COINS, count: 5, playerTurnDetails, @@ -120,41 +102,40 @@ describe('addLogEntry', () => { it('should throw an error when player index is provided for an action that does not require it', () => { expect(() => { - addLogEntry(mockGame, 0, GameLogActionWithCount.START_GAME, { count: 5 }); + addLogEntry(mockGame, 0, GameLogActionWithCount.END_GAME, { count: 5 }); }).toThrow('Player index is not relevant for this action'); }); it('should add a log entry with a valid NoPlayerActions action and playerIndex set to -1', () => { - const newEntry = addLogEntry(mockGame, -1, GameLogActionWithCount.START_GAME); + const newEntry = addLogEntry(mockGame, -1, GameLogActionWithCount.END_GAME); expect(mockGame.log).toContainEqual( expect.objectContaining({ playerIndex: -1, - action: GameLogActionWithCount.START_GAME, + action: GameLogActionWithCount.END_GAME, }) ); }); it('should throw an error when player index is provided for a NoPlayerActions action', () => { expect(() => { - addLogEntry(mockGame, 0, GameLogActionWithCount.START_GAME); + addLogEntry(mockGame, 0, GameLogActionWithCount.END_GAME); }).toThrow('Player index is not relevant for this action'); }); it('should throw an error when player index is out of range for a NoPlayerActions action', () => { expect(() => { - addLogEntry(mockGame, 99, GameLogActionWithCount.START_GAME); + addLogEntry(mockGame, 99, GameLogActionWithCount.END_GAME); }).toThrow('Player index is not relevant for this action'); }); it('should add a log entry with a valid NoPlayerActions action and playerIndex set to -1 with overrides', () => { - const newEntry = addLogEntry(mockGame, -1, GameLogActionWithCount.START_GAME, { + const newEntry = addLogEntry(mockGame, -1, GameLogActionWithCount.END_GAME, { correction: true, }); expect(mockGame.log).toContainEqual( expect.objectContaining({ playerIndex: -1, - action: GameLogActionWithCount.START_GAME, - correction: true, + action: GameLogActionWithCount.END_GAME, }) ); }); @@ -167,7 +148,6 @@ describe('addLogEntry', () => { expect(mockGame.log).toContainEqual( expect.objectContaining({ playerIndex: 0, - playerName: mockGame.players[0].name, action: GameLogActionWithCount.ADD_COINS, count: 5, correction: true, @@ -180,7 +160,6 @@ describe('addLogEntry', () => { expect(mockGame.log).toContainEqual( expect.objectContaining({ playerIndex: 0, - playerName: mockGame.players[0].name, action: GameLogActionWithCount.ADD_COINS, }) ); @@ -191,7 +170,6 @@ describe('addLogEntry', () => { expect(mockGame.log).toContainEqual( expect.objectContaining({ playerIndex: 0, - playerName: mockGame.players[0].name, action: GameLogActionWithCount.ADD_COINS, }) ); diff --git a/src/game/__tests__/dominion-lib-log-getTimeSpanFromStartGame.spec.ts b/src/game/__tests__/dominion-lib-log-getTimeSpanFromStartGame.spec.ts index 678cabe..2a83d85 100644 --- a/src/game/__tests__/dominion-lib-log-getTimeSpanFromStartGame.spec.ts +++ b/src/game/__tests__/dominion-lib-log-getTimeSpanFromStartGame.spec.ts @@ -7,7 +7,7 @@ describe('getTimeSpanFromStartGame', () => { const log: ILogEntry[] = [ { id: '1', - playerIndex: -1, + playerIndex: 0, timestamp: new Date('2023-01-01T00:00:00Z'), action: GameLogActionWithCount.START_GAME, }, @@ -21,7 +21,7 @@ describe('getTimeSpanFromStartGame', () => { const log: ILogEntry[] = [ { id: '1', - playerIndex: -1, + playerIndex: 0, timestamp: new Date('2023-01-01T00:00:00Z'), action: GameLogActionWithCount.START_GAME, }, @@ -35,7 +35,7 @@ describe('getTimeSpanFromStartGame', () => { const log: ILogEntry[] = [ { id: '1', - playerIndex: -1, + playerIndex: 0, timestamp: new Date('2023-01-01T01:00:00Z'), action: GameLogActionWithCount.START_GAME, }, @@ -49,7 +49,7 @@ describe('getTimeSpanFromStartGame', () => { const log: ILogEntry[] = [ { id: '1', - playerIndex: -1, + playerIndex: 0, timestamp: new Date('2023-01-01T00:00:00Z'), action: GameLogActionWithCount.START_GAME, }, @@ -63,7 +63,7 @@ describe('getTimeSpanFromStartGame', () => { const log: ILogEntry[] = [ { id: '1', - playerIndex: -1, + playerIndex: 0, timestamp: new Date('2023-01-01T00:00:00Z'), action: GameLogActionWithCount.START_GAME, }, @@ -77,7 +77,7 @@ describe('getTimeSpanFromStartGame', () => { const log: ILogEntry[] = [ { id: '1', - playerIndex: -1, + playerIndex: 0, timestamp: new Date('2023-01-01T00:00:00Z'), action: GameLogActionWithCount.START_GAME, }, @@ -91,7 +91,7 @@ describe('getTimeSpanFromStartGame', () => { const log: ILogEntry[] = [ { id: '1', - playerIndex: -1, + playerIndex: 0, timestamp: new Date('2023-01-01T00:00:00Z'), action: GameLogActionWithCount.START_GAME, }, @@ -105,7 +105,7 @@ describe('getTimeSpanFromStartGame', () => { const log: ILogEntry[] = [ { id: '1', - playerIndex: -1, + playerIndex: 0, timestamp: new Date('2023-01-01T00:00:00Z'), action: GameLogActionWithCount.START_GAME, }, @@ -131,7 +131,7 @@ describe('getTimeSpanFromStartGame', () => { const log: ILogEntry[] = [ { id: '1', - playerIndex: -1, + playerIndex: 0, timestamp: new Date('2023-01-01T00:00:00Z'), action: GameLogActionWithCount.START_GAME, }, @@ -169,7 +169,7 @@ describe('getTimeSpanFromStartGame', () => { const log: ILogEntry[] = [ { id: '1', - playerIndex: -1, + playerIndex: 0, timestamp: new Date('2023-01-01T00:00:00Z'), action: GameLogActionWithCount.START_GAME, }, @@ -201,7 +201,7 @@ describe('getTimeSpanFromStartGame', () => { const log: ILogEntry[] = [ { id: '1', - playerIndex: -1, + playerIndex: 0, timestamp: new Date('2023-01-01T00:00:00Z'), action: GameLogActionWithCount.START_GAME, }, diff --git a/src/game/__tests__/dominion-lib-log-logEntryToString.spec.ts b/src/game/__tests__/dominion-lib-log-logEntryToString.spec.ts index 3063c54..0852748 100644 --- a/src/game/__tests__/dominion-lib-log-logEntryToString.spec.ts +++ b/src/game/__tests__/dominion-lib-log-logEntryToString.spec.ts @@ -9,11 +9,10 @@ describe('logEntryToString', () => { id: faker.string.uuid(), timestamp: new Date(), playerIndex: 0, - playerName: 'Alice', action: GameLogActionWithCount.ADD_COINS, count: 5, }; - expect(logEntryToString(logEntry)).toBe(' Added 5 Coins'); + expect(logEntryToString(logEntry)).toBe('Added 5 Coins'); }); it('should return correct string without player name but with count', () => { @@ -32,10 +31,9 @@ describe('logEntryToString', () => { id: faker.string.uuid(), timestamp: new Date(), playerIndex: 0, - playerName: 'Bob', action: GameLogActionWithCount.ADD_COINS, }; - expect(logEntryToString(logEntry)).toBe(' Added Coins'); + expect(logEntryToString(logEntry)).toBe('Added Coins'); }); it('should return correct string without player name and count', () => { @@ -53,11 +51,10 @@ describe('logEntryToString', () => { id: faker.string.uuid(), timestamp: new Date(), playerIndex: 0, - playerName: 'Alice & Bob', action: GameLogActionWithCount.ADD_COINS, count: 3, }; - expect(logEntryToString(logEntry)).toBe(' Added 3 Coins'); + expect(logEntryToString(logEntry)).toBe('Added 3 Coins'); }); it('should handle undefined count correctly', () => { @@ -65,11 +62,10 @@ describe('logEntryToString', () => { id: faker.string.uuid(), timestamp: new Date(), playerIndex: 0, - playerName: 'Charlie', action: GameLogActionWithCount.ADD_COINS, count: undefined, }; - expect(logEntryToString(logEntry)).toBe(' Added Coins'); + expect(logEntryToString(logEntry)).toBe('Added Coins'); }); it('should handle correction log entry correctly', () => { @@ -77,12 +73,11 @@ describe('logEntryToString', () => { id: faker.string.uuid(), timestamp: new Date(), playerIndex: 0, - playerName: 'Dave', action: GameLogActionWithCount.ADD_COINS, count: 2, correction: true, }; - expect(logEntryToString(logEntry)).toBe(' Added 2 Coins (Correction)'); + expect(logEntryToString(logEntry)).toBe('Added 2 Coins'); }); it('should handle linked action log entry correctly', () => { @@ -90,12 +85,11 @@ describe('logEntryToString', () => { id: faker.string.uuid(), timestamp: new Date(), playerIndex: 0, - playerName: 'Eve', action: GameLogActionWithCount.ADD_COINS, count: 4, linkedActionId: 'some-linked-action-id', }; - expect(logEntryToString(logEntry)).toBe(' Added 4 Coins'); + expect(logEntryToString(logEntry)).toBe('Added 4 Coins'); }); it('should handle log entry with all fields correctly', () => { @@ -103,12 +97,11 @@ describe('logEntryToString', () => { id: faker.string.uuid(), timestamp: new Date(), playerIndex: 0, - playerName: 'Frank', action: GameLogActionWithCount.ADD_COINS, count: 6, correction: true, linkedActionId: 'some-linked-action-id', }; - expect(logEntryToString(logEntry)).toBe(' Added 6 Coins (Correction)'); + expect(logEntryToString(logEntry)).toBe('Added 6 Coins'); }); }); diff --git a/src/game/__tests__/dominion-lib-undo-applyLogAction.spec.ts b/src/game/__tests__/dominion-lib-undo-applyLogAction.spec.ts index f0708fd..d52114e 100644 --- a/src/game/__tests__/dominion-lib-undo-applyLogAction.spec.ts +++ b/src/game/__tests__/dominion-lib-undo-applyLogAction.spec.ts @@ -20,11 +20,10 @@ describe('applyLogAction', () => { const logEntry: ILogEntry = { action: GameLogActionWithCount.NEXT_TURN, - playerIndex: NO_PLAYER, + playerIndex: 1, id: '1', timestamp: new Date(), playerTurnDetails: [{ ...DefaultTurnDetails }, { ...DefaultTurnDetails }], - newPlayerIndex: 1, prevPlayerIndex: 0, }; @@ -51,10 +50,9 @@ describe('applyLogAction', () => { it('should handle NEXT_TURN action', () => { const logEntry: ILogEntry = { action: GameLogActionWithCount.NEXT_TURN, - playerIndex: NO_PLAYER, + playerIndex: 1, id: '1', timestamp: new Date(), - newPlayerIndex: 1, prevPlayerIndex: 0, }; @@ -68,10 +66,9 @@ describe('applyLogAction', () => { mockGame.currentPlayerIndex = 1; const logEntry: ILogEntry = { action: GameLogActionWithCount.NEXT_TURN, - playerIndex: NO_PLAYER, + playerIndex: 0, id: '1', timestamp: new Date(), - newPlayerIndex: 0, prevPlayerIndex: 1, }; @@ -116,7 +113,6 @@ describe('applyLogAction', () => { const logEntry: ILogEntry = { action: GameLogActionWithCount.ADD_PROPHECY, playerIndex: 0, - playerName: 'Player 1', count: 3, id: '1', timestamp: new Date(), @@ -248,24 +244,23 @@ describe('applyLogAction', () => { const logEntry: ILogEntry = { id: '1', - playerIndex: -1, + playerIndex: 2, timestamp: new Date(), action: GameLogActionWithCount.SELECT_PLAYER, - newPlayerIndex: 2, }; const updatedGame = applyLogAction(game, logEntry); expect(updatedGame.selectedPlayerIndex).toBe(2); }); - it('should not change the selectedPlayerIndex if newPlayerIndex is not provided', () => { + it('should change the selectedPlayerIndex for select_player', () => { const game: IGame = createMockGame(3, { - selectedPlayerIndex: 0, + selectedPlayerIndex: 2, }); const logEntry: ILogEntry = { id: '1', - playerIndex: -1, + playerIndex: 0, timestamp: new Date(), action: GameLogActionWithCount.SELECT_PLAYER, }; diff --git a/src/game/__tests__/dominion-lib-undo-canUndoAction.spec.ts b/src/game/__tests__/dominion-lib-undo-canUndoAction.spec.ts index 04225b7..3eccc65 100644 --- a/src/game/__tests__/dominion-lib-undo-canUndoAction.spec.ts +++ b/src/game/__tests__/dominion-lib-undo-canUndoAction.spec.ts @@ -63,7 +63,7 @@ describe('canUndoAction', () => { expect(consoleErrorSpy).not.toHaveBeenCalled(); }); - it('should return true for NEXT_TURN', () => { + it('should return true for NEXT_TURN when most recent action', () => { const game = createMockGame(2, { log: [ createLogEntry(GameLogActionWithCount.ADD_ACTIONS), @@ -76,6 +76,20 @@ describe('canUndoAction', () => { expect(consoleErrorSpy).not.toHaveBeenCalled(); }); + it('should return false for NEXT_TURN when not most recent action', () => { + const game = createMockGame(2, { + log: [ + createLogEntry(GameLogActionWithCount.START_GAME), + createLogEntry(GameLogActionWithCount.NEXT_TURN), + createLogEntry(GameLogActionWithCount.ADD_ACTIONS), + ], + }); + expect(undoModule.canUndoAction(game, 1)).toBe(false); + expect(removeTargetAndLinkedActionsSpy).toHaveBeenCalledTimes(0); + expect(reconstructGameStateSpy).toHaveBeenCalledTimes(0); + expect(consoleErrorSpy).not.toHaveBeenCalled(); + }); + it('should return true for a regular action that can be undone', () => { const game = createMockGame(2, { log: [createLogEntry(GameLogActionWithCount.ADD_ACTIONS, 1)], @@ -263,35 +277,90 @@ describe('canUndoAction', () => { ); }); - it.each(NoPlayerActions.filter((x) => x !== GameLogActionWithCount.NEXT_TURN))( - 'should return false when undoing a NoPlayerAction other than NEXT_TURN', - () => { + it.each(NoPlayerActions)('should return false when undoing a start_game', () => { + 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)], + log: [ + createLogEntry(GameLogActionWithCount.START_GAME), + createLogEntry(GameLogActionWithCount.NEXT_TURN), + createLogEntry(action), + ], }); reconstructGameStateSpy.mockImplementation(() => { // Simulate successful reconstruction }); - expect(undoModule.canUndoAction(game, 0)).toBe(false); + 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), + ], + }); + + 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', () => { const mainActionId = 'main-action'; const game = createMockGame(2, { log: [ + createLogEntry(GameLogActionWithCount.START_GAME), createLogEntry(GameLogActionWithCount.ADD_ACTIONS, 1, mainActionId), createLogEntry(GameLogActionWithCount.REMOVE_ACTIONS, 1, undefined, mainActionId), createLogEntry(GameLogActionWithCount.ADD_BUYS, 1, undefined, mainActionId), ], }); - expect(undoModule.canUndoAction(game, 0)).toBe(true); + expect(undoModule.canUndoAction(game, 1)).toBe(true); + expect(removeTargetAndLinkedActionsSpy).toHaveBeenCalledTimes(1); + expect(reconstructGameStateSpy).toHaveBeenCalledTimes(1); + 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, { + log: [ + createLogEntry(GameLogActionWithCount.START_GAME), + createLogEntry(GameLogActionWithCount.ADD_ACTIONS, 1, mainActionId), + createLogEntry(GameLogActionWithCount.REMOVE_ACTIONS, 1, undefined, mainActionId), + createLogEntry(GameLogActionWithCount.ADD_BUYS, 1, undefined, mainActionId), + ], + }); + + expect(undoModule.canUndoAction(game, 2)).toBe(true); expect(removeTargetAndLinkedActionsSpy).toHaveBeenCalledTimes(1); expect(reconstructGameStateSpy).toHaveBeenCalledTimes(1); expect(consoleErrorSpy).not.toHaveBeenCalled(); diff --git a/src/game/__tests__/dominion-lib-undo-reconstructGameState.spec.ts b/src/game/__tests__/dominion-lib-undo-reconstructGameState.spec.ts index 33de889..efa75df 100644 --- a/src/game/__tests__/dominion-lib-undo-reconstructGameState.spec.ts +++ b/src/game/__tests__/dominion-lib-undo-reconstructGameState.spec.ts @@ -23,7 +23,7 @@ describe('reconstructGameState', () => { }); }); - it('should return a new game state when given an empty log', () => { + it('should return an identical game state when given an existing game', () => { const result = reconstructGameState(baseGame); expect(result).toEqual(baseGame); }); @@ -84,9 +84,8 @@ describe('reconstructGameState', () => { id: faker.string.uuid(), timestamp: new Date(), action: GameLogActionWithCount.NEXT_TURN, - playerIndex: NO_PLAYER, + playerIndex: nextPlayerIndex, playerTurnDetails: [{ ...DefaultTurnDetails }, { ...DefaultTurnDetails }], - newPlayerIndex: nextPlayerIndex, prevPlayerIndex: baseGame.currentPlayerIndex, } as ILogEntry, ], @@ -115,12 +114,11 @@ describe('reconstructGameState', () => { id: faker.string.uuid(), timestamp: new Date(), action: GameLogActionWithCount.NEXT_TURN, - playerIndex: NO_PLAYER, + playerIndex: nextPlayerIndex, playerTurnDetails: [ { actions: 5, coins: 2, buys: 4 } as IPlayerGameTurnDetails, { ...DefaultTurnDetails }, ], - newPlayerIndex: nextPlayerIndex, prevPlayerIndex: baseGame.currentPlayerIndex, }, ], @@ -266,9 +264,8 @@ describe('reconstructGameState', () => { id: faker.string.uuid(), timestamp: new Date(), action: GameLogActionWithCount.NEXT_TURN, - playerIndex: NO_PLAYER, + playerIndex: 0, playerTurnDetails: [{ ...DefaultTurnDetails, coins: 3 }, { ...DefaultTurnDetails }], - newPlayerIndex: 0, prevPlayerIndex: 1, }, { @@ -285,9 +282,8 @@ describe('reconstructGameState', () => { id: faker.string.uuid(), timestamp: new Date(), action: GameLogActionWithCount.NEXT_TURN, - playerIndex: NO_PLAYER, + playerIndex: 1, playerTurnDetails: [{ ...DefaultTurnDetails }, { ...DefaultTurnDetails, actions: 3 }], - newPlayerIndex: 1, prevPlayerIndex: 0, }, { diff --git a/src/game/constants.ts b/src/game/constants.ts index 6ed2c0b..fa9806a 100644 --- a/src/game/constants.ts +++ b/src/game/constants.ts @@ -113,12 +113,9 @@ export const EmptyVictoryDetails: IVictoryDetails = { * A list of actions that do not affect player state. */ export const NoPlayerActions = [ - GameLogActionWithCount.START_GAME, GameLogActionWithCount.END_GAME, GameLogActionWithCount.SAVE_GAME, GameLogActionWithCount.LOAD_GAME, - GameLogActionWithCount.NEXT_TURN, - GameLogActionWithCount.SELECT_PLAYER, ]; export const AdjustmentActions = [ @@ -165,6 +162,13 @@ export const AdjustmentActions = [ GameLogActionWithCount.REMOVE_NEXT_TURN_COINS, ]; +export const ActionsWithPlayer = [ + ...AdjustmentActions, + GameLogActionWithCount.START_GAME, + GameLogActionWithCount.NEXT_TURN, + GameLogActionWithCount.SELECT_PLAYER, +]; + export const StepTransitions: Record = { [CurrentStep.AddPlayerNames]: CurrentStep.SelectFirstPlayer, [CurrentStep.SelectFirstPlayer]: CurrentStep.SetGameOptions, diff --git a/src/game/dominion-lib-log.ts b/src/game/dominion-lib-log.ts index 795538b..f71aec9 100644 --- a/src/game/dominion-lib-log.ts +++ b/src/game/dominion-lib-log.ts @@ -120,7 +120,6 @@ export function victoryFieldToGameLogAction( * @returns The string representation of the log entry */ export function logEntryToString(entry: ILogEntry): string { - const playerName = entry.playerName ? `<${entry.playerName}> ` : ''; let actionString = entry.action as string; if (entry.count !== undefined) { @@ -130,9 +129,7 @@ export function logEntryToString(entry: ILogEntry): string { actionString = actionString.replace(' {COUNT}', ''); } - const correctionString = entry.correction ? ' (Correction)' : ''; - - return `${playerName}${actionString}${correctionString}`; + return actionString; } /** @@ -228,13 +225,11 @@ export function addLogEntry( } else if (playerIndex > -1) { throw new Error('Player index is not relevant for this action'); } - const playerName = playerIndex > -1 ? game.players[playerIndex].name : undefined; const newLog: ILogEntry = { id: uuidv4(), timestamp: new Date(), action, playerIndex, - playerName, ...overrides, }; game.log.push(newLog); diff --git a/src/game/dominion-lib-undo-helpers.ts b/src/game/dominion-lib-undo-helpers.ts index e62c4c9..d7984b4 100644 --- a/src/game/dominion-lib-undo-helpers.ts +++ b/src/game/dominion-lib-undo-helpers.ts @@ -62,8 +62,9 @@ export function reconstructGameState(game: IGame): IGame { firstPlayerIndex: game.firstPlayerIndex, currentPlayerIndex: game.firstPlayerIndex, selectedPlayerIndex: game.firstPlayerIndex, - log: [], }); + // clear the log + reconstructedGame.log = []; for (let i = 0; i <= game.log.length - 1; i++) { reconstructedGame = applyLogAction(reconstructedGame, game.log[i]); diff --git a/src/game/dominion-lib-undo.ts b/src/game/dominion-lib-undo.ts index b458aa4..297093f 100644 --- a/src/game/dominion-lib-undo.ts +++ b/src/game/dominion-lib-undo.ts @@ -41,6 +41,7 @@ export function canUndoAction(game: IGame, logIndex: number): boolean { if (logIndex < 0 || logIndex >= game.log.length) { return false; } + const isMostRecent = logIndex === game.log.length - 1; if (game.currentStep !== CurrentStep.GameScreen) { return false; @@ -48,14 +49,21 @@ export function canUndoAction(game: IGame, logIndex: number): boolean { const actionToUndo = game.log[logIndex]; - // none of the No Player actions except Next Turn can be undone (start game, etc) + // 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.NEXT_TURN + NoPlayerActions.includes(actionToUndo.action) || + actionToUndo.action === GameLogActionWithCount.START_GAME || + actionToUndo.action === GameLogActionWithCount.SELECT_PLAYER ) { return false; } + // can only undo next_turn if it is the most recent action + if (actionToUndo.action === GameLogActionWithCount.NEXT_TURN && !isMostRecent) { + return false; + } + // Find the main action if this is a linked action let mainActionIndex = logIndex; if (actionToUndo.linkedActionId !== undefined) { @@ -121,9 +129,15 @@ export function undoAction(game: IGame, logIndex: number): { game: IGame; succes export function applyLogAction(game: IGame, logEntry: ILogEntry): IGame { let updatedGame = { ...game }; - if (logEntry.action === GameLogActionWithCount.NEXT_TURN) { + if (logEntry.action === GameLogActionWithCount.START_GAME) { + // set first player to the player who started the game + updatedGame.firstPlayerIndex = logEntry.playerIndex; + updatedGame.selectedPlayerIndex = logEntry.playerIndex; + } else if (logEntry.action === GameLogActionWithCount.NEXT_TURN) { // Move to next player - updatedGame = incrementTurnCountersAndPlayerIndices(updatedGame); + updatedGame.currentTurn = game.currentTurn + 1; + updatedGame.currentPlayerIndex = logEntry.playerIndex; + updatedGame.selectedPlayerIndex = logEntry.playerIndex; // Reset all players' turn counters to their newTurn values updatedGame.players = updatedGame.players.map((player) => ({ @@ -131,7 +145,7 @@ export function applyLogAction(game: IGame, logEntry: ILogEntry): IGame { turn: player.newTurn, })); } else if (logEntry.action === GameLogActionWithCount.SELECT_PLAYER) { - updatedGame.selectedPlayerIndex = logEntry.newPlayerIndex ?? updatedGame.selectedPlayerIndex; + updatedGame.selectedPlayerIndex = logEntry.playerIndex ?? updatedGame.selectedPlayerIndex; } else if ( logEntry.playerIndex !== NO_PLAYER && logEntry.playerIndex < updatedGame.players.length && diff --git a/src/game/dominion-lib.ts b/src/game/dominion-lib.ts index 5dbd61a..b7d9749 100644 --- a/src/game/dominion-lib.ts +++ b/src/game/dominion-lib.ts @@ -152,14 +152,7 @@ export const EmptyGameState: IGame = { currentPlayerIndex: NO_PLAYER, firstPlayerIndex: NO_PLAYER, selectedPlayerIndex: NO_PLAYER, - log: [ - { - id: uuidv4(), - timestamp: new Date(), - action: GameLogActionWithCount.START_GAME, - playerIndex: NO_PLAYER, - }, - ], + log: [], }; /** @@ -193,6 +186,14 @@ export const NewGameState = (gameStateWithOptions: IGame): IGame => { supply: initialSupply, currentStep: CurrentStep.GameScreen, currentTurn: 1, + log: [ + { + id: uuidv4(), + timestamp: new Date(), + playerIndex: gameStateWithOptions.firstPlayerIndex, + action: GameLogActionWithCount.START_GAME, + }, + ], }; // Distribute initial supply to players diff --git a/src/game/interfaces/log-entry-raw.ts b/src/game/interfaces/log-entry-raw.ts index fcd06e9..d2865ba 100644 --- a/src/game/interfaces/log-entry-raw.ts +++ b/src/game/interfaces/log-entry-raw.ts @@ -5,7 +5,6 @@ export interface ILogEntryRaw { timestamp: string; action: string; playerIndex: number; - playerName?: string; count?: number; correction?: boolean; linkedActionId?: string; @@ -14,11 +13,6 @@ export interface ILogEntryRaw { * Applicable to 'next turn' and 'select player' actions */ prevPlayerIndex?: number; - /** - * Index of the newly selected player - * Applicable to 'next turn' and 'select player' actions - */ - newPlayerIndex?: number; /** * Details of all player's turn counters at the time of this log entry * Used when undoing a "next turn" action diff --git a/src/game/interfaces/log-entry.ts b/src/game/interfaces/log-entry.ts index 0543b94..a52b2bb 100644 --- a/src/game/interfaces/log-entry.ts +++ b/src/game/interfaces/log-entry.ts @@ -6,7 +6,6 @@ export interface ILogEntry { timestamp: Date; action: GameLogActionWithCount; playerIndex: number; - playerName?: string; count?: number; correction?: boolean; linkedActionId?: string; @@ -15,11 +14,6 @@ export interface ILogEntry { * Applicable to 'next turn' and 'select player' actions */ prevPlayerIndex?: number; - /** - * Index of the newly selected player - * Applicable to 'next turn' and 'select player' actions - */ - newPlayerIndex?: number; /** * Details of all player's turn counters at the time of this log entry * Used when undoing a "next turn" action