Skip to content

Split TORCharacterStatsModel health calculation by character category; fix champion HP and start-time crash - #33

Draft
RandyChihuahua wants to merge 1 commit into
next_updatefrom
feature/PromotionHealthFix
Draft

RandyChihuahua wants to merge 1 commit into
next_updatefrom
feature/PromotionHealthFix

Conversation

@RandyChihuahua

Copy link
Copy Markdown
Contributor

Reorganises MaxHitpoints into four character categories — Troop, Player, Companion, Lord —
and fixes two bugs found on the way. Numbers are unchanged everywhere except those two fixes.

Bugs

Necromancer champion doubled instead of dying. The fallback for unreadable career-ability
scaling logged "Champion being set to 1 hp and being left to die", then did
value = ResultNumber - 1; Add(value) — yielding 2R−1, roughly double health. It now replaces
the number outright and returns. The log message was right; the arithmetic was missing a sign.

(bool)(model?.CampaignStartTime.IsNow) throws when the model is null — the ?. yields a null
bool? and the unboxing cast fails, so the null-safe operator caused the crash it looked like it
was preventing.

Why the split is shaped this way

Forest harmony deliberately does not hang off the hero-kind switch. Your own clan's lords can
ride in the main party and are not IsPlayerCompanion, so keying the Asrai block on kind would
have silently stripped their harmony debuff. It stays keyed on party membership.

Race bonuses were duplicated verbatim across the troop and hero paths (IsDwarf(Hero) and
IsDwarf(CharacterObject) are byte-identical) and now apply once for all four categories.
Reordering is safe — AddFactor applies to the accumulated total regardless of insertion point,
and the one order-sensitive read (the champion block) stays last.

Not done, deliberately: a static race→bonus dictionary. FaceGen.GetRaceOrDefault returns 0 for
an unregistered race, so a static initialiser running before races load would throw
TypeInitializationException on a duplicate key.

The IsTreeman() / else if (IsTreeSpirit()) pairing looks like a bug and isn't. tor_we_treeman
carries race="large_humanoid_monster" and the TreeSpirit attribute, so the else is what
keeps treemen at +1000 rather than +1100. Commented in place.

Other behaviour deltas

  • Forest harmony reads Hero.MainHero.GetForestHarmonyLevel() rather than
    PartyBelongedTo.LeaderHero... — same hero in every reachable case, matches how
    ForestHarmonyHelper reads it elsewhere, and removes a null LeaderHero dereference.
  • Oak of Ages was one AddFactor(0.1f) per unlocked upgrade, producing N identical tooltip rows.
    Now one row at 0.1f × count — same factor.
  • Every bare Add(…) gained a description, so the tooltip explains Everchosen, Orion, Tough, the
    troop tier step, the undead penalty and the race bonuses instead of showing an unattributed
    delta. Twelve new tor_stats_* strings.

Testing

Not yet run. No build path on the dev machine resolves this project's PackageReferences (non-SDK
csproj, no VS MSBuild), so a clean build wasn't obtainable; Roslyn binds the file with no
diagnostics of its own. 17 manual scenarios in docs/character-stats-refactor-test-plan.md
scenario 3b (champion fallback now 1 HP) and 13 (clan lord in main party keeps harmony) are the
ones that matter. Save-compatible: no saved fields added or renamed.

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.

1 participant