Refactor StallUpgradeManager to a declarative architecture#199
Conversation
- Replaced iterative Strategy pattern with a data-driven ScalingRule system. - Centralized stat scaling logic in STAT_RULES map using lambdas. - Simplified calculateValue, applyUpgrade, and getBenefitString. - Synchronized base stats in Registry.kt with documentation to ensure test consistency. - Updated fixes.md with REF-018. Co-authored-by: candour <4670475+candour@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughGameplay balance constants were adjusted in Registry.kt (enemy HP growth factor, CHICKEN_RICE/DURIAN damage, TIGER_MOM base HP). StallUpgradeManager.kt's stat scaling was refactored from per-stat ChangesBalance and scaling changes
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Build Successful! 🚀Note: This link will be removed when the PR is closed. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/com/messark/hawker/utils/StallUpgradeManager.kt (1)
20-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFragile
cost == 100coupling for the Chicken Rice DPS special case.
isChickenRiceDPSgates the linear+6.0scaling oncost == 100, which mirrors the currentCHICKEN_RICEcost inRegistry.kt. This PR itself tunes balance constants elsewhere, so a future cost change would silently flip Chicken Rice back to the*1.15multiplicative path with no compile error. Prefer keying purely on the stall type (or a named intent flag) rather than a magic cost value.♻️ Proposed refactor
- "Damage" to ScalingRule { current, l, type, cost, _ -> - val isChickenRiceDPS = type == StallType.CHICKEN_RICE && cost == 100 + "Damage" to ScalingRule { current, l, type, _, _ -> + val isChickenRiceDPS = type == StallType.CHICKEN_RICE val next = if (isChickenRiceDPS) current + 6.0 else (current * 1.15).roundToInt().toDouble() if (l % 10 == 0) (next * 1.25).roundToInt().toDouble() else next },Confirm whether
cost == 100was intentionally distinguishing a Chicken Rice variant before adopting this.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 422ee014-132f-4ebe-8efc-862a8543449e
📒 Files selected for processing (3)
app/src/main/java/com/messark/hawker/registry/Registry.ktapp/src/main/java/com/messark/hawker/utils/StallUpgradeManager.ktfixes.md
Refactored
StallUpgradeManager.ktto improve maintainability and efficiency by replacing the verbose strategy pattern with a declarative, data-driven architecture. Stat scaling rules are now centralized in aSTAT_RULESmap, and calculations are performed using idiomatic Kotlinfoldoperations. Additionally, base stats inRegistry.ktwere synchronized withSTALL_STATS.mdandCUSTOMER_STATS.mdto ensure unit test consistency and correct game balancing. Verified the changes withAliasNormalizationTestandMilestoneBoostTest.PR created automatically by Jules for task 15818129994225890729 started by @candour
Summary by CodeRabbit
Bug Fixes
Gameplay Improvements