bhlangonijr / chesslib

chess library for legal move generation, FEN/PGN parsing and more
Apache License 2.0
229 stars 80 forks source link

Illegal moves (the pawn can move backwards, a1 a3 from start etc ) #5

Closed evgentset closed 6 years ago

evgentset commented 6 years ago

First of all, many thanks for this library, it is very helpful. During using it I found a bug, I suppose. Steps to reproduce are:

  1. e2 e4
  2. e7 e5 (or any other move)
  3. e4 e2

Expected result - "e4 e2" is illegal move for pawn. The Pawn can never move backwards. Actual result - board.doMove(move, true) returns true.

I suppose that need to change something in method isMoveLegal(Move move, boolean fullValidation) to make similar moves illegal.

evgentset commented 6 years ago

Also I found that there are a lot of similar illegal moves which board.doMove(move, true) is allowed, like a1 a3 from start etc. So maybe I use it incorrect way? Because MoveGenerator.generateLegalMoves(board) is generated all legal moves correctly. Could anybody explain what went wrong?

bhlangonijr commented 6 years ago

Hi @evgentset, yes I agree the name of the function isMoveLegal is misleading in regards to what it actually does. This library is a by-product of an UI development and for checking the legality of a move I actually call MoveGenerator.generateLegalMoves(board) and match the results against the move user is trying to play before executing board.doMove(move). It means that you are free to use board#doMove(move) as long as the board is left in a valid state (e.g. side did not play a move leaving own king in check). You could use doMove() for moving pieces around to set up an arbitrary chessboard position using a drag & drop functionality in the UI, for example. Another useful usage is when you are fetching a move from a transposition table and replay it to Board: board.doMove(move, true) ensures the move is still valid for that given position. In that sense a more accurate name for this function would be isBoardValidFor(move) or something like that. I am open to suggestions though. Let me know what are your thoughts.

evgentset commented 6 years ago

ok, as you said, I've written util function to check it (koltin code, but java will be very similar):

fun isLegalMove(move: Move, board: Board): Boolean {
        val moveList = MoveGenerator.generateLegalMoves(board)
        return moveList.contains(move)
    }

So yes, it's just misleading for function name. You can mark these methods as deprecated in new library version, and copy them with another names. So users can easy migrate to another methods. I suppose it's acceptable solution for it.

Btw, I have not found function like List<Move> getLegalMovesForPieceInSquare(Square square) which also may be very helpful as well. This method should get Piece in Square and return legal moves for the Piece in some Sqare. I'm going to write similar function in future. If I have time I will send you pull request with it (I hope so).

Many thanks again, Cheers

bhlangonijr commented 6 years ago

Hi Evgen,

As a follow up I will add the method suggested and also rename the isLegalMove function.

Thank you for the suggestions, Ben

On Thu, Jul 19, 2018 at 11:03 AM, Evgen notifications@github.com wrote:

ok, as you said, I've written util function to check it (koltin code, but java will be very similar):

fun isLegalMove(move: Move, board: Board): Boolean { val moveList = MoveGenerator.generateLegalMoves(board) return moveList.contains(move) }

So yes, it's just misleading for function name. You can mark these methods as deprecated in new library version, and copy them with another names. So users can easy migrate to another methods. I suppose it's acceptable solution for it.

Btw, I have not found function like List getLegalMovesForPieceInSquare(Square square) which also may be very helpful as well. This method should get Piece in Square and return legal moves for the Piece in some Sqare. I'm going to write similar function in future. If I have time I will send you pull request with it (I hope so).

Many thanks again, Cheers

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bhlangonijr/chesslib/issues/5#issuecomment-406207526, or mute the thread https://github.com/notifications/unsubscribe-auth/AA6mDZ91zh1sLZZNiLea-xmpRp9vZP48ks5uIEtagaJpZM4VUmqD .