Conversation
There was a problem hiding this comment.
Pull request overview
This PR allows starting an offline computer game from an arbitrary position (via FEN), including from the analysis screen, and updates the new game dialog UX (side picker + mini-board preview) and controller logic to honor custom starting positions.
Changes:
- Extend
OfflineComputerGameScreenandOfflineComputerGameControllerto accept an optionalinitialFen, initialize the game from that position, and correctly decide whether the engine or player moves first based on the FEN turn. - Update the new game dialog UI to use an adaptive side picker and, when a custom FEN is provided, to show a mini
StaticChessboardpreview oriented to the chosen side. - Add an action in the analysis bottom sheet to “continue from here” into an offline computer game, and add comprehensive widget tests covering custom-start-position behavior and the new side picker helper.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/src/view/offline_computer/offline_computer_game_screen.dart | Adds initialFen support, shows the new game dialog immediately for custom positions, displays a mini preview board, switches side selection to a picker button, and forwards initialFen into game startup. |
| lib/src/model/offline_computer/offline_computer_game_controller.dart | Extends startNewGame/OfflineComputerGameState.initial to accept initialFen, initialize the Chess position and GameMeta (variant, initial FEN) from it, and trigger engine moves based on whose turn it actually is. |
| lib/src/view/analysis/analysis_screen.dart | Imports OfflineComputerGameScreen and adds a bottom sheet action that launches a computer game from the current analysis FEN. |
| test/view/offline_computer/offline_computer_game_screen_test.dart | Refactors side selection in tests via a selectSide helper, and adds tests for mini-board visibility, game state from custom FENs, engine vs. player first-move logic, and variant/initial FEN metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ), | ||
| if (analysisState.isComputerAnalysisAllowed) | ||
| BottomSheetAction( | ||
| makeLabel: (context) => const Text('Continue from here'), |
There was a problem hiding this comment.
This new bottom sheet action label is hard-coded as an English string instead of using the localized strings (e.g. context.l10n), whereas the neighboring actions like boardEditor and studyShareAndExport are localized. To keep the UI consistent and translatable, please replace this const Text('Continue from here') with a localized string from the existing l10n resources (or add one if needed).
| await tester.tap(find.text('White')); | ||
| case Side.black: | ||
| await tester.tap(find.text('Black')); |
There was a problem hiding this comment.
The switch (side) statement here has no break, return, or continue between the cases, so when side is Side.white the code will also execute the Side.black branch, tapping both options in the picker. This will either fail to compile (because Dart disallows fall-through between non-empty cases) or, if adjusted to compile, will end up selecting Black when White was requested; please add the appropriate control-flow (break/return) so only the intended side is tapped.
| await tester.tap(find.text('White')); | |
| case Side.black: | |
| await tester.tap(find.text('Black')); | |
| await tester.tap(find.text('White')); | |
| break; | |
| case Side.black: | |
| await tester.tap(find.text('Black')); | |
| break; |
1 participant
Allow to start an offline computer game from any position.