Title: Fix step control and score text display [fix]#2
Conversation
Details: 1. Fix fail of step control 2. Fix score text shows ellipsis instead real number. Type: Bugfix. Test: Local Passed. Related Issue: #1
📝 Walkthrough概览此更新修复了两个关键问题:分数步长设置不生效和分数显示不全。通过引入 SettingsProvider 的持久化机制管理全局分数步长,以及使用 FittedBox 动态缩放分数文本。同步发布了版本 0.0.2 的中文发布说明。 变更
序列图sequenceDiagram
actor User
participant Editor as ScoreStepsEditor
participant Provider as SettingsProvider
participant Prefs as SharedPreferences
participant Game as Game Screens<br/>(ScoreRow, ScorePanel)
User->>Editor: 修改分数步长并保存
Editor->>Prefs: 保存步长到 SharedPreferences
Editor->>Provider: 调用 reloadGlobalScoreSteps()
Provider->>Prefs: 读取 global_score_steps
Provider->>Provider: 更新内存中的 _globalScoreSteps
Provider->>Provider: 通知监听者
Game->>Provider: 读取 globalScoreSteps
Game->>Game: 使用新步长渲染按钮和面板
Note over User,Game: 新建游戏后立即使用修改后的步长
代码审查工作量估计🎯 3 (中等) | ⏱️ ~25 分钟 诗
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can generate a title for your PR based on the changes with custom instructions.Set the |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses two key bug fixes: rectifying the step control functionality for score adjustments and resolving an issue where long score numbers were truncated with an ellipsis. The changes centralize score step management within a new provider and enhance UI elements to properly display score text, improving overall usability and data presentation. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request successfully implements two fixes: making the score step controls configurable globally and ensuring long score numbers are displayed correctly without being truncated. The changes introduce a SettingsProvider to manage the global score steps, which are now persisted in SharedPreferences. The UI has been updated to use these global steps and to use FittedBox for better text display.
My review focuses on improving the robustness and maintainability of the new settings logic. I've pointed out a critical issue where malformed data in SharedPreferences could cause the app to crash and suggested adding try-catch blocks for JSON parsing. I've also recommended centralizing state management logic within the SettingsProvider and removing duplicated code and hardcoded values to improve the overall architecture.
| } | ||
| final stepsJson = prefs.getString('global_score_steps'); | ||
| if (stepsJson != null) { | ||
| _globalScoreSteps = (jsonDecode(stepsJson) as List<dynamic>).cast<int>(); |
There was a problem hiding this comment.
The jsonDecode call can throw a FormatException if the data in SharedPreferences is corrupted. This could crash the app on startup. It's crucial to wrap this in a try-catch block to handle potential parsing errors gracefully.
try {
_globalScoreSteps = (jsonDecode(stepsJson) as List<dynamic>).cast<int>();
} catch (e) {
// Log the error and fall back to the initial default.
print('Failed to parse global score steps: $e');
}| if (stepsJson != null) { | ||
| _globalScoreSteps = (jsonDecode(stepsJson) as List<dynamic>).cast<int>(); | ||
| } else { | ||
| _globalScoreSteps = [1, 2, 4, 8, 16, 32, 64, 128]; | ||
| } |
There was a problem hiding this comment.
Similar to _loadSettings, this jsonDecode call should be wrapped in a try-catch block to prevent crashes from malformed JSON. If parsing fails, you should fall back to the default score steps.
if (stepsJson != null) {
try {
_globalScoreSteps = (jsonDecode(stepsJson) as List<dynamic>).cast<int>();
} catch (e) {
print('Failed to parse global score steps: $e');
_globalScoreSteps = [1, 2, 4, 8, 16, 32, 64, 128];
}
} else {
_globalScoreSteps = [1, 2, 4, 8, 16, 32, 64, 128];
}| class SettingsProvider extends ChangeNotifier { | ||
| double _uiScale = 1.0; | ||
| bool _initialized = false; | ||
| List<int> _globalScoreSteps = [1, 2, 4, 8, 16, 32, 64, 128]; |
There was a problem hiding this comment.
The default score steps [1, 2, 4, 8, 16, 32, 64, 128] are hardcoded here, in reloadGlobalScoreSteps, and also in ScoreStepsEditor. To improve maintainability and avoid potential inconsistencies, consider defining this list as a static const in a shared location (e.g., SettingsProvider) and referencing it from all these places.
| Future<void> _saveSteps() async { | ||
| final prefs = await SharedPreferences.getInstance(); | ||
| await prefs.setString('global_score_steps', jsonEncode(_steps)); | ||
| if (mounted) { | ||
| Provider.of<SettingsProvider>(context, listen: false) | ||
| .reloadGlobalScoreSteps(); | ||
| } | ||
| } |
There was a problem hiding this comment.
The ScoreStepsEditor is directly manipulating SharedPreferences. It's better to centralize state management within the SettingsProvider. Instead of writing to SharedPreferences here and then telling the provider to reload, you should call the setGlobalScoreSteps method on SettingsProvider to update the steps. The provider is then responsible for persisting the change and notifying its listeners. This improves encapsulation and makes the code easier to maintain.
Future<void> _saveSteps() async {
if (mounted) {
await Provider.of<SettingsProvider>(context, listen: false)
.setGlobalScoreSteps(_steps);
}
}There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
lib/providers/settings_provider.dart (2)
27-30: 建议:添加 JSON 解析的错误处理。如果
SharedPreferences中存储的数据被损坏或格式不正确,jsonDecode或.cast<int>()可能会抛出异常,导致应用启动失败。♻️ 建议添加 try-catch 处理
final stepsJson = prefs.getString('global_score_steps'); if (stepsJson != null) { - _globalScoreSteps = (jsonDecode(stepsJson) as List<dynamic>).cast<int>(); + try { + _globalScoreSteps = (jsonDecode(stepsJson) as List<dynamic>).cast<int>(); + } catch (_) { + // 数据损坏时使用默认值 + _globalScoreSteps = [1, 2, 4, 8, 16, 32, 64, 128]; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/providers/settings_provider.dart` around lines 27 - 30, The JSON decode and cast of prefs.getString('global_score_steps') can throw and should be guarded; wrap the jsonDecode(...) and .cast<int>() logic in a try-catch inside the settings provider (where _globalScoreSteps is set from prefs.getString('global_score_steps')), catch FormatException/TypeError/Exception, log the error, fall back to a safe default for _globalScoreSteps (or clear the invalid pref), and avoid letting the exception propagate and crash app startup.
8-8: 建议:将默认步长值提取为常量。默认步长值
[1, 2, 4, 8, 16, 32, 64, 128]在多处重复出现(第 8 行、第 65 行,以及score_steps_editor.dart中),存在维护风险。♻️ 建议提取为静态常量
class SettingsProvider extends ChangeNotifier { + static const List<int> defaultScoreSteps = [1, 2, 4, 8, 16, 32, 64, 128]; + double _uiScale = 1.0; bool _initialized = false; - List<int> _globalScoreSteps = [1, 2, 4, 8, 16, 32, 64, 128]; + List<int> _globalScoreSteps = defaultScoreSteps;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/providers/settings_provider.dart` at line 8, Extract the hardcoded default steps list into a single constant and replace all usages with that constant: define a top-level or static constant (e.g., DefaultScoreSteps or SettingsProvider.defaultScoreSteps) containing [1,2,4,8,16,32,64,128] in lib/providers/settings_provider.dart, update the _globalScoreSteps initializer to reference that constant, and update the other occurrences (the code at line ~65 and the usage in score_steps_editor.dart) to import/use the same constant so the list is maintained in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/widgets/game/score_row.dart`:
- Around line 54-57: The score-steps migration is inconsistent: score_row.dart
now uses Provider.of<SettingsProvider>(context).globalScoreSteps (rendering
_ScoreStepButton with step and game) while transfer_dialog.dart still reads
game.scoreSteps and updateScoreSteps is never invoked; unify the source of
truth. Choose between per-game (game.scoreSteps + wire up updateScoreSteps and
UI to edit it) or global (SettingsProvider.globalScoreSteps) and update both
files to use the chosen API: if keeping global, replace game.scoreSteps reads in
transfer_dialog.dart with SettingsProvider.globalScoreSteps and remove/disable
updateScoreSteps; if keeping per-game, change score_row.dart to use
game.scoreSteps and add UI hooks to call GameProvider.updateScoreSteps where
_ScoreStepButton changes steps. Ensure references to SettingsProvider,
globalScoreSteps, game.scoreSteps, updateScoreSteps, and _ScoreStepButton are
updated accordingly so both score_row.dart and transfer_dialog.dart use the same
data source.
In `@lib/widgets/player_avatar.dart`:
- Around line 91-105: The FittedBox showing player.currentRoundChange inside the
Row has no width constraint so large values can prevent proper scaling; wrap
that FittedBox with a Flexible (or ConstrainedBox with a maxWidth) so it
receives a max width and the Text (player.currentRoundChange) can shrink
correctly; update the widget containing the FittedBox (reference the
FittedBox/Text using player.currentRoundChange in player_avatar.dart) to be
Flexible(child: FittedBox(...)) or apply a maxWidth constraint to ensure it
doesn't push/overflow the main score.
---
Nitpick comments:
In `@lib/providers/settings_provider.dart`:
- Around line 27-30: The JSON decode and cast of
prefs.getString('global_score_steps') can throw and should be guarded; wrap the
jsonDecode(...) and .cast<int>() logic in a try-catch inside the settings
provider (where _globalScoreSteps is set from
prefs.getString('global_score_steps')), catch
FormatException/TypeError/Exception, log the error, fall back to a safe default
for _globalScoreSteps (or clear the invalid pref), and avoid letting the
exception propagate and crash app startup.
- Line 8: Extract the hardcoded default steps list into a single constant and
replace all usages with that constant: define a top-level or static constant
(e.g., DefaultScoreSteps or SettingsProvider.defaultScoreSteps) containing
[1,2,4,8,16,32,64,128] in lib/providers/settings_provider.dart, update the
_globalScoreSteps initializer to reference that constant, and update the other
occurrences (the code at line ~65 and the usage in score_steps_editor.dart) to
import/use the same constant so the list is maintained in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 39d98c80-c771-4ae0-a838-3a80bd925bcf
📒 Files selected for processing (8)
assets/documentations/release_note/0.0.3.mdassets/documentations/release_note/archive/0.0.2.mdlib/providers/settings_provider.dartlib/screens/score_steps_editor.dartlib/widgets/game/score_row.dartlib/widgets/player_avatar.dartlib/widgets/score_panel.dartpubspec.yaml
| ...Provider.of<SettingsProvider>(context).globalScoreSteps.map((step) => _ScoreStepButton( | ||
| game: game, | ||
| step: step, | ||
| )), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 检查 game.scoreSteps 在其他地方的使用情况,以及是否有单局步长自定义的入口
# 查找 updateScoreSteps 的调用点
echo "=== updateScoreSteps 调用点 ==="
rg -n "updateScoreSteps" --type dart
# 查找 scoreSteps 的其他引用
echo -e "\n=== scoreSteps 在 GameProvider 中的使用 ==="
rg -n "scoreSteps" lib/providers/game_provider.dart
# 检查是否有 UI 入口允许用户设置单局步长
echo -e "\n=== 可能的单局步长设置 UI ==="
rg -n -C2 "scoreSteps" lib/screens/ lib/widgets/Repository: UtopiaXC/UtopiaScoreboard
Length of output: 2009
🏁 Script executed:
# Mencari pemanggilan actual ke updateScoreSteps (bukan hanya definisi)
echo "=== Pemanggilan updateScoreSteps (actual calls) ==="
rg -n "updateScoreSteps\(" --type dart
# Melihat konteks penuh dari game_provider.dart untuk updateScoreSteps
echo -e "\n=== Konteks updateScoreSteps method ==="
sed -n '495,505p' lib/providers/game_provider.dart
# Melihat score_row.dart secara lengkap untuk konteks
echo -e "\n=== Isi score_row.dart ==="
wc -l lib/widgets/game/score_row.dartRepository: UtopiaXC/UtopiaScoreboard
Length of output: 508
🏁 Script executed:
# Lihat full score_row.dart
echo "=== Full score_row.dart ==="
cat -n lib/widgets/game/score_row.dart
# Cek apakah ada UI untuk mengatur scoreSteps per game
echo -e "\n=== Cari settings atau dialog untuk scoreSteps ==="
rg -n "scoreSteps" lib/screens/ -A 2 -B 2
# Lihat full context dari transfer_dialog.dart untuk scoreSteps usage
echo -e "\n=== transfer_dialog.dart context (lines 70-80) ==="
sed -n '70,80p' lib/widgets/game/transfer_dialog.dartRepository: UtopiaXC/UtopiaScoreboard
Length of output: 5179
注意:scoreSteps 迁移不完整,存在使用源不一致的问题。
虽然 game.scoreSteps 是在 GameProvider 中定义的单局级设置,但进一步的分析表明:
- per-game 自定义功能实际不可用:
updateScoreSteps()方法已定义但从未被调用,且不存在用户界面允许自定义单局步长。 - 迁移不完整:此更改仅将
score_row.dart切换到globalScoreSteps,但transfer_dialog.dart仍然使用game.scoreSteps,导致两个相似功能的数据源不一致。
应统一这两个文件的实现方式,确保整个应用中步长配置来源保持一致。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/widgets/game/score_row.dart` around lines 54 - 57, The score-steps
migration is inconsistent: score_row.dart now uses
Provider.of<SettingsProvider>(context).globalScoreSteps (rendering
_ScoreStepButton with step and game) while transfer_dialog.dart still reads
game.scoreSteps and updateScoreSteps is never invoked; unify the source of
truth. Choose between per-game (game.scoreSteps + wire up updateScoreSteps and
UI to edit it) or global (SettingsProvider.globalScoreSteps) and update both
files to use the chosen API: if keeping global, replace game.scoreSteps reads in
transfer_dialog.dart with SettingsProvider.globalScoreSteps and remove/disable
updateScoreSteps; if keeping per-game, change score_row.dart to use
game.scoreSteps and add UI hooks to call GameProvider.updateScoreSteps where
_ScoreStepButton changes steps. Ensure references to SettingsProvider,
globalScoreSteps, game.scoreSteps, updateScoreSteps, and _ScoreStepButton are
updated accordingly so both score_row.dart and transfer_dialog.dart use the same
data source.
| FittedBox( | ||
| fit: BoxFit.scaleDown, | ||
| child: Text( | ||
| player.currentRoundChange > 0 | ||
| ? '+${player.currentRoundChange}' | ||
| : '${player.currentRoundChange}', | ||
| style: GoogleFonts.outfit( | ||
| color: player.currentRoundChange > 0 | ||
| ? Colors.greenAccent | ||
| : Colors.redAccent, | ||
| fontSize: 13, | ||
| fontWeight: FontWeight.bold, | ||
| ), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
增量积分的 FittedBox 缺少宽度约束,大数值时可能无法正确缩放。
与主积分不同,这里的 FittedBox 没有被 Flexible 包裹。在 Row 布局中,无约束的子组件会优先获取其固有尺寸,导致 FittedBox 没有最大宽度限制。当 currentRoundChange 是较大数值时(如 +999999),文本不会缩小,可能挤压主积分显示或导致溢出。
🔧 建议添加 Flexible 包裹
const SizedBox(width: 8),
- FittedBox(
- fit: BoxFit.scaleDown,
- child: Text(
+ Flexible(
+ child: FittedBox(
+ fit: BoxFit.scaleDown,
+ alignment: Alignment.centerLeft,
+ child: Text(
player.currentRoundChange > 0
? '+${player.currentRoundChange}'
: '${player.currentRoundChange}',
style: GoogleFonts.outfit(
color: player.currentRoundChange > 0
? Colors.greenAccent
: Colors.redAccent,
fontSize: 13,
fontWeight: FontWeight.bold,
),
+ ),
),
),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| FittedBox( | |
| fit: BoxFit.scaleDown, | |
| child: Text( | |
| player.currentRoundChange > 0 | |
| ? '+${player.currentRoundChange}' | |
| : '${player.currentRoundChange}', | |
| style: GoogleFonts.outfit( | |
| color: player.currentRoundChange > 0 | |
| ? Colors.greenAccent | |
| : Colors.redAccent, | |
| fontSize: 13, | |
| fontWeight: FontWeight.bold, | |
| ), | |
| ), | |
| ), | |
| Flexible( | |
| child: FittedBox( | |
| fit: BoxFit.scaleDown, | |
| alignment: Alignment.centerLeft, | |
| child: Text( | |
| player.currentRoundChange > 0 | |
| ? '+${player.currentRoundChange}' | |
| : '${player.currentRoundChange}', | |
| style: GoogleFonts.outfit( | |
| color: player.currentRoundChange > 0 | |
| ? Colors.greenAccent | |
| : Colors.redAccent, | |
| fontSize: 13, | |
| fontWeight: FontWeight.bold, | |
| ), | |
| ), | |
| ), | |
| ), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/widgets/player_avatar.dart` around lines 91 - 105, The FittedBox showing
player.currentRoundChange inside the Row has no width constraint so large values
can prevent proper scaling; wrap that FittedBox with a Flexible (or
ConstrainedBox with a maxWidth) so it receives a max width and the Text
(player.currentRoundChange) can shrink correctly; update the widget containing
the FittedBox (reference the FittedBox/Text using player.currentRoundChange in
player_avatar.dart) to be Flexible(child: FittedBox(...)) or apply a maxWidth
constraint to ensure it doesn't push/overflow the main score.
Details:
Type: Bugfix
Test: Local Passed.
Related Issue: 两个小建议 #1
Closes 两个小建议 #1
Summary by CodeRabbit
版本 0.0.3 发布说明
新增功能
修复
文档