From e0f1d023eebdba05319e4f3d9fd580e9104b8bb8 Mon Sep 17 00:00:00 2001 From: Eva Date: Sun, 31 May 2026 08:09:38 +0700 Subject: [PATCH] fix(charsheet): surface character Spell Save DC + Spell Attack Bonus and class features (#optimizer) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two read-model/display data-depth gaps an optimizer persona flagged as critical in a built-.app playtest. Both are DISPLAY gaps — the engine has the data, the character screen did not surface it. Read-model + viewer only; no engine writes. Bug 1 — Spells tab never showed the character-level Spell Save DC / Spell Attack Bonus. PR #410 resolved per-spell saveDc, but there was no once-at-the-top caster summary. Added _casting_ability/_spell_attack_bonus/_character_spellcasting to viewer/server.py (reusing the #410 DC formula; mirrors engine server.spell_save_dc: DC = 8 + prof + mod, attack = prof + mod), exposed as hero.spellcasting on the character surface, and rendered a SpellcastingHeader at the top of SpellsTab. Non-caster -> None -> header omitted (no fabricated DC 0). Bug 2 — Abilities tab read "No active abilities recorded" for a L3 Wizard. The engine DOES populate class/subclass features as NAMES (Character.features via srd_tables.features_through), already surfaced by the read-model as classFeatures — but AbilitiesTab only read hero.abilities (always []) and class features were only shown on the Feats tab. AbilitiesTab now renders hero.classFeatures + honest class/subclass/level context. Names only — the engine does not model feature DESCRIPTIONS or racial traits, so those stay absent (never fabricated). Tests: viewer/tests/test_charsheet_depth.py (8) — caster DC/attack present, non-caster omitted cleanly, DC/attack track prof+ability, class-feature names surfaced + empty-when-engine-has-none. Full viewer suite: 231 passed. Do NOT close on merge — verify on the next 5-persona sweep (optimizer >=7, 0 critical). --- viewer/openworlds/screen-character.jsx | 48 +++++- viewer/server.py | 59 +++++++- viewer/tests/test_charsheet_depth.py | 196 +++++++++++++++++++++++++ 3 files changed, 290 insertions(+), 13 deletions(-) create mode 100644 viewer/tests/test_charsheet_depth.py diff --git a/viewer/openworlds/screen-character.jsx b/viewer/openworlds/screen-character.jsx index 7c99848d..ea558f4a 100644 --- a/viewer/openworlds/screen-character.jsx +++ b/viewer/openworlds/screen-character.jsx @@ -658,21 +658,59 @@ function ResourcesStatus({ hero }) { ); } +// Title-case a class token ("wizard" -> "Wizard") for the honest class-context line. +function titleCaseWord(s) { + return String(s || "").replace(/\b\w/g, (m) => m.toUpperCase()); +} + function AbilitiesTab({ hero }) { + // `hero.abilities` is reserved for richly-modeled active-ability CARDS (name + detail); + // the engine does not populate those today, so it is empty. But the engine DOES carry the + // character's class/subclass features (as NAMES) in `hero.classFeatures` — those were only + // ever shown on the Feats tab, so the Abilities tab read "No active abilities recorded" for + // a real caster. Surface the class features here too (NAMES only — the engine does not model + // feature descriptions, so we never invent body text), with honest class/level context. const abilities = Array.isArray(hero.abilities) ? hero.abilities : []; const feats = Array.isArray(hero.feats) ? hero.feats : []; + const classFeatures = Array.isArray(hero.classFeatures) ? hero.classFeatures : []; + const classLine = [ + hero.level != null ? `Level ${hero.level}` : null, + hero.class ? titleCaseWord(hero.class) : null, + hero.archetype || null, + ].filter(Boolean).join(" · "); + const nothing = abilities.length === 0 && classFeatures.length === 0; return (
Special Abilities - {abilities.length > 0 ? ( -
+ {abilities.length > 0 && ( +
{abilities.map((a) => ( ))}
- ) : ( + )} + + {/* Class & subclass features the engine granted at this level (Arcane Recovery, a + School-of-Magic feature, etc.). Names come straight from the engine's `features` + list; detail is shown only when the data carries it (the engine models names, not + descriptions today — so most show name-only, never fabricated text). */} + {classFeatures.length > 0 && ( +
+ {classLine &&
{classLine}
} + {classFeatures.map((c) => ( +
+
{c.name}
+ {c.detail &&
{c.detail}
} +
+ ))} +
+ )} + + {nothing && (

- No active abilities recorded — this hero's edge is in their feats and class features. + {classLine + ? `No class, subclass, or racial features are recorded for this ${classLine} yet.` + : "No active abilities recorded — this hero's edge is in their feats and class features."}

)} @@ -1163,4 +1201,4 @@ function FeatsTab({ hero }) { ); } -Object.assign(window, { ScreenCharacter, AbilityScore, StatLine, ResourcesStatus, HeroEquipDoll, equippedStat, AbilitiesTab, SkillsTab, SpellsTab, SpellbookBrowser, SpellSlotTrack, SpellRules, SpellRuleChip, hasSpellRules, LineagePanel, FeatsTab, AbilityCard, FeatRow, RestPrepareModal, RestCard, ProficiencyDot, ProficiencyBadge, portraitScope, spellMeta }); +Object.assign(window, { ScreenCharacter, AbilityScore, StatLine, ResourcesStatus, HeroEquipDoll, equippedStat, AbilitiesTab, SkillsTab, SpellsTab, SpellcastingHeader, SpellbookBrowser, SpellSlotTrack, SpellRules, SpellRuleChip, hasSpellRules, LineagePanel, FeatsTab, AbilityCard, FeatRow, RestPrepareModal, RestCard, ProficiencyDot, ProficiencyBadge, portraitScope, spellMeta }); diff --git a/viewer/server.py b/viewer/server.py index 1f499a96..549622d3 100644 --- a/viewer/server.py +++ b/viewer/server.py @@ -3465,6 +3465,10 @@ def _slot_lvl(k): "stats": stats, "skills": skills, "spells": spells, + # Character-level casting summary (Spell Save DC + Spell Attack Bonus) for the top + # of the Spells tab. None for a non-caster (Fighter/Rogue) — the screen omits it + # rather than show a fake DC. Derived from the PC's casting ability + proficiency. + "spellcasting": _character_spellcasting(ch), "spellSlots": spell_slots, "classResources": class_resources, "conditions": conditions, @@ -3620,19 +3624,26 @@ def _spell_meta(name: str) -> dict: } -def _spell_save_dc(ch: dict) -> int | None: - """A caster's spell save DC = 8 + proficiency + casting-ability modifier, mirroring - engine ``server.spell_save_dc`` read-only from the snapshot. Returns None when the - character has no SRD caster class (a Fighter with stray spell names, an NPC) — we then - omit the DC rather than invent one. Honest: reads only engine-set abilities/prof.""" +def _casting_ability(ch: dict) -> str | None: + """The full ability key (e.g. "intelligence") the character casts with, from their + FIRST SRD caster class (mirror of engine srd_tables._CASTING_ABILITY). Returns None + for a non-caster (Fighter/Rogue/NPC with stray spell names) so callers omit DC/attack + rather than fabricate one. Honest: reads only the engine-set `classes` list.""" classes = ch.get("classes") if isinstance(ch.get("classes"), list) else [] - ability = None for cl in classes: if isinstance(cl, dict): a = _CASTING_ABILITY.get(_text(cl.get("name")).lower()) if a: - ability = a - break + return a + return None + + +def _spell_save_dc(ch: dict) -> int | None: + """A caster's spell save DC = 8 + proficiency + casting-ability modifier, mirroring + engine ``server.spell_save_dc`` read-only from the snapshot. Returns None when the + character has no SRD caster class (a Fighter with stray spell names, an NPC) — we then + omit the DC rather than invent one. Honest: reads only engine-set abilities/prof.""" + ability = _casting_ability(ch) if ability is None: return None abilities = ch.get("abilities") if isinstance(ch.get("abilities"), dict) else {} @@ -3641,6 +3652,38 @@ def _spell_save_dc(ch: dict) -> int | None: return 8 + prof + _ability_mod(abilities.get(ability)) +def _spell_attack_bonus(ch: dict) -> int | None: + """A caster's spell attack bonus = proficiency + casting-ability modifier, mirroring + engine ``server.spell_save_dc``'s `spell_attack_bonus` (server.py: prof + mod). Returns + None for a non-caster (no SRD caster class) so the UI omits it rather than show a fake + +0. Honest: reads only engine-set abilities/prof.""" + ability = _casting_ability(ch) + if ability is None: + return None + abilities = ch.get("abilities") if isinstance(ch.get("abilities"), dict) else {} + prof = _num(ch.get("proficiency_bonus")) + prof = int(prof) if prof is not None else 2 + return prof + _ability_mod(abilities.get(ability)) + + +def _character_spellcasting(ch: dict) -> dict | None: + """Character-level spellcasting summary for the TOP of the Spells tab — the once-at-the-top + Spell Save DC + Spell Attack Bonus a caster needs to plan (the way D&D Beyond shows them), + derived from the PC's spellcasting ability + proficiency. Returns None for a non-caster + (no SRD caster class) so the screen omits the block entirely — an honest Fighter shows + nothing, never a fabricated DC 0. Reuses the same #410 formula helpers (no new math).""" + ability = _casting_ability(ch) + if ability is None: + return None + return { + "ability": ability, + # short SRD code (int/wis/cha) for a compact "INT" badge in the UI + "abilityShort": ability[:3], + "spellSaveDc": _spell_save_dc(ch), + "spellAttackBonus": _spell_attack_bonus(ch), + } + + def _spell_card(name: str, time_label: str, save_dc: int | None) -> dict: """One spell's render card for the heroes screen: the name plus the engine's REAL SRD rules fields (level / school / range / casting time / duration / components / save / damage) diff --git a/viewer/tests/test_charsheet_depth.py b/viewer/tests/test_charsheet_depth.py new file mode 100644 index 00000000..8a60e58f --- /dev/null +++ b/viewer/tests/test_charsheet_depth.py @@ -0,0 +1,196 @@ +"""Character-sheet data-depth surface tests (optimizer-persona gaps). + +Two read-model gaps an optimizer flagged in a built-.app playtest: + + Bug 1 - the Spells tab never exposed the *character-level* Spell Save DC and Spell + Attack Bonus (only per-spell saveDc from PR #410). A caster could not plan. + A non-caster must NOT get a fabricated DC. + + Bug 2 - the Abilities tab read "No active abilities recorded" for a L3 wizard even + though the engine populates class/subclass features as NAMES in + Character.features (already surfaced by the read-model as `classFeatures`). + The fix surfaces those on the character surface so the Abilities tab can + render them; feature DESCRIPTIONS and RACIAL TRAITS are not modeled by the + engine, so they stay absent (never fabricated). + +Mirrors test_readmodel_surfaces.py: load server.py via importlib, drive the real +/character-surface route against a model-conformant snapshot written to a temp state dir. +""" + +import copy +import http.client +import importlib.util +import json +import os +import tempfile +import threading +import unittest +from pathlib import Path + + +_SERVER_PATH = Path(__file__).resolve().parents[1] / "server.py" +_SPEC = importlib.util.spec_from_file_location("viewer_server_charsheet", _SERVER_PATH) +assert _SPEC is not None +server = importlib.util.module_from_spec(_SPEC) +assert _SPEC.loader is not None +_SPEC.loader.exec_module(server) + + +# A model-conformant snapshot: a level-3 evocation Wizard (INT caster) + a level-4 Fighter +# (non-caster). The Wizard carries engine-populated class features (NAMES) in `features`. +_SNAPSHOT = { + "id": "camp_depth", + "title": "The Tower at Dusk", + "world_id": "stolen-marches", + "day": 3, + "party": ["elara", "thornwick"], + "characters": { + "elara": { + "id": "elara", "name": "Elara Moonwhisper", "kind": "player", "race": "High Elf", + "alignment": "Neutral Good", + "classes": [{"name": "Wizard", "level": 3, "subclass": "School of Evocation"}], + "abilities": {"strength": 8, "dexterity": 14, "constitution": 13, + "intelligence": 16, "wisdom": 12, "charisma": 10}, + "proficiency_bonus": 2, "armor_class": 12, "max_hp": 17, "current_hp": 17, + "spell_slots": {"1": {"maximum": 4, "used": 0}, "2": {"maximum": 2, "used": 0}}, + "spells_known": ["Fire Bolt", "Magic Missile", "Shield"], + "spells_prepared": ["Magic Missile", "Scorching Ray"], + # Engine-populated class/subclass feature NAMES (srd_tables.features_through). + "features": ["Arcane Recovery", "Evocation Savant", "Sculpt Spells"], + }, + "thornwick": { + "id": "thornwick", "name": "Thornwick", "kind": "player", "race": "Human", + "alignment": "Lawful Neutral", + "classes": [{"name": "Fighter", "level": 4, "subclass": "Champion"}], + "abilities": {"strength": 16, "dexterity": 12, "constitution": 14, + "intelligence": 10, "wisdom": 11, "charisma": 9}, + "proficiency_bonus": 2, "armor_class": 18, "max_hp": 36, "current_hp": 36, + "features": ["Second Wind", "Action Surge", "Improved Critical"], + }, + }, +} + + +class _QuietHandler(server._Handler): + def log_message(self, fmt: str, *args: object) -> None: + return + + +class CharsheetDepthTests(unittest.TestCase): + # ── direct unit coverage of the new read-model helpers ────────────────────── + + def test_caster_spellcasting_summary(self): + """Wizard L3, INT 16: DC = 8 + prof(2) + int_mod(+3) = 13; attack = prof + mod = +5.""" + cast = server._character_spellcasting(_SNAPSHOT["characters"]["elara"]) + self.assertIsNotNone(cast) + self.assertEqual(cast["ability"], "intelligence") + self.assertEqual(cast["abilityShort"], "int") + self.assertEqual(cast["spellSaveDc"], 13) + self.assertEqual(cast["spellAttackBonus"], 5) + + def test_noncaster_has_no_fabricated_spellcasting(self): + """A Fighter has no SRD caster class -> summary is None (no fake DC/attack).""" + self.assertIsNone(server._character_spellcasting(_SNAPSHOT["characters"]["thornwick"])) + self.assertIsNone(server._spell_save_dc(_SNAPSHOT["characters"]["thornwick"])) + self.assertIsNone(server._spell_attack_bonus(_SNAPSHOT["characters"]["thornwick"])) + + def test_dc_and_attack_track_proficiency_and_ability(self): + """Higher level + ability => higher DC/attack, proving derivation (not hardcoded).""" + higher = copy.deepcopy(_SNAPSHOT["characters"]["elara"]) + higher["classes"][0]["level"] = 5 + higher["proficiency_bonus"] = 3 + higher["abilities"]["intelligence"] = 18 + cast = server._character_spellcasting(higher) + # prof 3, int_mod(18) = +4 -> DC 8+3+4 = 15; attack 3+4 = +7 + self.assertEqual(cast["spellSaveDc"], 15) + self.assertEqual(cast["spellAttackBonus"], 7) + + # ── end-to-end via the real /character-surface route ──────────────────────── + + def setUp(self): + self._tmp = Path(self.enterContext(tempfile.TemporaryDirectory())) + self._old_state = os.environ.get("CLAWDND_STATE_DIR") + os.environ["CLAWDND_STATE_DIR"] = str(self._tmp) + _QuietHandler.campaign_id = "" + _QuietHandler.transcript_path = "" + _QuietHandler.chat_path = "" + _QuietHandler.pinned = False + self._httpd = server.ThreadingHTTPServer(("127.0.0.1", 0), _QuietHandler) + self._thread = threading.Thread(target=self._httpd.serve_forever, daemon=True) + self._thread.start() + self._host, self._port = self._httpd.server_address + + def tearDown(self): + self._httpd.shutdown() + self._httpd.server_close() + self._thread.join(timeout=2) + if self._old_state is None: + os.environ.pop("CLAWDND_STATE_DIR", None) + else: + os.environ["CLAWDND_STATE_DIR"] = self._old_state + + def _write(self, campaign_id: str, payload: dict) -> None: + cdir = self._tmp / "campaigns" / campaign_id + cdir.mkdir(parents=True) + (cdir / "snapshot.json").write_text(json.dumps(payload), encoding="utf-8") + + def _get_json(self, path: str) -> tuple[int, dict]: + conn = http.client.HTTPConnection(self._host, self._port, timeout=5) + try: + conn.request("GET", path) + resp = conn.getresponse() + body = resp.read() + return resp.status, (json.loads(body.decode("utf-8")) if body else {}) + finally: + conn.close() + + def _party(self, surface: dict) -> dict: + return {c["id"]: c for c in surface["party"]} + + def test_surface_exposes_caster_spell_dc_and_attack(self): + self._write("camp_depth", _SNAPSHOT) + status, surface = self._get_json("/character-surface?campaign=camp_depth") + self.assertEqual(status, 200) + elara = self._party(surface)["elara"] + cast = elara["spellcasting"] + self.assertIsNotNone(cast) + self.assertEqual(cast["spellSaveDc"], 13) + self.assertEqual(cast["spellAttackBonus"], 5) + self.assertEqual(cast["abilityShort"], "int") + + def test_surface_omits_spellcasting_for_non_caster(self): + self._write("camp_depth", _SNAPSHOT) + _status, surface = self._get_json("/character-surface?campaign=camp_depth") + thornwick = self._party(surface)["thornwick"] + # Key present for a stable shape, value None -> the Spells tab header omits itself. + self.assertIn("spellcasting", thornwick) + self.assertIsNone(thornwick["spellcasting"]) + + def test_surface_surfaces_engine_class_features(self): + """Bug 2: the engine's `features` NAMES reach the surface (as classFeatures) so the + Abilities tab can render them instead of 'No active abilities recorded'.""" + self._write("camp_depth", _SNAPSHOT) + _status, surface = self._get_json("/character-surface?campaign=camp_depth") + elara = self._party(surface)["elara"] + names = [c["name"] for c in elara["classFeatures"]] + self.assertIn("Arcane Recovery", names) + self.assertIn("Evocation Savant", names) + # subclass (School of Magic) is surfaced as the archetype, so the tab has context + self.assertEqual(elara["archetype"], "School of Evocation") + # honest: the engine models feature NAMES, not descriptions -> detail is empty + arcane = next(c for c in elara["classFeatures"] if c["name"] == "Arcane Recovery") + self.assertEqual(arcane["detail"], "") + + def test_surface_class_features_empty_when_engine_has_none(self): + """A character with no engine-populated features surfaces an empty list (honest), + not fabricated feature text.""" + snap = copy.deepcopy(_SNAPSHOT) + snap["characters"]["elara"]["features"] = [] + self._write("camp_empty", snap) + _status, surface = self._get_json("/character-surface?campaign=camp_empty") + elara = self._party(surface)["elara"] + self.assertEqual(elara["classFeatures"], []) + + +if __name__ == "__main__": + unittest.main()