feat: add playable trail run game to character tab - #55
Conversation
Transform the Character tab into a daily Trail Run mini-RPG where steps fuel movement along a 7-node trail and active calories fuel turn-based encounters. Adds domain entities, use cases, persistence, Riverpod providers, and UI for trail navigation, combat, and health-driven quests. Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.com>
|
Warning Review limit reached
Next review available in: 42 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. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis PR adds a "Trail Run" gamification mini-game: new domain entities (ActivityResources, EncounterState, TrailNode, AdventureSession, QuestObjective), use cases for computing resources, generating daily trails, resolving combat turns, and evaluating quest progress, repository persistence, Riverpod providers, and Flutter UI widgets integrated into the character page. ChangesTrail Run Mini-Game
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>
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (7)
lib/infrastructure/gamification/character_persistence_repository.dart (1)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImplementation looks correct; missing unit test coverage.
Logic mirrors the existing
loadCharacter/saveCharacterpattern correctly (key naming, JSON round-trip,StorageFailuremapping). However, no test file is included in this cohort forloadAdventureSession/saveAdventureSession, and the coding guidelines require repository contracts to be tested withmocktailfakes.As per coding guidelines, "Repository contracts must be tested with
mocktailfakes, and widget tests must useProviderScopewith overridden fakes rather than real repositories."#!/bin/bash # Check for existing unit tests covering CharacterPersistenceRepository / adventure session persistence fd -e dart . test | xargs rg -l 'CharacterPersistenceRepository|loadAdventureSession|saveAdventureSession' 2>/dev/nullAlso applies to: 13-13, 88-117
🤖 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/infrastructure/gamification/character_persistence_repository.dart` at line 4, Add missing unit test coverage for CharacterPersistenceRepository’s loadAdventureSession and saveAdventureSession behavior using mocktail fakes. Create tests that mirror the existing loadCharacter/saveCharacter repository contract checks: verify the storage key/JSON round-trip behavior and that StorageFailure is mapped correctly, and reference the loadAdventureSession/saveAdventureSession methods in CharacterPersistenceRepository so the tests stay aligned if implementation details move.Source: Coding guidelines
lib/domain/usecases/gamification/evaluate_quest_progress_usecase.dart (1)
48-62: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent guard:
progressFractionskips theisMeasurablecheck used byprogress/isComplete.Functionally harmless today (manual objectives return 0 progress via the switch, so dividing by target still yields 0), but the inconsistency is confusing to maintain — a future change to
progress()'s manual branch could silently breakprogressFractionwithout this guard catching it.♻️ Proposed fix
double progressFraction({ required Quest quest, HealthSummary? summary, List<WorkoutSession> todayRuns = const [], }) { final objective = quest.measurableObjective; - if (objective == null || objective.target <= 0) return 0; + if (objective == null || !objective.isMeasurable || objective.target <= 0) { + return 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/usecases/gamification/evaluate_quest_progress_usecase.dart` around lines 48 - 62, `progressFraction` is missing the same `isMeasurable` guard used by `progress` and `isComplete`, so make the early-return logic consistent in `evaluate_quest_progress_usecase.dart`. Update `progressFraction` to reject non-measurable quests before computing the fraction, using the existing `quest.isMeasurable` / `measurableObjective` checks so the behavior stays aligned with `progress()` and `isComplete()`. Keep the change localized to `progressFraction` and preserve the current clamp behavior for valid measurable quests.lib/domain/usecases/gamification/compute_activity_resources_usecase.dart (1)
24-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: implicit
numtyping forcalories/steps.
final calories = summary.activeCalories ?? 0;infersnum(LUB ofdouble?andintliteral) rather thandouble. It works correctly with~/, but an explicitdoubleannotation would make the intent clearer for reviewers unfamiliar with Dart's LUB inference.🤖 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/compute_activity_resources_usecase.dart` around lines 24 - 25, The `compute_activity_resources_usecase.dart` assignment for `steps` and especially `calories` relies on implicit LUB inference, so make the intent explicit in `compute_activity_resources` by annotating the values with their expected types. Update the local variables derived from `summary.steps` and `summary.activeCalories` so `calories` is clearly treated as a `double` (and keep `steps` clearly typed as needed), preserving the existing `~/` calculation while making the type choice obvious to readers.test/domain/usecases/gamification/resolve_encounter_turn_usecase_test.dart (1)
13-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
EvaluateQuestProgressUseCasetests duplicated in the wrong file.This file is named
resolve_encounter_turn_usecase_test.dart, but Lines 13-75 testEvaluateQuestProgressUseCase, which per the PR's cohort listing already has a dedicatedevaluate_quest_progress_usecase_test.dart. Having the same use case tested in two files risks drift (one file gets updated, the other doesn't) and hurts discoverability.🤖 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/domain/usecases/gamification/resolve_encounter_turn_usecase_test.dart` around lines 13 - 75, The test group in resolve_encounter_turn_usecase_test is exercising EvaluateQuestProgressUseCase instead of the encounter turn use case, and those cases already belong in the dedicated evaluate_quest_progress_usecase_test. Remove or move the duplicated EvaluateQuestProgressUseCase group and its quest-related tests from this file, and keep resolve_encounter_turn_usecase_test focused on ResolveEncounterTurnUseCase so each use case has a single source of truth.lib/domain/usecases/gamification/generate_daily_quests_usecase.dart (1)
18-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHealth-metric branch selection is non-deterministic across test runs, and inconsistent with
GenerateDailyTrailUseCase.
_healthMetricQuestderivesuseStepsfromnow.day.isEvenwherenow = DateTime.now()(set in_buildDeterministicQuests), andcall()also relies on the ambient clock rather than an injectedDateTime. UnlikeGenerateDailyTrailUseCase, which acceptsdateas an explicit parameter (seelib/domain/usecases/gamification/generate_daily_trail_usecase.dart), this use case cannot be tested deterministically for both the steps and active-calories branches — the branch executed depends on which day the test suite happens to run.Consider accepting
DateTime nowas a parameter (defaulting toDateTime.now()) to allow deterministic tests of both quest variants, consistent with the sibling use case's design.♻️ Suggested refactor for testability
Future<({List<Quest> quests, Failure? failure, bool usedModel})> call({ required RunnerCharacter character, required double readinessScore, + DateTime? now, }) async { final base = _buildDeterministicQuests( character: character, readiness: readinessScore, + now: now, ); ... } List<Quest> _buildDeterministicQuests({ required RunnerCharacter character, required double readiness, + DateTime? now, }) { final weakStat = character.stats.weakest; - final now = DateTime.now(); + final effectiveNow = now ?? DateTime.now(); ... }🤖 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_daily_quests_usecase.dart` around lines 18 - 58, The health-metric quest branch in GenerateDailyQuestsUseCase is tied to DateTime.now(), making the steps vs active-calories path non-deterministic and harder to test. Update call() and _buildDeterministicQuests() in GenerateDailyQuestsUseCase to accept an injected DateTime parameter (defaulting to DateTime.now()), then pass that same value into _healthMetricQuest so the branch selection is stable in tests. Keep the design aligned with GenerateDailyTrailUseCase by using the provided date/time input rather than the ambient clock.lib/features/character/providers/adventure_provider.dart (2)
17-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider
@freezedforAdventureViewState.Plain class lacks
copyWith/value equality, which coding guidelines require for data models inlib/**. Value equality also helps Riverpod skip redundant widget rebuilds when session/resources are unchanged.As per coding guidelines,
lib/**: "All data models must use@freezedfor immutability,copyWith, equality, andhashCode."♻️ Suggested refactor
-class AdventureViewState { - const AdventureViewState({ - required this.session, - required this.resources, - }); - - final AdventureSession session; - final ActivityResources resources; -} +@freezed +abstract class AdventureViewState with _$AdventureViewState { + const factory AdventureViewState({ + required AdventureSession session, + required ActivityResources resources, + }) = _AdventureViewState; +}Requires running
build_runnerand adding the freezed part directive/import.🤖 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/providers/adventure_provider.dart` around lines 17 - 25, `AdventureViewState` is a plain immutable holder that should be converted to a Freezed data model. Update the `AdventureViewState` class in `adventure_provider.dart` to use `@freezed` with the appropriate part directive so it gets `copyWith`, value equality, and `hashCode`, and keep its `session` and `resources` fields represented in the generated model. After the refactor, run `build_runner` so the generated Freezed code is created and Riverpod can rely on proper equality checks.Source: Coding guidelines
33-38: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueMove quest sync out of
build()
_syncMeasurableQuests()runs again whenever_awardXp()invalidatesrunnerCharacterProvider, sobuild()keeps re-reading quests/runs on every XP update. Completed quests are already filtered out, but this still mixes side effects intobuild(); consider moving the sync to an explicit action or separate notifier.🤖 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/providers/adventure_provider.dart` around lines 33 - 38, `AdventureProvider.build()` is doing quest synchronization as a side effect, which causes repeated re-syncs whenever `runnerCharacterProvider` is invalidated by `_awardXp()`. Move `_syncMeasurableQuests()` out of `build()` and into an explicit action or a separate notifier so `build()` stays read-only; use the existing `AdventureProvider`, `_syncMeasurableQuests`, and `_awardXp` entry points to relocate the logic without changing the quest filtering behavior.
🤖 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/activity_resources.dart`:
- Around line 2-39: ActivityResources is still a ձեռled immutable class, so it
lacks the required freezed-generated equality and copy behavior. Migrate the
ActivityResources model to use `@freezed` with a private constructor and a const
factory, keeping the derived getters available on the class. Remove the manual
copyWith and let freezed generate copyWith, ==, and hashCode, and ensure the
generated part file is added so the ActivityResources type behaves as a proper
value object.
In `@lib/domain/entities/gamification/adventure_session.dart`:
- Around line 5-95: AdventureSession is a persisted data model but it is still
implemented as a manual class, so it should be converted to use `@freezed` for
immutability/copyWith/equality and `@JsonSerializable` for its storage JSON
mapping. Update the AdventureSession definition and its fromJson/toJson handling
to rely on generated code, keep the existing fields and behaviors intact, and
then regenerate the model output with build_runner instead of maintaining the
serialization manually.
In `@lib/domain/entities/gamification/encounter_state.dart`:
- Around line 15-95: EncounterState is a persisted data model but is still
handwritten, so it should be converted to a Freezed/JSON-serializable model.
Update the EncounterState definition to use `@freezed` and `@JsonSerializable`, and
move its immutable fields, copyWith, equality, hashCode, and JSON mapping onto
the generated model instead of the manual implementation. Ensure the generated
serialization still covers enemyId, enemyMaxHp, enemyHp, turnCount, outcome,
blockingNextHit, focusedNextTurn, firstActionFree, combatLog, and isBoss, and
keep the EncounterState / EncounterOutcome symbols aligned so the persistence
code can continue using it unchanged.
- Around line 77-94: `EncounterState.fromJson` still assumes `enemy_max_hp` and
`enemy_hp` are always valid numeric values, so bad or missing save data can
throw during parsing. Update the parser in `EncounterState.fromJson` to
defensively read these fields, using safe defaults or validation like the other
optional fields, so corrupt payloads do not break encounter restoration. Focus
the fix on the `enemyMaxHp` and `enemyHp` assignments and keep the rest of the
factory tolerant of malformed JSON.
In `@lib/domain/entities/gamification/quest.dart`:
- Around line 9-41: QuestObjectiveKind and QuestObjective are introduced as
plain Dart types, but they should follow the project’s data-model conventions.
Update the QuestObjective model in the quest entity so it uses `@freezed` for
immutability/equality/copyWith and `@JsonSerializable` for persistence, matching
the patterns used by the sibling model classes. Keep the same fields and nested
usage inside Quest, and make sure the generated serialization still handles
QuestObjectiveKind and QuestObjective.fromJson/toJson correctly.
- Around line 33-39: QuestObjective.fromJson currently defaults a missing or
invalid target to 0, which can make malformed quests complete automatically in
EvaluateQuestProgressUseCase.isComplete(). Update QuestObjective.fromJson so
QuestObjectiveKind and target are validated together: reject or surface
invalid/missing measurable targets instead of silently defaulting, while still
allowing manual objectives to work through QuestObjectiveKind.manual. Use the
QuestObjective.fromJson factory and any related quest parsing path to ensure bad
quest data cannot produce a zero target.
In `@lib/domain/entities/gamification/trail_node.dart`:
- Around line 13-51: TrailNode is a persisted data model but is currently a ձեռ
manual class with custom copyWith/toJson/fromJson and no value equality. Update
TrailNode to follow the same model pattern as the other new entities by
converting it to a `@freezed` value type and adding `@JsonSerializable` so
serialization and equality are generated consistently. Keep the existing fields
and defaults, but move the implementation to the generated model definition
rather than maintaining manual JSON/copy logic.
In `@lib/domain/usecases/gamification/generate_daily_trail_usecase.dart`:
- Around line 47-51: The _roll and _seed logic in GenerateDailyTrailUsecase is
not web-safe because the current linear congruential step can exceed
JavaScript’s 53-bit precision and produce different results on Flutter web
versus VM. Update the randomization approach in these helpers to use a
32-bit-safe hash/PRNG or BigInt-based arithmetic so the same level/date input
produces identical output across platforms.
In `@lib/features/character/providers/adventure_provider.dart`:
- Around line 83-118: `advance()` (and `performCombatAction()`) can be entered
concurrently, which lets two rapid taps operate on the same stale `state.value`
before `_persist()` completes. Add an in-flight guard in `AdventureProvider` to
reject re-entrant calls while a previous action is running, and clear it in a
finally block after the async work finishes. Apply the same protection to both
`advance` and `performCombatAction` so they cannot double-spend resources or
resolve the same node twice.
- Around line 90-118: The `advance` flow in `adventure_provider.dart` is using a
stale `node` snapshot when deciding whether to set `trailCompleted`, so the
final node can be resolved twice. Update the completion check in `advance()` to
use the updated resolved state from the cloned `nodes`/session after
`_resolveNode()` (or re-read the node from the updated session) before calling
`copyWith(trailCompleted: true)`. Keep the fix anchored around `advance`,
`_resolveNode`, `session.copyWith`, and the `nextIndex` last-node check so the
trail completes immediately on the first arrival at the final node.
- Around line 40-47: Handle loadAdventureSession() and loadCharacter() errors
separately from null results so transient read failures do not fall through into
create-and-save or silent XP loss. In adventureProvider, update the branching
around loadAdventureSession() before the session regeneration path to
return/propagate on failure and only create a fresh session when the load
succeeds but no valid session exists. Also update _awardXp() to inspect the
loadCharacter() result, stop on errors, and avoid skipping XP updates silently.
- Around line 91-92: The clamp results in AdventureProvider are being used as if
they were ints, but int.clamp() returns num, which breaks indexing and copyWith
usage. Update the relevant expressions in AdventureProvider so the values
assigned to nextIndex and recovered are converted to int after clamp, and make
the same adjustment in the recovery path near the copyWith call to keep
session.nodes[...] and session.copyWith(...) type-safe.
---
Nitpick comments:
In `@lib/domain/usecases/gamification/compute_activity_resources_usecase.dart`:
- Around line 24-25: The `compute_activity_resources_usecase.dart` assignment
for `steps` and especially `calories` relies on implicit LUB inference, so make
the intent explicit in `compute_activity_resources` by annotating the values
with their expected types. Update the local variables derived from
`summary.steps` and `summary.activeCalories` so `calories` is clearly treated as
a `double` (and keep `steps` clearly typed as needed), preserving the existing
`~/` calculation while making the type choice obvious to readers.
In `@lib/domain/usecases/gamification/evaluate_quest_progress_usecase.dart`:
- Around line 48-62: `progressFraction` is missing the same `isMeasurable` guard
used by `progress` and `isComplete`, so make the early-return logic consistent
in `evaluate_quest_progress_usecase.dart`. Update `progressFraction` to reject
non-measurable quests before computing the fraction, using the existing
`quest.isMeasurable` / `measurableObjective` checks so the behavior stays
aligned with `progress()` and `isComplete()`. Keep the change localized to
`progressFraction` and preserve the current clamp behavior for valid measurable
quests.
In `@lib/domain/usecases/gamification/generate_daily_quests_usecase.dart`:
- Around line 18-58: The health-metric quest branch in
GenerateDailyQuestsUseCase is tied to DateTime.now(), making the steps vs
active-calories path non-deterministic and harder to test. Update call() and
_buildDeterministicQuests() in GenerateDailyQuestsUseCase to accept an injected
DateTime parameter (defaulting to DateTime.now()), then pass that same value
into _healthMetricQuest so the branch selection is stable in tests. Keep the
design aligned with GenerateDailyTrailUseCase by using the provided date/time
input rather than the ambient clock.
In `@lib/features/character/providers/adventure_provider.dart`:
- Around line 17-25: `AdventureViewState` is a plain immutable holder that
should be converted to a Freezed data model. Update the `AdventureViewState`
class in `adventure_provider.dart` to use `@freezed` with the appropriate part
directive so it gets `copyWith`, value equality, and `hashCode`, and keep its
`session` and `resources` fields represented in the generated model. After the
refactor, run `build_runner` so the generated Freezed code is created and
Riverpod can rely on proper equality checks.
- Around line 33-38: `AdventureProvider.build()` is doing quest synchronization
as a side effect, which causes repeated re-syncs whenever
`runnerCharacterProvider` is invalidated by `_awardXp()`. Move
`_syncMeasurableQuests()` out of `build()` and into an explicit action or a
separate notifier so `build()` stays read-only; use the existing
`AdventureProvider`, `_syncMeasurableQuests`, and `_awardXp` entry points to
relocate the logic without changing the quest filtering behavior.
In `@lib/infrastructure/gamification/character_persistence_repository.dart`:
- Line 4: Add missing unit test coverage for CharacterPersistenceRepository’s
loadAdventureSession and saveAdventureSession behavior using mocktail fakes.
Create tests that mirror the existing loadCharacter/saveCharacter repository
contract checks: verify the storage key/JSON round-trip behavior and that
StorageFailure is mapped correctly, and reference the
loadAdventureSession/saveAdventureSession methods in
CharacterPersistenceRepository so the tests stay aligned if implementation
details move.
In `@test/domain/usecases/gamification/resolve_encounter_turn_usecase_test.dart`:
- Around line 13-75: The test group in resolve_encounter_turn_usecase_test is
exercising EvaluateQuestProgressUseCase instead of the encounter turn use case,
and those cases already belong in the dedicated
evaluate_quest_progress_usecase_test. Remove or move the duplicated
EvaluateQuestProgressUseCase group and its quest-related tests from this file,
and keep resolve_encounter_turn_usecase_test focused on
ResolveEncounterTurnUseCase so each use case has a single source of truth.
🪄 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: 3fcb8222-c94d-455f-89f0-181fc003a6db
📒 Files selected for processing (28)
lib/core/constants/gamification_constants.dartlib/domain/entities/gamification/activity_resources.dartlib/domain/entities/gamification/adventure_session.dartlib/domain/entities/gamification/encounter_state.dartlib/domain/entities/gamification/quest.dartlib/domain/entities/gamification/trail_node.dartlib/domain/repositories/character_repository.dartlib/domain/usecases/gamification/compute_activity_resources_usecase.dartlib/domain/usecases/gamification/evaluate_quest_progress_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/features/character/presentation/pages/character_page.dartlib/features/character/presentation/widgets/activity_resources_bar.dartlib/features/character/presentation/widgets/encounter_panel.dartlib/features/character/presentation/widgets/quest_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/infrastructure/gamification/character_persistence_repository.dartlib/shared/providers/gamification_providers.darttest/domain/usecases/gamification/compute_activity_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/trail_run_game_panel_test.dart
| class ActivityResources { | ||
| const ActivityResources({ | ||
| required this.totalMovePoints, | ||
| required this.totalStamina, | ||
| required this.spentMovePoints, | ||
| required this.spentStamina, | ||
| this.bonusMoveGranted = false, | ||
| }); | ||
|
|
||
| final int totalMovePoints; | ||
| final int totalStamina; | ||
| final int spentMovePoints; | ||
| final int spentStamina; | ||
| final bool bonusMoveGranted; | ||
|
|
||
| int get availableMovePoints => | ||
| (totalMovePoints - spentMovePoints).clamp(0, totalMovePoints); | ||
|
|
||
| int get availableStamina => | ||
| (totalStamina - spentStamina).clamp(0, totalStamina); | ||
|
|
||
| bool get canAdvance => availableMovePoints > 0; | ||
|
|
||
| ActivityResources copyWith({ | ||
| int? totalMovePoints, | ||
| int? totalStamina, | ||
| int? spentMovePoints, | ||
| int? spentStamina, | ||
| bool? bonusMoveGranted, | ||
| }) => | ||
| ActivityResources( | ||
| totalMovePoints: totalMovePoints ?? this.totalMovePoints, | ||
| totalStamina: totalStamina ?? this.totalStamina, | ||
| spentMovePoints: spentMovePoints ?? this.spentMovePoints, | ||
| spentStamina: spentStamina ?? this.spentStamina, | ||
| bonusMoveGranted: bonusMoveGranted ?? this.bonusMoveGranted, | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Model doesn't use @freezed as required by guidelines.
ActivityResources is a hand-rolled immutable class with manual copyWith and no ==/hashCode override. Value comparisons (e.g., in Riverpod AsyncValue/provider rebuild checks, or test assertions) will fall back to identity equality.
As per coding guidelines, "All data models must use @freezed for immutability, copyWith, equality, and hashCode."
♻️ Sketch of a freezed migration
import 'package:freezed_annotation/freezed_annotation.dart';
part 'activity_resources.freezed.dart';
`@freezed`
class ActivityResources with _$ActivityResources {
const ActivityResources._();
const factory ActivityResources({
required int totalMovePoints,
required int totalStamina,
required int spentMovePoints,
required int spentStamina,
`@Default`(false) bool bonusMoveGranted,
}) = _ActivityResources;
int get availableMovePoints =>
(totalMovePoints - spentMovePoints).clamp(0, totalMovePoints);
int get availableStamina =>
(totalStamina - spentStamina).clamp(0, totalStamina);
bool get canAdvance => availableMovePoints > 0;
}Requires running dart run build_runner build --delete-conflicting-outputs.
🤖 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/activity_resources.dart` around lines 2 -
39, ActivityResources is still a ձեռled immutable class, so it lacks the
required freezed-generated equality and copy behavior. Migrate the
ActivityResources model to use `@freezed` with a private constructor and a const
factory, keeping the derived getters available on the class. Remove the manual
copyWith and let freezed generate copyWith, ==, and hashCode, and ensure the
generated part file is added so the ActivityResources type behaves as a proper
value object.
Source: Coding guidelines
| class AdventureSession { | ||
| const AdventureSession({ | ||
| required this.date, | ||
| required this.nodes, | ||
| required this.currentIndex, | ||
| required this.spentMovePoints, | ||
| required this.spentStamina, | ||
| this.activeEncounter, | ||
| this.trailCompleted = false, | ||
| this.bonusMoveUsed = false, | ||
| }); | ||
|
|
||
| final DateTime date; | ||
| final List<TrailNode> nodes; | ||
| final int currentIndex; | ||
| final int spentMovePoints; | ||
| final int spentStamina; | ||
| final EncounterState? activeEncounter; | ||
| final bool trailCompleted; | ||
| final bool bonusMoveUsed; | ||
|
|
||
| TrailNode? get currentNode => | ||
| currentIndex >= 0 && currentIndex < nodes.length | ||
| ? nodes[currentIndex] | ||
| : null; | ||
|
|
||
| bool get atTrailEnd => currentIndex >= nodes.length - 1; | ||
|
|
||
| AdventureSession copyWith({ | ||
| List<TrailNode>? nodes, | ||
| int? currentIndex, | ||
| int? spentMovePoints, | ||
| int? spentStamina, | ||
| EncounterState? activeEncounter, | ||
| bool clearEncounter = false, | ||
| bool? trailCompleted, | ||
| bool? bonusMoveUsed, | ||
| }) => | ||
| AdventureSession( | ||
| date: date, | ||
| nodes: nodes ?? this.nodes, | ||
| currentIndex: currentIndex ?? this.currentIndex, | ||
| spentMovePoints: spentMovePoints ?? this.spentMovePoints, | ||
| spentStamina: spentStamina ?? this.spentStamina, | ||
| activeEncounter: | ||
| clearEncounter ? null : (activeEncounter ?? this.activeEncounter), | ||
| trailCompleted: trailCompleted ?? this.trailCompleted, | ||
| bonusMoveUsed: bonusMoveUsed ?? this.bonusMoveUsed, | ||
| ); | ||
|
|
||
| ActivityResources resourcesFromTotals({ | ||
| required int totalMovePoints, | ||
| required int totalStamina, | ||
| }) => | ||
| ActivityResources( | ||
| totalMovePoints: totalMovePoints, | ||
| totalStamina: totalStamina, | ||
| spentMovePoints: spentMovePoints, | ||
| spentStamina: spentStamina, | ||
| bonusMoveGranted: bonusMoveUsed, | ||
| ); | ||
|
|
||
| Map<String, dynamic> toJson() => { | ||
| 'date': date.toIso8601String(), | ||
| 'nodes': nodes.map((n) => n.toJson()).toList(), | ||
| 'current_index': currentIndex, | ||
| 'spent_move_points': spentMovePoints, | ||
| 'spent_stamina': spentStamina, | ||
| 'active_encounter': activeEncounter?.toJson(), | ||
| 'trail_completed': trailCompleted, | ||
| 'bonus_move_used': bonusMoveUsed, | ||
| }; | ||
|
|
||
| factory AdventureSession.fromJson(Map<String, dynamic> json) => | ||
| AdventureSession( | ||
| date: DateTime.parse(json['date'] as String), | ||
| nodes: (json['nodes'] as List<dynamic>) | ||
| .map((n) => TrailNode.fromJson(n as Map<String, dynamic>)) | ||
| .toList(), | ||
| currentIndex: (json['current_index'] as num?)?.toInt() ?? 0, | ||
| spentMovePoints: (json['spent_move_points'] as num?)?.toInt() ?? 0, | ||
| spentStamina: (json['spent_stamina'] as num?)?.toInt() ?? 0, | ||
| activeEncounter: json['active_encounter'] != null | ||
| ? EncounterState.fromJson( | ||
| json['active_encounter'] as Map<String, dynamic>, | ||
| ) | ||
| : null, | ||
| trailCompleted: json['trail_completed'] as bool? ?? false, | ||
| bonusMoveUsed: json['bonus_move_used'] as bool? ?? false, | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
AdventureSession is the primary persisted aggregate but bypasses @freezed/@JsonSerializable.
Confirmed via character_persistence_repository.dart, this entity is loaded/saved directly through AdventureSession.fromJson/toJson with SharedPreferences. Per guidelines it must use @freezed (immutability/copyWith/equality/hashCode) and @JsonSerializable (storage-facing model), then regenerate via build_runner.
As per coding guidelines: "All data models must use @freezed..." / "All models that touch an API or storage must use @JsonSerializable." / "Run dart run build_runner build --delete-conflicting-outputs after model or provider changes, and never edit generated .g.dart or .freezed.dart files by hand."
🤖 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/adventure_session.dart` around lines 5 - 95,
AdventureSession is a persisted data model but it is still implemented as a
manual class, so it should be converted to use `@freezed` for
immutability/copyWith/equality and `@JsonSerializable` for its storage JSON
mapping. Update the AdventureSession definition and its fromJson/toJson handling
to rely on generated code, keep the existing fields and behaviors intact, and
then regenerate the model output with build_runner instead of maintaining the
serialization manually.
Source: Coding guidelines
| class EncounterState { | ||
| const EncounterState({ | ||
| required this.enemyId, | ||
| required this.enemyMaxHp, | ||
| required this.enemyHp, | ||
| required this.turnCount, | ||
| required this.outcome, | ||
| this.blockingNextHit = false, | ||
| this.focusedNextTurn = false, | ||
| this.firstActionFree = true, | ||
| this.combatLog = const [], | ||
| this.isBoss = false, | ||
| }); | ||
|
|
||
| final String enemyId; | ||
| final int enemyMaxHp; | ||
| final int enemyHp; | ||
| final int turnCount; | ||
| final EncounterOutcome outcome; | ||
| final bool blockingNextHit; | ||
| final bool focusedNextTurn; | ||
| final bool firstActionFree; | ||
| final List<String> combatLog; | ||
| final bool isBoss; | ||
|
|
||
| bool get isActive => outcome == EncounterOutcome.inProgress; | ||
|
|
||
| EncounterState copyWith({ | ||
| int? enemyHp, | ||
| int? turnCount, | ||
| EncounterOutcome? outcome, | ||
| bool? blockingNextHit, | ||
| bool? focusedNextTurn, | ||
| bool? firstActionFree, | ||
| List<String>? combatLog, | ||
| }) => | ||
| EncounterState( | ||
| enemyId: enemyId, | ||
| enemyMaxHp: enemyMaxHp, | ||
| enemyHp: enemyHp ?? this.enemyHp, | ||
| turnCount: turnCount ?? this.turnCount, | ||
| outcome: outcome ?? this.outcome, | ||
| blockingNextHit: blockingNextHit ?? this.blockingNextHit, | ||
| focusedNextTurn: focusedNextTurn ?? this.focusedNextTurn, | ||
| firstActionFree: firstActionFree ?? this.firstActionFree, | ||
| combatLog: combatLog ?? this.combatLog, | ||
| isBoss: isBoss, | ||
| ); | ||
|
|
||
| Map<String, dynamic> toJson() => { | ||
| 'enemy_id': enemyId, | ||
| 'enemy_max_hp': enemyMaxHp, | ||
| 'enemy_hp': enemyHp, | ||
| 'turn_count': turnCount, | ||
| 'outcome': outcome.name, | ||
| 'blocking_next_hit': blockingNextHit, | ||
| 'focused_next_turn': focusedNextTurn, | ||
| 'first_action_free': firstActionFree, | ||
| 'combat_log': combatLog, | ||
| 'is_boss': isBoss, | ||
| }; | ||
|
|
||
| factory EncounterState.fromJson(Map<String, dynamic> json) => EncounterState( | ||
| enemyId: json['enemy_id'] as String? ?? 'trail_grunt', | ||
| enemyMaxHp: (json['enemy_max_hp'] as num).toInt(), | ||
| enemyHp: (json['enemy_hp'] as num).toInt(), | ||
| turnCount: (json['turn_count'] as num?)?.toInt() ?? 0, | ||
| outcome: EncounterOutcome.values.firstWhere( | ||
| (o) => o.name == json['outcome'], | ||
| orElse: () => EncounterOutcome.inProgress, | ||
| ), | ||
| blockingNextHit: json['blocking_next_hit'] as bool? ?? false, | ||
| focusedNextTurn: json['focused_next_turn'] as bool? ?? false, | ||
| firstActionFree: json['first_action_free'] as bool? ?? true, | ||
| combatLog: (json['combat_log'] as List<dynamic>?) | ||
| ?.map((e) => e as String) | ||
| .toList() ?? | ||
| const [], | ||
| isBoss: json['is_boss'] as bool? ?? false, | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
EncounterState doesn't use @freezed/@JsonSerializable as mandated.
This class is persisted (nested in AdventureSession, saved via SharedPreferences per character_persistence_repository.dart), so it touches storage, yet uses hand-written copyWith/toJson/fromJson with no ==/hashCode.
As per coding guidelines: "All data models must use @freezed for immutability, copyWith, equality, and hashCode" and "All models that touch an API or storage must use @JsonSerializable."
🤖 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/encounter_state.dart` around lines 15 - 95,
EncounterState is a persisted data model but is still handwritten, so it should
be converted to a Freezed/JSON-serializable model. Update the EncounterState
definition to use `@freezed` and `@JsonSerializable`, and move its immutable fields,
copyWith, equality, hashCode, and JSON mapping onto the generated model instead
of the manual implementation. Ensure the generated serialization still covers
enemyId, enemyMaxHp, enemyHp, turnCount, outcome, blockingNextHit,
focusedNextTurn, firstActionFree, combatLog, and isBoss, and keep the
EncounterState / EncounterOutcome symbols aligned so the persistence code can
continue using it unchanged.
Source: Coding guidelines
| enum QuestObjectiveKind { | ||
| steps, | ||
| activeCalories, | ||
| runMinutes, | ||
| runDistanceKm, | ||
| manual, | ||
| } | ||
|
|
||
| class QuestObjective { | ||
| const QuestObjective({ | ||
| required this.kind, | ||
| required this.target, | ||
| }); | ||
|
|
||
| final QuestObjectiveKind kind; | ||
| final double target; | ||
|
|
||
| bool get isMeasurable => kind != QuestObjectiveKind.manual; | ||
|
|
||
| Map<String, dynamic> toJson() => { | ||
| 'kind': kind.name, | ||
| 'target': target, | ||
| }; | ||
|
|
||
| factory QuestObjective.fromJson(Map<String, dynamic> json) => QuestObjective( | ||
| kind: QuestObjectiveKind.values.firstWhere( | ||
| (k) => k.name == json['kind'], | ||
| orElse: () => QuestObjectiveKind.manual, | ||
| ), | ||
| target: (json['target'] as num?)?.toDouble() ?? 0, | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
New QuestObjective model repeats the non-freezed pattern.
QuestObjectiveKind/QuestObjective are newly introduced in this PR and persisted (nested inside Quest), but don't use @freezed/@JsonSerializable, extending the same guideline gap seen in the sibling model files.
As per coding guidelines: "All data models must use @freezed for immutability, copyWith, equality, and hashCode" and "All models that touch an API or storage must use @JsonSerializable."
🤖 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/quest.dart` around lines 9 - 41,
QuestObjectiveKind and QuestObjective are introduced as plain Dart types, but
they should follow the project’s data-model conventions. Update the
QuestObjective model in the quest entity so it uses `@freezed` for
immutability/equality/copyWith and `@JsonSerializable` for persistence, matching
the patterns used by the sibling model classes. Keep the same fields and nested
usage inside Quest, and make sure the generated serialization still handles
QuestObjectiveKind and QuestObjective.fromJson/toJson correctly.
Source: Coding guidelines
Harden adventure provider against concurrent taps and stale trail state, propagate persistence load/save failures, and fix int clamp typing. Add defensive JSON parsing for encounter HP and quest objectives, web-safe seededRoll utility, and targeted tests. Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.com>
|
🎉 This PR is included in version 1.12.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Summary
Transforms the Character tab from a static RPG profile into a playable Trail Run mini-RPG. Real-world health data now drives gameplay:
What changed
Domain layer
AdventureSession,TrailNode,EncounterState,ActivityResourcesQuestwithQuestObjectivefor measurable health-metric goalsComputeActivityResources,GenerateDailyTrail,ResolveEncounterTurn,EvaluateQuestProgressGameplay
Quests
UI
TrailRunGamePanelat top of Character tab with resource bar, trail map, and encounter panelValidation
flutter analyze— no issuesflutter test— 102 tests passingflutter build web— succeedsArchitecture
Follows clean architecture: domain use cases are pure Dart, persistence via extended
CharacterRepository, RiverpodAdventureSessionNotifierorchestrates game state, features never import infrastructure directly.Summary by CodeRabbit
New Features
Bug Fixes
Tests