Skip to content

fix(character): resolve default rolling method from config, not "standard"#150

Merged
QMalcolm merged 1 commit into
mainfrom
qmalcolm--fix-standard-rolling-method-fallback
Jul 19, 2026
Merged

fix(character): resolve default rolling method from config, not "standard"#150
QMalcolm merged 1 commit into
mainfrom
qmalcolm--fix-standard-rolling-method-fallback

Conversation

@QMalcolm

@QMalcolm QMalcolm commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

Closes #123

Why

roll_generated_value fell back to Map.get(rolling_methods, method_id || "standard") — domain vocabulary from dnd_5e_srd's TOML hardcoded in library source, violating the library/config boundary. A second system without a method named "standard" crashed with nil.dice for any methodless generated node; an unknown explicit method id crashed the same way.

What

The config mechanism already existed: rolling methods carry a default flag (dnd's standard sets default = true), parsed by the loader and then ignored by the library. Now:

  • Methodless generated nodes resolve to the method flagged default = true.
  • Unknown method id → clear raise naming the id; methodless node with no default → clear raise explaining the fix. No more nil.dice.

Load-time validation added (warn-and-degrade, matching loader posture):

  • The missing-method warning moved to a post-load pass — a methodless node is now legitimate when a default exists (possibly in another concept file), so the check needs the complete rolling_methods map. It fires only when no default is declared.
  • Unknown method reference on a generated node warns.
  • drop values other than "lowest" warn and are ignored (previously silent).
  • Multiple default = true methods warn as ambiguous.

The "rolling_method" reserved concept type and its keys (name/dice/drop/default) are now documented in the structural-vocabulary moduledoc; it was special-cased in code but missing from the reserved-types list. (Vocabulary constant centralization stays #125.)

Verification

  • grep '"standard"' apps/ex_ttrpg_dev/lib/ — zero hits.
  • 5 new/updated loader tests (no-default warning, default-satisfies-methodless, unknown method, unsupported drop, ambiguous defaults) + 3 new character tests (default-method roll works, unknown-method raise, no-default raise).
  • dnd_5e_srd still loads with zero warnings and generation behaves identically (existing tests unchanged).
  • Full suite (312 + 55), credo, dialyzer, CodeScene delta all pass.

Summary by Bito

  • Introduced a 'rolling_method' concept type to define character generation dice rolling rules, including support for defaults and drop-lowest mechanics.
  • Updated character generation to automatically resolve rolling methods, defaulting to the system-wide default if no method is specified.
  • Added validation and warning logic in the rule loader to detect ambiguous defaults, unsupported drop values, and missing rolling methods.
  • Improved error handling for unknown rolling methods during character generation.

…dard"

roll_generated_value fell back to Map.get(rolling_methods, method_id ||
"standard") — a rolling-method ID authored in dnd_5e_srd's TOML,
hardcoded in library source in violation of the library/config
boundary. A second system without a method named "standard" got a
nil.dice crash for any methodless generated node, and an unknown
explicit method id crashed the same way.

The config mechanism for this already existed: rolling methods carry a
"default" flag (dnd's standard method sets default = true) that the
loader parses and the library then ignored. Methodless generated nodes
now resolve to the method flagged default = true; both failure modes
(unknown method id, methodless node with no default) raise messages
naming the problem instead of nil.dice.

Load-time validation (consistent with the loader's warn-and-degrade
posture for concept-level mistakes):

- the missing-method warning moved from per-node parse time to a
  post-load pass, because a methodless node is now legitimate when a
  default method exists — possibly in a different concept file, so
  the check needs the fully-loaded rolling_methods map. It fires only
  when no default is declared.
- a node referencing an unknown rolling method warns.
- drop values other than "lowest" warn and are ignored (previously
  silently ignored, undocumented).
- multiple default = true methods warn as ambiguous.

Also documents the "rolling_method" reserved concept type and its keys
(name/dice/drop/default) in the structural-vocabulary moduledoc — it
was special-cased in process_type but absent from the reserved-types
list. Full centralization of vocabulary constants remains #125.

Closes #123
@QMalcolm
QMalcolm merged commit a2b8353 into main Jul 19, 2026
5 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.

fix(character): remove hardcoded "standard" rolling-method fallback

1 participant