refactor: introduce Node, Effect, and Decision structs - #161
Merged
Conversation
Part of #133. DAG nodes were shapeless maps whose schema existed only as the union of what Loader.parse_node produced and what Graph/Evaluator pattern-matched. Adding a node type or field meant auditing every %{type: :...} match by hand, with no compile-time help. ExTTRPGDev.RuleSystem.Node now declares the shape: type (:generated | :accumulator | :formula | :mapping) plus the per-type fields, with a typespec documenting which fields each type uses. Loader constructs %Node{} values; Graph, Evaluator, Characters (level threshold lookups), and Character (generated-value rolling) match on the struct, so a typo'd field is now a KeyError at the construction site rather than a silently non-matching clause. Node values are internal to a loaded system and never persisted, so no serialization concerns apply. Pending-choice entries (%{type: :pending}) are a different, caller-facing shape and deliberately remain maps.
Part of #133. Effects were shapeless maps whose schema existed only as the union of their producers: the Loader emitted %{source, target, value, when}, character effects carried only %{target, value}, and inventory_effects bolted :item_fields on via Map.put — a key the Evaluator then had to read defensively with Map.get(effect, :item_fields, %{}) because no other producer set it. ExTTRPGDev.RuleSystem.Effect declares the full shape, with :item_fields a real field defaulting to %{} and :source nil for effects the character carries directly. The Evaluator destructures %Effect{} in apply_effect instead of Map.get probing; inventory_effects uses map update on a matched %Effect{} so attaching item fields is checked; Loader/Graph/Characters/Character/Advancement construct and match on the struct throughout. Saved-character JSON is unchanged: effects persist as {"target", "value"} and from_json! rebuilds %Effect{} with default source/when/ item_fields, which is exactly what runtime effect construction produces — the existing round-trip test (character_test.exs) covers this. graph_test's repeated loader-data boilerplate is folded into a graph_data/2 helper while updating its literals (the two undefined-target tests tripped the duplication check once touched).
Closes #133. Decisions were %{scope, choice, selection} literals constructed and matched at ~15 sites across the library and CLI, with the JSON encoding knowledge living separately in Character's private serialize_decision/deserialize_decision. Nothing tied the sites together; a field rename would have been a grep hunt. ExTTRPGDev.Characters.Decision declares the shape (with a typespec documenting the scope forms), provides new/3, and owns the JSON encoding as to_json_map/from_json_map — moved verbatim from Character, so the saved-character format is unchanged. All construction and matching sites in Characters, Character, Advancement, and the CLI handlers/serializer now use the struct. A new test pins the exact saved-character JSON shape for decisions and effects (scope "type:id" strings, target "type:id:field" strings), so a future struct change cannot silently alter the on-disk format; the existing round-trip tests continue to cover load/save equality. The struct-literal conversions pushed characters_test.exs over its code-health size threshold, so the self-contained xp_to_next_level/2 describe block moves to test/characters/xp_to_next_level_test.exs, continuing the split-out pattern used for apply_award and resolve_progression_choice tests.
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #133.
What
Three commits, one struct each, replacing the shapeless maps whose schemas existed only as the union of their producers:
RuleSystem.Node—:type(:generated | :accumulator | :formula | :mapping) plus per-type fields, with a typespec documenting which fields each type uses. Loader constructs; Graph/Evaluator/Characters/Character match on the struct. Nodes are never persisted, so no serialization concerns. (Pending-choice entries —%{type: :pending}— are a different, caller-facing shape and deliberately remain maps.)RuleSystem.Effect— full shape declared, withitem_fieldsa real field defaulting to%{}(previously bolted on byinventory_effectsviaMap.putand read defensively by the Evaluator withMap.get(effect, :item_fields, %{})) andsource: nilfor effects the character carries directly. The Evaluator now destructures%Effect{}; attaching item fields is a checked struct update.Characters.Decision— shape +new/3+ JSON encoding asto_json_map/from_json_map, moved verbatim fromCharacter's privateserialize_decision/deserialize_decision. All ~15 construction/matching sites across the library and CLI use the struct.Format safety
Saved-character JSON is byte-identical to before. Beyond the existing round-trip tests (which now assert struct equality end-to-end), a new test pins the exact on-disk shape — decision scope
"type:id"strings, effect target"type:id:field"strings — so a future struct change cannot silently alter saved files.Notes
graph_data/2helper ingraph_test.exs, and thexp_to_next_level/2block moving to its own file (characters_test.exscrossed the 1000-line threshold).--warnings-as-errors(the type checker's struct-update check caught one capture-syntax issue during development), credo, and dialyzer clean.Summary by Bito