vampirc / vampirc-uci

A Universal Chess Interface (UCI) protocol parser and message generator.
https://vampirc.kejzar.si
Apache License 2.0
19 stars 7 forks source link

vampirc-uci doesn't recognize negative times #16

Closed vlad902 closed 4 years ago

vlad902 commented 4 years ago

Cute Chess lets human players go below 0 seconds without flagging and will then send the player's time as negative, e.g. 'go wtime -4061 btime 56826 movestogo 90'. The current parser only accepts positive numbers. To be honest, I'm not sure this is a vampirc-uci bug since by some definition negative numbers really shouldn't show up there, but in practice Cute Chess is probably one of the top chess interfaces so it may be worth supporting. Unfortunately it doesn't seem like there's a very well-defined specification to sort out whether this should be fixed on that parser end, or having Cute Chess do something like what xboard does in this situation (report opponent as having 1ms.)

vlad902 commented 4 years ago

Note that according [1] to using the timemargin setting with cutechess-cli to allow going over by some time amount will also allow negative times.

[1] https://groups.google.com/g/lczero/c/hcw-GttDRAY

MadMatt04 commented 4 years ago

Hm, I must admit I did fail to consider this possibility. I guess if CuteChess allows for it, we should at least consider it.

It does present a problem, though. Some versions ago I switched the representation of these values to the Rust standard library's Duration type, which does not allow for "negative" durations:

pub struct Duration { secs: u64, nanos: u32, // Always 0 <= nanos < NANOS_PER_SEC }

So I'm going to have to think about how to support this. It's probably going to have to be an API breaking change.

MadMatt04 commented 4 years ago

The chrono crate allows for negative Duration. We could consider switching to that.

It does introduce another dependency, though. An optional feature is probably not warranted here.

vlad902 commented 4 years ago

You could consider parsing negative value into a Duration::new(0, 0). Strictly speaking, it doesn't reflect the data returned; however, a negative duration is no better than a duration of 0 when you're using it to compute how long you should play for and is easier to test for (and more expected.)

MadMatt04 commented 4 years ago

I feel that just quietly changing the negative value to 0 might prove to be unexpected behaviour in the eyes of the users of the library. In addition to that, I feel that in certain cases knowing how much over the clock someone is could be useful information.

I think I'm gonna switch the representation to chrono::Duration. Seems to be the best possible solution.

MadMatt04 commented 4 years ago

Implemented in #18

vlad902 commented 4 years ago

I synced against master and verified that this change works in Cute Chess, thanks for the quick fix!

MadMatt04 commented 4 years ago

Now also available as a crate release v0.11.0.