Geras1mleo / Chess

C# .NET 6.0 Chess Library
MIT License
49 stars 15 forks source link

Not recognizing InsufficientMaterial EndGame #9

Closed manushand closed 1 year ago

manushand commented 1 year ago

Describe the bug A board with insufficient material is not being recognized as such, and its IsEndGame is false, etc.

To Reproduce

var board = ChessBoard.LoadFromFen("2b5/4kB2/8/8/8/8/5K2/8 b - - 0 16");
if (!board.IsIendGame)
    throw new ();

Expected behaviour Boards such as the one used in the reproduction example should be recognized as having insufficient material to proceed, and the game drawn, with board.EndGame.EndgameType == EndgameType.InsufficientMaterial, and with its board.ToPgn() string being given the ½-½, etc., etc.

Screenshots N/A

Additional context I know that you probably have a much better way to fix this in the library, but in case it is in any way useful, here is the code I added as a workaround in my application:

private static bool IsDrawnForInsufficientMaterial(this ChessBoard board)
{
    //  Insufficient Material isn't properly detected.  Honestly, I checked. Do it ourselves.
    //  If there are any pawns, rooks, or queens on the board, there is sufficient material.
    var fen = board.ToFen().Split()[0];
    if ("PQR".Any(fen.ToUpper().Contains))
    return false;
    // Otherwise, it's just bishops, knights, and kings.  Well, yeah, kings -- ignore them....
    var remainingPieces = new string(fen.Where(static c => c is > '8' and not ('K' or 'k')).ToArray());
    //  Kings only or King against a single bishop or knight is drawn.
    //  Otherwise, except for a bishop apiece, it's sufficient.
    //  In B vs b, it's a draw if both bishops are on light squares or both on dark squares.
    return remainingPieces.Length < 2
        || remainingPieces is "Bb" or "bB"
        && Regex.Replace(fen, "[2468]", string.Empty).Select(static (c, i) => c is 'B' or 'b' ? i : 0).Sum() % 2 is 0;
}
Geras1mleo commented 1 year ago

Well, i know about this one... #3 Also 50 moves rule and repetition is not supported yet Didn't expect to someone really using this lib haha :)

I cannot promise to make it shortly but I'll do my best!

manushand commented 1 year ago

Thanks so much! And I guess this is what you get for putting out quality software! :-)

BTW, I'm sorry to report that I have another very hairy bug to report to you, but it evades an easy way to reproduce it, so I have a nasty workaround in place. I'll let you know at some point all about it, once it stops evading my capture. Basically, there is a situation where a valid ChessBoard (created from .LoadFromFen), when told to .Move(oneOfItsOwnMovesReturnedFromBoard.Moves()) returns false and doesn't update the board. From all evidence, the only time I am seeing this is when the move is an en passant capture which (when coming out of board.Moves()) is not fully created -- with no .CapturedPiece, no .IsEnPassantCapture, etc.)

Geras1mleo commented 1 year ago

I'll take a look at this one

manushand commented 1 year ago

Hello -- I just realized that my workaround code for InsufficientMaterial is...well, insufficient. Unless I am mistaken, one player could have his King and only any number of bishops (say, five or six of them, even!) against an opposing king (who could himself have an army of bishops), and the game would be drawn for insufficient material if ALL of the bishops were on light squares or all on dark-squares. So we will both need better code than I provided in this ticket! :-)

Geras1mleo commented 1 year ago

Thanks so much! And I guess this is what you get for putting out quality software! :-)

BTW, I'm sorry to report that I have another very hairy bug to report to you, but it evades an easy way to reproduce it, so I have a nasty workaround in place. I'll let you know at some point all about it, once it stops evading my capture. Basically, there is a situation where a valid ChessBoard (created from .LoadFromFen), when told to .Move(oneOfItsOwnMovesReturnedFromBoard.Moves()) returns false and doesn't update the board. From all evidence, the only time I am seeing this is when the move is an en passant capture which (when coming out of board.Moves()) is not fully created -- with no .CapturedPiece, no .IsEnPassantCapture, etc.)

I have tested it and found a bug with SAN-notation, its already fixed. I suppose that CapturedPiece property is being set correctly, perhaps you are using an old version of library because IsEnPassantCapture property does not exist in the last one...

manushand commented 1 year ago

Yeah, I realized after submitting the report that .IsEnPassantCapture is an extension method that I wrote. :-)

On Fri, Jan 6, 2023 at 12:47 PM Sviatoslav @.***> wrote:

Thanks so much! And I guess this is what you get for putting out quality software! :-)

BTW, I'm sorry to report that I have another very hairy bug to report to you, but it evades an easy way to reproduce it, so I have a nasty workaround in place. I'll let you know at some point all about it, once it stops evading my capture. Basically, there is a situation where a valid ChessBoard (created from .LoadFromFen), when told to .Move(oneOfItsOwnMovesReturnedFromBoard.Moves()) returns false and doesn't update the board. From all evidence, the only time I am seeing this is when the move is an en passant capture which (when coming out of board.Moves()) is not fully created -- with no .CapturedPiece, no .IsEnPassantCapture, etc.)

I have tested https://github.com/Geras1mleo/Chess/blob/master/ChessUnitTests/UnitTests.cs#L475 it and found a bug with SAN-notation, its already fixed. I suppose that CapturedPiece property is being set correctly, perhaps you are using an old version of library because IsEnPassantCapture property does not exist in the last one...

— Reply to this email directly, view it on GitHub https://github.com/Geras1mleo/Chess/issues/9#issuecomment-1374052829, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASF4VN3THE23TQXICXI3JE3WRBZEFANCNFSM6AAAAAATHGCE4U . You are receiving this because you authored the thread.Message ID: @.***>

manushand commented 1 year ago

I looked at your unit test code, and dang, I promise I run the same thing and I do not get a pass. I'm on release 1.0.2 -- is that not the latest?

EDIT: I now realize that you were saying that you fixed it and released a new version (1.0.3, which I have now updated to), and sure enough, all better! Thank you very much!!

manushand commented 1 year ago

Hi again --

I updated my own workaround code that handles InsufficientMaterial per the realization I made a comment about earlier. In case it is of any help to you, this is what I am now using:

//  Insufficient Material isn't properly detected.  Honestly, I checked. Do it ourselves.
//  If there are any pawns, rooks, or queens on the board, there is sufficient material.
var fen = board.ToFen().Split()[0];
if ("PQR".Any(fen.ToUpper().Contains))
    return false;
//  Otherwise, it's just bishops, knights, and kings.  Well, yeah, kings -- ignore them....
var remaining = new string(fen.Where(static c => c is > '8' and not ('K' or 'k')).ToArray());
//  Kings only or King against a single bishop or knight is drawn.
//  Otherwise, except for only bishops, it's sufficient.
//  If bishops only, it's a draw if they're all on the same color.
var count = remaining.Length;
return count < 2
    || remaining.All(static c => c is 'B' or 'b')
    && new[] { 0, count }.Contains(Replace(fen, "[2468]", string.Empty).Select(static (c, i) => c is 'B' or 'b' ? i + 1 : 0)
                                       .Where(static i => i is not 0)
                                       .Count(static i => i % 2 is 0));
Geras1mleo commented 1 year ago

Hi again --

I updated my own workaround code that handles InsufficientMaterial per the realization I made a comment about earlier. In case it is of any help to you, this is what I am now using:

//    Insufficient Material isn't properly detected.  Honestly, I checked. Do it ourselves.
//    If there are any pawns, rooks, or queens on the board, there is sufficient material.
var fen = board.ToFen().Split()[0];
if ("PQR".Any(fen.ToUpper().Contains))
  return false;
// Otherwise, it's just bishops, knights, and kings.  Well, yeah, kings -- ignore them....
var remaining = new string(fen.Where(static c => c is > '8' and not ('K' or 'k')).ToArray());
//    Kings only or King against a single bishop or knight is drawn.
//    Otherwise, except for only bishops, it's sufficient.
//    If bishops only, it's a draw if they're all on the same color.
var count = remaining.Length;
return count < 2
  || remaining.All(static c => c is 'B' or 'b')
  && new[] { 0, count }.Contains(Replace(fen, "[2468]", string.Empty).Select(static (c, i) => c is 'B' or 'b' ? i + 1 : 0)
                                     .Where(static i => i is not 0)
                                     .Count(static i => i % 2 is 0));

Yes, thank you!

manushand commented 1 year ago

I can confirm that 1.0.3 fixed my en passant issue (which I reported as issue #10), and also was indeed the cause of the "hairy bug" that I mentioned earlier in this thread, and that you commented about, mentioning that your fix to en passant San may have addressed it. It sure did! Thank you once again!!! I am busy happily removing workarounds from my application now!

manushand commented 1 year ago

If you are looking for a good test position for any change you make to InsufficientMaterial calculation, you can get one by using my website that uses your library -- http://104.168.200.10:43860 --, changing the "Vocabulary" to PGN, and then giving it this problem to solve:

.. xx .x
xx xx xx
xx xx xx
&B    c&
xx xx xx
xx xx xx
/

It will take it a while (about 90 seconds) to solve it, but then it will give you the sequence of moves, and resulting FEN, that this string produces, which ends in an InsufficientMaterial position that your library does not detect (but my workaround code then does).

manushand commented 1 year ago

Thank you for working this! I look forward to the NuGet update that will include the new code!!

manushand commented 1 year ago

Hello -- Do you have a date on which you plan to deploy this fix in a new NuGet release? Or should I download the branch source and use it that way? (I would rather, of course, use it as a NuGet package.)

Thanks, Manus

Geras1mleo commented 1 year ago

Hello -- Do you have a date on which you plan to deploy this fix in a new NuGet release? Or should I download the branch source and use it that way? (I would rather, of course, use it as a NuGet package.)

Thanks, Manus

Today I've added threefold repetition rule and I will add 50 moves rule tomorrow, so you can expect new release tomorrow evening.

Cheers, Sviatoslav

manushand commented 1 year ago

Wow, that is great! Thank you so much!

Geras1mleo commented 1 year ago

Wow, that is great! Thank you so much!

New release with endgame rules, use AutoEndgameRules prop to configure! Please let me know if there are bugs or other issues. Thank you!

Cheers, Sviatoslav