refactor(characters): unify auto-resolve and tracked resolution appliers#155
Merged
Merged
Conversation
Closes #127. characters.ex carried two parallel families of choice-resolution appliers: apply_auto_{selection,sub_choice,value} used by auto_resolve_pending at generation time, and track_{selection,sub_choice,value}_resolution used by random_resolve_all for the --random-resolve flag. Each pair built the identical decision map, and the dispatching cond and the evaluate/find/recurse loop drivers were copy-pasted. Any change to decision shape had to be made twice. The tracked versions are now the single implementation, renamed resolve_{selection,sub_choice,value}. The two real differences between the families are parameterized on the shared resolve_all/5 driver: - a resolvability predicate (resolvable_at_level_1?/1 for generation, resolvable?/1 for resolve-all) - a value-method fun for value progressions (rolled_value_method/1 always rolls, matching the old apply_auto_value; random_value_method/1 keeps the rolled-or-average coin flip) auto_resolve_pending is now resolve_all with the resolutions list discarded. The auto path computes resolution records it never uses (two concept_metadata lookups per selection); this was deemed cheaper than threading a "skip tracking" flag through the shared code. Alternative considered and rejected: keeping apply_auto_* as thin wrappers over the resolve_* functions. Since both call sites go through the same dispatch cond, the wrappers would have added a layer with no callers other than the shared driver. No behavior change; existing tests pass unchanged.
![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 #127.
What
characters.exhad two parallel families of choice-resolution appliers building identical decision maps:apply_auto_{selection,sub_choice,value}(used byauto_resolve_pendingat generation) andtrack_{selection,sub_choice,value}_resolution(used byrandom_resolve_allfor--random-resolve). The dispatchingcondand the evaluate/find/recurse loop drivers were also copy-pasted.How
resolve_{selection,sub_choice,value}.resolve_all/5, parameterized on the two real differences:resolvable_at_level_1?/1vsresolvable?/1)rolled_value_method/1always rolls, matching the oldapply_auto_value;random_value_method/1keeps the rolled-or-average coin flip)auto_resolve_pending/2is nowresolve_allwith the resolutions list discarded; its public return shape is unchanged.Notes
The auto path now computes resolution records it discards (two
concept_metadatalookups per selection) — cheaper than threading a "skip tracking" flag through the shared code.No behavior change; existing tests pass unchanged (389 tests, credo and dialyzer clean).
Summary by Bito