Removes Engine.isPositionSet + Fixes bugs in UCI.debug & UCI.doPosition

src/main/java/com/fathzer/jchess/uci/Engine.java

@@ -91,10 +91,12 @@ default List<Option<?>> getOptions() {
 
 	/** Sets the start position.
 	 * @param fen The start position in the fen format.
+	 * @throws IllegalArgumentException if fen is illegal.
 	 */
 	void setStartPosition(String fen);
 	/** Moves a piece on the chess board.
 	 * @param move The move to apply.
+	 * @throws IllegalArgumentException if move is illegal.
 	 */
 	void move(UCIMove move);
 	/** Start searching for the best move.
@@ -106,9 +108,4 @@ default List<Option<?>> getOptions() {
 	 * @return A long running task able to compute the engine's move.
 	 */
 	LongRunningTask<GoReply> go(GoParameters params);
-
-	/** Tests whether a position is set.
-	 * @return true if a position is set
-	 */
-	boolean isPositionSet();
 }

src/main/java/com/fathzer/jchess/uci/UCI.java

@@ -53,6 +53,8 @@ public class UCI implements Runnable, AutoCloseable {
 	private boolean debugUCI = Boolean.getBoolean("debugUCI");
 	private Map<String, Option<?>> options;
 
+	private boolean isPositionSet;
+
 	public UCI(Engine defaultEngine) {
 		engines.put(defaultEngine.getId(), defaultEngine);
 		this.engine = defaultEngine;
@@ -147,6 +149,7 @@ protected void doIsReady(Deque<String> tokens) {
 
 	protected void doNewGame(Deque<String> tokens) {
 		getEngine().newGame();
+		isPositionSet = false;
 	}
 
 	protected void doPosition(Deque<String> tokens) {
@@ -165,13 +168,22 @@ protected void doPosition(Deque<String> tokens) {
 			return;
 		}
 		log("Setting board to FEN",fen);
-		getEngine().setStartPosition(fen);
-		tokens.stream().dropWhile(t->!MOVES.equals(t)).skip(1).forEach(this::doMove);
+		try {
+			getEngine().setStartPosition(fen);
+			tokens.stream().dropWhile(t->!MOVES.equals(t)).skip(1).forEach(this::doMove);
+			isPositionSet = true;
+		} catch (IllegalArgumentException e) {
+			debug("invalid position definition");
+		}
 	}
 
 	private void doMove(String move) {
 		log("Moving",move);
-		getEngine().move(UCIMove.from(move));
+		try {
+			getEngine().move(UCIMove.from(move));
+		} catch (IllegalArgumentException e) {
+			debug("invalid move "+move);
+		}
 	}
 
 	private String getFEN(Collection<String> tokens) {
@@ -189,7 +201,7 @@ protected boolean doBackground(Runnable task, Runnable stopper, Consumer<Excepti
 	}
 
 	protected void doGo(Deque<String> tokens) {
-		if (!engine.isPositionSet()) {
+		if (!isPositionSet()) {
 			debug("No position defined");
 		} else {
 			final Optional<GoParameters> goOptions = parse(GoParameters::new, GoParameters.PARSER, tokens);
@@ -240,7 +252,8 @@ protected void doEngine(Deque<String> tokens) {
 			if (newEngine.equals(this.engine)) {
 			 return;	
 			}
-			if (engine.isPositionSet()) {
+			if (isPositionSet()) {
+				isPositionSet = false;
 				debug("position is cleared by engine change");
 			}
 			this.engine = newEngine;
@@ -385,14 +398,27 @@ protected void out(CharSequence message) {
 		System.out.println(message);
 	}
 
+	/** Tests whether the debug mode is on.
+	 * @return true if debug mode is on.
+	 */
+	protected boolean isDebugMode() {
+		return debugUCI;
+	}
+
 	@SuppressWarnings("java:S106")
 	protected void debug(CharSequence message) {
     	log(":","info","UCI debug is", Boolean.toString(debugUCI),message.toString());
 		if (debugUCI) {
-			out("info string ");
-			out(message.toString());
+			out("info string "+message);
 		}
 	}
+
+	/** Tests whether a position is set.
+	 * @return true if a position is set
+	 */
+	protected boolean isPositionSet() {
+		return isPositionSet;
+	}
 
 	@Override
 	public void close() {

src/main/java/com/fathzer/jchess/uci/extended/Displayable.java

@@ -15,7 +15,7 @@ default String getBoardAsString() {
 
 	/** Returns the <a href="https://en.wikipedia.org/wiki/Forsyth%E2%80%93Edwards_Notation">FEN</a> representation of the board.
 	 * <br>Calling this method when no position is defined may lead to unpredictable results.
-	 * @return a string representing the chess board.
+	 * @return a string representing the chess board or null if no position is defined.
 	 */
 	String getFEN();
 }

src/main/java/com/fathzer/jchess/uci/extended/ExtendedUCI.java

@@ -35,13 +35,14 @@ public ExtendedUCI(Engine defaultEngine) {
 	}
 
 	protected void doDisplay(Deque<String> tokens) {
-		if (!engine.isPositionSet()) {
+		if (!isPositionSet()) {
 			debug(NO_POSITION_DEFINED);
 			return;
 		}
 		if (! (engine instanceof Displayable)) {
 			debug("position display is not supported by this engine");
 		}
+		//FIXME Engine could be non displayable here
 		final String result;
 		if (tokens.isEmpty()) {
 			result = ((Displayable)getEngine()).getBoardAsString();
@@ -55,7 +56,7 @@ protected void doDisplay(Deque<String> tokens) {
 	}
 
 	protected <M> void doPerft(Deque<String> tokens) {
-		if (!engine.isPositionSet()) {
+		if (!isPositionSet()) {
 			debug(NO_POSITION_DEFINED);
 			return;
 		}

src/main/java/com/fathzer/jchess/uci/extended/SpeedTest.java

@@ -72,7 +72,7 @@ public long run() {
 		final int size = policy.getSize();
 		final int accuracy = policy.getAccuracy();
 		final int depth = policy.getDepth();
-		final String fen = (uciEngine.isPositionSet() && uciEngine instanceof Displayable displayable) ? displayable.getFEN() : null;
+		final String fen = uciEngine instanceof Displayable displayable ? displayable.getFEN() : null;
 		try {
 			final long start = System.currentTimeMillis();
 			doSpeedTest();

src/main/java/com/fathzer/jchess/uci/helper/AbstractEngine.java

@@ -186,9 +186,4 @@ public B get() {
 	public IterativeDeepeningEngine<M, B> getEngine() {
 		return engine;
 	}
-
-	@Override
-	public boolean isPositionSet() {
-		return board!=null;
-	}
 }

src/test/java/com/fathzer/jchess/uci/UCITest.java

@@ -1,6 +1,8 @@
 package com.fathzer.jchess.uci;
 import static org.junit.jupiter.api.Assertions.*;
 
+import java.util.Arrays;
+import java.util.Collection;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.junit.jupiter.api.BeforeAll;
@@ -25,38 +27,104 @@ static void init() {
 
 	@BeforeEach
 	void clear() {
+		uci.post("ucinewgame", 100);
+		uci.post("debug on",100);
 		uci.clear();
 		engine.clear();
 	}
+
+	@Test
+	void unknownCommand() {
+		assertFalse(uci.post("cjhjhl",500));
+		assertFalse(uci.out().isEmpty());
+		assertDebug(uci.out());
+	}
+
+	@Test
+	void testDebugMode() {
+		uci.debug("test");
+		assertEquals(Arrays.asList("info string test"), uci.out());
+
+		// set debug with wrong arg
+		uci.out().clear();
+		uci.post("debug www", 100);
+		assertFalse(uci.out().isEmpty());
+		assertDebug(uci.out());
+
+		// set debug without arg
+		uci.out().clear();
+		uci.post("debug", 100);
+		assertFalse(uci.out().isEmpty());
+		assertDebug(uci.out());
+
+		// set debug off
+		uci.out().clear();
+		uci.post("debug off",100);
+		uci.debug("empty");
+		assertTrue(uci.out().isEmpty());
+
+		uci.out().clear();
+		uci.post("debug www", 100);
+		assertTrue(uci.out().isEmpty());
+	}
+
+	private void assertDebug(String string) {
+		assertTrue(string.startsWith("info string "), "expected \""+string+"\" started with \"info string \"");
+	}
+
+	private void assertDebug(Collection<String> strings) {
+		strings.stream().forEach(this::assertDebug);
+	}
 
 	@Test
-	void test() {
+	void testPositionAndNewGame() {
 		final AtomicReference<String> fen = new AtomicReference<>();
 
 		engine.setPositionConsumer(f -> fen.set(f));
-		assertFalse(uci.post("cjhjhl",500));
-		assertFalse(uci.getDebug().isEmpty());
-		assertNull(fen.get());
+		assertFalse(uci.isPositionSet());
 
+		// No position
 		uci.clear();
 		assertTrue(uci.post("position", 60000));
-		assertFalse(uci.getDebug().isEmpty());
+		assertFalse(uci.out().isEmpty());
+		assertDebug(uci.out());
 		assertNull(fen.get());
+		assertFalse(uci.isPositionSet());
 
+		// Illegal position kind
 		uci.clear();
 		assertTrue(uci.post("position kjmlkjm", 60000));
-		assertFalse(uci.getDebug().isEmpty());
+		assertFalse(uci.out().isEmpty());
+		assertDebug(uci.out());
 		assertNull(fen.get());
+		assertFalse(uci.isPositionSet());
 
+		// Start pos
 		uci.clear();
 		assertTrue(uci.post("position startpos", 60000));
-		assertTrue(uci.getDebug().isEmpty());
+		assertTrue(uci.out().isEmpty());
 		assertEquals("rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1", fen.get());
+		assertTrue(uci.isPositionSet());
 
+		// fen
 		fen.set(null);
+		uci.post("ucinewgame", 100);
+		uci.clear();
+		assertFalse(uci.isPositionSet());
 		assertTrue(uci.post("position fen toto", 60000));
-		assertTrue(uci.getDebug().isEmpty());
+		assertTrue(uci.out().isEmpty());
 		assertEquals("toto", fen.get());
+		assertTrue(uci.isPositionSet());
+
+		// Illegal fen
+		uci.post("ucinewgame", 100);
+		uci.clear();
+		engine.setPositionConsumer(f-> {throw new IllegalArgumentException();});
+		assertTrue(uci.post("position startpos", 60000));
+		assertFalse(uci.out().isEmpty());
+		assertDebug(uci.out());
+		assertFalse(uci.isPositionSet());
 
+		//TODO Test move related things
 	}
 }

src/test/java/com/fathzer/jchess/uci/util/InstrumentedEngine.java

@@ -13,7 +13,6 @@ public class InstrumentedEngine implements Engine {
 	private Consumer<String> positionConsumer;
 	private Consumer<UCIMove> moveConsumer;
 	private Function<GoParameters, LongRunningTask<GoReply>> goFunction;
-	private String fen;
 
 	@Override
 	public String getId() {
@@ -22,7 +21,6 @@ public String getId() {
 
 	@Override
 	public void setStartPosition(String fen) {
-		this.fen = fen;
 		positionConsumer.accept(fen);
 	}
 
@@ -36,11 +34,6 @@ public LongRunningTask<GoReply> go(GoParameters params) {
 		return goFunction.apply(params);
 	}
 
-	@Override
-	public boolean isPositionSet() {
-		return fen!=null;
-	}
-
 	public void setPositionConsumer(Consumer<String> positionConsumer) {
 		this.positionConsumer = positionConsumer;
 	}
@@ -54,7 +47,6 @@ public void setGoFunction(Function<GoParameters, LongRunningTask<GoReply>> goFun
 	}
 
 	public void clear() {
-		this.fen = null;
 		this.goFunction = null;
 		this.moveConsumer = null;
 		this.positionConsumer = null;

src/test/java/com/fathzer/jchess/uci/util/InstrumentedUCI.java

@@ -1,5 +1,8 @@
 package com.fathzer.jchess.uci.util;
 
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.ConcurrentHashMap;
@@ -20,15 +23,13 @@ private UnknownCommandException(String command) {
 	}
 
 	private final BlockingQueue<String> input;
-	private final BlockingQueue<String> output;
-	private final BlockingQueue<String> debug;
+	private final List<String> output;
 	private final Map<String, Throwable> exceptions;
 
 	public InstrumentedUCI(Engine defaultEngine) {
 		super(defaultEngine);
 		input = new LinkedBlockingQueue<String>();
-		output = new LinkedBlockingQueue<String>();
-		debug = new LinkedBlockingQueue<String>();
+		output = Collections.synchronizedList(new LinkedList<String>());
 		this.exceptions = new ConcurrentHashMap<>();
 	}
 
@@ -52,11 +53,6 @@ protected void err(String tag, Throwable e) {
 		exceptions.put(tag, e);
 	}
 
-	@Override
-	protected void debug(CharSequence message) {
-		debug.add(message.toString());
-	}
-
 	@Override
 	protected boolean doCommand(String command) {
 		boolean known;
@@ -99,16 +95,11 @@ public boolean post(String command, long timeOutMS) {
 	}
 
 	public void clear() {
-		debug.clear();
 		output.clear();
 		exceptions.clear();
 	}
 
-	public BlockingQueue<String> getOutput() {
+	public List<String> out() {
 		return output;
 	}
-
-	public BlockingQueue<String> getDebug() {
-		return debug;
-	}
 }