From 439593dd77d7203c25a5a3f229e8bcab7113e6e0 Mon Sep 17 00:00:00 2001 From: codeGlaze Date: Sun, 4 Oct 2026 18:05:55 +0000 Subject: [PATCH 1/8] Move the pick walker into a shared picks namespace; add remove-at, put-at orcpub.dnd.e5.picks (cljc) is one engine over a character's stored picks, for the character-rescue work (which must list and remove picks on the server, without the app) and the hidden-pick work (which sets picks aside and puts them back). walk-entries, walk-picks, picks-of and relink-picks move from the browser-only content_reconciliation.cljs, bodies token-identical, as walk (now public), walk-typed, keys-of and relink. Callers updated; no aliases. Their test moves to test/cljc, so it also runs in the JVM suite CI gates on. New: remove-at and put-at, addressing a pick by [selection key, entry key] pairs (the app's option-path format), never by index, so removing one pick does not move another. A multi-pick keeps its vector and its metadata; put-at returns nil when the path to the pick is gone and checks no rule. Tests fail when remove-at drops by position or put-at loses the metadata. entity/flatten-options (the build) and extract-content-keys (Missing Content) are not touched. --- src/cljc/orcpub/dnd/e5/picks.cljc | 108 ++++++++++++++++++ .../orcpub/dnd/e5/content_reconciliation.cljs | 50 +------- src/cljs/orcpub/dnd/e5/events.cljs | 3 +- test/cljc/orcpub/dnd/e5/picks_test.cljc | 95 +++++++++++++++ .../dnd/e5/content_reconciliation_test.cljs | 14 --- 5 files changed, 208 insertions(+), 62 deletions(-) create mode 100644 src/cljc/orcpub/dnd/e5/picks.cljc create mode 100644 test/cljc/orcpub/dnd/e5/picks_test.cljc diff --git a/src/cljc/orcpub/dnd/e5/picks.cljc b/src/cljc/orcpub/dnd/e5/picks.cljc new file mode 100644 index 000000000..956d7fc33 --- /dev/null +++ b/src/cljc/orcpub/dnd/e5/picks.cljc @@ -0,0 +1,108 @@ +(ns orcpub.dnd.e5.picks + "A character's stored picks (the entries under `::entity/options`): walking, reading, rewriting, + removing and putting back. Pure data, shared by the browser and the server. + GOTCHA: never depend on the iteration order of a set or map here; it differs between the JVM + and the browser (`testing-infrastructure.md`)." + (:require [orcpub.entity :as entity] + [orcpub.dnd.e5.library :as library])) + +(defn walk + "`opts` (::entity/options) with `(f path entry)` applied to every chosen entry, where `path` is + the vector of selection keys from the root down to it." + [opts f] + (letfn [(entry [path e] + (if (map? e) + (let [e' (f path e)] + (cond-> e' (map? (::entity/options e')) (update ::entity/options #(walk path %)))) + e)) + (walk [path o] + (into {} (map (fn [[sel v]] + (let [p (conj path sel)] + [sel (if (sequential? v) (mapv #(entry p %) v) (entry p v))]))) + o))] + (if (map? opts) (walk [] opts) opts))) + +(defn- walk-typed + "`opts` (::entity/options) with `f` applied to every pick that could be of `content-type` + (`library/pick-types`); `content-type` nil: every pick." + [opts content-type f] + (walk opts (fn [path e] + (if (or (nil? content-type) (seq (library/pick-types path [content-type]))) + (f e) + e)))) + +(defn keys-of + "Set of `character`'s pick keys that could be of `content-type` (see `walk-typed`)." + [character content-type] + (let [ks (atom #{})] + (walk-typed (::entity/options character) content-type + #(do (when-let [k (::entity/key %)] (swap! ks conj k)) %)) + @ks)) + +(defn relink + "`character` with each pick of `from` that could be of `content-type` rewritten to `to`, as + {:character :rewrote} (see `content-reconciliation/reconcile-former-keys`). `content-type` nil: + every pick." + [character content-type from to] + (let [rewrote (atom []) + opts (walk-typed (::entity/options character) content-type + #(if (= from (::entity/key %)) + (do (swap! rewrote conj {:from from :to to}) (assoc % ::entity/key to)) + %))] + {:character (cond-> character (seq @rewrote) (assoc ::entity/options opts)) + :rewrote @rewrote})) + +;; An address names one pick: [selection key, chosen entry's key] pairs from the root, the format +;; of the app's option paths (`entity/get-entity-path`), e.g. [:class :ranger :levels :level-7]. +;; A key, never an index: removing one pick must not move another's address. + +(defn- entry-index [v k] + (first (keep-indexed (fn [i e] (when (= k (::entity/key e)) i)) v))) + +(defn- options-path + "The `get-in` path to the ::entity/options map holding the last pair of `address`, or nil when + an entry on the way is not stored." + [character address] + (loop [path [::entity/options] [[sel k] & more] (partition 2 address)] + (if (empty? more) + path + (let [v (get-in character (conj path sel)) + step (if (sequential? v) + (when-let [i (entry-index v k)] [sel i]) + (when (= k (::entity/key v)) [sel]))] + (when step (recur (into path (conj step ::entity/options)) more)))))) + +(defn remove-at + "`character` without the pick at `address`, as {:character :removed}; `:removed` is + {:address :entry :multi?}, what `put-at` takes, or nil when nothing is stored there. + A multi-pick keeps its (possibly empty) vector; a single pick's selection is dissoc'd." + [character address] + {:pre [(even? (count address)) (seq address)]} + (let [sel (nth address (- (count address) 2)) + k (last address) + opath (options-path character address) + v (when opath (get-in character (conj opath sel)))] + (cond + (and (sequential? v) (entry-index v k)) + {:character (assoc-in character (conj opath sel) + (with-meta (vec (remove #(= k (::entity/key %)) v)) (meta v))) + :removed {:address address :entry (nth v (entry-index v k)) :multi? true}} + + (and (map? v) (= k (::entity/key v))) + {:character (update-in character opath dissoc sel) + :removed {:address address :entry v :multi? false}} + + :else {:character character :removed nil}))) + +(defn put-at + "`character` with `removed` (from `remove-at`) stored again at its address, or nil when an entry + on the way to it is no longer stored. A multi-pick gets it appended; a single pick is replaced. + GOTCHA: checks no rule; whether it still fits is the caller's question." + [character {:keys [address entry multi?]}] + (let [sel (nth address (- (count address) 2)) + opath (options-path character address)] + (when opath + (if multi? + (update-in character (conj opath sel) + (fn [v] (if (sequential? v) (with-meta (conj (vec v) entry) (meta v)) [entry]))) + (assoc-in character (conj opath sel) entry))))) diff --git a/src/cljs/orcpub/dnd/e5/content_reconciliation.cljs b/src/cljs/orcpub/dnd/e5/content_reconciliation.cljs index 4cb463cfb..52b5f7e6a 100644 --- a/src/cljs/orcpub/dnd/e5/content_reconciliation.cljs +++ b/src/cljs/orcpub/dnd/e5/content_reconciliation.cljs @@ -9,6 +9,7 @@ [orcpub.template :as t] [orcpub.dnd.e5.library-links :as links] [orcpub.dnd.e5.library :as library] + [orcpub.dnd.e5.picks :as picks] [orcpub.dnd.e5.classes :as class5e])) ;; ============================================================================ @@ -445,51 +446,6 @@ {:flat (former-key-index plugins offered) :typed (typed-former-key-index plugins offered-by-type)}) -(defn- walk-entries - "`opts` (::entity/options) with `(f path entry)` applied to every chosen entry, where `path` is - the vector of selection keys from the root down to it." - [opts f] - (letfn [(entry [path e] - (if (map? e) - (let [e' (f path e)] - (cond-> e' (map? (::entity/options e')) (update ::entity/options #(walk path %)))) - e)) - (walk [path o] - (into {} (map (fn [[sel v]] - (let [p (conj path sel)] - [sel (if (sequential? v) (mapv #(entry p %) v) (entry p v))]))) - o))] - (if (map? opts) (walk [] opts) opts))) - -(defn- walk-picks - "`opts` (::entity/options) with `f` applied to every pick that could be of `content-type` - (`library/pick-types`); `content-type` nil: every pick." - [opts content-type f] - (walk-entries opts (fn [path e] - (if (or (nil? content-type) (seq (library/pick-types path [content-type]))) - (f e) - e)))) - -(defn picks-of - "Set of `character`'s pick keys that could be of `content-type` (see `walk-picks`)." - [character content-type] - (let [ks (atom #{})] - (walk-picks (::entity/options character) content-type - #(do (when-let [k (::entity/key %)] (swap! ks conj k)) %)) - @ks)) - -(defn relink-picks - "`character` with each pick of `from` that could be of `content-type` rewritten to `to`, as - {:character :rewrote} (see `reconcile-former-keys`). `content-type` nil: every pick." - [character content-type from to] - (let [rewrote (atom []) - opts (walk-picks (::entity/options character) content-type - #(if (= from (::entity/key %)) - (do (swap! rewrote conj {:from from :to to}) (assoc % ::entity/key to)) - %))] - {:character (cond-> character (seq @rewrote) (assoc ::entity/options opts)) - :rewrote @rewrote})) - (defn relink-to-ask "The index into `relinks` (db/pending-relinks) of the first rename to ask `character` about, or nil: a saved character not yet asked, holding the renamed item's old key as a pick of its type, @@ -500,7 +456,7 @@ (when id (first (keep-indexed (fn [i {:keys [content-type from to asked]}] (when (and (not (contains? asked id)) - (contains? (picks-of character content-type) from) + (contains? (picks/keys-of character content-type) from) (holds? content-type to) (holds? content-type from)) i)) relinks))))) @@ -514,7 +470,7 @@ (if (or (and (empty? flat) (every? empty? (vals typed))) (nil? (::entity/options character))) {:character character :rewrote []} (let [rewrote (atom []) - opts (walk-entries + opts (picks/walk (::entity/options character) (fn [path e] (let [t (library/pick-homes path) diff --git a/src/cljs/orcpub/dnd/e5/events.cljs b/src/cljs/orcpub/dnd/e5/events.cljs index cf414cadc..905b90f35 100644 --- a/src/cljs/orcpub/dnd/e5/events.cljs +++ b/src/cljs/orcpub/dnd/e5/events.cljs @@ -94,6 +94,7 @@ default-subclass]] [orcpub.dnd.e5.autosave-fx :as autosave-fx] [orcpub.dnd.e5.library :as library] + [orcpub.dnd.e5.picks :as picks] [orcpub.dnd.e5.event-utils :as event-utils] [orcpub.dnd.e5.compute :as compute] [re-frame.core :refer [reg-event-db reg-event-fx reg-fx inject-cofx path @@ -2066,7 +2067,7 @@ ;; rewrite to picks of that type; a race and a subrace can share a key. Goes through ;; :set-character, so the heals and the rebuild run as on any load. (let [{:keys [character]} - (content-recon/relink-picks (:character db) content-type from-key to-key)] + (picks/relink (:character db) content-type from-key to-key)] {:dispatch-n [[:set-character character] [:show-message (str "Relinked " (name from-key) " to " (name to-key) diff --git a/test/cljc/orcpub/dnd/e5/picks_test.cljc b/test/cljc/orcpub/dnd/e5/picks_test.cljc new file mode 100644 index 000000000..511567647 --- /dev/null +++ b/test/cljc/orcpub/dnd/e5/picks_test.cljc @@ -0,0 +1,95 @@ +(ns orcpub.dnd.e5.picks-test + (:require [clojure.test :refer [deftest testing is]] + [orcpub.entity :as entity] + [orcpub.dnd.e5.picks :as picks])) + +(def ranger + "A ranger 7 / fighter 1 with a race, a multi-pick skill list and a single pick at level 7." + {:db/id 7 + ::entity/options + {:race {::entity/key :elf ::entity/options {:subrace {::entity/key :wood-elf}}} + :class [{::entity/key :ranger + ::entity/options + {:levels [{::entity/key :level-7 + ::entity/options {:defensive-tactics {::entity/key :steel-will}}}] + :skill-proficiency (with-meta [{::entity/key :athletics} + {::entity/key :insight} + {::entity/key :stealth}] + {:kept true})}} + {::entity/key :fighter + ::entity/options {:fighting-style [{::entity/key :defense}]}}]}}) + +(def steel-will [:class :ranger :levels :level-7 :defensive-tactics :steel-will]) +(def insight [:class :ranger :skill-proficiency :insight]) + +(deftest relinking-a-race-leaves-a-subrace-with-the-same-key-alone + (let [character {:db/id 7 + ::entity/options {:race {::entity/key :aarakocra + ::entity/options {:subrace {::entity/key :aarakocra}}}}} + {:keys [character rewrote]} (picks/relink character :orcpub.dnd.e5/races :aarakocra :aarakocra-2)] + (is (= :aarakocra-2 (get-in character [::entity/options :race ::entity/key]))) + (is (= :aarakocra (get-in character [::entity/options :race ::entity/options :subrace ::entity/key]))) + (is (= 1 (count rewrote)))) + (let [character {::entity/options {:race {::entity/key :bird + ::entity/options {:subrace {::entity/key :aarakocra}}}}}] + (is (= #{:aarakocra} (picks/keys-of character :orcpub.dnd.e5/subraces))) + (is (= #{:bird} (picks/keys-of character :orcpub.dnd.e5/races)) + "a subrace pick does not make the question about the race"))) + +(deftest walk-visits-every-pick-with-its-selection-path + (let [seen (atom [])] + (picks/walk (::entity/options ranger) (fn [path e] (swap! seen conj [path (::entity/key e)]) e)) + (is (= #{[[:race] :elf] [[:race :subrace] :wood-elf] [[:class] :ranger] [[:class] :fighter] + [[:class :levels] :level-7] [[:class :levels :defensive-tactics] :steel-will] + [[:class :skill-proficiency] :athletics] [[:class :skill-proficiency] :insight] + [[:class :skill-proficiency] :stealth] [[:class :fighting-style] :defense]} + (set @seen))) + (is (= 10 (count @seen)) "each pick once"))) + +(deftest remove-at-takes-out-exactly-one-pick + (testing "a single pick: its selection goes" + (let [{:keys [character removed]} (picks/remove-at ranger steel-will)] + (is (= {:address steel-will :entry {::entity/key :steel-will} :multi? false} removed)) + (is (not (contains? (get-in character [::entity/options :class 0 ::entity/options :levels 0 + ::entity/options]) + :defensive-tactics))))) + (testing "a multi-pick: by key, the others keep their order and the vector its metadata" + (let [{:keys [character removed]} (picks/remove-at ranger insight) + v (get-in character [::entity/options :class 0 ::entity/options :skill-proficiency])] + (is (= [:athletics :stealth] (map ::entity/key v))) + (is (= {:kept true} (meta v))) + (is (true? (:multi? removed))))) + (testing "the same key in another class is a different address" + (let [{:keys [removed]} (picks/remove-at ranger [:class :fighter :fighting-style :defense])] + (is (= {::entity/key :defense} (:entry removed))))) + (testing "nothing stored there: the character is unchanged" + (doseq [address [[:class :ranger :skill-proficiency :perception] + [:class :wizard :skill-proficiency :insight] + [:feats :alert]]] + (let [{:keys [character removed]} (picks/remove-at ranger address)] + (is (nil? removed) (str address)) + (is (= ranger character) (str address))))) + (testing "everything else is untouched" + (let [{:keys [character]} (picks/remove-at ranger steel-will)] + (is (= (disj (picks/keys-of ranger nil) :steel-will) (picks/keys-of character nil)))))) + +(deftest put-at-reverses-remove-at + (testing "a single pick comes back as it was" + (let [{:keys [character removed]} (picks/remove-at ranger steel-will)] + (is (= ranger (picks/put-at character removed))))) + (testing "a multi-pick comes back at the end, metadata kept" + (let [{:keys [character removed]} (picks/remove-at ranger insight) + back (picks/put-at character removed) + v (get-in back [::entity/options :class 0 ::entity/options :skill-proficiency])] + (is (= [:athletics :stealth :insight] (map ::entity/key v))) + (is (= {:kept true} (meta v))))) + (testing "into an emptied multi-pick" + (let [{:keys [character removed]} (picks/remove-at ranger [:class :fighter :fighting-style :defense])] + (is (= [] (get-in character [::entity/options :class 1 ::entity/options :fighting-style]))) + (is (= [{::entity/key :defense}] + (get-in (picks/put-at character removed) + [::entity/options :class 1 ::entity/options :fighting-style]))))) + (testing "nowhere to go: nil" + (let [{:keys [removed]} (picks/remove-at ranger steel-will) + no-ranger (assoc-in ranger [::entity/options :class] [(get-in ranger [::entity/options :class 1])])] + (is (nil? (picks/put-at no-ranger removed)))))) diff --git a/test/cljs/orcpub/dnd/e5/content_reconciliation_test.cljs b/test/cljs/orcpub/dnd/e5/content_reconciliation_test.cljs index aa8268fd2..4cb2899b1 100644 --- a/test/cljs/orcpub/dnd/e5/content_reconciliation_test.cljs +++ b/test/cljs/orcpub/dnd/e5/content_reconciliation_test.cljs @@ -724,20 +724,6 @@ (is (= [{:key :tidecall :content-label "Spell"}] (map #(select-keys % [:key :content-label]) (:items report)))))) -(deftest relinking-a-race-leaves-a-subrace-with-the-same-key-alone - (let [character {:db/id 7 - ::entity/options {:race {::entity/key :aarakocra - ::entity/options {:subrace {::entity/key :aarakocra}}}}} - {:keys [character rewrote]} (reconcile/relink-picks character :orcpub.dnd.e5/races :aarakocra :aarakocra-2)] - (is (= :aarakocra-2 (get-in character [::entity/options :race ::entity/key]))) - (is (= :aarakocra (get-in character [::entity/options :race ::entity/options :subrace ::entity/key]))) - (is (= 1 (count rewrote)))) - (let [character {::entity/options {:race {::entity/key :bird - ::entity/options {:subrace {::entity/key :aarakocra}}}}}] - (is (= #{:aarakocra} (reconcile/picks-of character :orcpub.dnd.e5/subraces))) - (is (= #{:bird} (reconcile/picks-of character :orcpub.dnd.e5/races)) - "a subrace pick does not make the question about the race"))) - (deftest a-background-picked-by-its-name-heals-to-its-stored-key (let [plugins {"Pak" {:orcpub.dnd.e5/backgrounds {:folk-hero-pk {:name "Folk Hero" :key :folk-hero-pk}}}}] (is (= :folk-hero-pk (get (reconcile/former-key-index plugins #{}) :folk-hero))) From 88d06f5d3f48d5d1ce266fdc0557f54667b53872 Mon Sep 17 00:00:00 2001 From: codeGlaze Date: Sun, 4 Oct 2026 18:05:55 +0000 Subject: [PATCH 2/8] Branch changelog for refactor/picks-namespace --- docs/branch-changelog.md | 46 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 docs/branch-changelog.md diff --git a/docs/branch-changelog.md b/docs/branch-changelog.md new file mode 100644 index 000000000..e38cf4988 --- /dev/null +++ b/docs/branch-changelog.md @@ -0,0 +1,46 @@ + + +# Branch changelog — `refactor/picks-namespace` + + + +## Why this branch exists + + + +One engine over a character's stored picks, shared by the character-rescue work (which must list +and remove picks on the server, without the app) and the hidden-pick work (which moves picks that +stop applying out of the character and back). Three walkers over stored picks already exist; this +moves the heals' one into shared code unchanged and adds the two operations both pieces need, +rather than writing a fourth. The build's walker and the Missing Content walker are not touched. + +## Added + +- **Remove and put back one stored pick by its address** — `picks/remove-at` and `picks/put-at`, the base for repairing a character and for setting picks aside (`439593dd`). + +## Fixed + +## Changed + +- **The pick walker moved into shared code** — `walk-entries`, `walk-picks`, `picks-of` and `relink-picks` now live in `orcpub.dnd.e5.picks` (as `walk`, `walk-typed`, `keys-of`, `relink`), unchanged, so the server can use them and their tests run in CI (`439593dd`). From b33c765ac7af440c1365db58c9e1db6cd77469ce Mon Sep 17 00:00:00 2001 From: codeGlaze Date: Sun, 4 Oct 2026 18:51:40 +0000 Subject: [PATCH 3/8] picks: name the shape flag :multiselect?; document the storage shapes The removed-pick record's flag is now :multiselect?, the template's own word (::t/multiselect?) for a selection that stores a vector. The namespace and remove-at/put-at docstrings state the shapes: a multiselect keeps its vector, emptied by its last pick as unticking does; a single pick's selection is dissoc'd; the flag tells put-at which, since neither shows it afterwards. --- src/cljc/orcpub/dnd/e5/picks.cljc | 20 ++++++++++++-------- test/cljc/orcpub/dnd/e5/picks_test.cljc | 4 ++-- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/cljc/orcpub/dnd/e5/picks.cljc b/src/cljc/orcpub/dnd/e5/picks.cljc index 956d7fc33..da4e2b12a 100644 --- a/src/cljc/orcpub/dnd/e5/picks.cljc +++ b/src/cljc/orcpub/dnd/e5/picks.cljc @@ -1,6 +1,8 @@ (ns orcpub.dnd.e5.picks "A character's stored picks (the entries under `::entity/options`): walking, reading, rewriting, - removing and putting back. Pure data, shared by the browser and the server. + removing and putting back. Pure data, shared by the browser and the server. A selection stores + its picks as a vector when it is `::t/multiselect?` (kept, empty, after its last pick goes), + otherwise as the one entry map; an absent selection was never chosen. GOTCHA: never depend on the iteration order of a set or map here; it differs between the JVM and the browser (`testing-infrastructure.md`)." (:require [orcpub.entity :as entity] @@ -74,8 +76,9 @@ (defn remove-at "`character` without the pick at `address`, as {:character :removed}; `:removed` is - {:address :entry :multi?}, what `put-at` takes, or nil when nothing is stored there. - A multi-pick keeps its (possibly empty) vector; a single pick's selection is dissoc'd." + {:address :entry :multiselect?}, what `put-at` takes, or nil when nothing is stored there. + A multiselect keeps its vector, emptied if this was its last pick, as unticking does; a single + pick's selection is dissoc'd. `:multiselect?` records which, since neither shows it after." [character address] {:pre [(even? (count address)) (seq address)]} (let [sel (nth address (- (count address) 2)) @@ -86,23 +89,24 @@ (and (sequential? v) (entry-index v k)) {:character (assoc-in character (conj opath sel) (with-meta (vec (remove #(= k (::entity/key %)) v)) (meta v))) - :removed {:address address :entry (nth v (entry-index v k)) :multi? true}} + :removed {:address address :entry (nth v (entry-index v k)) :multiselect? true}} (and (map? v) (= k (::entity/key v))) {:character (update-in character opath dissoc sel) - :removed {:address address :entry v :multi? false}} + :removed {:address address :entry v :multiselect? false}} :else {:character character :removed nil}))) (defn put-at "`character` with `removed` (from `remove-at`) stored again at its address, or nil when an entry - on the way to it is no longer stored. A multi-pick gets it appended; a single pick is replaced. + on the way to it is no longer stored. `:multiselect?` true appends it to the selection's vector + (creating one); false makes it the selection's entry, replacing any there. GOTCHA: checks no rule; whether it still fits is the caller's question." - [character {:keys [address entry multi?]}] + [character {:keys [address entry multiselect?]}] (let [sel (nth address (- (count address) 2)) opath (options-path character address)] (when opath - (if multi? + (if multiselect? (update-in character (conj opath sel) (fn [v] (if (sequential? v) (with-meta (conj (vec v) entry) (meta v)) [entry]))) (assoc-in character (conj opath sel) entry))))) diff --git a/test/cljc/orcpub/dnd/e5/picks_test.cljc b/test/cljc/orcpub/dnd/e5/picks_test.cljc index 511567647..2cd430b7b 100644 --- a/test/cljc/orcpub/dnd/e5/picks_test.cljc +++ b/test/cljc/orcpub/dnd/e5/picks_test.cljc @@ -49,7 +49,7 @@ (deftest remove-at-takes-out-exactly-one-pick (testing "a single pick: its selection goes" (let [{:keys [character removed]} (picks/remove-at ranger steel-will)] - (is (= {:address steel-will :entry {::entity/key :steel-will} :multi? false} removed)) + (is (= {:address steel-will :entry {::entity/key :steel-will} :multiselect? false} removed)) (is (not (contains? (get-in character [::entity/options :class 0 ::entity/options :levels 0 ::entity/options]) :defensive-tactics))))) @@ -58,7 +58,7 @@ v (get-in character [::entity/options :class 0 ::entity/options :skill-proficiency])] (is (= [:athletics :stealth] (map ::entity/key v))) (is (= {:kept true} (meta v))) - (is (true? (:multi? removed))))) + (is (true? (:multiselect? removed))))) (testing "the same key in another class is a different address" (let [{:keys [removed]} (picks/remove-at ranger [:class :fighter :fighting-style :defense])] (is (= {::entity/key :defense} (:entry removed))))) From ad3c612b1239cac689aaf019a90e20e426a48cd0 Mon Sep 17 00:00:00 2001 From: codeGlaze Date: Sun, 4 Oct 2026 18:51:40 +0000 Subject: [PATCH 4/8] Clear integration's 50 lint warnings and 3 infos Behaviour-preserving only: - locals that shadowed core or library vars renamed for what they hold: old/new pairs -> before/after (library, library_links, e5 re-key), after -> plugins-after (homebrew and pack deletion), var -> env-var, get -> lookup, integer? -> integer-setting? (config report), import -> imported-pack, choice-tags -> tags-by-selection, and test locals; map keys unchanged ({env-var :var ...}); - unused refers and imports removed; the seven in equipment_subs were used only by the commented-out ORPHANED remote-item block and come back with it; - three nested lets merged, two redundant str calls and a duplicate require dropped, export_paths_test refers clojure.test explicitly. lein lint: 0 errors, 0 warnings. --- src/clj/orcpub/config.clj | 35 +++++---- src/clj/orcpub/pdf.clj | 2 +- src/clj/orcpub/styles/core.clj | 2 +- src/cljc/orcpub/dnd/e5/library.cljc | 54 ++++++------- src/cljc/orcpub/dnd/e5/library_links.cljc | 20 ++--- src/cljs/orcpub/character_builder.cljs | 4 +- src/cljs/orcpub/dnd/e5.cljc | 6 +- src/cljs/orcpub/dnd/e5/autosave_fx.cljs | 2 +- src/cljs/orcpub/dnd/e5/equipment_subs.cljs | 9 +-- src/cljs/orcpub/dnd/e5/events.cljs | 77 +++++++++---------- test/clj/orcpub/config_report_test.clj | 6 +- test/clj/orcpub/export_paths_test.clj | 2 +- test/cljc/orcpub/dnd/e5/library_test.cljc | 8 +- .../cljc/orcpub/dnd/e5/share_bundle_test.cljc | 8 +- .../dnd/e5/homebrew_save_lifecycle_test.cljs | 17 ++-- 15 files changed, 124 insertions(+), 128 deletions(-) diff --git a/src/clj/orcpub/config.clj b/src/clj/orcpub/config.clj index 5ce767099..310464fbf 100644 --- a/src/clj/orcpub/config.clj +++ b/src/clj/orcpub/config.clj @@ -262,16 +262,17 @@ change was picked up, ignored as a typo, or never set. Secret values are dropped here, not at print time, so nothing downstream can leak one by accident." [] - (for [{:keys [var get secret? redact? critical? note group fix]} settings] - (let [raw (env-raw var) - integer? (some? (some #(= var (:var %)) tunables)) - parsed (when (and raw integer?) - (try (Integer/parseInt (str/trim raw)) (catch NumberFormatException _ nil))) - ignored? (boolean (and raw integer? (not (and parsed (pos? parsed))))) - value (cond secret? nil - get (get) - :else raw)] - {:var var :group group :note note :critical? critical? :fix fix + (for [{env-var :var lookup :get :keys [secret? redact? critical? note group fix]} settings] + (let [raw (env-raw env-var) + integer-setting? (some? (some #(= env-var (:var %)) tunables)) + parsed (when (and raw integer-setting?) + (try (Integer/parseInt (str/trim raw)) + (catch NumberFormatException _ nil))) + ignored? (boolean (and raw integer-setting? (not (and parsed (pos? parsed))))) + value (cond secret? nil + lookup (lookup) + :else raw)] + {:var env-var :group group :note note :critical? critical? :fix fix :value (if (and value redact?) (redact-secrets value) value) :secret? secret? :raw (when-not secret? raw) @@ -289,10 +290,10 @@ ([rows actual] (let [;; A secret is reported as present or absent and NEVER by value. `-` where we have ;; no number at all, so the VALUE column never argues with SOURCE beside it. - val (fn [{:keys [var value secret? set?]}] + val (fn [{env-var :var :keys [value secret? set?]}] (cond secret? (if set? "set" "NOT SET") value (str value) - (get actual var) (str (get actual var)) + (get actual env-var) (str (get actual env-var)) :else "-")) ;; A secret's VALUE column already says set / NOT SET; repeating DEFAULT beside it ;; says nothing, and "DEFAULT" next to "NOT SET" reads as though there is a @@ -316,19 +317,19 @@ (cons (str " [" (str/upper-case (or (:group (first gr)) "other")) "]") (map #(trim (format fmt (:var %) (val %) (source %) (or (:note %) ""))) gr))) (partition-by :group rows)) - [(str " [BRANDING]") + [" [BRANDING]" (format " %s %s of %s set" (apply str (repeat w " ")) brand (count branding-vars))] (when-let [bad (seq (filter :ignored? rows))] (cons rule - (for [{:keys [var raw]} bad] + (for [{env-var :var :keys [raw]} bad] (format " (!) %s=%s was ignored: not a positive integer. The default above is in use." - var raw)))) + env-var raw)))) ;; A missing SIGNATURE means every login fails. That is not a row to be spotted. ;; Say what broke AND how to fix it. A boot line that names a symptom and leaves the ;; remedy to be guessed just moves the work. (when-let [miss (seq (filter #(and (:critical? %) (not (:set? %))) rows))] - (mapcat (fn [{:keys [var note fix]}] - (let [body (str/split-lines (or fix (str var " is NOT SET -- " (or note "required"))))] + (mapcat (fn [{env-var :var :keys [note fix]}] + (let [body (str/split-lines (or fix (str env-var " is NOT SET -- " (or note "required"))))] (cons rule ;; Continuations line up under the first line's text, not under the ;; marker, so the block reads as one paragraph. diff --git a/src/clj/orcpub/pdf.clj b/src/clj/orcpub/pdf.clj index 66cdd9b20..023111002 100644 --- a/src/clj/orcpub/pdf.clj +++ b/src/clj/orcpub/pdf.clj @@ -21,7 +21,7 @@ PDAppearanceStream) (org.apache.pdfbox.pdmodel.common PDRectangle) (org.apache.pdfbox.cos COSName COSDictionary) - (org.apache.pdfbox.pdmodel PDPage PDDocument PDPageContentStream PDResources) + (org.apache.pdfbox.pdmodel PDPage PDPageContentStream PDResources) ;; APPEND appends operators to a page's existing content stream, so ;; template artwork survives. The alternative overwrites it. (org.apache.pdfbox.pdmodel PDPageContentStream$AppendMode) diff --git a/src/clj/orcpub/styles/core.clj b/src/clj/orcpub/styles/core.clj index d08f666b7..cec5a3c12 100644 --- a/src/clj/orcpub/styles/core.clj +++ b/src/clj/orcpub/styles/core.clj @@ -804,7 +804,7 @@ ;; The outer shadow is a low glow, not a selected-field highlight. {:position :relative :border-color :transparent - :box-shadow (str "0 0 14px rgba(240, 161, 0, 0.13)")} + :box-shadow "0 0 14px rgba(240, 161, 0, 0.13)"} [:&:before {:content "''" :position :absolute diff --git a/src/cljc/orcpub/dnd/e5/library.cljc b/src/cljc/orcpub/dnd/e5/library.cljc index 4b8a72b0e..94bd26e3f 100644 --- a/src/cljc/orcpub/dnd/e5/library.cljc +++ b/src/cljc/orcpub/dnd/e5/library.cljc @@ -45,12 +45,12 @@ {} (content-groups plugins))) (defn broken-links - "Links in `new` that name an item `old` held and `new` does not, as + "Links in `after` that name an item `before` held and `after` does not, as [{:source :type :key :name :target :target-type}]. Links to a [type key] in `retargeting` are being moved elsewhere and are not counted." - [old new retargeting] - (let [was (held old) now (held new)] - (for [[src ct items] (content-groups new) + [before after retargeting] + (let [was (held before) now (held after)] + (for [[src ct items] (content-groups after) [k item] items :when (map? item) link links/links @@ -63,13 +63,13 @@ {:source src :type ct :key k :name (:name item) :target target :target-type to}))) (defn- invalid-keys - "[source type key] of every item `new` stores under a key the loader would set aside, that - `old` did not already hold there." - [old new] - (for [[src ct items] (content-groups new) + "[source type key] of every item `after` stores under a key the loader would set aside, that + `before` did not already hold there." + [before after] + (for [[src ct items] (content-groups after) k (keys items) :when (and (not (common/keyword-starts-with-letter? k)) - (not (contains? (get-in old [src ct]) k)))] + (not (contains? (get-in before [src ct]) k)))] [src ct k])) (def ^:private built-in-names @@ -77,15 +77,15 @@ {:orcpub.dnd.e5/languages (into #{} (map :name) langs/languages)}) (defn- follow-renamed-names - "`new` with each `:by :name` link naming an item `old` knew by another name renamed to match, - when nothing in `new` or built-in content still answers to the old name." - [old new] + "`after` with each `:by :name` link naming an item `before` knew by another name renamed to match, + when nothing in `after` or built-in content still answers to the old name." + [before after] (let [names (reduce (fn [acc [_ ct items]] (update acc ct (fnil into #{}) (keep :name) (filter map? (vals items)))) - built-in-names (content-groups new)) - renames (for [[src ct items] (content-groups new) + built-in-names (content-groups after)) + renames (for [[src ct items] (content-groups after) [k item] items - :let [was (get-in old [src ct k :name])] + :let [was (get-in before [src ct k :name])] :when (and (string? was) (string? (:name item)) (not= was (:name item)) (not (contains? (get names ct) was)))] [ct was (:name item)]) @@ -100,27 +100,27 @@ item name-links))) {} items))) p (content-groups p))) - new renames))) + after renames))) (defn commit - "What storing `new` in place of `old` does: {:plugins (normalize new) :broken [...]}, or + "What storing `after` in place of `before` does: {:plugins (normalize after) :broken [...]}, or {:refused {:broken [...] :invalid [...]}} when it would add a key the loader sets aside or leave a link pointing at nothing. `deleting?` lets a deliberate removal through and reports the links it broke; `retargeting` is a collection of [type key] whose links are being moved; `restoring?` lets keys the loader sets aside through (it sets them aside again)." - [old new {:keys [deleting? retargeting restoring?]}] - (let [stored (follow-renamed-names old (normalize new)) - broken (vec (broken-links old stored (set retargeting))) - invalid (if restoring? [] (vec (invalid-keys old stored)))] + [before after {:keys [deleting? retargeting restoring?]}] + (let [stored (follow-renamed-names before (normalize after)) + broken (vec (broken-links before stored (set retargeting))) + invalid (if restoring? [] (vec (invalid-keys before stored)))] (if (or (seq invalid) (and (seq broken) (not deleting?))) {:refused {:broken broken :invalid invalid}} {:plugins stored :broken broken}))) (defn overwritten - "Items `old` and `new` both hold at the same source, type and key, whose content differs, as - [{:source :type :key :name}]. The name is the one `new` gives it." - [old new] - (let [o (normalize old) n (normalize new)] + "Items `before` and `after` both hold at the same source, type and key, whose content differs, as + [{:source :type :key :name}]. The name is the one `after` gives it." + [before after] + (let [o (normalize before) n (normalize after)] (for [[src ct items] (content-groups n) [k item] items :let [was (get-in o [src ct k])] @@ -354,13 +354,13 @@ "Picks in `character`'s options, under a choice tagged with one of `tags` (per `choice-tags`), that `offered` lacks, as [{:key :selection :tag}]. Skips entries carrying their own `::entity/value`." - [character offered choice-tags tags] + [character offered tags-by-selection tags] (letfn [(walk [opts] (for [[sel v] opts e (if (sequential? v) v [v]) :when (map? e) x (cons (let [k (::entity/key e) - tag (some tags (get choice-tags sel))] + tag (some tags (get tags-by-selection sel))] (when (and tag (keyword? k) (not (contains? e ::entity/value)) (not (offers? offered k))) diff --git a/src/cljc/orcpub/dnd/e5/library_links.cljc b/src/cljc/orcpub/dnd/e5/library_links.cljc index 1828c6c8b..32e7f8431 100644 --- a/src/cljc/orcpub/dnd/e5/library_links.cljc +++ b/src/cljc/orcpub/dnd/e5/library_links.cljc @@ -100,19 +100,19 @@ x))) (defn retarget - "`item` with every target of `link` equal to `old` replaced by `new`. Leaves the item unchanged + "`item` with every target of `link` equal to `before` replaced by `after`. Leaves the item unchanged where it does not hold the link. GOTCHA: a `:by :name` link is left alone; renaming a key does not change a name." - [link item old new] + [link item before after] (if (= :name (:by link)) item - (rewrite item (:path link) #(if (= % old) new %)))) + (rewrite item (:path link) #(if (= % before) after %)))) (defn rename-target - "`item` with every name `old` that the `:by :name` `link` holds replaced by `new`." - [link item old new] + "`item` with every name `before` that the `:by :name` `link` holds replaced by `after`." + [link item before after] (if (= :name (:by link)) - (rewrite item (:path link) #(if (= % old) new %)) + (rewrite item (:path link) #(if (= % before) after %)) item)) (def former-key-cap @@ -142,9 +142,9 @@ (into [(first ks)] (take-last (dec former-key-cap) ks))))))) (defn repoint - "`plugin` (one source, {content-type {key item}}) with every link naming `old` in `to-type` - retargeted to `new`, in every item that can hold it." - [plugin to-type old new] + "`plugin` (one source, {content-type {key item}}) with every link naming `before` in `to-type` + retargeted to `after`, in every item that can hold it." + [plugin to-type before after] (let [relevant (filter #(= to-type (:to %)) links)] (reduce-kv (fn [p ct items] @@ -152,7 +152,7 @@ (assoc p ct (reduce-kv (fn [m k item] (assoc m k (reduce #(if (and (map? %1) (holds? %2 ct)) - (retarget %2 %1 old new) + (retarget %2 %1 before after) %1) item relevant))) {} items)) diff --git a/src/cljs/orcpub/character_builder.cljs b/src/cljs/orcpub/character_builder.cljs index 486e1e288..51bf9832a 100644 --- a/src/cljs/orcpub/character_builder.cljs +++ b/src/cljs/orcpub/character_builder.cljs @@ -2338,12 +2338,12 @@ "Asks, once, which item a character meant after an import renamed the library's copy of it. Stays until the author answers." [] - (when-let [{:keys [from to-name from-name import]} @(subscribe [:orcpub.dnd.e5/relink-question])] + (when-let [{imported-pack :import :keys [from to-name from-name]} @(subscribe [:orcpub.dnd.e5/relink-question])] (let [item (or from-name (name from))] [:div.decision-callout [:div.message-title (str "Which \u201c" item "\u201d did this character mean?")] [:div.message-detail - (str "Importing " (if import (str "\u201c" import "\u201d") "a pack") " added a different \u201c" + (str "Importing " (if imported-pack (str "\u201c" imported-pack "\u201d") "a pack") " added a different \u201c" item "\u201d. Yours is now called \u201c" to-name "\u201d.")] [:div.decision-actions [:button.form-button {:on-click #(dispatch [:orcpub.dnd.e5/answer-relink :switch])} "Use yours"] diff --git a/src/cljs/orcpub/dnd/e5.cljc b/src/cljs/orcpub/dnd/e5.cljc index 6c8e84a71..80631b4a5 100644 --- a/src/cljs/orcpub/dnd/e5.cljc +++ b/src/cljs/orcpub/dnd/e5.cljc @@ -164,10 +164,10 @@ {:plugin {} :renames []} plugin)] ;; Repoint after every group is re-keyed, so a link in any group is reached. - (reduce (fn [p [ct old new]] + (reduce (fn [p [ct before after]] (-> p - (update-in [ct new] links/record-former-key old) - (links/repoint ct old new))) + (update-in [ct after] links/record-former-key before) + (links/repoint ct before after))) rekeyed renames)))) diff --git a/src/cljs/orcpub/dnd/e5/autosave_fx.cljs b/src/cljs/orcpub/dnd/e5/autosave_fx.cljs index 1fe19345c..ca4a76234 100644 --- a/src/cljs/orcpub/dnd/e5/autosave_fx.cljs +++ b/src/cljs/orcpub/dnd/e5/autosave_fx.cljs @@ -3,7 +3,7 @@ (:require [orcpub.dnd.e5.character :as char5e] [orcpub.dnd.e5.content-reconciliation :as content-recon] [orcpub.dnd.e5.library :as library] - [re-frame.core :refer [reg-fx reg-event-db reg-event-fx dispatch subscribe]] + [re-frame.core :refer [reg-fx reg-event-fx dispatch subscribe]] [reagent.core :as r])) ;; timeout in ms during which we wait for further changes; if diff --git a/src/cljs/orcpub/dnd/e5/equipment_subs.cljs b/src/cljs/orcpub/dnd/e5/equipment_subs.cljs index 5b79773af..ef6772f6c 100644 --- a/src/cljs/orcpub/dnd/e5/equipment_subs.cljs +++ b/src/cljs/orcpub/dnd/e5/equipment_subs.cljs @@ -1,5 +1,5 @@ (ns orcpub.dnd.e5.equipment-subs - (:require [re-frame.core :refer [reg-sub reg-sub-raw dispatch #_subscribe]] + (:require [re-frame.core :refer [reg-sub #_subscribe]] [orcpub.common :as common] [orcpub.template :as t] [orcpub.dnd.e5.spell-subs] @@ -17,14 +17,11 @@ [orcpub.dnd.e5.equipment :as equipment5e] [orcpub.dnd.e5.spells :as spells5e] [orcpub.route-map :as routes] - [orcpub.dnd.e5.event-utils :as event-utils :refer [url-for-route auth-headers - handle-api-response]] + [orcpub.dnd.e5.event-utils :as event-utils] [orcpub.dnd.e5.api-subs :as api-subs] [reagent.ratom :as ra] [clojure.string :as s] - [orcpub.dnd.e5.http-safe :as http] - [cljs.core.async :refer [local-store dev-mode->local-store @@ -1482,8 +1481,8 @@ (fn [{:keys [db]} [_ item source key confirmed?]] (let [source (or source (:option-pack item)) key (or key (:key item)) - after (when (and source key) (update-in (:plugins db) [source plugin-key] dissoc key)) - broken (when after (:broken (library/commit (:plugins db) after {:deleting? true})))] + plugins-after (when (and source key) (update-in (:plugins db) [source plugin-key] dissoc key)) + broken (when plugins-after (:broken (library/commit (:plugins db) plugins-after {:deleting? true})))] (cond (not (and source key (get-in db [:plugins source plugin-key key]))) {:dispatch [:show-error-message @@ -1495,7 +1494,7 @@ broken [event-key item source key true]) :else - {:dispatch [::e5/set-plugins after {:deleting? true}]}))))) + {:dispatch [::e5/set-plugins plugins-after {:deleting? true}]}))))) (reg-delete-homebrew ::spells/delete-spell @@ -4931,15 +4930,15 @@ :when (and (qualified-keyword? ct) (map? items))] (count items)))) n-fixed (count-items kept-items) - n-left (count-items still-bad)] + n-left (count-items still-bad) + {db' :db ok? :ok? refused :dispatch-n} (commit-library db live {})] ;; Persist live + quarantine together so they never disagree, and show now. - (let [{db' :db ok? :ok? refused :dispatch-n} (commit-library db live {})] - (if-not ok? - {:db db' :dispatch-n refused} - (do (set-rejected-plugins new-rejected) - {:db (assoc db' :quarantined-plugins new-rejected) - :dispatch [(if (pos? n-fixed) :show-warning-message :show-error-message) - (restore-message source-name bad kept-items still-bad)]})))))))) + (if-not ok? + {:db db' :dispatch-n refused} + (do (set-rejected-plugins new-rejected) + {:db (assoc db' :quarantined-plugins new-rejected) + :dispatch [(if (pos? n-fixed) :show-warning-message :show-error-message) + (restore-message source-name bad kept-items still-bad)]}))))))) ;; Permanently discard a quarantined source the user won't repair (a stale one never ;; self-clears). Drops it from BOTH the persisted rejected store and the panel. @@ -5151,24 +5150,24 @@ (let [renamed (orcbrew-val/rename-key-in-plugin (get plugins option-pack) plugin-key old-key new-key) new-plugins (assoc plugins option-pack renamed) - moved (get-in renamed [plugin-key new-key])] + moved (get-in renamed [plugin-key new-key]) + offer (library/repoint-offer new-plugins plugin-key old-key new-key option-pack)] ;; Keep the author's item, re-keyed, not the library copy, so unsaved edits survive; ;; `:former-keys` comes across so the next save does not write it back out. Links in this ;; source follow now; links in other sources are offered. - (let [offer (library/repoint-offer new-plugins plugin-key old-key new-key option-pack)] - {:dispatch-n - [[:set-builder-field-errors {}] - [::e5/set-plugins new-plugins - {:retargeting [[plugin-key old-key]] - :on-success [::e5/builder-saved item-key plugin-key - (assoc item :key new-key :former-keys (:former-keys moved)) - (origin-of (assoc moved :option-pack option-pack :key new-key)) - [:show-message - {:title (str "Key changed to " new-key) - :details [(str "Characters that stored " old-key - " are rebound when they next load.") - (repoint-offer-line offer)]} - (if (seq offer) :sticky 10000)]]}]]})))))) + {:dispatch-n + [[:set-builder-field-errors {}] + [::e5/set-plugins new-plugins + {:retargeting [[plugin-key old-key]] + :on-success [::e5/builder-saved item-key plugin-key + (assoc item :key new-key :former-keys (:former-keys moved)) + (origin-of (assoc moved :option-pack option-pack :key new-key)) + [:show-message + {:title (str "Key changed to " new-key) + :details [(str "Characters that stored " old-key + " are rebound when they next load.") + (repoint-offer-line offer)]} + (if (seq offer) :sticky 10000)]]}]]}))))) (defn- log-export-warnings [plugin-name validation] (when (seq (:warnings validation)) @@ -5520,12 +5519,12 @@ (reg-event-fx ::e5/delete-plugin (fn [{:keys [db]} [_ source-name confirmed?]] - (let [after (dissoc (:plugins db) source-name) - broken (:broken (library/commit (:plugins db) after {:deleting? true}))] + (let [plugins-after (dissoc (:plugins db) source-name) + broken (:broken (library/commit (:plugins db) plugins-after {:deleting? true}))] (if (and (seq broken) (not confirmed?)) (still-used-fx (str "content from \u201c" source-name "\u201d") broken [::e5/delete-plugin source-name true]) - {:dispatch [::e5/set-plugins after {:deleting? true}]})))) + {:dispatch [::e5/set-plugins plugins-after {:deleting? true}]})))) (reg-event-fx ::e5/toggle-plugin @@ -5807,15 +5806,15 @@ collapsed {source-name (apply merge-with (fn [a b] (if (and (map? a) (map? b)) (merge a b) b)) (vals shared))} - live (e5/merge-all-plugins (:plugins db) collapsed)] - (let [{db' :db ok? :ok? refused :dispatch-n} (commit-library db live {})] - (if ok? - {:db (dissoc db' :shared-plugins :shared-content-info) - :dispatch [:show-message - (str "Saved this character's custom content to your library as \"" - source-name "\".")]} - ;; the shared content stays on view, so Keep can be tried again - {:db db' :dispatch-n refused}))))))) + live (e5/merge-all-plugins (:plugins db) collapsed) + {db' :db ok? :ok? refused :dispatch-n} (commit-library db live {})] + (if ok? + {:db (dissoc db' :shared-plugins :shared-content-info) + :dispatch [:show-message + (str "Saved this character's custom content to your library as \"" + source-name "\".")]} + ;; the shared content stays on view, so Keep can be tried again + {:db db' :dispatch-n refused})))))) ;; Dismiss the shared-content banner without keeping (content stays view-only for ;; this session; the overlay itself is cleared on the next character route). diff --git a/test/clj/orcpub/config_report_test.clj b/test/clj/orcpub/config_report_test.clj index 747b0b8a0..a2ff55530 100644 --- a/test/clj/orcpub/config_report_test.clj +++ b/test/clj/orcpub/config_report_test.clj @@ -166,9 +166,9 @@ (is (not (some #(and (= v (:var %)) (:critical? %)) config/settings)) (str v " is marked critical but has a working default")))) (testing "every critical setting explains how to fix it, not just what broke" - (doseq [{:keys [var fix]} (filter :critical? config/settings)] - (is (some? fix) (str var " is critical with no remedy")) - (is (str/includes? fix "Fix:") (str var "'s message has no Fix: line"))))) + (doseq [{env-var :var :keys [fix]} (filter :critical? config/settings)] + (is (some? fix) (str env-var " is critical with no remedy")) + (is (str/includes? fix "Fix:") (str env-var "'s message has no Fix: line"))))) (deftest the-card-cap-is-whole-sheets (testing "nine cards to a sheet, so a cap that is not a multiple of nine leaves a ragged diff --git a/test/clj/orcpub/export_paths_test.clj b/test/clj/orcpub/export_paths_test.clj index 304f25c45..2e644b583 100644 --- a/test/clj/orcpub/export_paths_test.clj +++ b/test/clj/orcpub/export_paths_test.clj @@ -7,7 +7,7 @@ ClojureScript source: they fail when a NEW way to write a file appears, or when an existing writer stops going through the shared entry points, so the next divergence surfaces on the file instead of in someone's .orcbrew." - (:require [clojure.test :refer :all] + (:require [clojure.test :refer [deftest is testing]] [clojure.java.io :as io] [clojure.string :as s])) diff --git a/test/cljc/orcpub/dnd/e5/library_test.cljc b/test/cljc/orcpub/dnd/e5/library_test.cljc index 3751e1e5a..f8ddac79c 100644 --- a/test/cljc/orcpub/dnd/e5/library_test.cljc +++ b/test/cljc/orcpub/dnd/e5/library_test.cljc @@ -199,11 +199,11 @@ "two items claiming the old key: no suggestion, since it would be a guess"))) (deftest renaming-a-language-carries-the-new-name-into-races - (let [old {"Pak" {:orcpub.dnd.e5/languages {:tidetongue-tp {:name "Tidetongue"}} - :orcpub.dnd.e5/races {:tidefolk {:name "Tidefolk" :languages #{"Tidetongue" "Common"}}}}} - new (assoc-in old ["Pak" :orcpub.dnd.e5/languages :tidetongue-tp :name] "Tide-tongue")] + (let [before {"Pak" {:orcpub.dnd.e5/languages {:tidetongue-tp {:name "Tidetongue"}} + :orcpub.dnd.e5/races {:tidefolk {:name "Tidefolk" :languages #{"Tidetongue" "Common"}}}}} + after (assoc-in before ["Pak" :orcpub.dnd.e5/languages :tidetongue-tp :name] "Tide-tongue")] (is (= #{"Tide-tongue" "Common"} - (get-in (:plugins (library/commit old new {})) ["Pak" :orcpub.dnd.e5/races :tidefolk :languages]))))) + (get-in (:plugins (library/commit before after {})) ["Pak" :orcpub.dnd.e5/races :tidefolk :languages]))))) (deftest a-renamed-language-is-not-followed-while-its-old-name-still-answers (let [old {"Pak" {:orcpub.dnd.e5/languages {:elvish-tp {:name "Elvish"}}} diff --git a/test/cljc/orcpub/dnd/e5/share_bundle_test.cljc b/test/cljc/orcpub/dnd/e5/share_bundle_test.cljc index 35b241d91..130d11f1a 100644 --- a/test/cljc/orcpub/dnd/e5/share_bundle_test.cljc +++ b/test/cljc/orcpub/dnd/e5/share_bundle_test.cljc @@ -59,17 +59,17 @@ (deftest a-key-two-content-types-share-is-bundled-as-what-was-picked ;; :orc is a homebrew race in one pack and an unrelated homebrew language in another. - (let [plugins {"Races" {:orcpub.dnd.e5/races {:orc {:name "Orc"}}} - "Tongues" {:orcpub.dnd.e5/languages {:orc {:name "Orcish"}}}} + (let [library-plugins {"Races" {:orcpub.dnd.e5/races {:orc {:name "Orc"}}} + "Tongues" {:orcpub.dnd.e5/languages {:orc {:name "Orcish"}}}} race-only {::entity/options {:race {::entity/key :orc}}} both {::entity/options {:race {::entity/key :orc ::entity/options {:languages [{::entity/key :orc}]}}}}] (testing "picked as a race: the race, and not the language" - (let [bundle (sb/extract-bundle race-only plugins)] + (let [bundle (sb/extract-bundle race-only library-plugins)] (is (contains? (get-in bundle ["Races" :orcpub.dnd.e5/races]) :orc)) (is (nil? (get bundle "Tongues"))))) (testing "picked as a race and, under a language choice, as a language: both" - (let [bundle (sb/extract-bundle both plugins)] + (let [bundle (sb/extract-bundle both library-plugins)] (is (contains? (get-in bundle ["Races" :orcpub.dnd.e5/races]) :orc)) (is (contains? (get-in bundle ["Tongues" :orcpub.dnd.e5/languages]) :orc)))))) diff --git a/test/cljs/orcpub/dnd/e5/homebrew_save_lifecycle_test.cljs b/test/cljs/orcpub/dnd/e5/homebrew_save_lifecycle_test.cljs index 870fd8a44..726f2e253 100644 --- a/test/cljs/orcpub/dnd/e5/homebrew_save_lifecycle_test.cljs +++ b/test/cljs/orcpub/dnd/e5/homebrew_save_lifecycle_test.cljs @@ -15,15 +15,14 @@ [orcpub.dnd.e5.languages :as langs5e] [orcpub.dnd.e5.selections :as selections5e] [orcpub.dnd.e5.orcbrew-validation :as orcbrew-val] + ;; …and every subscription: the collision the save refuses is the one pinned at the + ;; bottom of this file. [orcpub.dnd.e5.spell-subs :as subs5e] [orcpub.dnd.e5.spells :as spells5e] [orcpub.common :as common] [orcpub.dnd.e5.content-reconciliation :as reconcile] ;; Side effect: registers every event handler under test. - [orcpub.dnd.e5.events :as events] - ;; …and every subscription: the collision the save refuses is the one pinned at the - ;; bottom of this file. - [orcpub.dnd.e5.spell-subs])) + [orcpub.dnd.e5.events :as events])) (def ^:private SRC "Lifecycle Pak") (def ^:private ct :orcpub.dnd.e5/languages) @@ -173,7 +172,7 @@ (defn- moved-through "An item carried through a chain of deliberate key changes, oldest first." [ks] - (reduce (fn [item k] (reconcile/record-former-key (assoc item :key k) (:key item))) + (reduce (fn [item step-key] (reconcile/record-former-key (assoc item :key step-key) (:key item))) {:key (first ks) :option-pack SRC} (rest ks))) @@ -370,10 +369,10 @@ (defn- open-from-library! "Open a stored item the way My Content's edit button does, so the builder records where it came from." - [source k] + [source item-key] ;; My Content passes the ROW's address alongside the item -- the source holding it and the key ;; it answers to -- because neither is reliably on the item itself. - (dispatch! [::langs5e/edit-language (get-in @app-db [:plugins source ct k]) source k ct])) + (dispatch! [::langs5e/edit-language (get-in @app-db [:plugins source ct item-key]) source item-key ct])) (defn- retarget! [source] (swap! app-db assoc-in [::langs5e/builder-item :option-pack] source)) @@ -486,8 +485,8 @@ (def ^:private B "Pak B") (defn- lib [& sources] - (into {} (for [[src k nm] (partition 3 sources)] - [src {ct {k {:key k :name nm :option-pack src}}}]))) + (into {} (for [[src item-key nm] (partition 3 sources)] + [src {ct {item-key {:key item-key :name nm :option-pack src}}}]))) (defn- dest [plugins recorded option-pack key item] (:action (events/save-destination plugins recorded ct option-pack key item))) From 3cbe12dfe629bd0c59f49afb561aa257b948bd2e Mon Sep 17 00:00:00 2001 From: codeGlaze Date: Sun, 4 Oct 2026 18:51:59 +0000 Subject: [PATCH 5/8] Fold the refactor/picks-namespace changelog into Summer Patch --- CHANGELOG.md | 11 ++++++++++ docs/branch-changelog.md | 46 ---------------------------------------- 2 files changed, 11 insertions(+), 46 deletions(-) delete mode 100644 docs/branch-changelog.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 10f57bae7..997f77dfa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1154,6 +1154,17 @@ one 10px gutter, and the header's tab menus open on screen again. - **The full browser test run takes 5.5 minutes instead of 8.5** — the bundle builds while the test server starts, so every test can start in parallel; the summary shows peak memory, the limit on running more at once (`7da2e4e8`). +### refactor/picks-namespace + +**Added** + +- **Remove and put back one stored pick by its address** — `picks/remove-at` and `picks/put-at`, the base for repairing a character and for setting picks aside (`439593dd`, `b33c765a`). + +**Changed** + +- **Lint is clean** — the 50 warnings and 3 infos on `integration` are fixed, not suppressed: shadowing locals renamed for what they hold, unused requires removed, nested lets merged; no behaviour change (`ad3c612b`). +- **The pick walker moved into shared code** — `walk-entries`, `walk-picks`, `picks-of` and `relink-picks` now live in `orcpub.dnd.e5.picks` (as `walk`, `walk-typed`, `keys-of`, `relink`), unchanged, so the server can use them and their tests run in CI (`439593dd`). + ## [breaking/2026-stack-modernization] ### Infrastructure diff --git a/docs/branch-changelog.md b/docs/branch-changelog.md deleted file mode 100644 index e38cf4988..000000000 --- a/docs/branch-changelog.md +++ /dev/null @@ -1,46 +0,0 @@ - - -# Branch changelog — `refactor/picks-namespace` - - - -## Why this branch exists - - - -One engine over a character's stored picks, shared by the character-rescue work (which must list -and remove picks on the server, without the app) and the hidden-pick work (which moves picks that -stop applying out of the character and back). Three walkers over stored picks already exist; this -moves the heals' one into shared code unchanged and adds the two operations both pieces need, -rather than writing a fourth. The build's walker and the Missing Content walker are not touched. - -## Added - -- **Remove and put back one stored pick by its address** — `picks/remove-at` and `picks/put-at`, the base for repairing a character and for setting picks aside (`439593dd`). - -## Fixed - -## Changed - -- **The pick walker moved into shared code** — `walk-entries`, `walk-picks`, `picks-of` and `relink-picks` now live in `orcpub.dnd.e5.picks` (as `walk`, `walk-typed`, `keys-of`, `relink`), unchanged, so the server can use them and their tests run in CI (`439593dd`). From 909655cf12a14b622b9c240dd788e82e23755cad Mon Sep 17 00:00:00 2001 From: codeGlaze Date: Sun, 4 Oct 2026 18:59:52 +0000 Subject: [PATCH 6/8] picks/put-at: never append a second copy of a key; list picks-test in the runner put-at for a multiselect now leaves the vector as it is when it already holds the pick's key (the player picked it again while it was set aside), instead of appending a duplicate that a later remove-at would take out together. The new test fails without the check. picks-test is added to orcpub.test-runner's list, so the per-test reporter mode runs it too; the full browser suite already did (546 tests, now 2828 assertions with the new one). --- src/cljc/orcpub/dnd/e5/picks.cljc | 8 ++++++-- test/cljc/orcpub/dnd/e5/picks_test.cljc | 25 +++++++++++++++++++------ test/cljs/orcpub/test_runner.cljs | 2 ++ 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/cljc/orcpub/dnd/e5/picks.cljc b/src/cljc/orcpub/dnd/e5/picks.cljc index da4e2b12a..fa181b354 100644 --- a/src/cljc/orcpub/dnd/e5/picks.cljc +++ b/src/cljc/orcpub/dnd/e5/picks.cljc @@ -100,7 +100,8 @@ (defn put-at "`character` with `removed` (from `remove-at`) stored again at its address, or nil when an entry on the way to it is no longer stored. `:multiselect?` true appends it to the selection's vector - (creating one); false makes it the selection's entry, replacing any there. + (creating one), unless that vector already holds its key; false makes it the selection's entry, + replacing any there. GOTCHA: checks no rule; whether it still fits is the caller's question." [character {:keys [address entry multiselect?]}] (let [sel (nth address (- (count address) 2)) @@ -108,5 +109,8 @@ (when opath (if multiselect? (update-in character (conj opath sel) - (fn [v] (if (sequential? v) (with-meta (conj (vec v) entry) (meta v)) [entry]))) + (fn [v] + (cond (not (sequential? v)) [entry] + (entry-index v (::entity/key entry)) v + :else (with-meta (conj (vec v) entry) (meta v))))) (assoc-in character (conj opath sel) entry))))) diff --git a/test/cljc/orcpub/dnd/e5/picks_test.cljc b/test/cljc/orcpub/dnd/e5/picks_test.cljc index 2cd430b7b..b311270ef 100644 --- a/test/cljc/orcpub/dnd/e5/picks_test.cljc +++ b/test/cljc/orcpub/dnd/e5/picks_test.cljc @@ -26,12 +26,15 @@ (let [character {:db/id 7 ::entity/options {:race {::entity/key :aarakocra ::entity/options {:subrace {::entity/key :aarakocra}}}}} - {:keys [character rewrote]} (picks/relink character :orcpub.dnd.e5/races :aarakocra :aarakocra-2)] + {:keys [character rewrote]} + (picks/relink character :orcpub.dnd.e5/races :aarakocra :aarakocra-2)] (is (= :aarakocra-2 (get-in character [::entity/options :race ::entity/key]))) - (is (= :aarakocra (get-in character [::entity/options :race ::entity/options :subrace ::entity/key]))) + (is (= :aarakocra + (get-in character [::entity/options :race ::entity/options :subrace ::entity/key]))) (is (= 1 (count rewrote)))) - (let [character {::entity/options {:race {::entity/key :bird - ::entity/options {:subrace {::entity/key :aarakocra}}}}}] + (let [character {::entity/options + {:race {::entity/key :bird + ::entity/options {:subrace {::entity/key :aarakocra}}}}}] (is (= #{:aarakocra} (picks/keys-of character :orcpub.dnd.e5/subraces))) (is (= #{:bird} (picks/keys-of character :orcpub.dnd.e5/races)) "a subrace pick does not make the question about the race"))) @@ -84,12 +87,22 @@ (is (= [:athletics :stealth :insight] (map ::entity/key v))) (is (= {:kept true} (meta v))))) (testing "into an emptied multi-pick" - (let [{:keys [character removed]} (picks/remove-at ranger [:class :fighter :fighting-style :defense])] + (let [{:keys [character removed]} + (picks/remove-at ranger [:class :fighter :fighting-style :defense])] (is (= [] (get-in character [::entity/options :class 1 ::entity/options :fighting-style]))) (is (= [{::entity/key :defense}] (get-in (picks/put-at character removed) [::entity/options :class 1 ::entity/options :fighting-style]))))) + (testing "the key re-picked meanwhile: no second copy" + (let [{:keys [character removed]} (picks/remove-at ranger insight) + repicked (update-in character + [::entity/options :class 0 ::entity/options :skill-proficiency] + conj {::entity/key :insight}) + v (get-in (picks/put-at repicked removed) + [::entity/options :class 0 ::entity/options :skill-proficiency])] + (is (= [:athletics :stealth :insight] (map ::entity/key v))))) (testing "nowhere to go: nil" (let [{:keys [removed]} (picks/remove-at ranger steel-will) - no-ranger (assoc-in ranger [::entity/options :class] [(get-in ranger [::entity/options :class 1])])] + no-ranger (assoc-in ranger [::entity/options :class] + [(get-in ranger [::entity/options :class 1])])] (is (nil? (picks/put-at no-ranger removed)))))) diff --git a/test/cljs/orcpub/test_runner.cljs b/test/cljs/orcpub/test_runner.cljs index 66a9355f7..1adc1cd60 100644 --- a/test/cljs/orcpub/test_runner.cljs +++ b/test/cljs/orcpub/test_runner.cljs @@ -5,6 +5,7 @@ [orcpub.dnd.e5.event-utils-test] [orcpub.dnd.e5.compute-test] [orcpub.dnd.e5.hunter-evasion-test] + [orcpub.dnd.e5.picks-test] ;; The spell page packer and row annotations run in the browser -- ;; the builder decides the layout -- so their tests run here too. [orcpub.dnd.e5.spell-packing-test] @@ -36,6 +37,7 @@ 'orcpub.dnd.e5.event-utils-test 'orcpub.dnd.e5.compute-test 'orcpub.dnd.e5.hunter-evasion-test + 'orcpub.dnd.e5.picks-test 'orcpub.dnd.e5.spell-packing-test 'orcpub.image-url-test 'orcpub.whats-new-test From 29e9e2b692e2a34a0c9402d064fe237e779263f5 Mon Sep 17 00:00:00 2001 From: codeGlaze Date: Sun, 4 Oct 2026 19:08:10 +0000 Subject: [PATCH 7/8] Drop the unused hostile-locales from pedestal_test Defined with the tests in 402ea5e5 and never used: the file explains the locale defect cannot be reproduced in one JVM, and the guard is running the suite under a non-English JVM. It was the one lint warning the locale-safety merge brought in; lint is back to 0 warnings. --- test/clj/orcpub/pedestal_test.clj | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/clj/orcpub/pedestal_test.clj b/test/clj/orcpub/pedestal_test.clj index 8a2d798ed..61616eb68 100644 --- a/test/clj/orcpub/pedestal_test.clj +++ b/test/clj/orcpub/pedestal_test.clj @@ -17,12 +17,6 @@ [java.time.format DateTimeFormatter] [java.util Locale])) -(def ^:private hostile-locales - [(Locale/forLanguageTag "es-ES") - (Locale/forLanguageTag "de-DE") - (Locale/forLanguageTag "tr-TR") - (Locale/forLanguageTag "ja-JP")]) - ;; A real Last-Modified from the Font Awesome webjar, and the ETag the English ;; locale has always produced for it. Pinning the exact value matters: if it ;; ever changes, every cached ETag in the wild is invalidated. From 453aff1d7153641a4348987fa614365d50dae762 Mon Sep 17 00:00:00 2001 From: codeGlaze Date: Sun, 4 Oct 2026 21:14:50 +0000 Subject: [PATCH 8/8] picks/put-at: never overwrite a one-pick selection either; nil when nothing is written put-at now behaves the same for both shapes: it writes the pick back only into an empty place. A multiselect vector already holding the key, or a one-pick selection already holding any pick, is left alone and put-at returns nil, as it does when the path is gone. A pick the player made while the old one was set aside is never overwritten or duplicated; nil tells the caller the held copy is retired. Tests: the re-picked key and a one-pick selection chosen again both return nil; each fails when its guard is removed. --- src/cljc/orcpub/dnd/e5/picks.cljc | 31 ++++++++++++++----------- test/cljc/orcpub/dnd/e5/picks_test.cljc | 15 ++++++++---- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/src/cljc/orcpub/dnd/e5/picks.cljc b/src/cljc/orcpub/dnd/e5/picks.cljc index fa181b354..49f5beb2a 100644 --- a/src/cljc/orcpub/dnd/e5/picks.cljc +++ b/src/cljc/orcpub/dnd/e5/picks.cljc @@ -98,19 +98,24 @@ :else {:character character :removed nil}))) (defn put-at - "`character` with `removed` (from `remove-at`) stored again at its address, or nil when an entry - on the way to it is no longer stored. `:multiselect?` true appends it to the selection's vector - (creating one), unless that vector already holds its key; false makes it the selection's entry, - replacing any there. + "`character` with `removed` (from `remove-at`) stored again at its address: appended to a + `:multiselect?` vector (created if absent), else as the selection's entry. Nil when nothing is + written: an entry on the way is gone, the vector already holds its key, or the one-pick + selection already holds a pick. Never overwrites or duplicates a pick. GOTCHA: checks no rule; whether it still fits is the caller's question." [character {:keys [address entry multiselect?]}] (let [sel (nth address (- (count address) 2)) - opath (options-path character address)] - (when opath - (if multiselect? - (update-in character (conj opath sel) - (fn [v] - (cond (not (sequential? v)) [entry] - (entry-index v (::entity/key entry)) v - :else (with-meta (conj (vec v) entry) (meta v))))) - (assoc-in character (conj opath sel) entry))))) + opath (options-path character address) + v (when opath (get-in character (conj opath sel)))] + (cond + (nil? opath) nil + multiselect? (cond (not (sequential? v)) + (assoc-in character (conj opath sel) [entry]) + + (entry-index v (::entity/key entry)) nil + + :else + (assoc-in character (conj opath sel) + (with-meta (conj (vec v) entry) (meta v)))) + (some? v) nil + :else (assoc-in character (conj opath sel) entry)))) diff --git a/test/cljc/orcpub/dnd/e5/picks_test.cljc b/test/cljc/orcpub/dnd/e5/picks_test.cljc index b311270ef..f97ab6c52 100644 --- a/test/cljc/orcpub/dnd/e5/picks_test.cljc +++ b/test/cljc/orcpub/dnd/e5/picks_test.cljc @@ -93,14 +93,19 @@ (is (= [{::entity/key :defense}] (get-in (picks/put-at character removed) [::entity/options :class 1 ::entity/options :fighting-style]))))) - (testing "the key re-picked meanwhile: no second copy" + (testing "the key re-picked meanwhile: nothing written, nil" (let [{:keys [character removed]} (picks/remove-at ranger insight) repicked (update-in character [::entity/options :class 0 ::entity/options :skill-proficiency] - conj {::entity/key :insight}) - v (get-in (picks/put-at repicked removed) - [::entity/options :class 0 ::entity/options :skill-proficiency])] - (is (= [:athletics :stealth :insight] (map ::entity/key v))))) + conj {::entity/key :insight})] + (is (nil? (picks/put-at repicked removed))))) + (testing "a one-pick selection chosen again meanwhile: never overwritten, nil" + (let [{:keys [character removed]} (picks/remove-at ranger steel-will) + chose-other (assoc-in character + [::entity/options :class 0 ::entity/options :levels 0 + ::entity/options :defensive-tactics] + {::entity/key :escape-the-horde})] + (is (nil? (picks/put-at chose-other removed))))) (testing "nowhere to go: nil" (let [{:keys [removed]} (picks/remove-at ranger steel-will) no-ranger (assoc-in ranger [::entity/options :class]