Damage as a percentage of HP, which the case did not read - #2306
Damage as a percentage of HP, which the case did not read#2306denislauri1999 wants to merge 1 commit into
Conversation
BCard type 37 subtype 31, "Decreases the opponent's HP by %s%%". 112 of the 122 declarations on skills are this one subtype, and nothing read it: the skills carrying it did none of what they declare. The percentages in the file run from 5% to 100%, with 90% twelve times - these are boss skills that are meant to take most of a health bar in one blow. ONE ASSUMPTION IS OURS, and it is the same one the sibling codebase carries so the two do not drift: the percentage comes off MAXIMUM HP. The file says only "HP by %s%%" and does not distinguish. Off current HP the loss would halve and halve again without ever finishing anything, which is not what a skill declaring 90% is for. It is part of the same subtraction as the blow, so it can kill and the ordinary death path handles it. That is a deliberate difference from the sibling, which floors the victim at 1 HP - a workaround for applying the effect outside the blow, where a kill would bypass the death sequence. Nothing in the file says the effect must leave its victim standing. Subtype 32 hits the caster and no skill in the file declares it, so it is left out rather than written blind against no data. Reading the percentage off current HP instead of maximum fails two tests.
|
Warning Review limit reachedNext included review available in 17 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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 |
|
Heads-up for whoever merges: this compiles on its own but not alongside another of my open PRs, and
I found it by merging all nine of today's branches onto a scratch branch: with those two lines fixed the lot builds at zero warnings and passes 983 tests. Pairwise |
1 participant
BCard type 37 subtype 31 — "Decreases the opponent's HP by %s%%."
112 of the 122 declarations on skills are this one subtype, and nothing read it: the skills carrying it did none of what they declare. The percentages in the file run from 5% to 100%, with 90% appearing twelve times — these are boss skills meant to take most of a health bar in one blow.
One assumption is ours
The percentage comes off maximum HP. The file says only "HP by %s%%" and does not distinguish, so this is the one point that does not come from it. Off current HP the loss would halve and halve again without ever finishing anything, which is not what a skill declaring 90% is for.
It is the same assumption the sibling codebase already carries, so the two do not drift.
One difference from the sibling, on purpose
It can kill. The loss is part of the same subtraction as the blow, so the ordinary death path handles it.
The sibling floors the victim at 1 HP, but that is a workaround for where it applies the effect — outside the blow, where a kill would bypass the death sequence. Here that does not arise, and nothing in the file says the effect must leave its victim standing.
Left out
Subtype 32 hits the caster instead, and no skill in the file declares it. Written blind against no data it would be a guess, so it is not here.
Checked by breaking it
Reading the percentage off current HP instead of maximum fails two of the five tests. One test also documents an existing guard I ran into: a blow whose ordinary damage rolls to zero is already treated as a miss, so the percentage rides along with a blow that landed rather than standing on its own.