A melee-only hit rate bonus fed the ranged rate too

[denislauri1999]

What

BCard type 4 declares three scopes and only the first was folded in — into both rates at once:

subtype	client text
11 / 12	"Hit rate of all attacks is increased by %s."
21 / 22	"Hit rate of melee attacks is increased by %s."
31 / 32	"Hit rate of ranged attacks is increased by %s."

So a melee-only bonus did nothing, and neither did a ranged-only one: 13 declarations of 4/21 and 4/31 across items and cards had no effect at all.

Type 2 sitting next to it already keeps the three defences apart the same way, so this is the offensive half of a split that was half done, written to match.

What is left out, and why

4/41-42, concentration during a magic attack. CombatStats has no field for it, and exposing one that nothing reads would be worse than the gap.

Testing

• Builds with 0 warnings; full suite green — 996 tests, five of them new.

• The new tests assert the negative half as well as the positive: a melee bonus must leave DistanceRate unchanged, and a ranged bonus must leave HitRate unchanged. A leak into the other field raises nothing, so that is the assertion that earns its place.

• Not verified in a client. What somebody with a client would need to check - kept here rather than in manual-test-plan.md, since it belongs to this one change:

  • A buff with subtype 21 makes melee attacks land more often and leaves ranged alone
  • A buff with subtype 31 makes ranged attacks land more often and leaves melee alone
  • A subtype 11 buff raises both

Summary by CodeRabbit

• Bug Fixes

  • Melee and ranged hit-rate bonuses now apply only to their corresponding attack ranges.
  • General hit-rate bonuses continue to affect both melee and ranged attacks.
  • Hit-rate penalties now correctly reduce the applicable attack range.

• Documentation

  • Added manual test coverage for range-specific hit-rate effects.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b583ba9f-e601-48a6-9161-4fd0ca8bbd45

📥 Commits

Reviewing files that changed from the base of the PR and between 85d3ed3 and e8e55ab.

📒 Files selected for processing (3)

• documentation/manual-test-plan.md
• src/NosCore.GameObject/Services/BattleService/BattleStatsProvider.cs
• test/NosCore.GameObject.Tests/Services/BattleService/HitRateByRangeTests.cs

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.

---

Walkthrough

BattleStatsProvider now applies melee and ranged hit-rate card effects to their respective accuracy statistics. Tests and manual checks cover increases, decreases, shared All effects, and stacking behavior.

Changes

Range-specific hit-rate calculation

Layer / File(s)	Summary
Route range-specific hit-rate effects src/NosCore.GameObject/Services/BattleService/BattleStatsProvider.cs	The provider tracks melee and ranged hit-rate values separately. It applies each value to its corresponding accuracy statistic, while shared values affect both.
Validate range-specific results test/NosCore.GameObject.Tests/Services/BattleService/HitRateByRangeTests.cs, documentation/manual-test-plan.md	Automated tests and manual checks verify increased, decreased, shared, and stacked target-card effects.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to e8e55

The change correctly separates melee-only and ranged-only hit-rate bonuses while preserving all-attack bonuses for both ranges. No actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: erwan-joly

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the primary bug fixed by the pull request: a melee-only hit rate bonus incorrectly affected the ranged rate.

Full details: Docstring Coverage

Explanation

Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[erwan-joly]

this file should be removed those are for conversation in git pr

[erwan-joly]

actually there is useful stuff there so maybe we should just keep the thing that are relevant for 1 pr in the conversation and keep the global useful one like mounts, combat etc

[denislauri1999]

Done — the three checks moved into the PR description, and the file keeps only what is useful across PRs.

On the second half: master's copy still carries six sections that belong to one PR each (types 40, 37/31, 16, 13/14, 34/35, and the combo/equipment ones are the general kind you mean). Happy to prune those in a separate small PR and leave Setup, Combat basics, Mounts, Class change, Equipment gates and the regression sweep — say the word and I'll open it.

[erwan-joly]

need to rebase