From 1f3d5664bf952eb311faa34d1878aa66ccd90d8d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 18:14:34 +0000 Subject: [PATCH] Take the evaluated move back when practice mode's search is interrupted extended_compute_move (practice-mode evaluation) makes each candidate move, searches, and unmakes it. If the opponent has to pass after the candidate, it runs a second search for the same side. When an undo or redo interrupted that second search (force_return), the loop left without unmake_move, so the candidate stayed on the board and every following undo/redo worked on a wrong position. This is the WthorReplayTest flake "WThor game 155 undo of ply 28 (g8) did not clear that square": after ply 28 White has to pass after most of Black's moves, so an undo arriving during the evaluation often hit the second search. New host test undoDuringAPracticeEvaluationLeavesTheBoardIntact undoes ply 28 of that game at varying points in the evaluation and compares the board with the known position after ply 27. It failed on the first attempt before this fix (a4 left on the board) and passes 60 of 60 attempts with it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V5G4cWSLBkiNNK4KC3zvXM --- project/src/main/jni/zebra/game.c | 7 ++- .../shurik/droidzebra/HostJniSmokeTest.java | 59 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/project/src/main/jni/zebra/game.c b/project/src/main/jni/zebra/game.c index 2d0d517..3ec0f98 100644 --- a/project/src/main/jni/zebra/game.c +++ b/project/src/main/jni/zebra/game.c @@ -865,8 +865,13 @@ extended_compute_move( int side_to_move, int book_only, this_eval.res = WON_POSITION; } - if ( force_return ) + if ( force_return ) { + /* The second search above (the opponent had to pass) can be + interrupted too - take THIS_MOVE back before leaving, like the + check after the first search does. */ + unmake_move( side_to_move, this_move ); break; + } else evaluated_list[index].eval = this_eval; diff --git a/project/src/test/java/com/shurik/droidzebra/HostJniSmokeTest.java b/project/src/test/java/com/shurik/droidzebra/HostJniSmokeTest.java index c512a90..aac30a7 100644 --- a/project/src/test/java/com/shurik/droidzebra/HostJniSmokeTest.java +++ b/project/src/test/java/com/shurik/droidzebra/HostJniSmokeTest.java @@ -210,6 +210,9 @@ public void onGameStateReady(GameState gameState) { // case exercised separately below. private static final int PASSLESS_GAME_INDEX = 4; + // See undoDuringAPracticeEvaluationLeavesTheBoardIntact. + private static final int PASS_AFTER_EVALUATED_MOVE_GAME_INDEX = 155; + @Test public void playsFullGameThenUndoesAndRedoesCleanly() throws Exception { File nativeLib = findNativeLib(); @@ -611,6 +614,62 @@ public void undoDuringAPracticeEvaluationIsTakenRightAway() throws Exception { } } + // WThor game 155 after ply 28 (White's only move, g8): Black to move, and + // after many of Black's moves (h8 among them) White has to pass. Practice + // mode evaluates each of those moves with a second search for Black. An + // undo that interrupted that second search used to leave the move on the + // board, so the position after the undo was wrong (seen in CI as + // WthorReplayTest's "undo of ply 28 (g8) did not clear that square"). + @Test + public void undoDuringAPracticeEvaluationLeavesTheBoardIntact() throws Exception { + File nativeLib = findNativeLib(); + Assume.assumeTrue("host libdroidzebra not built (run project/hostjni's Makefile first) - " + + "skipping, this is expected on workflows that don't build it", nativeLib != null); + File assetsDir = findAssetsDir(); + Assume.assumeTrue("could not locate project/src/main/assets from " + new File(".").getAbsolutePath(), + assetsDir != null); + File wthorFile = findWthorFile(); + Assume.assumeTrue("could not locate " + WTHOR_FILE_NAME + " from " + new File(".").getAbsolutePath(), + wthorFile != null); + + byte[] file = Files.readAllBytes(wthorFile.toPath()); + byte[] fullGameMoves = decodeGameMoveInts(file, PASS_AFTER_EVALUATED_MOVE_GAME_INDEX); + String fullGameText = decodeGameMoveText(file, PASS_AFTER_EVALUATED_MOVE_GAME_INDEX); + int ply = 28; + String beforeUndo = fullGameText.substring(0, 2 * ply); + String afterUndo = fullGameText.substring(0, 2 * (ply - 1)); + ZebraEngine engine = ZebraEngine.get(new TestGameContext(filesDir.getRoot(), assetsDir)); + try { + GameState reference = loadGame(engine, java.util.Arrays.copyOf(fullGameMoves, ply - 1), + new EngineConfig(FUNCTION_HUMAN_VS_HUMAN, 1, 1, 1, + false, null, false, false, false, 0, 0, 0)); + waitForMoveSequence(reference, afterUndo); + waitForUserInputWait(engine); + byte[][] expectedBoard = captureBoard(reference); + + GameState gameState = loadGame(engine, java.util.Arrays.copyOf(fullGameMoves, ply), + new EngineConfig(FUNCTION_HUMAN_VS_HUMAN, 10, 10, 10, + false, null, false, true, false, 0, 0, 0)); + waitForMoveSequence(gameState, beforeUndo); + java.util.Random random = new java.util.Random(PASS_AFTER_EVALUATED_MOVE_GAME_INDEX); + for (int attempt = 0; attempt < 60; attempt++) { + long deadline = System.currentTimeMillis() + WAIT_TIMEOUT_MILLIS; + while (engine.getState() != ZebraEngine.ENGINE_STATE.ES_PLAY_IN_PROGRESS + && System.currentTimeMillis() < deadline) { + Thread.sleep(1); + } + // Spread the undo over the whole evaluation. + Thread.sleep(random.nextInt(60)); + undoOnce(engine, gameState, afterUndo, ply); + assertEquals("board after undo #" + attempt + " of ply " + ply, + "", diffBoards(expectedBoard, captureBoard(gameState))); + redoOnce(engine, gameState, beforeUndo, ply); + } + } finally { + engine.forceStopGame(); + } + } + // A stop that comes while the engine is busy but about to ask for input // (here: from a board update, which the engine sends right before it // asks) used to be lost - the exit event was only posted if the engine