feat: replace trail run with summit camp game - #71
Merged
Merged
Conversation
Replace the Trail Run board-game/combat loop with Summit Camp, a health-fueled base builder where steps, calories, sleep, and runs power four distinct resources. - Add camp domain entities, use cases, and persistence - Build camp grid UI with expand, build, rest, and expedition actions - Redesign daily quests to three measurable pillar missions - Update dashboard glance card with summit altitude and resources - Remove trail/combat code and tests Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.com>
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughThis PR replaces the "Trail Run" gamification mini-game with a new "Summit Camp" system. New constants, entities (CampBuildingType, PlacedBuilding, CampResources, CampTile, CampState, ExpeditionEvent), use cases, providers, and UI widgets are introduced, while trail/encounter/adventure entities and use cases are removed. Repository, persistence, and dashboard/character page integrations are updated accordingly. ChangesSummit Camp Feature
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.com>
Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.com>
YKDBontekoe
marked this pull request as ready for review
July 6, 2026 20:35
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/features/character/camp_game_panel_test.dart (1)
1-35: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftTest file doesn't test
CampGamePanel.Despite the filename, this only tests
CampResourcesBarin isolation.CampGamePanelitself — which drivescampSessionProvider,healthSummaryProvider,recentRunsProvider, tile selection, and the build sheet flow — has no widget test with aProviderScopeoverriding fakes, as required by the coding guidelines: "widget tests must useProviderScopewith overridden fakes rather than real repositories." The AI summary/commit message describes this as "a corresponding widget test" for the panel, but coverage doesn't match that scope.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/features/character/camp_game_panel_test.dart` around lines 1 - 35, The test coverage is for CampResourcesBar, not CampGamePanel, so add a widget test that targets CampGamePanel itself. Build the panel under a ProviderScope and override the relevant providers such as campSessionProvider, healthSummaryProvider, and recentRunsProvider with fakes/mocks so it does not hit real repositories. Verify the panel’s key UI and flow behavior, including tile selection and the build sheet interaction, using the CampGamePanel widget rather than the CampResourcesBar widget.Source: Coding guidelines
🧹 Nitpick comments (13)
lib/features/character/presentation/widgets/camp_grid.dart (1)
93-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueIcon-only tiles lack accessible labels.
Locked/built/empty tile states are conveyed only via bare
Iconwidgets with no semantic label, so screen reader users can't distinguish tile status or building type.♿ Suggested fix
child: Center( child: isLocked - ? Icon(Icons.lock_outline, size: 14, color: kynos.tertiaryLabel) + ? Semantics( + label: 'Locked tile', + child: Icon(Icons.lock_outline, size: 14, color: kynos.tertiaryLabel), + ) : building != null - ? Icon( - _buildingIcon(building!.type), - size: 18, - color: kynos.label, - ) - : Icon( - Icons.terrain_outlined, - size: 16, - color: kynos.secondaryLabel, - ), + ? Semantics( + label: building!.type.label, + child: Icon(_buildingIcon(building!.type), size: 18, color: kynos.label), + ) + : Semantics( + label: 'Empty tile', + child: Icon(Icons.terrain_outlined, size: 16, color: kynos.secondaryLabel), + ), ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/character/presentation/widgets/camp_grid.dart` around lines 93 - 107, The icon-only tile states in camp_grid.dart are not accessible because the locked, building, and empty cases use bare Icon widgets without any semantic description. Update the widget branch in the tile builder to wrap each state in appropriate accessibility semantics or provide labels for the Icon(s) so screen readers can distinguish locked, building type, and empty terrain tiles; use the existing state checks around isLocked, building, and _buildingIcon to attach the correct label.lib/features/character/presentation/widgets/summit_progress_card.dart (1)
15-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDirect
DateTime.now()call reduces testability.Computing
isSundayinline withDateTime.now()makes this widget harder to test deterministically (can't inject a fixed date) and ties the "summit push" day to device-local time rather than a shared clock/week-boundary source used elsewhere in the domain.♻️ Suggested fix — accept as a parameter
class SummitProgressCard extends StatelessWidget { - const SummitProgressCard({super.key, required this.camp}); + const SummitProgressCard({super.key, required this.camp, DateTime? now}) + : _now = now; final CampState camp; + final DateTime? _now; `@override` Widget build(BuildContext context) { final kynos = context.kynosTheme; - final isSunday = DateTime.now().weekday == DateTime.sunday; + final isSunday = (_now ?? DateTime.now()).weekday == DateTime.sunday;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/character/presentation/widgets/summit_progress_card.dart` around lines 15 - 18, The SummitProgressCard widget computes isSunday directly inside build with DateTime.now(), which makes the summit-day logic hard to test and tied to device-local time. Move this date decision out of build by injecting the current date or a clock-derived value into SummitProgressCard, then use that injected value to determine isSunday so tests can supply a fixed date and the shared time source can control the boundary.test/infrastructure/gamification/character_persistence_repository_test.dart (1)
14-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for legacy adventure-key cleanup.
Round-trip, empty, and corrupt-JSON cases are covered, but the new legacy-migration behavior (removing
kynos_adventure_session_v1on load) isn't tested.✅ Suggested additional test
test('removes legacy adventure session data on load', () async { SharedPreferences.setMockInitialValues({ 'kynos_adventure_session_v1': '{"some":"legacy"}', }); await repo.loadCampState(); final prefs = await SharedPreferences.getInstance(); expect(prefs.getString('kynos_adventure_session_v1'), isNull); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/infrastructure/gamification/character_persistence_repository_test.dart` around lines 14 - 49, Add a test in CharacterPersistenceRepository camp state coverage for the legacy migration path: when loadCampState() runs and SharedPreferences contains kynos_adventure_session_v1, the legacy key should be removed afterward. Extend the existing repository tests alongside the round-trip and corrupt-JSON cases, using CharacterPersistenceRepository and SharedPreferences to verify the cleanup behavior on load.lib/domain/entities/gamification/camp_building.dart (1)
42-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd value equality to
PlacedBuilding.No
==/hashCodeoverride means two structurally identical instances (e.g. after JSON round-trip) are not equal, which can break Riverpod state-change detection and any test that comparesPlacedBuildingvalues directly. Since domain entities cannot depend onfreezed(per the zero-dependency rule forlib/domain/), implement equality manually.♻️ Proposed manual equality
PlacedBuilding copyWith({ CampBuildingType? type, int? level, int? row, int? col, }) => PlacedBuilding( type: type ?? this.type, level: level ?? this.level, row: row ?? this.row, col: col ?? this.col, ); + + `@override` + bool operator ==(Object other) => + identical(this, other) || + other is PlacedBuilding && + type == other.type && + level == other.level && + row == other.row && + col == other.col; + + `@override` + int get hashCode => Object.hash(type, level, row, col);As per coding guidelines, "All data models must use
@freezedfor immutability,copyWith, equality, andhashCode," which requires equality support even where@freezeditself can't be used.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/domain/entities/gamification/camp_building.dart` around lines 42 - 87, `PlacedBuilding` currently has no value equality, so identical instances are treated as different objects. Update the `PlacedBuilding` class to manually override `==` and `hashCode` using its fields (`type`, `level`, `row`, `col`), keeping behavior consistent with `copyWith`, `toJson`, and `fromJson`. Make sure equality is structural so JSON round-trips and direct comparisons work correctly in state management and tests.Source: Coding guidelines
lib/domain/entities/gamification/camp_resources.dart (1)
2-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame missing value-equality gap as
PlacedBuilding.
CampResourcesalso lacks==/hashCode, which affects equality-based comparisons of camp state downstream (e.g. Riverpod rebuild checks, test assertions).♻️ Proposed manual equality
CampResources withSpent({ int momentum = 0, int fuel = 0, int focus = 0, int spirit = 0, }) => copyWith( spentMomentum: spentMomentum + momentum, spentFuel: spentFuel + fuel, spentFocus: spentFocus + focus, spentSpirit: spentSpirit + spirit, ); + + `@override` + bool operator ==(Object other) => + identical(this, other) || + other is CampResources && + totalMomentum == other.totalMomentum && + totalFuel == other.totalFuel && + totalFocus == other.totalFocus && + totalSpirit == other.totalSpirit && + spentMomentum == other.spentMomentum && + spentFuel == other.spentFuel && + spentFocus == other.spentFocus && + spentSpirit == other.spentSpirit && + restMultiplier == other.restMultiplier; + + `@override` + int get hashCode => Object.hash( + totalMomentum, + totalFuel, + totalFocus, + totalSpirit, + spentMomentum, + spentFuel, + spentFocus, + spentSpirit, + restMultiplier, + );As per coding guidelines, "All data models must use
@freezedfor immutability,copyWith, equality, andhashCode."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/domain/entities/gamification/camp_resources.dart` around lines 2 - 74, CampResources is missing value-based equality, so instances with the same resource values still compare by identity and can break state comparisons and tests. Update the CampResources model to use the project’s standard immutable data-model approach with `@freezed`, or otherwise add proper == and hashCode support consistent with copyWith/withSpent so equality checks on totalMomentum, totalFuel, totalFocus, totalSpirit, spentMomentum, spentFuel, spentFocus, spentSpirit, and restMultiplier work correctly.Source: Coding guidelines
lib/domain/entities/gamification/camp_state.dart (1)
98-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate hardcoded
weeklyGoalvalue.
100is repeated inCampState.initial()andforCurrentWeek(). Extract to a shared constant (ideally fromgamification_constants.dart, which this PR stack introduces) so both paths stay in sync if the weekly target is tuned later.♻️ Proposed fix
+ static const int defaultWeeklyGoal = 100; + final int gridSize;return CampState( gridSize: size, tiles: tiles, buildings: const [], weeklyAltitude: 0, - weeklyGoal: 100, + weeklyGoal: defaultWeeklyGoal, weekStart: weekStart,return copyWith( weekStart: start, weeklyAltitude: 0, - weeklyGoal: 100, + weeklyGoal: defaultWeeklyGoal, );Also applies to: 119-131
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/domain/entities/gamification/camp_state.dart` around lines 98 - 111, The `CampState.initial()` and `CampState.forCurrentWeek()` builders both hardcode the same `weeklyGoal` value, so extract that value into a shared constant and use it in both places. Prefer referencing the new shared constant from `gamification_constants.dart` so `CampState` stays aligned if the weekly target changes later.lib/domain/usecases/gamification/expand_camp_tile_usecase.dart (1)
62-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMagic number for altitude gain.
weeklyAltitude + 2hardcodes the tile-expand altitude reward inline, while the Momentum cost usesGamificationConstants.momentumPerTileExpand. Consider adding a matching constant (e.g.altitudeGainPerTileExpand) for consistency and easier balance tuning.♻️ Proposed fix
return ExpandCampTileResult( camp: camp.copyWith( tiles: updatedTiles, spentMomentum: camp.spentMomentum + GamificationConstants.momentumPerTileExpand, - weeklyAltitude: camp.weeklyAltitude + 2, + weeklyAltitude: + camp.weeklyAltitude + GamificationConstants.altitudeGainPerTileExpand, ), );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/domain/usecases/gamification/expand_camp_tile_usecase.dart` around lines 62 - 69, The tile-expand altitude reward in ExpandCampTileUsecase is hardcoded with a magic number, unlike the momentum cost which uses GamificationConstants.momentumPerTileExpand. Replace the inline weeklyAltitude increment in ExpandCampTileResult/camp.copyWith with a dedicated GamificationConstants value such as altitudeGainPerTileExpand, and use that constant consistently in the expansion logic so the reward is easy to tune.lib/domain/usecases/gamification/build_camp_structure_usecase.dart (1)
63-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMagic number for max building level.
existing.level < 3hardcodes the level cap; consider aGamificationConstants.maxBuildingLevelto keep this in sync withCampBuildingType.upgradeFuelCost/summitContribution, which presumably also assume a 3-level cap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/domain/usecases/gamification/build_camp_structure_usecase.dart` around lines 63 - 78, The max building level is hardcoded in the build flow, so update the `BuildCampStructureUseCase` upgrade branch to use a shared constant instead of `existing.level < 3`. Introduce or reuse `GamificationConstants.maxBuildingLevel` and make sure the `failure` message path still triggers when the cap is reached; also verify any related `CampBuildingType.upgradeFuelCost` and `summitContribution` logic stays aligned with the same cap.lib/domain/usecases/gamification/rest_camp_usecase.dart (1)
52-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMagic number for altitude gain.
weeklyAltitude + 3mirrors the same inline-constant concern raised forExpandCampTileUseCase; consider centralizing all altitude-gain values inGamificationConstants.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/domain/usecases/gamification/rest_camp_usecase.dart` around lines 52 - 60, The rest camp update uses an inline altitude increment, which should be centralized instead of hardcoded. Update RestCampUsecase’s camp.copyWith call to use a dedicated altitude-gain constant from GamificationConstants rather than adding 3 directly, and align this with the same centralized pattern used for other altitude adjustments like in ExpandCampTileUseCase. Ensure the new constant is reused wherever weeklyAltitude gains are applied so the value stays consistent across the gamification flow.lib/shared/providers/camp_providers.dart (2)
213-231: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueSilent failure swallowing in
_awardXp.When
loadCharacter/saveCharacterfail, the method just returns — the expedition still completes and persists, but the XP/stat reward is silently lost with no log trace.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/shared/providers/camp_providers.dart` around lines 213 - 231, The _awardXp method is swallowing load/save failures, so XP/stat rewards can disappear without any trace. Update _awardXp in camp_providers.dart to log or otherwise surface errors from characterRepositoryProvider.loadCharacter() and repo.saveCharacter() before returning, using the existing _awardXp, loadCharacter, and saveCharacter flow as the hook points. Keep the early returns, but add clear failure reporting so reward loss is observable when the expedition completes.
167-167: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMagic number
3for spirit cost duplicated.The expedition spirit cost is hard-coded in two places. Consider extracting a named constant (e.g.,
GamificationConstants.expeditionSpiritCost) to avoid drift if the cost is tuned later.Also applies to: 199-199
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/shared/providers/camp_providers.dart` at line 167, The expedition spirit cost is duplicated as a hard-coded magic number in camp provider logic, which can drift if the value changes. Extract the shared cost into a named constant such as GamificationConstants.expeditionSpiritCost, then update the checks in the relevant camp provider methods (including the current canSpendSpirit guard and the other matching use) to reference that constant instead of literal 3.lib/domain/usecases/gamification/generate_camp_quests_usecase.dart (2)
10-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnused
characterparameter.
characteris a required parameter ofcall()but is never referenced anywhere in the method body or in_momentumQuest/_fuelQuest/_focusQuest. Either wire it into quest personalization (e.g., class-specific stat rewards/titles) or drop the parameter to avoid a misleading API surface.Also applies to: 52-104
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/domain/usecases/gamification/generate_camp_quests_usecase.dart` around lines 10 - 14, The required character parameter in GenerateCampQuestsUsecase.call is currently unused, so either thread RunnerCharacter through the quest-building flow or remove it from the API. Update call, _momentumQuest, _fuelQuest, and _focusQuest consistently so the character is actually referenced for personalization, or simplify the signature and all call sites if it is not needed.
60-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winOnly one momentum-quest branch is unit tested.
_momentumQuestalternates between the steps objective and the exercise-minutes objective based onnow.day.isEven. The provided test (camp_game_usecase_test.dart) only exercisesreferenceTime: DateTime(2026, 7, 6)(day 6, even → steps branch); the exercise-minutes branch (odd day) has no coverage in the supplied tests.As per coding guidelines, "Every use-case in
domain/usecases/must have a corresponding unit test."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/domain/usecases/gamification/generate_camp_quests_usecase.dart` around lines 60 - 104, The _momentumQuest branch in GenerateCampQuestsUsecase is only covered for the even-day steps path, so add a unit test that uses an odd `referenceTime` to exercise the exercise-minutes path. Update the existing `camp_game_usecase_test.dart` coverage to call the same use case and verify the returned `Quest` from `_momentumQuest` has `QuestObjectiveKind.exerciseMinutes`, the expected title/objective text, and the correct target chosen from `GamificationConstants.questExerciseMinNormal` or `questExerciseMinEasy` based on readiness.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/domain/entities/gamification/expedition_event.dart`:
- Around line 29-46: The stat-delta parsing in ExpeditionEvent.fromJson is too
permissive in the wrong place and unsafe in another: entry.value is cast
directly to num, and unknown stat names are currently mapped to
CharacterStatId.endurance. Update the parsing so malformed or missing values are
handled safely like the other fields, and do not substitute unrecognized keys
with a real stat; instead skip invalid entries or otherwise avoid merging them
into an existing CharacterStatId. Focus the fix inside ExpeditionEvent.fromJson
where rawDeltas, deltas, and CharacterStatId.values.firstWhere are used.
In `@lib/domain/usecases/gamification/advance_weekly_summit_usecase.dart`:
- Around line 4-31: Add a dedicated unit test suite for
AdvanceWeeklySummitUseCase, since this use case currently has no corresponding
coverage. Create tests for the call method that verify the Sunday bonus path,
bonusAltitude accumulation, and the weeklyGoal resync behavior when
CampState.weeklyGoal differs from GamificationConstants.weeklySummitGoal. Use
the AdvanceWeeklySummitUseCase class and its call entrypoint to locate the
logic, and assert the updated CampState values returned by
forCurrentWeek/copyWith.
In `@lib/domain/usecases/gamification/build_camp_structure_usecase.dart`:
- Around line 5-15: BuildCampStructureResult is using a raw String for failure,
which breaks the domain error contract. Update the BuildCampStructureResult
model in build_camp_structure_usecase.dart to store a Failure from
core/errors/failures.dart instead of String, and adjust isSuccess to reflect
that contract. Make sure any code that creates or consumes
BuildCampStructureResult (including the related BuildCampStructureUsecase flow)
now passes and handles the Failure sealed class consistently, matching the
pattern used in ExpandCampTileResult.
In `@lib/domain/usecases/gamification/expand_camp_tile_usecase.dart`:
- Around line 5-15: The ExpandCampTileResult type is using a raw String for
failure instead of the domain Failure contract. Update ExpandCampTileResult to
carry a Failure? from core/errors/failures.dart, and adjust isSuccess to reflect
the presence of a Failure rather than a string. Make sure the related
ExpandCampTile use case and any callers that inspect failure are updated to work
with the Failure sealed class instead of doing string-based handling.
In `@lib/domain/usecases/gamification/resolve_expedition_usecase.dart`:
- Around line 45-54: The stat delta updates in resolve_expedition_usecase.dart
are overwriting when the same CharacterStatId is hit by multiple branches.
Update the logic around statDeltas in the expedition use case so each bonus
accumulates instead of assigning directly, especially for the endurance,
willpower, and character.stats.weakest paths. Preserve the intended totals by
merging increments for the same key rather than replacing prior values.
In `@lib/domain/usecases/gamification/rest_camp_usecase.dart`:
- Around line 4-14: The RestCampResult model is carrying failures as a raw
String instead of using the domain Failure hierarchy. Update RestCampResult to
depend on the Failure sealed class from core/errors/failures.dart, and change
its failure field and any related success checks in RestCampResult to work with
Failure rather than String so this use case follows the same error pattern as
the other camp use cases.
In `@lib/features/character/presentation/widgets/camp_game_panel.dart`:
- Around line 126-144: The async callbacks in `_onTileTap` use the outer
`context` after an async gap, which can fail if the widget is unmounted. Add a
`context.mounted` guard before calling `Navigator.of(context).pop()` and before
any later use of `context` in the `onExpand` and `onBuild` handlers around
`CampBuildSheet.show` and the `campSessionProvider` notifier calls. Keep the
navigation and build/expand flow the same, but only proceed when the widget is
still mounted.
In `@lib/shared/providers/daily_quests_provider.dart`:
- Around line 45-55: The persistence result from repo.saveQuests in
DailyQuestsProvider is being ignored, so save failures are silently dropped.
Update the quest generation flow in the provider method that reads
generateCampQuestsUseCaseProvider to check the return value of
repo.saveQuests(quests) and surface or throw on failure, matching the previous
behavior instead of always returning quests as if they were saved.
---
Outside diff comments:
In `@test/features/character/camp_game_panel_test.dart`:
- Around line 1-35: The test coverage is for CampResourcesBar, not
CampGamePanel, so add a widget test that targets CampGamePanel itself. Build the
panel under a ProviderScope and override the relevant providers such as
campSessionProvider, healthSummaryProvider, and recentRunsProvider with
fakes/mocks so it does not hit real repositories. Verify the panel’s key UI and
flow behavior, including tile selection and the build sheet interaction, using
the CampGamePanel widget rather than the CampResourcesBar widget.
---
Nitpick comments:
In `@lib/domain/entities/gamification/camp_building.dart`:
- Around line 42-87: `PlacedBuilding` currently has no value equality, so
identical instances are treated as different objects. Update the
`PlacedBuilding` class to manually override `==` and `hashCode` using its fields
(`type`, `level`, `row`, `col`), keeping behavior consistent with `copyWith`,
`toJson`, and `fromJson`. Make sure equality is structural so JSON round-trips
and direct comparisons work correctly in state management and tests.
In `@lib/domain/entities/gamification/camp_resources.dart`:
- Around line 2-74: CampResources is missing value-based equality, so instances
with the same resource values still compare by identity and can break state
comparisons and tests. Update the CampResources model to use the project’s
standard immutable data-model approach with `@freezed`, or otherwise add proper ==
and hashCode support consistent with copyWith/withSpent so equality checks on
totalMomentum, totalFuel, totalFocus, totalSpirit, spentMomentum, spentFuel,
spentFocus, spentSpirit, and restMultiplier work correctly.
In `@lib/domain/entities/gamification/camp_state.dart`:
- Around line 98-111: The `CampState.initial()` and `CampState.forCurrentWeek()`
builders both hardcode the same `weeklyGoal` value, so extract that value into a
shared constant and use it in both places. Prefer referencing the new shared
constant from `gamification_constants.dart` so `CampState` stays aligned if the
weekly target changes later.
In `@lib/domain/usecases/gamification/build_camp_structure_usecase.dart`:
- Around line 63-78: The max building level is hardcoded in the build flow, so
update the `BuildCampStructureUseCase` upgrade branch to use a shared constant
instead of `existing.level < 3`. Introduce or reuse
`GamificationConstants.maxBuildingLevel` and make sure the `failure` message
path still triggers when the cap is reached; also verify any related
`CampBuildingType.upgradeFuelCost` and `summitContribution` logic stays aligned
with the same cap.
In `@lib/domain/usecases/gamification/expand_camp_tile_usecase.dart`:
- Around line 62-69: The tile-expand altitude reward in ExpandCampTileUsecase is
hardcoded with a magic number, unlike the momentum cost which uses
GamificationConstants.momentumPerTileExpand. Replace the inline weeklyAltitude
increment in ExpandCampTileResult/camp.copyWith with a dedicated
GamificationConstants value such as altitudeGainPerTileExpand, and use that
constant consistently in the expansion logic so the reward is easy to tune.
In `@lib/domain/usecases/gamification/generate_camp_quests_usecase.dart`:
- Around line 10-14: The required character parameter in
GenerateCampQuestsUsecase.call is currently unused, so either thread
RunnerCharacter through the quest-building flow or remove it from the API.
Update call, _momentumQuest, _fuelQuest, and _focusQuest consistently so the
character is actually referenced for personalization, or simplify the signature
and all call sites if it is not needed.
- Around line 60-104: The _momentumQuest branch in GenerateCampQuestsUsecase is
only covered for the even-day steps path, so add a unit test that uses an odd
`referenceTime` to exercise the exercise-minutes path. Update the existing
`camp_game_usecase_test.dart` coverage to call the same use case and verify the
returned `Quest` from `_momentumQuest` has `QuestObjectiveKind.exerciseMinutes`,
the expected title/objective text, and the correct target chosen from
`GamificationConstants.questExerciseMinNormal` or `questExerciseMinEasy` based
on readiness.
In `@lib/domain/usecases/gamification/rest_camp_usecase.dart`:
- Around line 52-60: The rest camp update uses an inline altitude increment,
which should be centralized instead of hardcoded. Update RestCampUsecase’s
camp.copyWith call to use a dedicated altitude-gain constant from
GamificationConstants rather than adding 3 directly, and align this with the
same centralized pattern used for other altitude adjustments like in
ExpandCampTileUseCase. Ensure the new constant is reused wherever weeklyAltitude
gains are applied so the value stays consistent across the gamification flow.
In `@lib/features/character/presentation/widgets/camp_grid.dart`:
- Around line 93-107: The icon-only tile states in camp_grid.dart are not
accessible because the locked, building, and empty cases use bare Icon widgets
without any semantic description. Update the widget branch in the tile builder
to wrap each state in appropriate accessibility semantics or provide labels for
the Icon(s) so screen readers can distinguish locked, building type, and empty
terrain tiles; use the existing state checks around isLocked, building, and
_buildingIcon to attach the correct label.
In `@lib/features/character/presentation/widgets/summit_progress_card.dart`:
- Around line 15-18: The SummitProgressCard widget computes isSunday directly
inside build with DateTime.now(), which makes the summit-day logic hard to test
and tied to device-local time. Move this date decision out of build by injecting
the current date or a clock-derived value into SummitProgressCard, then use that
injected value to determine isSunday so tests can supply a fixed date and the
shared time source can control the boundary.
In `@lib/shared/providers/camp_providers.dart`:
- Around line 213-231: The _awardXp method is swallowing load/save failures, so
XP/stat rewards can disappear without any trace. Update _awardXp in
camp_providers.dart to log or otherwise surface errors from
characterRepositoryProvider.loadCharacter() and repo.saveCharacter() before
returning, using the existing _awardXp, loadCharacter, and saveCharacter flow as
the hook points. Keep the early returns, but add clear failure reporting so
reward loss is observable when the expedition completes.
- Line 167: The expedition spirit cost is duplicated as a hard-coded magic
number in camp provider logic, which can drift if the value changes. Extract the
shared cost into a named constant such as
GamificationConstants.expeditionSpiritCost, then update the checks in the
relevant camp provider methods (including the current canSpendSpirit guard and
the other matching use) to reference that constant instead of literal 3.
In `@test/infrastructure/gamification/character_persistence_repository_test.dart`:
- Around line 14-49: Add a test in CharacterPersistenceRepository camp state
coverage for the legacy migration path: when loadCampState() runs and
SharedPreferences contains kynos_adventure_session_v1, the legacy key should be
removed afterward. Extend the existing repository tests alongside the round-trip
and corrupt-JSON cases, using CharacterPersistenceRepository and
SharedPreferences to verify the cleanup behavior on load.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 78422db0-28c2-4f22-9883-b95c42e649bb
📒 Files selected for processing (59)
CODEMAP.mdlib/core/constants/gamification_constants.dartlib/domain/entities/gamification/activity_resources.dartlib/domain/entities/gamification/adventure_session.dartlib/domain/entities/gamification/camp_building.dartlib/domain/entities/gamification/camp_resources.dartlib/domain/entities/gamification/camp_state.dartlib/domain/entities/gamification/camp_tile.dartlib/domain/entities/gamification/encounter_state.dartlib/domain/entities/gamification/expedition_event.dartlib/domain/entities/gamification/quest.dartlib/domain/entities/gamification/trail_node.dartlib/domain/repositories/character_repository.dartlib/domain/usecases/gamification/advance_weekly_summit_usecase.dartlib/domain/usecases/gamification/build_camp_structure_usecase.dartlib/domain/usecases/gamification/compute_activity_resources_usecase.dartlib/domain/usecases/gamification/compute_camp_resources_usecase.dartlib/domain/usecases/gamification/evaluate_quest_progress_usecase.dartlib/domain/usecases/gamification/expand_camp_tile_usecase.dartlib/domain/usecases/gamification/generate_camp_quests_usecase.dartlib/domain/usecases/gamification/generate_daily_quests_usecase.dartlib/domain/usecases/gamification/generate_daily_trail_usecase.dartlib/domain/usecases/gamification/resolve_encounter_turn_usecase.dartlib/domain/usecases/gamification/resolve_expedition_usecase.dartlib/domain/usecases/gamification/rest_camp_usecase.dartlib/features/character/presentation/pages/character_page.dartlib/features/character/presentation/widgets/activity_resources_bar.dartlib/features/character/presentation/widgets/camp_build_sheet.dartlib/features/character/presentation/widgets/camp_game_panel.dartlib/features/character/presentation/widgets/camp_grid.dartlib/features/character/presentation/widgets/camp_resources_bar.dartlib/features/character/presentation/widgets/encounter_panel.dartlib/features/character/presentation/widgets/expedition_card.dartlib/features/character/presentation/widgets/summit_progress_card.dartlib/features/character/presentation/widgets/trail_map.dartlib/features/character/presentation/widgets/trail_run_game_panel.dartlib/features/character/providers/adventure_provider.dartlib/features/character/providers/adventure_provider.g.dartlib/features/dashboard/presentation/pages/dashboard_page.dartlib/features/dashboard/presentation/widgets/character_glance_card.dartlib/features/dashboard/presentation/widgets/daily_quest_teaser.dartlib/infrastructure/gamification/character_persistence_repository.dartlib/shared/providers/camp_providers.dartlib/shared/providers/camp_providers.g.dartlib/shared/providers/daily_quests_provider.dartlib/shared/providers/gamification_providers.darttest/domain/entities/gamification/gamification_entity_json_test.darttest/domain/usecases/gamification/camp_actions_usecase_test.darttest/domain/usecases/gamification/camp_game_usecase_test.darttest/domain/usecases/gamification/compute_activity_resources_usecase_test.darttest/domain/usecases/gamification/compute_camp_resources_usecase_test.darttest/domain/usecases/gamification/evaluate_quest_progress_usecase_test.darttest/domain/usecases/gamification/generate_daily_quests_usecase_test.darttest/domain/usecases/gamification/generate_daily_trail_usecase_test.darttest/domain/usecases/gamification/resolve_encounter_turn_usecase_test.darttest/features/character/camp_game_panel_test.darttest/features/character/trail_map_test.darttest/features/character/trail_run_game_panel_test.darttest/infrastructure/gamification/character_persistence_repository_test.dart
💤 Files with no reviewable changes (20)
- test/domain/usecases/gamification/generate_daily_trail_usecase_test.dart
- lib/features/character/presentation/widgets/trail_map.dart
- test/domain/usecases/gamification/compute_activity_resources_usecase_test.dart
- test/domain/usecases/gamification/generate_daily_quests_usecase_test.dart
- lib/domain/entities/gamification/adventure_session.dart
- lib/domain/usecases/gamification/generate_daily_trail_usecase.dart
- lib/domain/usecases/gamification/compute_activity_resources_usecase.dart
- lib/domain/usecases/gamification/generate_daily_quests_usecase.dart
- lib/features/character/presentation/widgets/trail_run_game_panel.dart
- lib/domain/entities/gamification/encounter_state.dart
- test/features/character/trail_map_test.dart
- lib/features/character/presentation/widgets/activity_resources_bar.dart
- lib/features/character/presentation/widgets/encounter_panel.dart
- test/features/character/trail_run_game_panel_test.dart
- test/domain/usecases/gamification/resolve_encounter_turn_usecase_test.dart
- lib/domain/entities/gamification/trail_node.dart
- lib/domain/entities/gamification/activity_resources.dart
- lib/features/character/providers/adventure_provider.g.dart
- lib/domain/usecases/gamification/resolve_encounter_turn_usecase.dart
- lib/features/character/providers/adventure_provider.dart
Comment on lines
+29
to
+46
| factory ExpeditionEvent.fromJson(Map<String, dynamic> json) { | ||
| final rawDeltas = json['stat_deltas'] as Map<String, dynamic>? ?? {}; | ||
| final deltas = <CharacterStatId, int>{}; | ||
| for (final entry in rawDeltas.entries) { | ||
| final id = CharacterStatId.values.firstWhere( | ||
| (s) => s.name == entry.key, | ||
| orElse: () => CharacterStatId.endurance, | ||
| ); | ||
| deltas[id] = (entry.value as num).toInt(); | ||
| } | ||
| return ExpeditionEvent( | ||
| title: json['title'] as String? ?? 'Expedition', | ||
| narrative: json['narrative'] as String? ?? '', | ||
| xpReward: (json['xp_reward'] as num?)?.toInt() ?? 0, | ||
| statDeltas: deltas, | ||
| summitBonus: (json['summit_bonus'] as num?)?.toInt() ?? 0, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Unsafe cast and silent stat misattribution in fromJson.
Two issues in the stat-delta parsing:
- Line 37 casts
entry.value as numwithout a null-safe?, unlike every other field here — a malformed/legacy persisted value will throw instead of degrading gracefully. - Lines 33-36 fall back an unrecognized stat name to
endurance, which can silently merge/overwrite an unrelated stat's delta rather than just dropping the bad entry.
🛡️ Proposed fix
final rawDeltas = json['stat_deltas'] as Map<String, dynamic>? ?? {};
final deltas = <CharacterStatId, int>{};
for (final entry in rawDeltas.entries) {
- final id = CharacterStatId.values.firstWhere(
- (s) => s.name == entry.key,
- orElse: () => CharacterStatId.endurance,
- );
- deltas[id] = (entry.value as num).toInt();
+ final id = CharacterStatId.values
+ .where((s) => s.name == entry.key)
+ .firstOrNull;
+ if (id == null) continue;
+ deltas[id] = (entry.value as num?)?.toInt() ?? 0;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/domain/entities/gamification/expedition_event.dart` around lines 29 - 46,
The stat-delta parsing in ExpeditionEvent.fromJson is too permissive in the
wrong place and unsafe in another: entry.value is cast directly to num, and
unknown stat names are currently mapped to CharacterStatId.endurance. Update the
parsing so malformed or missing values are handled safely like the other fields,
and do not substitute unrecognized keys with a real stat; instead skip invalid
entries or otherwise avoid merging them into an existing CharacterStatId. Focus
the fix inside ExpeditionEvent.fromJson where rawDeltas, deltas, and
CharacterStatId.values.firstWhere are used.
Comment on lines
+4
to
+31
| class AdvanceWeeklySummitUseCase { | ||
| const AdvanceWeeklySummitUseCase(); | ||
|
|
||
| CampState call({ | ||
| required CampState camp, | ||
| required DateTime reference, | ||
| int bonusAltitude = 0, | ||
| }) { | ||
| var updated = camp.forCurrentWeek(reference); | ||
| final isSunday = reference.weekday == DateTime.sunday; | ||
| final sundayBonus = | ||
| isSunday ? GamificationConstants.sundaySummitBonus : 0; | ||
|
|
||
| if (bonusAltitude > 0 || sundayBonus > 0) { | ||
| updated = updated.copyWith( | ||
| weeklyAltitude: updated.weeklyAltitude + bonusAltitude + sundayBonus, | ||
| ); | ||
| } | ||
|
|
||
| if (updated.weeklyGoal != GamificationConstants.weeklySummitGoal) { | ||
| updated = updated.copyWith( | ||
| weeklyGoal: GamificationConstants.weeklySummitGoal, | ||
| ); | ||
| } | ||
|
|
||
| return updated; | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Missing dedicated unit test for AdvanceWeeklySummitUseCase.
The provided test files (camp_actions_usecase_test.dart, compute_camp_resources_usecase_test.dart) don't cover this use case. As per coding guidelines, "Every use-case in domain/usecases/ must have a corresponding unit test."
Want me to draft a test covering the Sunday bonus, bonusAltitude accumulation, and weeklyGoal resync branches?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/domain/usecases/gamification/advance_weekly_summit_usecase.dart` around
lines 4 - 31, Add a dedicated unit test suite for AdvanceWeeklySummitUseCase,
since this use case currently has no corresponding coverage. Create tests for
the call method that verify the Sunday bonus path, bonusAltitude accumulation,
and the weeklyGoal resync behavior when CampState.weeklyGoal differs from
GamificationConstants.weeklySummitGoal. Use the AdvanceWeeklySummitUseCase class
and its call entrypoint to locate the logic, and assert the updated CampState
values returned by forCurrentWeek/copyWith.
Source: Coding guidelines
Comment on lines
+5
to
+15
| class BuildCampStructureResult { | ||
| const BuildCampStructureResult({ | ||
| required this.camp, | ||
| this.failure, | ||
| }); | ||
|
|
||
| final CampState camp; | ||
| final String? failure; | ||
|
|
||
| bool get isSuccess => failure == null; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Failure represented as raw String instead of the Failure sealed class.
Same concern as in ExpandCampTileResult: this violates the domain error contract.
As per coding guidelines, "Domain errors must extend the Failure sealed class in core/errors/failures.dart."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/domain/usecases/gamification/build_camp_structure_usecase.dart` around
lines 5 - 15, BuildCampStructureResult is using a raw String for failure, which
breaks the domain error contract. Update the BuildCampStructureResult model in
build_camp_structure_usecase.dart to store a Failure from
core/errors/failures.dart instead of String, and adjust isSuccess to reflect
that contract. Make sure any code that creates or consumes
BuildCampStructureResult (including the related BuildCampStructureUsecase flow)
now passes and handles the Failure sealed class consistently, matching the
pattern used in ExpandCampTileResult.
Source: Coding guidelines
Comment on lines
+5
to
+15
| class ExpandCampTileResult { | ||
| const ExpandCampTileResult({ | ||
| required this.camp, | ||
| this.failure, | ||
| }); | ||
|
|
||
| final CampState camp; | ||
| final String? failure; | ||
|
|
||
| bool get isSuccess => failure == null; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Failure represented as raw String instead of the Failure sealed class.
Coding guidelines require domain errors to extend Failure. This result type instead exposes a plain String? failure, which diverges from the established error-handling contract and pushes ad-hoc string matching/display into the UI layer.
As per coding guidelines, "Domain errors must extend the Failure sealed class in core/errors/failures.dart."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/domain/usecases/gamification/expand_camp_tile_usecase.dart` around lines
5 - 15, The ExpandCampTileResult type is using a raw String for failure instead
of the domain Failure contract. Update ExpandCampTileResult to carry a Failure?
from core/errors/failures.dart, and adjust isSuccess to reflect the presence of
a Failure rather than a string. Make sure the related ExpandCampTile use case
and any callers that inspect failure are updated to work with the Failure sealed
class instead of doing string-based handling.
Source: Coding guidelines
Comment on lines
+45
to
+54
| final statDeltas = <CharacterStatId, int>{}; | ||
| if (distanceKm >= 5) { | ||
| statDeltas[CharacterStatId.endurance] = 2; | ||
| } | ||
| if (durationMin >= 30) { | ||
| statDeltas[CharacterStatId.willpower] = 1; | ||
| } | ||
| if (distanceKm >= 3) { | ||
| statDeltas[character.stats.weakest] = 1; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Stat deltas overwrite instead of accumulate.
Since distanceKm >= 5 also satisfies distanceKm >= 3, both the endurance branch and the "weakest stat" branch can fire for the same run. If character.stats.weakest happens to be endurance (or willpower, similarly overlapping with the duration branch), the later assignment on line 53 silently overwrites the earlier one, reducing the intended reward (e.g., endurance +2 becomes +1).
🐛 Proposed fix to accumulate stat deltas
final statDeltas = <CharacterStatId, int>{};
+ void addStat(CharacterStatId id, int amount) {
+ statDeltas.update(id, (v) => v + amount, ifAbsent: () => amount);
+ }
if (distanceKm >= 5) {
- statDeltas[CharacterStatId.endurance] = 2;
+ addStat(CharacterStatId.endurance, 2);
}
if (durationMin >= 30) {
- statDeltas[CharacterStatId.willpower] = 1;
+ addStat(CharacterStatId.willpower, 1);
}
if (distanceKm >= 3) {
- statDeltas[character.stats.weakest] = 1;
+ addStat(character.stats.weakest, 1);
}📝 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.
Suggested change
| final statDeltas = <CharacterStatId, int>{}; | |
| if (distanceKm >= 5) { | |
| statDeltas[CharacterStatId.endurance] = 2; | |
| } | |
| if (durationMin >= 30) { | |
| statDeltas[CharacterStatId.willpower] = 1; | |
| } | |
| if (distanceKm >= 3) { | |
| statDeltas[character.stats.weakest] = 1; | |
| } | |
| final statDeltas = <CharacterStatId, int>{}; | |
| void addStat(CharacterStatId id, int amount) { | |
| statDeltas.update(id, (v) => v + amount, ifAbsent: () => amount); | |
| } | |
| if (distanceKm >= 5) { | |
| addStat(CharacterStatId.endurance, 2); | |
| } | |
| if (durationMin >= 30) { | |
| addStat(CharacterStatId.willpower, 1); | |
| } | |
| if (distanceKm >= 3) { | |
| addStat(character.stats.weakest, 1); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/domain/usecases/gamification/resolve_expedition_usecase.dart` around
lines 45 - 54, The stat delta updates in resolve_expedition_usecase.dart are
overwriting when the same CharacterStatId is hit by multiple branches. Update
the logic around statDeltas in the expedition use case so each bonus accumulates
instead of assigning directly, especially for the endurance, willpower, and
character.stats.weakest paths. Preserve the intended totals by merging
increments for the same key rather than replacing prior values.
Comment on lines
+4
to
+14
| class RestCampResult { | ||
| const RestCampResult({ | ||
| required this.camp, | ||
| this.failure, | ||
| }); | ||
|
|
||
| final CampState camp; | ||
| final String? failure; | ||
|
|
||
| bool get isSuccess => failure == null; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Failure represented as raw String instead of the Failure sealed class.
Same concern as the other camp use cases.
As per coding guidelines, "Domain errors must extend the Failure sealed class in core/errors/failures.dart."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/domain/usecases/gamification/rest_camp_usecase.dart` around lines 4 - 14,
The RestCampResult model is carrying failures as a raw String instead of using
the domain Failure hierarchy. Update RestCampResult to depend on the Failure
sealed class from core/errors/failures.dart, and change its failure field and
any related success checks in RestCampResult to work with Failure rather than
String so this use case follows the same error pattern as the other camp use
cases.
Source: Coding guidelines
Comment on lines
+126
to
+144
| await CampBuildSheet.show( | ||
| context, | ||
| camp: viewState.camp, | ||
| row: row, | ||
| col: col, | ||
| availableFuel: viewState.resources.availableFuel, | ||
| availableMomentum: viewState.resources.availableMomentum, | ||
| onExpand: () async { | ||
| Navigator.of(context).pop(); | ||
| await ref.read(campSessionProvider.notifier).expandTile(row, col); | ||
| }, | ||
| onBuild: (type) async { | ||
| Navigator.of(context).pop(); | ||
| await ref | ||
| .read(campSessionProvider.notifier) | ||
| .buildStructure(row: row, col: col, type: type); | ||
| }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Missing context.mounted guard across async gaps.
_onTileTap is async and the onExpand/onBuild callbacks use the outer context (via Navigator.of(context).pop()) after awaiting CampBuildSheet.show/notifier calls. If the widget is unmounted in the meantime (e.g., user navigates away while the sheet is open), this can throw. As per coding guidelines, Do not use BuildContext across async gaps without a mounted guard.
🛡️ Suggested fix
onExpand: () async {
+ if (!context.mounted) return;
Navigator.of(context).pop();
await ref.read(campSessionProvider.notifier).expandTile(row, col);
},
onBuild: (type) async {
+ if (!context.mounted) return;
Navigator.of(context).pop();
await ref
.read(campSessionProvider.notifier)
.buildStructure(row: row, col: col, type: type);
},📝 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.
Suggested change
| await CampBuildSheet.show( | |
| context, | |
| camp: viewState.camp, | |
| row: row, | |
| col: col, | |
| availableFuel: viewState.resources.availableFuel, | |
| availableMomentum: viewState.resources.availableMomentum, | |
| onExpand: () async { | |
| Navigator.of(context).pop(); | |
| await ref.read(campSessionProvider.notifier).expandTile(row, col); | |
| }, | |
| onBuild: (type) async { | |
| Navigator.of(context).pop(); | |
| await ref | |
| .read(campSessionProvider.notifier) | |
| .buildStructure(row: row, col: col, type: type); | |
| }, | |
| ); | |
| } | |
| await CampBuildSheet.show( | |
| context, | |
| camp: viewState.camp, | |
| row: row, | |
| col: col, | |
| availableFuel: viewState.resources.availableFuel, | |
| availableMomentum: viewState.resources.availableMomentum, | |
| onExpand: () async { | |
| if (!context.mounted) return; | |
| Navigator.of(context).pop(); | |
| await ref.read(campSessionProvider.notifier).expandTile(row, col); | |
| }, | |
| onBuild: (type) async { | |
| if (!context.mounted) return; | |
| Navigator.of(context).pop(); | |
| await ref | |
| .read(campSessionProvider.notifier) | |
| .buildStructure(row: row, col: col, type: type); | |
| }, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/features/character/presentation/widgets/camp_game_panel.dart` around
lines 126 - 144, The async callbacks in `_onTileTap` use the outer `context`
after an async gap, which can fail if the widget is unmounted. Add a
`context.mounted` guard before calling `Navigator.of(context).pop()` and before
any later use of `context` in the `onExpand` and `onBuild` handlers around
`CampBuildSheet.show` and the `campSessionProvider` notifier calls. Keep the
navigation and build/expand flow the same, but only proceed when the widget is
still mounted.
Source: Coding guidelines
Comment on lines
+45
to
+55
| final useCase = ref.read(generateCampQuestsUseCaseProvider); | ||
| final quests = useCase( | ||
| character: character, | ||
| readinessScore: readiness, | ||
| ); | ||
|
|
||
| if (result.failure != null) { | ||
| throw result.failure!; | ||
| if (quests.isNotEmpty) { | ||
| await repo.saveQuests(quests); | ||
| } | ||
|
|
||
| if (result.quests.isNotEmpty) { | ||
| await repo.saveQuests(result.quests); | ||
| } | ||
|
|
||
| return result.quests; | ||
| return quests; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
repo.saveQuests failure result is discarded.
Unlike the previous flow (which threw on failure), the return value of repo.saveQuests(quests) on line 52 isn't checked. If persistence fails, quests are silently not saved with no error surfaced — the app will just regenerate them next load rather than notifying the caller.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/shared/providers/daily_quests_provider.dart` around lines 45 - 55, The
persistence result from repo.saveQuests in DailyQuestsProvider is being ignored,
so save failures are silently dropped. Update the quest generation flow in the
provider method that reads generateCampQuestsUseCaseProvider to check the return
value of repo.saveQuests(quests) and surface or throw on failure, matching the
previous behavior instead of always returning quests as if they were saved.
…73) Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.com>
|
🎉 This PR is included in version 1.15.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
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
2 participants
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.
Summary
Replaces the Trail Run board-game/combat loop with Summit Camp — a health-fueled base builder on the Character tab.
What changed
Removed
Validation
flutter analyze— 0 errors (4 info-level style hints)flutter test— 148 tests passingflutter build web— succeedsSummary by CodeRabbit
New Features
Bug Fixes