When we are in check and we move the king then testing with
pl_move_is_legal(m, pinned) is not enough becuase we cannot
rely on attackers_to() but we have to explicitly remove the
king form the occupied bitboard to catch as invalid moves like
b1a1 when opposite queen is on c1.
Our move generator already produces correct evasions so we
just need to add the extra verification to move_is_legal().
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It is interesting the fact that we need to test for
move_is_castle(m) anyway and not relying on testing
if destination square is attacked. Indeed the latter
condition fails if the castling rook is attacked,
castling is coded as "king captures the rook" but it
is legal in that case.
Verified no functional change with beginning of the series.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
The structure of move is changed so should also the two
functions. It happens that it works by accident !
Bug spotted by Marek Kwiatkowski
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
It seems we have a lot of totally useless code !
After 8577 games 1504 - 1451 - 5622 ELO +2 (+- 4.4)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Also let do_setup_move() don't reuse same StateInfo so that
we can remove the check about different StateInfo objects
before memcpy() in do_move.
Functional change due to harmless additionals
do_move() / undo_move() steps.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Keep the isChess960 flag inside Position so that is
copied with the Position, but esplicitly highlight the
fact that a FEN string has not enough information to detect
Chess960 in general case. To do this add a boolean argument
isChess960 to from_fen() function so to self document this
shortcoming of FEN notation.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Introduced by me in before 1.9 and found by Tord that says:
The 'isChess960' slot in the 'Position' class is currently
set depending on the initial files of the rooks, and not on the value
of the UCI_Chess960 parameter. This is incorrect, as there are lots of
Chess960 positions where the rooks start on the usual files. As a
consequence (unless I am missing something), Stockfish will occasionally
output castling moves as e1g1/e1c1 rather than the correct e1h1/e1a1 format
in Chess960 games. It is possible that some or even most GUIs are robust
enough to accept both notations, but I wouldn't bet on it. And in any case,
Stockfish's behavior clearly violates the protocol.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And also store the node counter in Position and not in Thread.
This will allow to properly count nodes also in sub trees with
SMP active.
This requires a surprisingly high number of changes
in a lot of places to make it work properly.
No functional change but node count changed for obvious reasons.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Greatly cleanup SEE code and now it is also a bit
faster on gcc, about +0.6%.
Thanks to Mike Whiteley new SEE code that gave me
fresh ideas on how to cleanup this old stuff.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We don't need that !
We can infere from starting fen string if we are in
a Chess960 game or not. And note that this is a per-position
property, not an application wide one.
A nice trick is to use a custom manipulator (that is an
enum actually) to keep using the handy operator<<() on the
move when sending to std::cout, yes, I have indulged a
bit here ;-)
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
There is a functional change because we now skip
more moves and because do_move() / undo_move() is
well known to be not reversible we end up with a
change in node count, although there is actually
no change but a bit speed up.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
To be uniform across the sources. As a nice side effect
I quickly spotted a couple of needed renames:
captured_piece() -> captured_piece_type()
st->capture -> st->capturedType
Proposed by Ralph and done with QtCreator
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Complete rewrite the function and extend compatibility
also to X-FEN notation for Chess960.
We are now able to read standard FEN, Shredder-FEN and X-FEN.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
We don't need to pass side_to_move because we can get
it directly from the position object.
Note that in benchmark we always used to pass '0' and
it was a bug, but with no effect because was used only
in time[] and increment[], set always to 0 for both
colors.
Also additional small cleanup while there.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Also renamed history access value in something more
in line with the meaning.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
Search ply and game ply are rwo different things !
Revert bogus commit.
No functional change on bench, but it changes in real games
when engine sends all the moves up to current one.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
This is the best place because when we split we do a
copy of the position and also threadID, once set in a
given position, never changes anymore.
Forbid use of Position's default and copy c'tor to avoid
nasty bugs in case a position is created without explicitly
setting the threadID.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
And avoid a redundant one passed as argument in
search calls.
Also renamed gamePly in ply to better clarify this
is used as search ply and is set to zero at the
beginning of the search.
No functional change.
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
