What the attacker does to the defender's resistances - #2308
What the attacker does to the defender's resistances#2308denislauri1999 wants to merge 1 commit into
Conversation
BCard type 14, "Changes enemy's resistances": 210 declarations, and the mirror
of the type 13 just added. That one is the resistance of whoever is being hit;
this is what the one hitting does to it, so it rides on the attacker and is read
against the defender's own number.
11: Increases the enemy's elemental resistances by %s.
12: Reduces the enemy's elemental resistances by %s.
21..52: the same, one element at a time.
A "reduces by 20" folds to -20, so the damage step simply adds the two. The sign
is the whole point: read backwards, resistance-piercing effects would protect
the target instead, and nothing would report it.
Written on the fold rather than beside the equipment, so the 194 declarations on
items arrive with no further work once the worn pieces' BCards reach the same
place.
Three of the seven tests go through BattleStatsProvider and not only through the
damage step. The first four set the field by hand, and a mutation that folded
the water value into the fire field left all four green - the fold needed tests
of its own, which is the same hole one level up that a review found earlier
today.
Flipping the sign fails two; folding the wrong element fails one.
|
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 (4)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. WalkthroughThe battle system now collects enemy elemental resistance modifiers from buffs, stores them in ChangesEnemy elemental resistance modifiers
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change is localized to attacker-side resistance handling and damage calculation, with targeted coverage included. No actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant ActiveBuffs
participant BattleStatsProvider
participant CombatStats
participant DamageCalculator
participant Defender
ActiveBuffs->>BattleStatsProvider: provide enemy resistance buffs
BattleStatsProvider->>CombatStats: store accumulated elemental modifiers
Defender->>DamageCalculator: provide elemental resistance
CombatStats->>DamageCalculator: provide matching enemy resistance modifier
DamageCalculator->>DamageCalculator: calculate elemental damage
🚥 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 |
|
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 14, "Changes enemy's resistances" — 210 declarations, and the mirror of type 13 in #2307. That one is the resistance of whoever is being hit; this is what the one hitting does to it, so it rides on the attacker and is read against the defender's own number.
A "reduces by 20" folds to
-20, so the damage step simply adds the two. The sign is the whole point: read backwards, resistance-piercing effects would protect the target instead, and nothing would report it — the fight goes on and the numbers are just wrong.Written on the fold rather than beside the equipment, so the 194 declarations on items arrive with no further work once the worn pieces' BCards reach the same place.
The tests needed a second layer
Four of the seven go through the damage step with the field set by hand. A mutation that folded the water value into the fire field left all four green — so three more go through
BattleStatsProvideritself. Testing the new code and not the step that fills it is a hole I had pointed out to me on #2304 earlier today.Flipping the sign fails two; folding the wrong element fails one.
One known conflict, and it is trivial
This conflicts with #2305 on
CombatStats.cs, and only there. Both PRs add optional parameters to the same positional record, and C# requires those to come last — so both insertions land at the same point and git cannot merge them. The resolution is keep both blocks; nothing has to be decided.I checked every other pair among my open PRs with
git merge-tree(36 combinations) and this is the only one left.Summary by CodeRabbit
New Features
Bug Fixes