Fix concurrency bug in analysis

cordova.polyfills.js

@@ -15,6 +15,7 @@
     trackEvent: noop
   };
 
+  window.AppVersion = 'dev';
 
   // push
   function oneSignalInit() {

src/dts/lichess.d.ts

@@ -35,9 +35,9 @@ interface LichessOptions {
 }
 
 interface Window {
-  lichess: LichessOptions;
-  moment: any;
-  shouldRotateToOrientation: () => boolean;
+  lichess: LichessOptions
+  moment: any
+  shouldRotateToOrientation: () => boolean
   handleOpenURL: (url: string) => void
   AppVersion: { version: string }
 }

src/js/chess.ts

@@ -1,6 +1,6 @@
 import { askWorker } from './utils/worker';
 
-const worker = new Worker('vendor/scalachessjs.js');
+const worker = new Worker('vendor/scalachess.js');
 
 // warmup
 worker.postMessage({ topic: 'init', payload: { variant: 'standard'}})
@@ -39,7 +39,7 @@ export interface MoveRequest {
 
 export interface MoveResponse {
   situation: GameSituation
-  path: string
+  path?: string
 }
 
 export interface DropRequest {
@@ -85,20 +85,24 @@ export interface PgnReadResponse {
   replay: Array<GameSituation>
 }
 
+function uniqId() {
+  return String(performance.now());
+}
+
 export function init(payload: InitRequest): Promise<InitResponse> {
   return askWorker(worker, { topic: 'init', payload });
 }
 
 export function dests(payload: DestsRequest): Promise<DestsResponse> {
-  return askWorker(worker, { topic: 'dests', payload });
+  return askWorker(worker, { topic: 'dests', payload, reqid: uniqId() });
 }
 
 export function move(payload: MoveRequest): Promise<MoveResponse> {
-  return askWorker(worker, { topic: 'move', payload });
+  return askWorker(worker, { topic: 'move', payload, reqid: uniqId() });
 }
 
 export function drop(payload: DropRequest): Promise<MoveResponse> {
-  return askWorker(worker, { topic: 'drop', payload });
+  return askWorker(worker, { topic: 'drop', payload, reqid: uniqId() });
 }
 
 export function threefoldTest(payload: ThreefoldTestRequest): Promise<ThreefoldTestResponse> {

src/js/ui/analyse/AnalyseCtrl.ts

@@ -7,7 +7,7 @@ import sound from '../../sound';
 import socket from '../../socket';
 import * as gameApi from '../../lichess/game';
 import settings from '../../settings';
-import { isEmptyObject, handleXhrError, oppositeColor, noop, hasNetwork } from '../../utils';
+import { isEmptyObject, handleXhrError, oppositeColor, hasNetwork } from '../../utils';
 import promotion from '../shared/offlineRound/promotion';
 import continuePopup from '../shared/continuePopup';
 import { notesCtrl } from '../shared/round/notes';
@@ -69,19 +69,6 @@ export default class AnalyseCtrl {
     this.menu = menu.controller(this);
     this.continuePopup = continuePopup.controller();
 
-    this.vm = {
-      shouldGoBack,
-      path: null,
-      pathStr: '',
-      step: null,
-      cgConfig: null,
-      variationMenu: null,
-      flip: false,
-      analysisProgress: false,
-      showBestMove: settings.analyse.showBestMove(),
-      showComments: settings.analyse.showComments()
-    };
-
     this.analyse = new Analyse(this.data);
     this.ceval = cevalCtrl(this.data.game.variant.key, this.allowCeval(), this.onCevalMsg);
     this.evalSummary = this.data.analysis ? evalSummary.controller(this) : null;
@@ -96,8 +83,18 @@ export default class AnalyseCtrl {
         treePath.default(this.analyse.lastPly()) :
         treePath.default(this.analyse.firstPly());
 
-    this.vm.path = initialPath;
-    this.vm.pathStr = treePath.write(initialPath);
+    this.vm = {
+      shouldGoBack,
+      path: initialPath,
+      pathStr: treePath.write(initialPath),
+      step: null,
+      cgConfig: null,
+      variationMenu: null,
+      flip: false,
+      analysisProgress: false,
+      showBestMove: settings.analyse.showBestMove(),
+      showComments: settings.analyse.showComments()
+    };
 
     this.showGround();
     this.initCeval();
@@ -107,21 +104,6 @@ export default class AnalyseCtrl {
     }
   }
 
-  public setData(data: AnalysisData) {
-    this.data = data;
-
-    this.analyse = new Analyse(this.data);
-    this.ceval = cevalCtrl(this.data.game.variant.key, this.allowCeval(), this.onCevalMsg);
-
-    const initialPath = treePath.default(0);
-    this.vm.step = null;
-    this.vm.path = initialPath;
-    this.vm.pathStr = treePath.write(initialPath);
-
-    this.showGround();
-    this.initCeval();
-  }
-
   public connectGameSocket = () => {
     if (hasNetwork()) {
       socket.createGame(
@@ -256,7 +238,7 @@ export default class AnalyseCtrl {
     if (prom) move.promotion = prom;
     chess.move(move)
     .then(this.addStep)
-    .catch(console.error.bind(console));
+    .catch(err => console.error('send move error', move, err));
   }
 
   private userMove = (orig: Pos, dest: Pos, capture: boolean) => {
@@ -279,7 +261,7 @@ export default class AnalyseCtrl {
       .then(this.addStep)
       .catch(err => {
         // catching false drops here
-        console.error(err);
+        console.error('wrong drop', err);
         this.jump(this.vm.path);
       });
     } else this.jump(this.vm.path);
@@ -301,7 +283,7 @@ export default class AnalyseCtrl {
     this.explorer.loading(true);
   }
 
-  public addStep = ({ situation, path}: chess.MoveResponse) => {
+  public addStep = ({ situation, path }: chess.MoveResponse) => {
     const step = {
       ply: situation.ply,
       dests: situation.dests,
@@ -333,7 +315,7 @@ export default class AnalyseCtrl {
     const id = path[0].variation;
     this.analyse.deleteVariation(ply, id);
     if (treePath.contains(path, this.vm.path)) this.jumpToMain(ply - 1);
-    this.toggleVariationMenu(null);
+    this.toggleVariationMenu();
   }
 
   public promoteVariation = (path: Path) => {
@@ -342,7 +324,7 @@ export default class AnalyseCtrl {
     this.analyse.promoteVariation(ply, id);
     if (treePath.contains(path, this.vm.path))
       this.jump(this.vm.path.splice(1));
-    this.toggleVariationMenu(null);
+    this.toggleVariationMenu();
   }
 
   public currentAnyEval = () => {
@@ -372,16 +354,18 @@ export default class AnalyseCtrl {
         dest: <Pos>res.ceval.best.slice(2, 4)
       }
 
-      chess.move(m)
-      .then((data: chess.MoveResponse) => {
-        step.ceval.bestSan = data.situation.pgnMoves[0];
-        if (res.work.path === this.vm.path) {
-          redraw();
-        }
-      })
-      .catch((err) => {
-        console.error('ceval move err', m, err);
-      });
+      if (step.ceval.best !== res.ceval.best) {
+        chess.move(m)
+        .then((data: chess.MoveResponse) => {
+          step.ceval.bestSan = data.situation.pgnMoves[0];
+          if (res.work.path === this.vm.path) {
+            redraw();
+          }
+        })
+        .catch((err) => {
+          console.error('ceval move err', m, err);
+        });
+      }
     });
   }
 
@@ -424,7 +408,7 @@ export default class AnalyseCtrl {
   public sharePGN = () => {
     if (this.source === 'online') {
       getPGN(this.data.game.id)
-      .then(pgn => window.plugins.socialsharing.share(pgn))
+      .then((pgn: string) => window.plugins.socialsharing.share(pgn))
       .catch(handleXhrError);
     } else {
       const endSituation = this.data.steps[this.data.steps.length - 1];
@@ -465,7 +449,7 @@ export default class AnalyseCtrl {
           if (this.gameOver()) this.ceval.stop();
         }
       })
-      .catch(console.error.bind(console));
+      .catch(err => console.error('get dests error', err));
     }
   }
 

src/js/ui/analyse/ceval/cevalEngine.ts

@@ -25,7 +25,6 @@ export default function cevalEngine(opts: Opts) {
 
   // we may have several start requests queued while we wait for previous
   // eval to complete
-  // we'll simply take the last request
   let startQueue: Array<CevalWork> = []
 
   function processOutput(text: string, work: CevalWork) {
@@ -101,13 +100,14 @@ export default function cevalEngine(opts: Opts) {
     .then(() => send('go depth ' + opts.maxDepth));
   }
 
+  // take the last work in queue and clear the queue just after
+  // to ensure we send to stockfish only one position to evaluate at a time
   function doStart() {
     const work = startQueue.pop();
     if (work) {
-      launchEval(work);
       startQueue = [];
+      launchEval(work);
     }
-    else console.error('no work queued to start eval');
   }
 
   return {
@@ -120,7 +120,7 @@ export default function cevalEngine(opts: Opts) {
         .then(() => Stockfish.init(), () => Stockfish.init())
         .then(() => init(variant))
       })
-      .catch(console.error.bind(console));
+      .catch(err => console.error('stockfish init error', err));
     },
 
     start(work: CevalWork) {

src/js/ui/analyse/interfaces.ts

@@ -144,7 +144,6 @@ export interface AnalyseCtrlInterface {
   userJump(path: Path, direction?: 'forward' | 'backward'): void
   nextStepBest(): string | null
   currentAnyEval(): Ceval | RemoteEval
-  setData(data: AnalysisData): void
   explorerMove(uci: string): void
   debouncedScroll(): void
   gameOver(): boolean

src/js/ui/analyse/path.ts

@@ -20,7 +20,7 @@ export default {
   },
 
   write(path: Path): string {
-    return path.map(function(step) {
+    return path.map((step: PathObj) => {
       return step.variation ? step.ply + ':' + step.variation : step.ply;
     }).join(',');
   },

src/js/utils/worker.ts

@@ -1,6 +1,7 @@
 export interface WorkerMessage<T> {
-  topic: string;
-  payload?: T;
+  topic: string
+  payload?: T
+  reqid?: string
 }
 
 export function tellWorker<A>(worker: Worker, topic: string, payload?: A): void {
@@ -14,10 +15,12 @@ export function tellWorker<A>(worker: Worker, topic: string, payload?: A): void
 export function askWorker<A, B>(worker: Worker, msg: WorkerMessage<A>): Promise<B> {
   return new Promise(function(resolve, reject) {
     function listen(e: MessageEvent) {
-      if (e.data.topic === msg.topic) {
+      if (e.data.topic === msg.topic && (msg.reqid === undefined || e.data.reqid === msg.reqid)) {
         worker.removeEventListener('message', listen);
         resolve(e.data.payload);
-      } else if (e.data.topic === 'error' && e.data.payload.callerTopic === msg.topic) {
+      } else if (e.data.topic === 'error' && e.data.payload.callerTopic === msg.topic && (
+        msg.reqid === undefined || e.data.reqid === msg.reqid
+      )) {
         worker.removeEventListener('message', listen);
         reject(e.data.payload.error);
       }