Merge pull request #1275 from lichess-org/fix_start_engine

Fix engine evaluation started from new analysis screens
lichess-org · Dec 19, 2024 · ed56971 · ed56971
2 parents 2ea2b5e + 47fba32
commit ed56971
Show file tree

Hide file tree

Showing 6 changed files with 116 additions and 175 deletions.
diff --git a/lib/src/model/analysis/analysis_controller.dart b/lib/src/model/analysis/analysis_controller.dart
@@ -191,20 +191,11 @@ class AnalysisController extends _$AnalysisController implements PgnTreeNotifier
     );
 
     if (analysisState.isEngineAvailable) {
-      evaluationService
-          .initEngine(
-            _evaluationContext,
-            options: EvaluationOptions(
-              multiPv: prefs.numEvalLines,
-              cores: prefs.numEngineCores,
-              searchTime: prefs.engineSearchTime,
-            ),
-          )
-          .then((_) {
-            _startEngineEvalTimer = Timer(const Duration(milliseconds: 250), () {
-              _startEngineEval();
-            });
-          });
+      evaluationService.initEngine(_evaluationContext, options: _evaluationOptions).then((_) {
+        _startEngineEvalTimer = Timer(const Duration(milliseconds: 250), () {
+          _startEngineEval();
+        });
+      });
     }
 
     return analysisState;
@@ -213,6 +204,9 @@ class AnalysisController extends _$AnalysisController implements PgnTreeNotifier
   EvaluationContext get _evaluationContext =>
       EvaluationContext(variant: _variant, initialPosition: _root.position);
 
+  EvaluationOptions get _evaluationOptions =>
+      ref.read(analysisPreferencesProvider).evaluationOptions;
+
   void onUserMove(NormalMove move, {bool shouldReplace = false}) {
     if (!state.requireValue.position.isLegal(move)) return;
 
@@ -360,17 +354,9 @@ class AnalysisController extends _$AnalysisController implements PgnTreeNotifier
     );
 
     if (state.requireValue.isEngineAvailable) {
-      final prefs = ref.read(analysisPreferencesProvider);
       await ref
           .read(evaluationServiceProvider)
-          .initEngine(
-            _evaluationContext,
-            options: EvaluationOptions(
-              multiPv: prefs.numEvalLines,
-              cores: prefs.numEngineCores,
-              searchTime: prefs.engineSearchTime,
-            ),
-          );
+          .initEngine(_evaluationContext, options: _evaluationOptions);
       _startEngineEval();
     } else {
       _stopEngineEval();
@@ -381,15 +367,7 @@ class AnalysisController extends _$AnalysisController implements PgnTreeNotifier
   void setNumEvalLines(int numEvalLines) {
     ref.read(analysisPreferencesProvider.notifier).setNumEvalLines(numEvalLines);
 
-    ref
-        .read(evaluationServiceProvider)
-        .setOptions(
-          EvaluationOptions(
-            multiPv: numEvalLines,
-            cores: ref.read(analysisPreferencesProvider).numEngineCores,
-            searchTime: ref.read(analysisPreferencesProvider).engineSearchTime,
-          ),
-        );
+    ref.read(evaluationServiceProvider).setOptions(_evaluationOptions);
 
     _root.updateAll((node) => node.eval = null);
 
@@ -406,31 +384,15 @@ class AnalysisController extends _$AnalysisController implements PgnTreeNotifier
   void setEngineCores(int numEngineCores) {
     ref.read(analysisPreferencesProvider.notifier).setEngineCores(numEngineCores);
 
-    ref
-        .read(evaluationServiceProvider)
-        .setOptions(
-          EvaluationOptions(
-            multiPv: ref.read(analysisPreferencesProvider).numEvalLines,
-            cores: numEngineCores,
-            searchTime: ref.read(analysisPreferencesProvider).engineSearchTime,
-          ),
-        );
+    ref.read(evaluationServiceProvider).setOptions(_evaluationOptions);
 
     _startEngineEval();
   }
 
   void setEngineSearchTime(Duration searchTime) {
     ref.read(analysisPreferencesProvider.notifier).setEngineSearchTime(searchTime);
 
-    ref
-        .read(evaluationServiceProvider)
-        .setOptions(
-          EvaluationOptions(
-            multiPv: ref.read(analysisPreferencesProvider).numEvalLines,
-            cores: ref.read(analysisPreferencesProvider).numEngineCores,
-            searchTime: searchTime,
-          ),
-        );
+    ref.read(evaluationServiceProvider).setOptions(_evaluationOptions);
 
     _startEngineEval();
   }
@@ -553,6 +515,14 @@ class AnalysisController extends _$AnalysisController implements PgnTreeNotifier
     }
   }
 
+  void _refreshCurrentNode() {
+    state = AsyncData(
+      state.requireValue.copyWith(
+        currentNode: AnalysisCurrentNode.fromNode(_root.nodeAt(state.requireValue.currentPath)),
+      ),
+    );
+  }
+
   Future<(UciPath, FullOpening)?> _fetchOpening(Node fromNode, UciPath path) async {
     if (!kOpeningAllowedVariants.contains(_variant)) return null;
 
@@ -572,15 +542,16 @@ class AnalysisController extends _$AnalysisController implements PgnTreeNotifier
 
     final curState = state.requireValue;
     if (curState.currentPath == path) {
-      state = AsyncData(
-        curState.copyWith(currentNode: AnalysisCurrentNode.fromNode(_root.nodeAt(path))),
-      );
+      _refreshCurrentNode();
     }
   }
 
-  void _startEngineEval() {
+  Future<void> _startEngineEval() async {
     final curState = state.requireValue;
     if (!curState.isEngineAvailable) return;
+    await ref
+        .read(evaluationServiceProvider)
+        .ensureEngineInitialized(_evaluationContext, options: _evaluationOptions);
     ref
         .read(evaluationServiceProvider)
         .start(
@@ -589,7 +560,13 @@ class AnalysisController extends _$AnalysisController implements PgnTreeNotifier
           initialPositionEval: _root.eval,
           shouldEmit: (work) => work.path == state.valueOrNull?.currentPath,
         )
-        ?.forEach((t) => _root.updateAt(t.$1.path, (node) => node.eval = t.$2));
+        ?.forEach((t) {
+          final (work, eval) = t;
+          _root.updateAt(work.path, (node) => node.eval = eval);
+          if (work.path == curState.currentPath && eval.searchTime >= work.searchTime) {
+            _refreshCurrentNode();
+          }
+        });
   }
 
   void _debouncedStartEngineEval() {
@@ -601,12 +578,7 @@ class AnalysisController extends _$AnalysisController implements PgnTreeNotifier
   void _stopEngineEval() {
     ref.read(evaluationServiceProvider).stop();
     // update the current node with last cached eval
-    final curState = state.requireValue;
-    state = AsyncData(
-      curState.copyWith(
-        currentNode: AnalysisCurrentNode.fromNode(_root.nodeAt(curState.currentPath)),
-      ),
-    );
+    _refreshCurrentNode();
   }
 
   void _listenToServerAnalysisEvents() {

diff --git a/lib/src/model/analysis/analysis_preferences.dart b/lib/src/model/analysis/analysis_preferences.dart
@@ -99,6 +99,9 @@ class AnalysisPrefs with _$AnalysisPrefs implements Serializable {
   factory AnalysisPrefs.fromJson(Map<String, dynamic> json) {
     return _$AnalysisPrefsFromJson(json);
   }
+
+  EvaluationOptions get evaluationOptions =>
+      EvaluationOptions(multiPv: numEvalLines, cores: numEngineCores, searchTime: engineSearchTime);
 }
 
 Duration _searchTimeDefault() {

diff --git a/lib/src/model/broadcast/broadcast_game_controller.dart b/lib/src/model/broadcast/broadcast_game_controller.dart
@@ -120,20 +120,11 @@ class BroadcastGameController extends _$BroadcastGameController implements PgnTr
     );
 
     if (broadcastState.isLocalEvaluationEnabled) {
-      evaluationService
-          .initEngine(
-            _evaluationContext,
-            options: EvaluationOptions(
-              multiPv: prefs.numEvalLines,
-              cores: prefs.numEngineCores,
-              searchTime: ref.read(analysisPreferencesProvider).engineSearchTime,
-            ),
-          )
-          .then((_) {
-            _startEngineEvalTimer = Timer(const Duration(milliseconds: 250), () {
-              _startEngineEval();
-            });
-          });
+      evaluationService.initEngine(_evaluationContext, options: _evaluationOptions).then((_) {
+        _startEngineEvalTimer = Timer(const Duration(milliseconds: 250), () {
+          _startEngineEval();
+        });
+      });
     }
 
     return broadcastState;
@@ -251,6 +242,9 @@ class BroadcastGameController extends _$BroadcastGameController implements PgnTr
   EvaluationContext get _evaluationContext =>
       EvaluationContext(variant: Variant.standard, initialPosition: _root.position);
 
+  EvaluationOptions get _evaluationOptions =>
+      ref.read(analysisPreferencesProvider).evaluationOptions;
+
   void onUserMove(NormalMove move) {
     if (!state.hasValue) return;
 
@@ -388,17 +382,9 @@ class BroadcastGameController extends _$BroadcastGameController implements PgnTr
     );
 
     if (state.requireValue.isLocalEvaluationEnabled) {
-      final prefs = ref.read(analysisPreferencesProvider);
       await ref
           .read(evaluationServiceProvider)
-          .initEngine(
-            _evaluationContext,
-            options: EvaluationOptions(
-              multiPv: prefs.numEvalLines,
-              cores: prefs.numEngineCores,
-              searchTime: ref.read(analysisPreferencesProvider).engineSearchTime,
-            ),
-          );
+          .initEngine(_evaluationContext, options: _evaluationOptions);
       _startEngineEval();
     } else {
       _stopEngineEval();
@@ -411,15 +397,7 @@ class BroadcastGameController extends _$BroadcastGameController implements PgnTr
 
     ref.read(analysisPreferencesProvider.notifier).setNumEvalLines(numEvalLines);
 
-    ref
-        .read(evaluationServiceProvider)
-        .setOptions(
-          EvaluationOptions(
-            multiPv: numEvalLines,
-            cores: ref.read(analysisPreferencesProvider).numEngineCores,
-            searchTime: ref.read(analysisPreferencesProvider).engineSearchTime,
-          ),
-        );
+    ref.read(evaluationServiceProvider).setOptions(_evaluationOptions);
 
     _root.updateAll((node) => node.eval = null);
 
@@ -435,31 +413,15 @@ class BroadcastGameController extends _$BroadcastGameController implements PgnTr
   void setEngineCores(int numEngineCores) {
     ref.read(analysisPreferencesProvider.notifier).setEngineCores(numEngineCores);
 
-    ref
-        .read(evaluationServiceProvider)
-        .setOptions(
-          EvaluationOptions(
-            multiPv: ref.read(analysisPreferencesProvider).numEvalLines,
-            cores: numEngineCores,
-            searchTime: ref.read(analysisPreferencesProvider).engineSearchTime,
-          ),
-        );
+    ref.read(evaluationServiceProvider).setOptions(_evaluationOptions);
 
     _startEngineEval();
   }
 
   void setEngineSearchTime(Duration searchTime) {
     ref.read(analysisPreferencesProvider.notifier).setEngineSearchTime(searchTime);
 
-    ref
-        .read(evaluationServiceProvider)
-        .setOptions(
-          EvaluationOptions(
-            multiPv: ref.read(analysisPreferencesProvider).numEvalLines,
-            cores: ref.read(analysisPreferencesProvider).numEngineCores,
-            searchTime: searchTime,
-          ),
-        );
+    ref.read(evaluationServiceProvider).setOptions(_evaluationOptions);
 
     _startEngineEval();
   }
@@ -540,10 +502,13 @@ class BroadcastGameController extends _$BroadcastGameController implements PgnTr
     }
   }
 
-  void _startEngineEval() {
+  Future<void> _startEngineEval() async {
     if (!state.hasValue) return;
 
     if (!state.requireValue.isLocalEvaluationEnabled) return;
+    await ref
+        .read(evaluationServiceProvider)
+        .ensureEngineInitialized(_evaluationContext, options: _evaluationOptions);
     ref
         .read(evaluationServiceProvider)
         .start(
@@ -552,7 +517,21 @@ class BroadcastGameController extends _$BroadcastGameController implements PgnTr
           initialPositionEval: _root.eval,
           shouldEmit: (work) => work.path == state.valueOrNull?.currentPath,
         )
-        ?.forEach((t) => _root.updateAt(t.$1.path, (node) => node.eval = t.$2));
+        ?.forEach((t) {
+          final (work, eval) = t;
+          _root.updateAt(work.path, (node) => node.eval = eval);
+          if (work.path == state.requireValue.currentPath && eval.searchTime >= work.searchTime) {
+            _refreshCurrentNode();
+          }
+        });
+  }
+
+  void _refreshCurrentNode() {
+    state = AsyncData(
+      state.requireValue.copyWith(
+        currentNode: AnalysisCurrentNode.fromNode(_root.nodeAt(state.requireValue.currentPath)),
+      ),
+    );
   }
 
   void _debouncedStartEngineEval() {
@@ -566,11 +545,7 @@ class BroadcastGameController extends _$BroadcastGameController implements PgnTr
 
     ref.read(evaluationServiceProvider).stop();
     // update the current node with last cached eval
-    state = AsyncData(
-      state.requireValue.copyWith(
-        currentNode: AnalysisCurrentNode.fromNode(_root.nodeAt(state.requireValue.currentPath)),
-      ),
-    );
+    _refreshCurrentNode();
   }
 
   ({Duration? parentClock, Duration? clock}) _getClocks(UciPath path) {

diff --git a/lib/src/model/engine/evaluation_service.dart b/lib/src/model/engine/evaluation_service.dart
@@ -49,6 +49,8 @@ class EvaluationService {
 
   /// Initialize the engine with the given context and options.
   ///
+  /// If the engine is already initialized, it is disposed first.
+  ///
   /// An optional [engineFactory] can be provided, it defaults to Stockfish.
   ///
   /// If [options] is not provided, the default options are used.
@@ -59,7 +61,7 @@ class EvaluationService {
     Engine Function() engineFactory = StockfishEngine.new,
     EvaluationOptions? options,
   }) async {
-    if (context != _context) {
+    if (_context != null || _engine != null) {
       await disposeEngine();
     }
     _context = context;
@@ -81,6 +83,17 @@ class EvaluationService {
     });
   }
 
+  /// Ensure the engine is initialized with the given context and options.
+  Future<void> ensureEngineInitialized(
+    EvaluationContext context, {
+    Engine Function() engineFactory = StockfishEngine.new,
+    EvaluationOptions? options,
+  }) async {
+    if (_engine == null || _context != context) {
+      await initEngine(context, engineFactory: engineFactory, options: options);
+    }
+  }
+
   void setOptions(EvaluationOptions options) {
     stop();
     _options = options;

diff --git a/lib/src/model/puzzle/puzzle_controller.dart b/lib/src/model/puzzle/puzzle_controller.dart
@@ -447,8 +447,9 @@ class PuzzleController extends _$PuzzleController {
     return pgn;
   }
 
-  void _startEngineEval() {
+  Future<void> _startEngineEval() async {
     if (!state.isEngineEnabled) return;
+    await ref.read(evaluationServiceProvider).ensureEngineInitialized(state.evaluationContext);
     _engineEvalDebounce(
       () => ref
           .read(evaluationServiceProvider)
@@ -463,6 +464,9 @@ class PuzzleController extends _$PuzzleController {
             _gameTree.updateAt(work.path, (node) {
               node.eval = eval;
             });
+            if (work.path == state.currentPath && eval.searchTime >= work.searchTime) {
+              state = state.copyWith(node: _gameTree.branchAt(state.currentPath).view);
+            }
           }),
     );
   }