WinBoard terminates game when reading the word draw

[Sven]

If my engine prints output during search that looks like this:

Code: Select all

# nps=136533 bf=4.1 nodesQS=6786 (82.8%) nodesDepth0=4545 (55.5%) nodesDraw=0 ( 0.0%) nodesHashHit=146 ( 1.8%)

then please guess how WinBoard reacts.

1. ...?
2. ...?
3. ...?

No, all wrong

WinBoard terminates the game with a draw. Can you see why? The line starts with a '#' which explicitly advises newer WinBoard versions to ignore the whole line according to the spec. However, it does not ignore it, no matter whether I specifiy the "/engineDebugOutput 1" command line option or not.

If I change the word "nodesDraw" into something that does not contain the word "draw" in any upper-lower-case combination then the game is not terminated (I did not try the words "result", "resign", "mated", or "quit", however ...).

I found this behaviour with quite an old WinBoard version (4.3.14k) as well as with the very recent 4.4.2b, so I guess it's an older bug.

Two problems involved IMO:
1) not ignoring "# comment" lines;
2) taking the occurrence of the string "draw" anywhere within an input line as a draw claim, which in turn triggers WB to terminate the game if it is configured this way.

I can get a decent ELO with those 50% against all world's top engines So I'm beginning to like that pro-amateur feature ...

Extract from winboard.debug:

Code: Select all

1344 <first &#58; 4 23 0 994 e2e4 b8c6 g1f3 e7e5
1344 <first &#58; 4 0 1 2276 e2e4 b8c6 b1c3 e7e5
1391 <first &#58; # stopped search &#40;timeout&#41;
1391 <first &#58; 5 0 6 8192 e2e4 b8c6 b1c3 e7e5
1391 <first &#58; # nps=136533 bf=4.1 nodesQS=6786 &#40;82.8%) nodesDepth0=4545 &#40;55.5%) nodesDraw=0 ( 0.0%) nodesHashHit=146 ( 1.8%)
1391 <first &#58; GameEnds&#40;39, Draw, 5&#41;
1391 >first &#58; result 1/2-1/2 &#123;Draw&#125;
1391 >second&#58; result 1/2-1/2 &#123;Draw&#125;

Sven

[hgm]

Does the engine send feature debug=1?

The /enginedebugOutput option only affects how non-compliant output will appear in the debug file. It has no effect on how WinBoard handles the commands. Its only purpose is to make life easier on software that uses winboard.debug to extract information from it, such as TLCS/TLCV.

Historically WinBoard had the policy to cater to non-compliant engines, trying to correct engine bugs by special code. So there is a whole section of code testing for undocumented nonsense some engines coould send in the past, trying to make sense out of it. The 'Draw' thing is one of those (only works when you spell it with a capital...). In the past we already have some problems with that, when Polyglot would be sending UCI options in WB format, as these often contain the word 'Draw'. (e.g. feature option="Accept Draws -combo Never /// *Sometimes /// Always".) So at least I moved the non-compliant section to after checking for any existing command.

[Sven]

Does your engine send feature debug=1?

No. I did not realize the existence of this "feature" setting up to now, I had only seen the command line option "/engineDebugOutput" which controls if and how command lines are copied to the debug output file. I'll fix this of course. I thought that specifying "/debug" and "/engineDebugOutput 1" were enough but obviously this is not the case.

This explains problem 1 but not the main problem itself, recognizing a string like "bla bla nodesDraw=2384907650884356" as a draw claim.

Sven

[govert]

Would it be possible to provide the non-compliant checking as an option?

This would solve 2 things:
A) The issue in this thread
B) It would ensure that engines that aspire to be fully compliant actually are compliant. No risk for an engine to "accidentally work" due to the helpful non-compliant code...

[Sven]

Would it be possible to provide the non-compliant checking as an option?

This would solve 2 things:
A) The issue in this thread
B) It would ensure that engines that aspire to be fully compliant actually are compliant. No risk for an engine to "accidentally work" due to the helpful non-compliant code...

I support your proposal in general but I don't quite understand your point A. One part of the issue in this thread is the reaction of WinBoard to non-compliant engine output, and especially the way how WinBoard thinks about how a draw claim looks like. If an engine sends a string "Hello WinBoard, do you think I can draw against Rybka?" and if we forget the stuff about debug output, "feature debug" and some cmdline options for a moment, then it looks 100% inappropriate to me that the GUI terminates the game with a draw just because the word "draw" appears right in the middle. It may ignore it, react with some annoying error popup (I wouldn't like that, though), or even punish the engine with a loss of game, but not make it a draw game. This is a bug IMO.

Sven

[Sven]

From the current WinBoard protocol spec, section 13:

To support older engines, certain additional commands from the engine to xboard are also recognized. (These are commands by themselves, not values to be placed in the comment field of the PGN result code.) These forms are not recommended for new engines; use the PGN result code commands or the resign command instead.
Command Interpreted as
White resigns 0-1 {White resigns}
Black resigns 1-0 {Black resigns}
White 1-0 {White mates}
Black 0-1 {Black mates}
Draw 1/2-1/2 {Draw}
computer mates 1-0 {White mates} or 0-1 {Black mates}
opponent mates 1-0 {White mates} or 0-1 {Black mates}
computer resigns 0-1 {White resigns} or 1-0 {Black resigns}
game is a draw 1/2-1/2 {Draw}
checkmate 1-0 {White mates} or 0-1 {Black mates}

Commands in the above table are recognized if they begin a line and arbitrary characters follow, so (for example) "White mates" will be recognized as "White", and "game is a draw by the 50 move rule" will be recognized as "game is a draw". All the commands are case-sensitive.

The current implementation in backend.c, function HandleMachineMove(), uses strstr() to check for presence of the strings "Draw" and "game is a draw". All other special cases listed above are handled by using strcmp() or strncmp(). Since the spec explicitly states "are recognized if they begin a line and arbitrary characters follow", I think that this inconsistency should be corrected.

Sven

[Sven]

If an engine sends a string "Hello WinBoard, do you think I can draw against Rybka?"

At least I have learned that only "Draw" with uppercase 'D' currently triggers the problem, not the lowercase "draw".

Sven

[hgm]

Well, in principle I don't like this non-compliant stuff at all. I did not put it in or added to it, I just inherited it. I also have no idea why in this case strstr is used rather than strcmp. So far I suspected that this was because some of the engines on behalf of which this non-compliance was supported did print either something before or after it on the same line. There might be a reason for it, or it might be a bug. I don't know how to find out.

An option -forceCompliancy would be easy enough to implement: just treat everything that is not matched by the time the end of HandleMachineMove is reached (or in fact the code block of non-compliant matches) as a resign comand.

It would not be very useful, though; almost no existing engine would pass the test, so people that would actually want to play any games that did not end in an immediate forfeit would be forced to switch it off. Many engines print non-compliant garbage before they receive the xboard command, but there is no way to figure out if an engine sent something before or after the GUI sent something. I could of course only start to judge after receiving a feature command, but not every engine is v2. For v1 I could start after the feature time-out, though. Problem is that on restart of an engine (-xreuse) there is no feature time-out.

Another problem with such an option would be that the GUI implementing it could no longer use it after protocol extensions. So I wonder if it is worth the trouble. The only use I see for it is that it could be used by engine builders to check if their engine is compliant. But I wonder if they really need such an option for that. It would probably be more helpful to them to add one more setting of engineDebugOutput, where non-compliant lines would be prefixed in the debug file not by "# " to make them look like compliant comment lines of engines that set feature debug=1, but in stead by something like "### NON-COMPLIANT! ### ".

But no matter how silly one might consider WinBoard's reaction to non-compliant lines containing the word "Draw" in them, the root cause of the problem here is the sending of non-compliant lines in the first place. Commenting them out with "# " only makes them compliant AFTER sending feature debug=1, which was apparently not done. And sending non-compliant lines will lead to undefined behavior, of which declaring a draw is a valid example. Playing with /testClaims=true would have forfeited the game, and thus goes already much more in the direction of a /forceCompliancy option with existing means.

[Sven]

Once more. Compliant or not, debug or not: interpreting a line containing "Draw" in the middle as a draw claim is a bug, nothing else. No matter whether you inherited it, I don't blame you for it of course, but the bug is there. I showed that the spec says these exceptions are only valid at the beginning of the line, for good reasons as I think.

You don't want a UNIX shell to recognize the "rm" command each time the user types a command containing the string "form", for example.

I can pretty well live without a new option to enforce compliance. It was not my key point, and now I share your doubts that such an option would be useful. Without it, *ignoring* non-compliant input is the only thing the GUI can do IMO.

And sending non-compliant lines will lead to undefined behavior

Can't accept this. Sounds like you don't want to defend against garbage. Better ignore what you don't understand. This has been the policy of WinBoard before, you can read it in the interface spec, and I still believe it's a good policy.

A couple of exceptions have made it into the spec for historical reasons, but I hope that list is "closed" now, and also I think it should be implemented exactly as specified, which is currently not the case.

So please, let's now focus on the question whether you accept the strstr() as a bug or not. Let's move away from my initial problem which could be solved by sending "feature debug=1" as you pointed out (thanks for that hint ).

Now what would have to happen to create problems with any existing engine by changing the strstr() into strncmp() in WinBoard, as specified? I guess that there are at most one or two engines in the world needing this WinBoard behaviour that does not conform to its own spec, to be able to claim a draw.

Sven

[hgm]

Receiving non-compliant input must by necessity result in undefined behavior, or it would be impossible to extend the protocol. What is non-compliant now, could be in the protcol specs next year. If the protocol would guarantee that it was ignored, you would break compatibilty by that, because you will not add a new command for the purpose of being ignored.

I think it is also desirable to hve an opportunity to add undocumented commands, not mentioned in the specs, because they do not completely do what I want yet, or because I want to keep the possibility open to change their behavior. And changing behavior without breaking compatibility is only possible as long as the behavior is still undefined.

As to the usage of strstr: yes, I think that is bad programming, and could be improved. But making changes just to alter behavior that is not specified in the first place and can only occur due to engines violating the specs does have a lower priority than creating new specified functionality.