nkarve / surge

A fast bitboard-based chess move generator in C++
MIT License
63 stars 15 forks source link

Cannot create castling move from string #11

Open lennertcl opened 3 years ago

lennertcl commented 3 years ago

With the current implementation, I have not succeeded in generating a castling move. O-O / O-O-O doesn't work. e1h1 sends the king to h1 on top of the rook (the rook is removed).

Move(const std::string& move) {
this->move = (create_square(File(move[0] - 'a'), Rank(move[1] - '1')) << 6) |
    create_square(File(move[2] - 'a'), Rank(move[3] - '1'));
}

I would like to pull request with a check for O-O or O-O-O. An update to some function to move the pieces might also be necessary.

I would also like to add a namespace (surge/nkarve_surge?), because I had a clash with some other library that also used a constant named HFILE.

nkarve commented 3 years ago

Yes, that function is quite dysfunctional and in general should not be used. For instance, it cannot even handle captures correctly, let alone double pushes, castling or promotions. May I enquire what you're using this for? My philosophy was that, in general, all Moves should be created using the Move(Square from, Square to, MoveFlags flags) constructor. If you were to use this for e.g. converting end user input to a move, I sort of left the implementation details to the programmer, because

So although upgrading this to incorporate castling is simple, it isn't for the other move types, and so I don't want to introduce much reliance on this function. In fact its existence is likely a relic of debugging, I might have to remove it or deprecate it. However, let me know what your intended purpose with this feature is, perhaps an alternative solution is available.

lennertcl commented 3 years ago

My intented purpose was indeed to convert user input to a move. I will write my own function to generate the move correctly.

I would still add the namespace. (I already added it to the version I am currently using, because I literally cannot use it otherwise)

nkarve commented 3 years ago

Yes, the namespace is a good idea