New classic level - #29
Merged
Merged
Conversation
📝 WalkthroughWalkthroughMigrates level identifiers from integers to strings to support fractional IDs (e.g., 0.1, 0.2). Adds level 0.2, bumps app and database versions, includes DB migration converting INTEGER level_id → TEXT, and updates providers, UI, router, widgets, and tests to use String level IDs. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Router
participant GameScreen
participant GameProvider
participant Database
Note over Database: On app upgrade (DB v5→v6) migration runs
User->>Router: navigate /game/:levelId (e.g., "0.2")
Router->>GameScreen: instantiate with levelId (String)
GameScreen->>GameProvider: request build/loadLevel(levelId: "0.2")
GameProvider->>Database: getSavedGameState(levelId: "0.2") / isLevelCompleted("0.2")
Database-->>GameProvider: return TEXT-keyed data
GameProvider-->>GameScreen: provide game state
GameScreen->>User: render level "0.2"
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@lib/core/database/database_helper.dart`:
- Around line 66-108: The migration block guarded by "if (oldVersion < 6)" must
be executed inside a database transaction to avoid partial failure: replace the
current sequence of await db.execute(...) calls for creating
completed_levels_new, saved_games_new, the INSERT ... SELECT migrations, DROP
TABLE completed_levels/saved_games and the ALTER TABLE ... RENAME operations
with a single transactional call (e.g. await db.transaction((txn) async { ... })
) and run each DDL/DML against the transaction handle
(txn.execute/txn.rawInsert) so any exception triggers an automatic rollback and
prevents loss of completed_levels and saved_games data.
In `@lib/core/utils/market_helper.dart`:
- Around line 41-44: The Makefile's set-google-play sed pattern no longer
matches because the code default was changed to return
GeneralConsts.otherAppsGooglePlayLink; update the sed in the set-google-play
target so it matches either variant and forces the return to
GeneralConsts.otherAppsGooglePlayLink (e.g. use a regex that matches
GeneralConsts.otherAppsRustoreLink|GeneralConsts.otherAppsGooglePlayLink and
replaces with GeneralConsts.otherAppsGooglePlayLink); also ensure the
set-rustore target is updated symmetrically to always replace either symbol with
GeneralConsts.otherAppsRustoreLink so _getCurrentOtherAppsUrl and the
GeneralConsts.otherAppsGooglePlayLink/otherAppsRustoreLink swaps work reliably.
🧹 Nitpick comments (3)
lib/presentation/screens/game/game_screen.dart (1)
271-271: Consider:indexOfreturns -1 if level not found.If
widget.levelIdis not in thelevelslist (e.g., during async loading race),displayLevelNumberwill be -1. Line 288 handles this by showing "0", but this may be confusing to users if the level truly isn't found.lib/presentation/providers/levels_provider.dart (1)
18-23: Note: Non-numeric level IDs will sort to position 0.The
double.tryParse(a) ?? 0fallback means any level ID that isn't a valid number (e.g., "tutorial") will be treated as 0 for sorting purposes. This clusters all non-numeric IDs at the start, which may not be intentional.If non-numeric IDs are expected in the future, consider a more explicit ordering strategy.
test/levels_uniqueness_test.dart (1)
20-30: Consider extracting duplicated level ID parsing logic.The level ID extraction and sorting logic (lines 20-30) is duplicated from
levels_provider.dart. If the parsing logic changes in one place, tests might drift out of sync.Consider extracting this into a shared utility function or directly using
levelsProviderin tests.♻️ Example: Extract shared utility
// In a shared utility file (e.g., lib/utils/level_utils.dart) List<String> extractAndSortLevelIds(List<String> levelPaths) { final levelIds = levelPaths.map((path) { return path.split('/').last.replaceAll('level_', '').replaceAll('.json', ''); }).toList(); levelIds.sort((a, b) { final aNum = double.tryParse(a) ?? 0; final bNum = double.tryParse(b) ?? 0; return aNum.compareTo(bNum); }); return levelIds; }
Comment on lines
+66
to
+108
| if (oldVersion < 6) { | ||
| // Migrate level_id from INTEGER to TEXT to support fractional levels (0.1, 0.2, etc.) | ||
| // Also migrate level 0 to 0.1 for users who already completed it | ||
|
|
||
| // Create new tables with TEXT level_id | ||
| await db.execute(''' | ||
| CREATE TABLE completed_levels_new ( | ||
| level_id TEXT PRIMARY KEY, | ||
| completed_at TEXT NOT NULL, | ||
| moves_count INTEGER NOT NULL | ||
| ) | ||
| '''); | ||
|
|
||
| await db.execute(''' | ||
| CREATE TABLE saved_games_new ( | ||
| level_id TEXT PRIMARY KEY, | ||
| board_state TEXT NOT NULL, | ||
| moves_count INTEGER NOT NULL, | ||
| saved_at TEXT NOT NULL | ||
| ) | ||
| '''); | ||
|
|
||
| // Migrate data, converting level 0 to 0.1 | ||
| await db.execute(''' | ||
| INSERT INTO completed_levels_new (level_id, completed_at, moves_count) | ||
| SELECT CASE WHEN level_id = 0 THEN '0.1' ELSE CAST(level_id AS TEXT) END, | ||
| completed_at, moves_count | ||
| FROM completed_levels | ||
| '''); | ||
|
|
||
| await db.execute(''' | ||
| INSERT INTO saved_games_new (level_id, board_state, moves_count, saved_at) | ||
| SELECT CASE WHEN level_id = 0 THEN '0.1' ELSE CAST(level_id AS TEXT) END, | ||
| board_state, moves_count, saved_at | ||
| FROM saved_games | ||
| '''); | ||
|
|
||
| // Drop old tables and rename new ones | ||
| await db.execute('DROP TABLE completed_levels'); | ||
| await db.execute('DROP TABLE saved_games'); | ||
| await db.execute('ALTER TABLE completed_levels_new RENAME TO completed_levels'); | ||
| await db.execute('ALTER TABLE saved_games_new RENAME TO saved_games'); | ||
| } |
There was a problem hiding this comment.
Wrap migration in a transaction to prevent data loss on partial failure.
The migration performs multiple DDL operations sequentially. If any statement fails after DROP TABLE (lines 104-105) but before ALTER TABLE ... RENAME (lines 106-107), the database will be left in an inconsistent state with potential data loss.
🛡️ Proposed fix: Wrap in transaction
if (oldVersion < 6) {
// Migrate level_id from INTEGER to TEXT to support fractional levels (0.1, 0.2, etc.)
// Also migrate level 0 to 0.1 for users who already completed it
+ await db.transaction((txn) async {
+ // Create new tables with TEXT level_id
+ await txn.execute('''
+ CREATE TABLE completed_levels_new (
+ level_id TEXT PRIMARY KEY,
+ completed_at TEXT NOT NULL,
+ moves_count INTEGER NOT NULL
+ )
+ ''');
- // Create new tables with TEXT level_id
- await db.execute('''
- CREATE TABLE completed_levels_new (
- level_id TEXT PRIMARY KEY,
- completed_at TEXT NOT NULL,
- moves_count INTEGER NOT NULL
- )
- ''');
-
- await db.execute('''
- CREATE TABLE saved_games_new (
- level_id TEXT PRIMARY KEY,
- board_state TEXT NOT NULL,
- moves_count INTEGER NOT NULL,
- saved_at TEXT NOT NULL
- )
- ''');
-
- // Migrate data, converting level 0 to 0.1
- await db.execute('''
- INSERT INTO completed_levels_new (level_id, completed_at, moves_count)
- SELECT CASE WHEN level_id = 0 THEN '0.1' ELSE CAST(level_id AS TEXT) END,
- completed_at, moves_count
- FROM completed_levels
- ''');
-
- await db.execute('''
- INSERT INTO saved_games_new (level_id, board_state, moves_count, saved_at)
- SELECT CASE WHEN level_id = 0 THEN '0.1' ELSE CAST(level_id AS TEXT) END,
- board_state, moves_count, saved_at
- FROM saved_games
- ''');
-
- // Drop old tables and rename new ones
- await db.execute('DROP TABLE completed_levels');
- await db.execute('DROP TABLE saved_games');
- await db.execute('ALTER TABLE completed_levels_new RENAME TO completed_levels');
- await db.execute('ALTER TABLE saved_games_new RENAME TO saved_games');
+ await txn.execute('''
+ CREATE TABLE saved_games_new (
+ level_id TEXT PRIMARY KEY,
+ board_state TEXT NOT NULL,
+ moves_count INTEGER NOT NULL,
+ saved_at TEXT NOT NULL
+ )
+ ''');
+
+ // Migrate data, converting level 0 to 0.1
+ await txn.execute('''
+ INSERT INTO completed_levels_new (level_id, completed_at, moves_count)
+ SELECT CASE WHEN level_id = 0 THEN '0.1' ELSE CAST(level_id AS TEXT) END,
+ completed_at, moves_count
+ FROM completed_levels
+ ''');
+
+ await txn.execute('''
+ INSERT INTO saved_games_new (level_id, board_state, moves_count, saved_at)
+ SELECT CASE WHEN level_id = 0 THEN '0.1' ELSE CAST(level_id AS TEXT) END,
+ board_state, moves_count, saved_at
+ FROM saved_games
+ ''');
+
+ // Drop old tables and rename new ones
+ await txn.execute('DROP TABLE completed_levels');
+ await txn.execute('DROP TABLE saved_games');
+ await txn.execute('ALTER TABLE completed_levels_new RENAME TO completed_levels');
+ await txn.execute('ALTER TABLE saved_games_new RENAME TO saved_games');
+ });
}🤖 Prompt for AI Agents
In `@lib/core/database/database_helper.dart` around lines 66 - 108, The migration
block guarded by "if (oldVersion < 6)" must be executed inside a database
transaction to avoid partial failure: replace the current sequence of await
db.execute(...) calls for creating completed_levels_new, saved_games_new, the
INSERT ... SELECT migrations, DROP TABLE completed_levels/saved_games and the
ALTER TABLE ... RENAME operations with a single transactional call (e.g. await
db.transaction((txn) async { ... }) ) and run each DDL/DML against the
transaction handle (txn.execute/txn.rawInsert) so any exception triggers an
automatic rollback and prevents loss of completed_levels and saved_games data.
Comment on lines
41
to
+44
| static String _getCurrentOtherAppsUrl() { | ||
| // Check which URL is currently active | ||
| // The Makefile will modify this to switch between markets | ||
| return GeneralConsts.otherAppsRustoreLink; // This will be changed by make | ||
| return GeneralConsts.otherAppsGooglePlayLink; // This will be changed by make |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify Makefile/mk scripts still target the correct token after this change
echo "=== Searching for Makefile/mk files ==="
fd -e Makefile -e mk | head -20
echo -e "\n=== Searching in Makefiles for market-related tokens ==="
rg -n "market_helper|otherAppsGooglePlayLink|otherAppsRuStoreLink|rustore|play\.google\.com" -g 'Makefile*' -g '*.mk' --max-count=20
echo -e "\n=== Checking market_helper.dart constants ==="
rg -n "otherAppsGooglePlayLink|otherAppsRuStoreLink|RuStore" lib/core/utils/market_helper.dartRepository: khlebobul/pegma
Length of output: 2602
Update the Makefile sed pattern for Google Play builds to match the new default.
The set-google-play target (line 33) uses:
sed -i '' -E "s|return GeneralConsts\\.otherAppsRustoreLink;|return GeneralConsts.otherAppsGooglePlayLink;|"
This no longer matches the default return statement after your change. While RuStore builds work correctly (line 39), the Google Play build logic is broken—relying on the code already being at the correct default. Update line 33 to replace otherAppsGooglePlayLink when needed, or adjust the default logic to ensure both build paths work reliably.
🤖 Prompt for AI Agents
In `@lib/core/utils/market_helper.dart` around lines 41 - 44, The Makefile's
set-google-play sed pattern no longer matches because the code default was
changed to return GeneralConsts.otherAppsGooglePlayLink; update the sed in the
set-google-play target so it matches either variant and forces the return to
GeneralConsts.otherAppsGooglePlayLink (e.g. use a regex that matches
GeneralConsts.otherAppsRustoreLink|GeneralConsts.otherAppsGooglePlayLink and
replaces with GeneralConsts.otherAppsGooglePlayLink); also ensure the
set-rustore target is updated symmetrically to always replace either symbol with
GeneralConsts.otherAppsRustoreLink so _getCurrentOtherAppsUrl and the
GeneralConsts.otherAppsGooglePlayLink/otherAppsRustoreLink swaps work reliably.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
Change type
Summary by CodeRabbit
Version 1.8.0
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.