Evaluators symmetry 293

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

@@ -35,7 +35,7 @@ static int getNotPawnInitialRank( Side side ) {
 	}
 
 	static Position generate( Rules rules ) {
-		final Position result = new Position( Side.WHITE );
+		final Position result = new Position( Side.WHITE, null );
 		result.setRules( rules );
 
 		final Set< String > initialRookFiles = new HashSet<>( Arrays.asList( "a", "h" ) );

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

@@ -70,7 +70,16 @@ public class Position implements GameState< Move, Position > {
 
 	private Set< Side > hasHRookMoved = new HashSet<>();
 
+	//this should be replaced by position history which would be needed for repetition rules
+	//now it's used for Castling evaluator, it's some extension of historical information
+	//remove me in https://github.com/lrozenblyum/chess/issues/321
+	private Set< Side > hasCastlingExecuted = new HashSet<>();
+
 	private Side sideToMove;
+	//technically this historical information should be filled in by .getPrevious().getSideToMove()
+	//due to mutability of sideToMove AND need to support consistency for test-only method
+	//setSideToMove, this field is mutable as well
+	private Side movedSide;
 
 	private Result gameResult;
 	private boolean terminal;
@@ -92,6 +101,10 @@ void setHasHRookMoved( Side side ) {
 		this.hasHRookMoved.add( side );
 	}
 
+	void setHasCastlingExecuted( Side side ) {
+		this.hasCastlingExecuted.add( side );
+	}
+
 	void setEnPassantFile( Character enPassantFile ) {	this.enPassantFile = enPassantFile; }
 
 	//temporary state in game (which could change)
@@ -115,11 +128,17 @@ void setHasHRookMoved( Side side ) {
 	 *
 	 * By default en passant file is absent
 	 *
-	 * @param sideToMove side which turn will be now, null for terminal positions
+	 * @param sideToMove side which turn will be now, null is prohibited.
+	 *                   To init terminal position use another constructor.
 	 *
 	 */
 	public Position( Side sideToMove ) {
+		this( sideToMove, sideToMove.opposite() );
+	}
+
+	public Position( Side sideToMove, Side movedSide ) {
 		this.sideToMove = sideToMove;
+		this.movedSide = movedSide;
 		this.rules = Rules.DEFAULT;
 	}
 
@@ -331,12 +350,17 @@ private Set<String> getSquaresAttackedByRook( String square ) {
 		return result;
 	}
 
+	public Set<String> getSquaresOccupied() {
+		//risk for immutability?
+		return pieces.keySet();
+	}
+
 	public Set<String> getSquaresOccupiedBySide( Side neededSide ) {
 		return getSquaresOccupiedBySideToStream( neededSide ).collect( toSet() );
 	}
 
 	private Stream<String> getSquaresOccupiedBySideToStream( Side neededSide ) {
-		return pieces.keySet().stream().filter( square -> this.isOccupiedBy( square, neededSide ) );
+		return getSquaresOccupied().stream().filter( square -> this.isOccupiedBy( square, neededSide ) );
 	}
 
 	private boolean isKingInCheck( Side side ) {
@@ -662,14 +686,18 @@ public boolean hasKingMoved( Side side ) {
 		return hasKingMoved.contains( side );
 	}
 
+	public boolean hasCastlingExecuted( Side side ) {
+		return hasCastlingExecuted.contains( side );
+	}
+
 	/**
 	 * Copy pieces from current position to the destination
 	 * Copy state (like info if the king has moved)
 	 * @param position destination position
 	 */
 	void copyStateTo( Position position ) {
 		//cloning position
-		for ( String square : pieces.keySet() ) {
+		for ( String square : getSquaresOccupied() ) {
 			//looks safe as both keys and pieces are IMMUTABLE
 			position.pieces.put( square, pieces.get( square ) );
 		}
@@ -679,6 +707,7 @@ void copyStateTo( Position position ) {
 		position.hasKingMoved = new HashSet<>( this.hasKingMoved );
 		position.hasARookMoved = new HashSet<>( this.hasARookMoved );
 		position.hasHRookMoved = new HashSet<>( this.hasHRookMoved );
+		position.hasCastlingExecuted = new HashSet<>( this.hasCastlingExecuted );
 
 		//little overhead but ensuring we really copy the FULL state
 		position.waitingForAcceptDraw = this.waitingForAcceptDraw;
@@ -894,8 +923,17 @@ public Side getSideToMove() {
 		return sideToMove;
 	}
 
+	public Side getMovedSide() {
+		return movedSide;
+	}
+
 	void setSideToMove( Side sideToMove ) {
+		setSideToMove( sideToMove, sideToMove.opposite() );
+	}
+
+	void setSideToMove(Side sideToMove, Side lastMovedSide) {
 		this.sideToMove = sideToMove;
+		this.movedSide = lastMovedSide;
 	}
 
 	/**

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

@@ -83,10 +83,10 @@ public PositionBuilder winningSide( Side side ) {
 		return this;
 	}
 
-	public PositionBuilder draw() {
+	public PositionBuilder draw( Side lastMovedSide ) {
 		this.position.setTerminal( null );
 		//see the reason of this in PositionBuilder
-		this.position.setSideToMove( null );
+		this.position.setSideToMove( null, lastMovedSide );
 		return this;
 	}
 }

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

@@ -145,7 +145,7 @@ private void validateStandardMove( Move move ) {
 	}
 
 	private Position createTerminalPosition(Side winningSide, Result gameResult) {
-		final Position terminalPosition = new Position( null );
+		final Position terminalPosition = new Position( null, source.getSideToMove() );
 		source.copyStateTo( terminalPosition );
 		//TODO: should checkmate move also set this flag?
 		terminalPosition.setTerminal(winningSide);
@@ -196,7 +196,13 @@ private Position processKingMove( String squareFrom, String move ) {
 			newPosition.moveUnconditionally( "a" + rank, "d" + rank );
 		}
 
-		newPosition.setHasKingMoved( this.source.getSide( squareFrom ) );
+		Side ourSide = this.source.getSide( squareFrom );
+
+		if ( isCastlingKingSide || isCastlingQueenSide ) {
+			newPosition.setHasCastlingExecuted( ourSide );
+		}
+
+		newPosition.setHasKingMoved(ourSide);
 		return newPosition;
 	}
 

src/main/java/com/leokom/chess/player/legal/brain/common/SideEvaluator.java

@@ -0,0 +1,15 @@
+package com.leokom.chess.player.legal.brain.common;
+
+import com.leokom.chess.engine.Position;
+import com.leokom.chess.engine.Side;
+
+@FunctionalInterface
+public interface SideEvaluator {
+    /**
+     * Evaluate position from point of view of the given side
+     * @param position position to analyze
+     * @param side side of interest
+     * @return a position evaluation
+     */
+    double evaluatePosition( Position position, Side side );
+}

src/main/java/com/leokom/chess/player/legal/brain/denormalized/AttackEvaluator.java

@@ -1,9 +1,14 @@
 package com.leokom.chess.player.legal.brain.denormalized;
 
+import com.google.common.collect.Sets;
 import com.leokom.chess.engine.Move;
 import com.leokom.chess.engine.Position;
 import com.leokom.chess.engine.Side;
 import com.leokom.chess.player.legal.brain.common.Evaluator;
+import com.leokom.chess.player.legal.brain.internal.common.SymmetricEvaluator;
+
+import java.util.Set;
+import java.util.stream.Stream;
 
 /**
  * Author: Leonid
@@ -12,7 +17,41 @@
 class AttackEvaluator implements Evaluator {
 	@Override
 	public double evaluateMove( Position position, Move move ) {
-		final Side ourSide = position.getSideToMove();
-		return AttackIndexCalculator.getAttackIndex( position.move( move ), ourSide );
+		Position targetPosition = position.move(move);
+		return new SymmetricEvaluator( new AttackSideEvaluator() ).evaluate( targetPosition );
+	}
+
+	private static class AttackSideEvaluator implements com.leokom.chess.player.legal.brain.common.SideEvaluator {
+		//TODO: technically, in case of a terminal position, it should return empty result
+		//REFACTOR: too generic to encapsulate into Position?
+		private static Set<String> getPiecesAttackedBy( Position position, Side attackerSide ) {
+			Set<String> defenderSquares = position.getSquaresOccupiedBySide( attackerSide.opposite() );
+			return Sets.intersection( position.getSquaresAttackedBy( attackerSide ),
+					defenderSquares );
+		}
+
+		/*
+		 * Backlog for improvements:
+		 * - king has become a main target for attacks
+		 */
+		@Override
+		public double evaluatePosition(Position position, Side side) {
+			final Set< String > squaresAttacked = getPiecesAttackedBy( position, side );
+
+			// sum of piece values
+			// if a piece is protected - index of piece value is reduced
+			float result = 0;
+			for ( String attackedSquare : squaresAttacked ) {
+				//REFACTOR: probably bad dependency on another brain - extract common utility
+				int pieceValue = MaterialEvaluator.getValue( position.getPieceType( attackedSquare ) );
+
+				final Stream< String > protectors = position.getSquaresAttackingSquare( side.opposite(), attackedSquare );
+
+				//+1 to avoid / 0, more protectors is better
+				result += pieceValue / ( protectors.count() + 1.0 );
+			}
+			return result;
+		}
 	}
+
 }