The last percentage division without a guard - #2310
Conversation
`GenerateStatInfo` divides by MaxHp and MaxMp with nothing checking them, while every other place that computes the same two percentages checks first - including one five hundred lines up in the same file. It never threw and never would: the division is on floats, so a maximum of zero gives NaN rather than an exception, and the cast then puts a meaningless number in the two fields the client draws as the health and mana bars. No log, no symptom to search for, a bar reading whatever that number happened to be. I swept the rest rather than fixing only what I tripped over. Every other float division of this shape in src/ is guarded: AliveEntityExtension, MonsterBundleExtensions, NpcBundleExtensions, VisualEntityExtension, the earlier one in this same file, RollBaseDamage in DamageCalculator, and the reward share in RewardService, whose caller returns early on a total of zero. This was the only one left. The fallback of 100 matches what the guarded computation five hundred lines up already uses, rather than inventing a second convention. Removing the guard fails one of the three tests.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. Walkthrough
ChangesStat percentage handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change guards health and mana percentage calculations against zero maximum values and uses the existing 100% fallback convention, with focused tests covering the behavior. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
GenerateStatInfodivides byMaxHpandMaxMpwith nothing checking them, while every other place that computes the same two percentages checks first — including one five hundred lines up in the same file:It never threw and never would. The division is on floats, so a maximum of zero gives
NaNrather than an exception, and the cast then puts a meaningless number in the two fields the client draws as the health and mana bars. No log, no symptom to search for.I swept the rest rather than fixing only what I tripped over
Every other float division of this shape in
src/is guarded:AliveEntityExtension102-106MaxHp > 0 ? … : 100MonsterBundleExtensions33MaxHp > 0 ? … : 100NpcBundleExtensions33MaxHp > 0 ? … : 100VisualEntityExtension36?? 1PlayerBundleExtensions238-239MaxHp > 0 ? … : 100DamageCalculator.RollBaseDamagemax <= minRewardService98totalDamage <= 0GenerateStatInfowas the only one left.The fallback of 100 is the one the guarded computation in the same file already uses, rather than inventing a second convention for the same question.
Removing the guard fails one of the three tests.
Summary by CodeRabbit
Bug Fixes
Tests