Make the BCard (type, subtype) pair one key

[erwan-joly]

Draft, to show the shape. Nothing here is load-bearing yet — say the word and I'll split it, rename things, or take the parser side further.

Why the current shape hurts

A subtype only means something next to its type: every type reuses 11/12/21…52. That leaves three problems:

• CardType and AdditionalTypes are two hierarchies held together by an identifier convention nothing checks. Type 61 proves it — CardType has no member for it at all (// 61-80 Missingno = 61, is commented out), so the Type61 enum Name the 19 BCard types nothing could refer to #2324 added paired with nothing and no code could reach it.
• The reading code casts the safety away. sub == (byte)AdditionalTypes.Damage.MeleeIncreased against a Defence card compiles fine — both are 21.
• It forces control flow where a table belongs: switch on type, if/else ladder on subtype.

What this does

The pair is the identity, so it becomes one value — type * 100 + subtype, readable as-is in a debugger (2811 = type 28, subtype 11):

public enum BCardEffect : short
{
    CaptureCaptureAnimal = 2811,
}

public static BCardEffect Effect(this BCardDto card) => (BCardEffect)(card.Type * 100 + card.SubType);

Every name is carried over unchanged from the nested enums, so none of #2324's naming work is lost — 1074 members, AdditionalTypes deleted.

The fold in BattleStatsProvider stops being branches:

[BCardEffect.AttackPowerAllAttacksIncreased] = (t, v) => t.AttackAll += v,
[BCardEffect.AttackPowerAllAttacksDecreased] = (t, v) => t.AttackAll -= v,

foreach (var card in source)
{
    if (CardEffects.TryGetValue(card.Effect(), out var apply))
    {
        apply(totals, ScaleByLevel(card, target.Level));
    }
}

108 lines of switch/else if become 68 table rows — one per effect, and a new effect is a row rather than a branch in the right arm of the right switch. HitQueue, DeathBCardHandler and CaptureService convert the same way; CaptureService's predicate goes from two casts to b.Effect() == BCardEffect.CaptureCaptureAnimal.

The alignment test

documentation/dat/BCard.dat.md records what BCard.dat declares — 1283 effects with the client sentence for each — sitting next to the five parser docs already in that folder. BCardVocabularyTests then checks BCardEffect against it exactly the way LogLanguageTests checks language keys against the resources:

test	direction	runs
EveryEffectIsInTheVocabulary	no member is invented	always
EveryNamedRowMatchesItsEffect	the doc and the enum agree on the value	always
EveryDeclaredEffectIsNamed	every declared effect has a name	OPTIONAL-TEST, like CheckEveryLanguageValueSet

The third one is opt-in for the same reason the translation one is: the vocabulary isn't finished. It fails by design and prints the work left, with the client's own words:

type 28 subtype 21 has no BCardEffect member: Can hunt sheep.
type 12 subtype 51 has no BCardEffect member: Absorb %s%% of your max HP as damage.

What it turns up

All pre-existing, none of it changed here:

• 231 declared effects have no name, across 51 of the 129 types. Type 28 has two of ten — sheep hunting, acorn throwing, pet training experience and the extra-essence chance are all unnamed.
• 22 members are declared by no BCard.dat on record — every member of types 130 and 131. The parser folder's file stops at VNUM 129 (an older copy on disk stops at 115), so either Name the 19 BCard types nothing could refer to #2324's author had a newer client than the one we parse, or those are invented. Worth deciding before anything reads them.
• Type 61's vocabulary is now reachable (Type61*), where before nothing could name it.

Testing

• Builds with 0 warnings; suite green as CI runs it — 984 tests, 2 new. The existing battle tests cover BattleStatsProvider's numbers and pass unchanged, which is what says the fold still folds the same way.
• Not played in game — this is a rename plus a lookup, with no new behaviour.

Deliberately not here

• Naming the 231. Mechanical once the doc exists, but it is a separate read.
• The handler-per-effect split for the behavioural ones, keyed off the same enum via the RegisterAssemblyTypes scan.
• Importing BCard.dat in the parser so the doc regenerates itself; today I generated it once from the parser folder.

[coderabbitai]

Warning

Review limit reached

Next included review available in 7 minutes.

View limit details

Limit 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.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: abcb072e-ec72-4ec7-bc11-d1fbc21e2219

📥 Commits

Reviewing files that changed from the base of the PR and between ce7224b and c4131a3.

📒 Files selected for processing (13)

• documentation/dat/BCard.dat.md
• src/NosCore.Data/Enumerations/Buff/AdditionalTypes.cs
• src/NosCore.Data/Enumerations/Buff/BCardEffect.cs
• src/NosCore.Data/Enumerations/Buff/BCardEffectExtensions.cs
• src/NosCore.GameObject/Messaging/Handlers/Battle/DeathBCardHandler.cs
• src/NosCore.GameObject/Services/BattleService/BattleStatsProvider.cs
• src/NosCore.GameObject/Services/BattleService/CaptureService.cs
• src/NosCore.GameObject/Services/BattleService/HitQueue.cs
• test/NosCore.GameObject.Tests/Services/BattleService/ElementResistanceTests.cs
• test/NosCore.GameObject.Tests/Services/BattleService/EnemyElementResistanceTests.cs
• test/NosCore.GameObject.Tests/Services/BattleService/HitQueueTests.cs
• test/NosCore.GameObject.Tests/Services/EquipmentService/EquipmentStatsServiceTests.cs
• test/NosCore.Parser.Tests/BCardVocabularyTests.cs

---

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.}