SebLague / Chess-Challenge

Create your own tiny chess bot!
https://www.youtube.com/watch?v=Ne40a5LkK6A
MIT License
1.78k stars 1.07k forks source link

[Critical Bug] The Token Counter is bugged #489

Open toanth opened 9 months ago

toanth commented 9 months ago

I'm still trying to create a minimum working example, but what I've found so far is that this line: bool negativeTokens = move is { TargetSquare.Rank: 1} can somehow reduce the total number of tokens as if it consisted of a negative number of tokens. As the name implies, move is an object of type Move, and when I added a similar line to my program I realized that the token counter had decreased. Interestingly, the location where this line is added changes how many tokens are subtracted, or if this bugs works at all. It's also possible that a small number of tokens (less than the actual token count of this line) gets added. This bug isn't restricted to this specific line of course, but this is the smallest example I've found so far, and it has also been confirmed by other people in the discord server) I'll hopefully have a fully reproducible example soon, but I wanted to created this issue asap.

toanth commented 9 months ago

Steps to reproduce: Adding this line in the move loop of the default EvilBot incorrectly increases the token count from 142 to 151. Adding it to the MoveIsCheckmate function instead correctly increases the token count to 153.

Algorhythm-sxv commented 9 months ago

This appears to have something to do with 'syntax trivia'.

If you add this code to line 57 inside the foreach loop:

var a = token.ToString().Replace("\n", "\\n");
var b = token.TrailingTrivia.ToString().Replace("\n", "\\n");
Console.WriteLine($"'{a}' has trailing trivia '{b}'");

and then run it on this code:

using ChessChallenge.API;

public class MyBot : IChessBot
{
    public Move Think(Board board, Timer timer)
    {
        bool tokenTest = Move.NullMove is { TargetSquare.Rank: 1 };
        return default;
    }
}

you can observe that somehow, it causes the line after the pattern match to count as "syntax trivia":

'}' has trailing trivia ''
';' has trailing trivia '\n        return default;\n'
'}' has trailing trivia '\n}'
'' has trailing trivia ''

which https://learn.microsoft.com/en-us/dotnet/csharp/roslyn-sdk/work-with-syntax#syntax-trivia defines as

Syntax trivia represent the parts of the source text that are largely insignificant for normal understanding of the code, such as white space, comments, and preprocessor directives.

In this case, the return statement is very much significant to understanding the code, and it isn't whitespace, a comment, or a preprocessor directive, so I'm inclined to believe this is a bug in the parser. This should be somewhat mitigated by adding descendIntoTrivia: true to the DescendantTokens call, which should make it visit trivia tokens.

ryanheath commented 9 months ago

I think it is a bug in the parser too. The returned tokens also differ in other places that should not have changed.

See the images below; Left original source, mid in the move loop, right in the MoveIsCheckmate

You can see tokens are missing in mid compared to the original.

image

Here you can see tokens are inserted and missing in right compared to the original.

image

SebLague commented 9 months ago

Thanks for reporting, that's really strange! I will investigate (and will look into descendintotrivia as a potential solution @Algorhythm-sxv)

SebLague commented 8 months ago

In case anyone is still wondering, it turns out the issue is because I accidentally used an outdated version of Roslyn. Updating to 4.7 fixes the problem. Sorry about the blunder!