fix(loaders): coerce combatant level with Pydantic-consistent rules
Feat resolution read level with an isinstance(int) check and fell back to 1, while Pydantic accepts numeric strings and lossless floats: a level-4 sheet authored as "4" got Toughness for level 1. _parse_level now runs the value through TypeAdapter(int) (same lax coercion as the model) and raises CombatantLoadError for values the model would reject.
This commit is contained in:
@@ -19,7 +19,7 @@ import logging
|
|||||||
import re
|
import re
|
||||||
from typing import TYPE_CHECKING
|
from typing import TYPE_CHECKING
|
||||||
|
|
||||||
from pydantic import ValidationError
|
from pydantic import TypeAdapter, ValidationError
|
||||||
|
|
||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
@@ -187,6 +187,25 @@ class CombatantLoadError(Exception):
|
|||||||
self.reason = reason
|
self.reason = reason
|
||||||
|
|
||||||
|
|
||||||
|
_LEVEL_ADAPTER: TypeAdapter[int] = TypeAdapter(int)
|
||||||
|
|
||||||
|
|
||||||
|
def _parse_level(raw_level: Json, path: Path) -> int:
|
||||||
|
"""Coerce the JSON ``level`` with the same lax rules Pydantic applies.
|
||||||
|
|
||||||
|
Feat resolution (Toughness) needs the level before model validation;
|
||||||
|
reading it with a bare ``isinstance(int)`` check disagreed with
|
||||||
|
Pydantic, which also accepts numeric strings ("4") and lossless
|
||||||
|
floats (4.0) -- the feat then silently used level 1 on a level-4
|
||||||
|
sheet. A value Pydantic would reject raises CombatantLoadError here.
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
return _LEVEL_ADAPTER.validate_python(raw_level)
|
||||||
|
except ValidationError as exc:
|
||||||
|
msg = f"level must be an integer, got {raw_level!r}"
|
||||||
|
raise CombatantLoadError(path, msg) from exc
|
||||||
|
|
||||||
|
|
||||||
def _resolve_features(
|
def _resolve_features(
|
||||||
raw_features: list[Json], level: int, path: Path
|
raw_features: list[Json], level: int, path: Path
|
||||||
) -> list[AbilitySpec]:
|
) -> list[AbilitySpec]:
|
||||||
@@ -301,8 +320,7 @@ def load_combatant(path: Path) -> Combatant:
|
|||||||
# Resolve features (list of strings → list of AbilitySpec). A present
|
# Resolve features (list of strings → list of AbilitySpec). A present
|
||||||
# but non-list value (e.g. a bare string) used to be popped and dropped
|
# but non-list value (e.g. a bare string) used to be popped and dropped
|
||||||
# silently, so the combatant lost every feat; it is now a data error.
|
# silently, so the combatant lost every feat; it is now a data error.
|
||||||
raw_level = data.get("level")
|
level = _parse_level(data.get("level", 1), path)
|
||||||
level = raw_level if isinstance(raw_level, int) and not isinstance(raw_level, bool) else 1
|
|
||||||
had_features = "features" in data
|
had_features = "features" in data
|
||||||
raw_features = data.pop("features", None)
|
raw_features = data.pop("features", None)
|
||||||
if had_features and not isinstance(raw_features, list):
|
if had_features and not isinstance(raw_features, list):
|
||||||
|
|||||||
@@ -146,6 +146,28 @@ class TestFeatures:
|
|||||||
c = load_combatant(_write(tmp_path, data))
|
c = load_combatant(_write(tmp_path, data))
|
||||||
assert c.features[0].effects == toughness(7).effects
|
assert c.features[0].effects == toughness(7).effects
|
||||||
|
|
||||||
|
def test_toughness_numeric_string_level(self, tmp_path: Path) -> None:
|
||||||
|
data = _minimal_payload(level="4", features=["Toughness"])
|
||||||
|
c = load_combatant(_write(tmp_path, data))
|
||||||
|
assert c.level == 4
|
||||||
|
assert c.features[0].effects == toughness(4).effects
|
||||||
|
|
||||||
|
def test_toughness_lossless_float_level(self, tmp_path: Path) -> None:
|
||||||
|
data = _minimal_payload(level=4.0, features=["Toughness"])
|
||||||
|
c = load_combatant(_write(tmp_path, data))
|
||||||
|
assert c.level == 4
|
||||||
|
assert c.features[0].effects == toughness(4).effects
|
||||||
|
|
||||||
|
def test_non_numeric_level_raises(self, tmp_path: Path) -> None:
|
||||||
|
data = _minimal_payload(level="abc", features=["Toughness"])
|
||||||
|
with pytest.raises(CombatantLoadError, match="level must be an integer"):
|
||||||
|
load_combatant(_write(tmp_path, data))
|
||||||
|
|
||||||
|
def test_fractional_level_raises(self, tmp_path: Path) -> None:
|
||||||
|
data = _minimal_payload(level=4.5, features=["Toughness"])
|
||||||
|
with pytest.raises(CombatantLoadError, match="level must be an integer"):
|
||||||
|
load_combatant(_write(tmp_path, data))
|
||||||
|
|
||||||
def test_level_default_one(self, tmp_path: Path) -> None:
|
def test_level_default_one(self, tmp_path: Path) -> None:
|
||||||
data = _minimal_payload(level=1, features=["Toughness"])
|
data = _minimal_payload(level=1, features=["Toughness"])
|
||||||
c = load_combatant(_write(tmp_path, data))
|
c = load_combatant(_write(tmp_path, data))
|
||||||
|
|||||||
Reference in New Issue
Block a user