Skip to content

fix(bcard): the client spells it "hidden", not "hiden" - #2373

Merged
erwan-joly merged 2 commits into
NosCoreIO:masterfrom
denislauri1999:pr/bcard-hidden-typo
Aug 31, 2026
Merged

fix(bcard): the client spells it "hidden", not "hiden"#2373
erwan-joly merged 2 commits into
NosCoreIO:masterfrom
denislauri1999:pr/bcard-hidden-typo

Conversation

@denislauri1999

@denislauri1999 denislauri1999 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Six BCardEffect members carry the typo Hiden, and documentation/dat/BCard.dat.md repeats it.

The client's own text has the word spelled correctly - "Attacks on hidden enemies", "Damage from hidden enemies", "Reveals hidden enemies within %s spaces" - and the documentation table quotes those sentences two columns away from the misspelled names.

Rename only: no behaviour, no new members, and nothing else in the tree refers to them.

What was tested, and how. The solution builds with 0 errors, and dotnet test test/NosCore.Parser.Tests --filter "FullyQualifiedName~BCardVocabularyTests&TestCategory!=OPTIONAL-TEST" passes - which matters here because EveryNamedRowMatchesItsEffect compares the enum against the documentation table, so a rename applied to only one of the two would fail it.

What was NOT tested. Nothing was played. These are enum members and a documentation table.

Note on ordering. This is stacked on #2372, which adds one more member with the same typo so that it matches its neighbours at the time; this PR then fixes all six together. Reviewing #2372 first keeps the diff here to the rename.

Summary by CodeRabbit

  • Documentation

    • Updated effect references with clearer names and client text for previously unnamed effects.
    • Corrected “Hidden” effect terminology for consistency.
  • Refactor

    • Added identifiers for several negated effects, including focus, skill, damage, visibility, and ability-related effects.
    • Standardized effect naming to improve clarity and consistency.

BCard.dat declares 24 (type, subtype) pairs that BCardEffect does not name. Seven of
them can be read; this adds those seven and leaves the other seventeen alone.

A pair's text lives in LISTn-1 / LISTn-2, where the second number is the high or the low
half. For these seven the high half has a client text and the low half has none, so the
low half is the negation of a sentence that is written down rather than a mechanic that
has to be guessed. For the other seventeen the high half is mute too.

Negated rather than Increase/Decrease because the text decides: where the client says
"increased / reduced" the names say Increase/Decrease (176 pairs), and where one
sentence covers both halves the low one is X + Negated (186 of 194). All seven high
halves describe a capability - "Allows you to use Full Moon skills", "Reveals hidden
enemies within %s spaces" - and a capability is negated, not reduced.

documentation/dat/BCard.dat.md is updated in the same commit so
EveryEffectIsInTheVocabulary and EveryNamedRowMatchesItsEffect stay in step.

Tested: the two mandatory BCardVocabularyTests pass; the opt-in EveryDeclaredEffectIsNamed
goes from 24 unnamed pairs to 17, measured with a rebuild between the two runs.
Six BCardEffect members carry the typo, and the documentation table repeats it. The
client's own text has the word spelled correctly - "Attacks on hidden enemies", "Damage
from hidden enemies", "Reveals hidden enemies within %s spaces" - and BCard.dat.md
quotes those sentences two columns away from the misspelled names.

Rename only. No behaviour, no new members, and nothing else in the tree refers to them.

Tested: the solution builds with 0 errors, and the two mandatory BCardVocabularyTests
pass. Nothing was played - these are enum members and a documentation table.

Stacked on the seven-names PR, which adds one more member with the same typo so that it
matches its neighbours; this commit fixes all six together.
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown

Review Change Stack

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: 5c705f6d-f49c-4b28-a6b7-be3ef496b810

📥 Commits

Reviewing files that changed from the base of the PR and between 671fa07 and b6e6825.

📒 Files selected for processing (2)
  • documentation/dat/BCard.dat.md
  • src/NosCore.Data/Enumerations/Buff/BCardEffect.cs

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


Walkthrough

The BCardEffect enum adds names for previously unnamed negated effects and corrects Type88 spelling. The BCard.dat reference table mirrors these enum changes.

Changes

BCard effect naming

Layer / File(s) Summary
BCardEffect enum updates
src/NosCore.Data/Enumerations/Buff/BCardEffect.cs
Adds negated Type63, Type88, and Type91 members. Renames Type88 members from Hiden to Hidden.
BCard.dat reference alignment
documentation/dat/BCard.dat.md
Adds explicit effect names and applies the corrected Type88 spelling in the reference table.

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

Merge Risk: ⚪ Minimal · up to b6e68

This change corrects misspelled public effect names and their documentation without changing runtime behavior or effect identities. No actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: erwan-joly

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the primary change: correcting the Hiden spelling to Hidden in BCard effects. It is concise and directly related to the documented changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
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.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. (1 skipped: 1 unsupported.)

✨ 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

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

@erwan-joly
erwan-joly merged commit 31a8fad into NosCoreIO:master Aug 31, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants