feat: persist ongoing OTB games #2470
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements persistence for ongoing over-the-board (OTB) games, allowing players to resume their games when they return to the OTB screen after leaving it. The implementation uses local file storage to save game state and clock times, automatically loading saved games on screen initialization.
- Added
OverTheBoardGameStorageclass for persisting and loading OTB game state - Modified
OverTheBoardScreento automatically load saved games on initialization and save games when losing focus - Updated clock and controller to support loading games with custom time values
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/src/model/over_the_board/over_the_board_game_storage.dart | New storage class that handles saving and loading OTB games to/from local JSON file |
| lib/src/view/over_the_board/over_the_board_screen.dart | Added initialization logic to load saved games, FocusDetector to save on focus loss, and updated exit dialog message |
| lib/src/model/over_the_board/over_the_board_game_controller.dart | Added loadOngoingGame method to restore a saved game state |
| lib/src/model/over_the_board/over_the_board_clock.dart | Extended setupClock to accept optional time parameters for loading saved clock states |
| lib/src/model/game/over_the_board_game.dart | Added fromJson factory method for deserialization |
| test/view/over_the_board/over_the_board_screen_test.dart | Added test for loading saved games and updated existing tests to mock the storage provider |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
This is cool, does it persist if say battery dies mid game |
Currently it only saves when the OTB screen loses focus (either by closing the screen or by closing the app entirely). I think if we run out of battery there would be no time to save (phone would be dead immediately). Could be fixed by saving the game at every move instead, but I decided against this because it's a lot of unnecessary I/O to write to the file at every move, especially with faster time controls. |
| } | ||
| }); | ||
|
|
||
| final clockState = ref.read(overTheBoardClockProvider); |
There was a problem hiding this comment.
Don't use ref.read in build();
If this is meant to be used in a callback, then use ref.read directly in the callback.
There was a problem hiding this comment.
As mentioned in my response to Copilot, this throws an exception in tests (In the emulator it only happens ~50% of the time):
00:07 +0: Playing over the board (offline) Checkmate and Rematch
══╡ EXCEPTION CAUGHT BY SCHEDULER LIBRARY ╞═════════════════════════════════════════════════════════
The following StateError was thrown during a scheduler callback:
Bad state: Using "ref" when a widget is about to or has been unmounted is unsafe.
Ref relies on BuildContext, and BuildContext is unsafe to use when the widget is deactivated.
To safely refer to the state of providers inside State.dispose(), save the provider state in a field
of your State class.
When the exception was thrown, this was the stack:
#0 ConsumerStatefulElement._assertNotDisposed (package:flutter_riverpod/src/core/consumer.dart:432:7)
#1 ConsumerStatefulElement.read (package:flutter_riverpod/src/core/consumer.dart:511:5)
#2 _BodyState.build.<anonymous closure> (package:lichess_mobile/src/view/over_the_board/over_the_board_screen.dart:189:28)
#3 _FocusDetectorState._notifyFocusLoss (package:lichess_mobile/src/utils/focus_detector.dart:142:25)
#4 _FocusDetectorState._notifyVisibilityStatusChange (package:lichess_mobile/src/utils/focus_detector.dart:129:7)
#5 _FocusDetectorState.build.<anonymous closure> (package:lichess_mobile/src/utils/focus_detector.dart:106:7)
#6 RenderVisibilityDetectorBase._fireCallback (package:visibility_detector/src/render_visibility_detector.dart:87:26)
#7 RenderVisibilityDetectorBase._scheduleUpdate.<anonymous closure> (package:visibility_detector/src/render_visibility_detector.dart:150:7)
#8 RenderVisibilityDetectorBase._processCallbacks (package:visibility_detector/src/render_visibility_detector.dart:62:15)
#9 RenderVisibilityDetectorBase._scheduleUpdate.<anonymous closure> (package:visibility_detector/src/render_visibility_detector.dart:162:11)
#10 SchedulerBinding._invokeFrameCallback (package:flutter/src/scheduler/binding.dart:1434:15)
#11 SchedulerBinding.handleDrawFrame (package:flutter/src/scheduler/binding.dart:1361:11)
#12 AutomatedTestWidgetsFlutterBinding.scheduleWarmUpFrame (package:flutter_test/src/binding.dart:1432:5)
#13 _runWidget (package:flutter/src/widgets/binding.dart:1643:7)
#14 runApp (package:flutter/src/widgets/binding.dart:1576:3)
#15 TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:1066:7)
<asynchronous suspension>
<asynchronous suspension>
(elided one frame from package:stack_trace)
There was a problem hiding this comment.
you need to check if the widget is still mounted before using ref
There was a problem hiding this comment.
if the widget is unmounted during onFocusLost and it is not normal you should try to find the reason why
There was a problem hiding this comment.
I couldn't really figure out why sometimes this would happen, Claude suggested it's probably a race condition between the flutter framework and the visibility_detector package we're using.
It only happens when I exit the game via the back-button, but not if I put the app into the background.
I now changed the callback from onFocusLost to onForegroundLost (to handle the case where the user puts the app into the background, and at some point the app would be killed because of low RAM) and added another callback instead when the user presses the Yes Button of the confirmation dialog
There was a problem hiding this comment.
iirc onFocusLost also handle the case where the user puts the app into background; will try to debug it myself when I have the time
|
I rebased this to test it @tom-anders |
3 participants
Closes #2466
otb.webm