refactor: centralize structural vocabulary in a Vocabulary module#153
Merged
Conversation
Reserved type IDs and structural key names were raw string literals
scattered across the library — "character_progression" alone appeared
at 12 sites in characters.ex, and the "choice_#{n}" progression choice
ID was constructed in 4 places. Renaming or extending the vocabulary
meant a grep-sweep with no compiler support.
ExTTRPGDev.RuleSystem.Vocabulary now owns every structural name:
reserved type IDs (character_progression / roll / rolling_method), the
structural metadata key set (moved from Loader's attribute), and the
progression decision shape (progression_scope/1 and
progression_choice_id/1). Semantics stay documented in Loader's
Structural Vocabulary section; Vocabulary's moduledoc points there.
Decisions:
- Consumers bind names to module attributes at compile time
(@progression_type Vocabulary.progression_type()) because function
calls are not allowed in pattern-match positions. Single source is
preserved: the literal exists only in Vocabulary.
- The four identical progression-decision constructions (which also
each recomputed the next choice number) collapse into one
next_progression_decision/3 constructor in Characters. A Decision
struct remains #133's territory.
- Loader binds the structural-keys MapSet to an attribute rather than
calling Vocabulary per-call: dialyzer loses MapSet's opaqueness
across the module boundary and rejects the cross-module value in
MapSet.union/2 (call_without_opaque).
- The CLI app's own "character_progression" references are untouched:
it is allowed to be system-aware per the project boundary doc, and
migrating it can ride along with the server handler split (#132).
No behavior change; existing tests pass unchanged.
Closes #125
![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 #125
What
ExTTRPGDev.RuleSystem.Vocabularynow owns every structural name the library defines:progression_type/0,roll_type/0,rolling_method_type/0Loader's module attribute)progression_scope/1andprogression_choice_id/1(with doctests)"character_progression"appeared at 12 sites incharacters.exand"choice_#{n}"at 4 — all now flow through Vocabulary. The four identical decision constructions (each also recomputing the next choice number) collapse into onenext_progression_decision/3constructor. Semantics remain documented in Loader's "Structural Vocabulary" section, which Vocabulary's moduledoc references.Notable decisions
@progression_type Vocabulary.progression_type()) since function calls can't appear in patterns. The literal still exists only in Vocabulary.MapSet's opaqueness across module boundaries and flags the cross-module value inMapSet.union/2. Documented in a comment."character_progression"references stay — the app is allowed to be system-aware, and migrating it can ride along with the handler split (refactor(server): split dispatch into handler modules and standardize error formatting #132).Decisionstruct remains refactor: introduce Node, Effect, and Decision structs #133's territory.Verification
No behavior change: full suite passes unchanged (311 + 55 tests, +2 new Vocabulary doctests), credo and dialyzer clean, grep confirms zero structural literals left in library logic outside
vocabulary.ex.Summary by Bito