Merge pull request #205 from lrozenblyum/Validator

Validator

src/main/java/com/leokom/chess/engine/Position.java

@@ -857,6 +857,17 @@ private boolean isObligatoryDraw() {
 		return movesTillDraw.isPresent() && pliesCount >= movesTillDraw.getAsInt() * PLIES_IN_MOVE;
 	}
 
+	/**
+	 * Check whether the given move is legal in current position
+	 * @param move move to check
+	 * @return true if the move is legal, false otherwise
+	 */
+	public boolean isLegal( Move move ) {
+		//NOTE: great point to OPTIMIZE, we don't need calculating the whole set
+		//lazy calculation via lambdas could greatly improve performance
+		return getMoves().contains( move );
+	}
+
 	/**
 	 * @return legal non-special moves
 	 */

src/main/java/com/leokom/chess/player/winboard/WinboardCommander.java

@@ -56,7 +56,11 @@ interface WinboardCommander {
 	 */
 	void stalemateDraw();
 
-
+    /**
+     * Inform Winboard that an illegal move has been executed
+	 * @param winboardMove original move received from Winboard that is illegal
+     */
+    void illegalMove( String winboardMove );
 
 	void onProtover( ProtoverListener protoverListener );
 	void onQuit( QuitListener listener );
@@ -67,4 +71,6 @@ interface WinboardCommander {
 	void onForce( ForceListener listener );
 	void onNew( NewListener listener );
 
+
+
 }

src/main/java/com/leokom/chess/player/winboard/WinboardCommanderImpl.java

@@ -143,6 +143,28 @@ public void checkmate( Side winningSide ) {
 		communicator.send( prefix + " {Checkmate}" );
 	}
 
+	/**
+	 * @inheritDoc
+	 *
+	 * The implementation doesn't provide the optional REASON.
+	 */
+	@Override
+	public void illegalMove( String winboardMove ) {
+		//Spec: If your engine receives a MOVE command that is recognizably a move but is not legal in the current position, your engine must print an error message in one of the above formats so that xboard can pass the error on to the user and retract the move. The (REASON) is entirely optional. Examples:
+		//Illegal move: e2e4
+		//Illegal move (in check): Nf3
+		//Illegal move (moving into check): e1g1
+
+		//LR: practically I don't see ANY usefulness from passing back the winboardMove
+		//it's not reflected on UI so far (maybe topical for network games?)
+		//anyway I follow the spec.
+
+		//in contrary to Winboard UI, Arena UI shows the illegal move,
+		//so it's useful. However Arena UI got stuck after this situation, I sent a message to the
+		//developers with the issue report.
+		communicator.send( "Illegal move: " + winboardMove );
+	}
+
 	@Override
 	public void onXBoard( XBoardListener listener ) {
 		listenersWithoutParams.put( "xboard", listener );

src/main/java/com/leokom/chess/player/winboard/WinboardPlayer.java

@@ -259,15 +259,14 @@ private class WinboardUserMoveListener implements UserMoveListener {
 		 */
 		@Override
 		public void execute( String move ) {
-			String translatedMove = move;
-			if ( isPromotion( move ) ) {
-				translatedMove = translatedMove.substring( 0, PROMOTION_MOVE_LENGTH - 1 ) + translatedMove.substring( PROMOTION_MOVE_LENGTH - 1 ).toUpperCase();
-			}
+			final Move engineMove = translateMove( move );
 
-			String squareFrom = translatedMove.substring( 0, SQUARE_FROM_LENGTH );
-			String destination = translatedMove.substring( 2 );
+			if ( !position.isLegal( engineMove ) ) {
+				//the original move should be passed back to the UI
+				commander.illegalMove( move );
+				return;
+			}
 
-			final Move engineMove = new Move( squareFrom, destination );
 			position = position.move( engineMove );
 
 			detectGameOver();
@@ -278,5 +277,22 @@ public void execute( String move ) {
 			//only THEN inform the opponent!
 			opponent.opponentMoved( engineMove );
 		}
+
+		/**
+		 * Translate move from Winboard to LeokomChess engine move
+		 * @param move winboard move
+		 * @return engine move
+		 */
+		private Move translateMove( String move ) {
+			String translatedMove = move;
+			if ( isPromotion( move ) ) {
+				translatedMove = translatedMove.substring( 0, PROMOTION_MOVE_LENGTH - 1 ) + translatedMove.substring( PROMOTION_MOVE_LENGTH - 1 ).toUpperCase();
+			}
+
+			String squareFrom = translatedMove.substring( 0, SQUARE_FROM_LENGTH );
+			String destination = translatedMove.substring( 2 );
+
+			return new Move( squareFrom, destination );
+		}
 	}
 }

src/test/java/com/leokom/chess/player/winboard/WinBoardCommanderSendTest.java

@@ -59,4 +59,10 @@ public void obligatoryDraw() {
 		commander.obligatoryDrawByMovesCount( 75 );
 		verify( communicator ).send( "1/2-1/2 {Draw by 75 moves rule}" );
 	}
+
+	@Test
+	public void illegalMove() {
+		commander.illegalMove( "e2e7" );
+		verify( communicator ).send( "Illegal move: e2e7" );
+	}
 }

src/test/java/com/leokom/chess/player/winboard/WinBoardPlayerIntegrationTest.java

@@ -112,6 +112,7 @@ public void userMoveNoException() {
 	public void promotionCorrectlyTranslatedToCommonStandard() {
 		PositionBuilder position = new PositionBuilder()
 				.addPawn( Side.WHITE, "f7" )
+				.add( Side.BLACK, "g8", PieceType.QUEEN )
 				.setSideOf( "f7" );
 
 		player.setPosition( position.build() );
@@ -136,6 +137,12 @@ public void promotionCorrectlyTranslatedFromCommonStandard() {
 
 	@Test
 	public void castlingCorrectlyTranslatedToPlayer() {
+		PositionBuilder position = new PositionBuilder()
+				.add( Side.WHITE, "e1", PieceType.KING )
+				.add( Side.WHITE, "h1", PieceType.ROOK );
+
+		player.setPosition( position.build() );
+
 		assertTranslationOfReceivedCommandToMoveForOpponent(
 				"usermove e1g1", new Move( "e1", "g1" ) );
 	}

src/test/java/com/leokom/chess/player/winboard/WinBoardPlayerTest.java

@@ -48,6 +48,40 @@ private void executeMoveFromUI( WinboardCommander commander, String move ) {
 		moveListener.execute( move );
 	}
 
+	@Test
+	public void errorMoveIsDetected() {
+		WinboardCommander commander = mock( WinboardCommander.class );
+		final WinboardPlayer player = new WinboardPlayer( commander );
+		player.setOpponent( mock( Player.class ) );
+
+		executeMoveFromUI( commander, "e2e5" );
+
+		verify( commander ).illegalMove( "e2e5" );
+	}
+
+	@Test
+	public void correctMoveIsNotReportedAsError() {
+		WinboardCommander commander = mock( WinboardCommander.class );
+		final WinboardPlayer player = new WinboardPlayer( commander );
+		player.setOpponent( mock( Player.class ) );
+
+		executeMoveFromUI( commander, "e2e4" );
+
+		verify( commander, never() ).illegalMove( any() );
+	}
+
+	@Test
+	public void noPositionUpdateForAnotherPlayerInErrorCase() {
+		WinboardCommander commander = mock( WinboardCommander.class );
+		final WinboardPlayer player = new WinboardPlayer( commander );
+		Player opponent = mock(Player.class);
+		player.setOpponent( opponent );
+
+		executeMoveFromUI( commander, "e2e5" );
+
+		verify( opponent, never() ).opponentMoved( any() );
+	}
+
 	@Test
 	public void offerDrawTransmittedToTheOpponent() {
 		WinboardCommander commander = mock( WinboardCommander.class );