Rewrite the buffering on position change.

There was seemingly a small window where we could get evaluations
on the previous position and attribute them to the current one,
without actually noticing. Try rewriting this for the Nth time
and hope it doesn't mess up anything.

diff --git a/Engine.pm b/Engine.pm
index c364ae5daff3a800082d5abb00c3d5cdc43a41b4..6ec914fb42c8bf7e98909d36e9c5a7aebf1fad4a 100644 (file)
--- a/Engine.pm
+++ b/Engine.pm
@@ -29,6 +29,7 @@ sub open {
                ev => $ev,
                cb => $cb,
                seen_uciok => 0,
+               stopping => 0,
        };
 
        print $uciwrite "uci\n";

diff --git a/config.pm b/config.pm
index 7de44078afdfb91e5b4c85f11e4d78bfccdc5634..44ed56b0b50555498e80fede05077e3981c85464 100644 (file)
--- a/config.pm
+++ b/config.pm
@@ -57,7 +57,6 @@ our %engine2_config = (
 );
 our $engine2_grpc_backend = undef;  # Not used by us, but will be communicated to serve-analysis.js.
 
-our $uci_assume_full_compliance = 0;                    # dangerous :-)
 our $update_max_interval = 1.0;
 our @masters = (
        'Sesse',

diff --git a/remoteglot.pl b/remoteglot.pl
index 35fe887382eaabb2cc0e0dd7be9a8809c9bf4eea..2b81e5c9566bd2ae7c6069e2401407ed7ef1486d 100755 (executable)
--- a/remoteglot.pl
+++ b/remoteglot.pl
@@ -74,7 +74,7 @@ my $engine = open_engine($remoteglotconf::engine_cmdline, 'E1', sub { handle_uci
 my $engine2 = open_engine($remoteglotconf::engine2_cmdline, 'E2', sub { handle_uci(@_, 0); });
 my $last_move;
 my $last_text = '';
-my ($pos_waiting, $pos_calculating, $pos_calculating_second_engine);
+my ($pos_calculating, $pos_calculating_second_engine);
 
 uciprint($engine, "setoption name UCI_AnalyseMode value true");
 while (my ($key, $value) = each %remoteglotconf::engine_config) {
@@ -144,6 +144,11 @@ sub handle_uci {
 
        $line =~ s/  / /g;  # Sometimes needed for Zappa Mexico
        print UCILOG localtime() . " $engine->{'tag'} <= $line\n";
+
+       # If we've sent a stop command, gobble up lines until we see bestmove.
+       return if ($engine->{'stopping'} && $line !~ /^bestmove/);
+       $engine->{'stopping'} = 0;
+
        if ($line =~ /^info/) {
                my (@infos) = split / /, $line;
                shift @infos;
@@ -156,24 +161,6 @@ sub handle_uci {
 
                parse_ids($engine, @ids);
        }
-       if ($line =~ /^bestmove/) {
-               if ($primary) {
-                       return if (!$remoteglotconf::uci_assume_full_compliance);
-                       if (defined($pos_waiting)) {
-                               uciprint($engine, "position fen " . $pos_waiting->fen());
-                               uciprint($engine, "go infinite");
-
-                               $pos_calculating = $pos_waiting;
-                               $pos_waiting = undef;
-                       }
-               } else {
-                       $engine2->{'info'} = {};
-                       my $pos = $pos_waiting // $pos_calculating;
-                       uciprint($engine2, "position fen " . $pos->fen());
-                       uciprint($engine2, "go infinite");
-                       $pos_calculating_second_engine = $pos;
-               }
-       }
        output();
 }
 
@@ -189,7 +176,7 @@ sub handle_fics {
                $t->cmd("moves");
        }
        if ($line =~ /^Movelist for game /) {
-               my $pos = $pos_waiting // $pos_calculating;
+               my $pos = $pos_calculating;
                if (defined($pos)) {
                        @uci_movelist = ();
                        @pretty_movelist = ();
@@ -222,10 +209,9 @@ sub handle_fics {
        if ($getting_movelist &&
            $line =~ /^\s+ \{.*\} \s+ (?: \* | 1\/2-1\/2 | 0-1 | 1-0 )/x) {
                # End of movelist.
-               for my $pos ($pos_waiting, $pos_calculating) {
-                       next if (!defined($pos));
-                       if ($pos->fen() eq $pos_for_movelist->fen()) {
-                               $pos->{'history'} = \@pretty_movelist;
+               if (defined($pos_calculating)) {
+                       if ($pos_calculating->fen() eq $pos_for_movelist->fen()) {
+                               $pos_calculating->{'history'} = \@pretty_movelist;
                        }
                }
                $getting_movelist = 0;
@@ -356,22 +342,15 @@ sub handle_position {
        my ($pos) = @_;
        find_clock_start($pos, $pos_calculating);
                
-       # if this is already in the queue, ignore it (just update the result)
-       if (defined($pos_waiting) && $pos->fen() eq $pos_waiting->fen()) {
-               $pos_waiting->{'result'} = $pos->{'result'};
-               return;
-       }
-
-       # if we're already chewing on this and there's nothing else in the queue,
-       # also ignore it
-       if (!defined($pos_waiting) && defined($pos_calculating) &&
-           $pos->fen() eq $pos_calculating->fen()) {
+       # If we're already chewing on this and there's nothing else in the queue,
+       # ignore it.
+       if (defined($pos_calculating) && $pos->fen() eq $pos_calculating->fen()) {
                $pos_calculating->{'result'} = $pos->{'result'};
                return;
        }
 
-       # if we're already thinking on something, stop and wait for the engine
-       # to approve
+       # If we're already thinking on something, stop and wait for the engine
+       # to approve.
        if (defined($pos_calculating)) {
                # Store the final data we have for this position in the history,
                # with the precise clock information we just got from the new
@@ -386,33 +365,27 @@ sub handle_position {
                delete $pos_calculating->{'black_clock_target'};
                output_json(1);
 
-               if (!defined($pos_waiting)) {
-                       uciprint($engine, "stop");
-               }
-               if ($remoteglotconf::uci_assume_full_compliance) {
-                       $pos_waiting = $pos;
-               } else {
-                       uciprint($engine, "position fen " . $pos->fen());
-                       uciprint($engine, "go infinite");
-                       $pos_calculating = $pos;
-               }
-       } else {
-               # it's wrong just to give the FEN (the move history is useful,
-               # and per the UCI spec, we should really have sent "ucinewgame"),
-               # but it's easier
-               uciprint($engine, "position fen " . $pos->fen());
-               uciprint($engine, "go infinite");
-               $pos_calculating = $pos;
+               # Ask the engine to stop; we will throw away its data until it
+               # sends us "bestmove", signaling the end of it.
+               $engine->{'stopping'} = 1;
+               uciprint($engine, "stop");
        }
 
+       # It's wrong to just give the FEN (the move history is useful,
+       # and per the UCI spec, we should really have sent "ucinewgame"),
+       # but it's easier, and it works around a Stockfish repetition issue.
+       uciprint($engine, "position fen " . $pos->fen());
+       uciprint($engine, "go infinite");
+       $pos_calculating = $pos;
+
        if (defined($engine2)) {
                if (defined($pos_calculating_second_engine)) {
+                       $engine2->{'stopping'} = 1;
                        uciprint($engine2, "stop");
-               } else {
-                       uciprint($engine2, "position fen " . $pos->fen());
-                       uciprint($engine2, "go infinite");
-                       $pos_calculating_second_engine = $pos;
                }
+               uciprint($engine2, "position fen " . $pos->fen());
+               uciprint($engine2, "go infinite");
+               $pos_calculating_second_engine = $pos;
                $engine2->{'info'} = {};
        }
 
@@ -533,7 +506,7 @@ sub prettyprint_pv_no_cache {
 sub prettyprint_pv {
        my ($pos, @pvs) = @_;
 
-       my $cachekey = $pos->fen() . join('', @pvs);
+       my $cachekey = join('', @pvs);
        if (exists($pos->{'prettyprint_cache'}{$cachekey})) {
                return @{$pos->{'prettyprint_cache'}{$cachekey}};
        } else {
@@ -1350,7 +1323,7 @@ sub find_clock_start {
 
 sub schedule_tb_lookup {
        return if (!defined($remoteglotconf::tb_serial_key));
-       my $pos = $pos_waiting // $pos_calculating;
+       my $pos = $pos_calculating;
        return if (exists($tb_cache{$pos->fen()}));
 
        # If there's more than seven pieces, there's not going to be an answer,