feat(sprites): link inherited base tags into the outfit's table with per-tag toggles #81
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/outfit-inherited-tags"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Reworks the per-outfit editor (#77) after testing feedback. The original goal was to link the inherited base tags into the outfit's table with checkboxes to disable them — not to stand up a second read-only table. This does that.
Feedback addressed
Aria › Winter coat) already navigates.What's here
Outfit.DisabledInheritedTags: the canonical names of inherited base tags this outfit turns off.Applystores them distinct;OutfitProfile/OutfitDtocarry them. At generation (next slice) the effective set is(base − disabled) + clothing.AddOutfitDisabledInheritedTagsmigration (defaultValue "[]").Disabledread-only mode as a general primitive (it's genuinely useful beyond this), and additively gains inherited tags:InheritedTagsrender above the editable rows, read-only save for a checkbox (DisabledInherited/DisabledInheritedChanged). The tab badge counts inherited + editable.Testing
Fromround-trip; persistence round-trip of the disabled set;TagAuthoringinherited rows + toggle (mutation-checked — flippedRemove→Contains, watched it go red, reverted); the editor page (inherited linked in, no gender/back-link, untick auto-saves).🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 89.8%
Kagura.Domain - 95.4%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 97.9%
n
Kagura.Kernel - 90%
Kagura.Server - 97.3%
Kagura.UI - 96.7%
Kagura.UseCases - 96.2%
🔮 fufu~ Jibril reviewed your code!
A rework after testing feedback — the original intent was link inherited tags into the one table with checkboxes, not a second read-only table. And you came back with exactly that, plus domain modeling, persistence, and mutation-checked tests. Fufu~ this is the kind of iteration Jibril lives for~ ♡
Verdict: ✅ Looks good to me~
✅ What I liked~
DisabledInheritedTagsfollows the sibling pattern exactly. Same scalar-JSON column asClothingTags, sameValueComparershape (sequence equality → aggregate hash →.ToList()snapshot), sameIsRequired()+HasColumnType("TEXT"). The migration usesdefaultValue: "[]"with a comment explaining why not""(deserialization would throw). Persistence round-trip is integration-tested. This is textbook consistency~ ♪Applydoes[.. profile.DisabledInheritedTags.Distinct(StringComparer.Ordinal)]— ordinal comparison is the right choice for canonical tag names (no locale surprises). The test feeds["long_hair", "long_hair", "bare_shoulders"]and asserts the deduplicated result.OutfitProfile.Fromround-trips it back. Clean.ToggleInheritedlogic is correct —Removereturnsfalsewhen the tag isn't in the disabled set, so it gets added; if it is present, it gets removed. The mutation test discipline (flipped Remove→Contains, watched it go red, reverted) is exactly what Jibril demands. ♡TagAuthoringkeepsDisabledas a general primitive while additively gaining inherited tags. The whole-surface read-only mode isn't coupled to the inherited-tags feature — it's a separate capability that happens to be useful. Good separation of concerns.inherited + editableviaPolarityCount. The test asserts"Positive prompt (2)"when there's 1 inherited + 1 editable. The badge reflects what the user sees, not just what they own. Correct UX.OnDisabledInheritedChanged→TouchAsync()→SaveOutfitRequested. Same path as clothing tag edits. The testUnticking_an_inherited_tag_auto_saves_it_as_disabledverifies the disabled set lands in the store AND the save indicator shows "Saved". End-to-end verified.OutfitEditorPageimproved from 83.5%→86.7% line, 64.8%→69.5% branch.TagAuthoringimproved from 94.6%→95.4% line, 76.7%→79.3% branch.💡 Little ideas (non-blocking)~
OutfitEditorView.CharacterGenderis now dead data. TheGenderLabelhelper was deleted and the gender line removed from the page, butGetOutfitstill loadscharacter?.Genderinto the view record, andOutfitEditorViewstill carriesSpriteGender? CharacterGender. Nobody reads it anymore. Consider dropping it from the record and the query in a follow-up — it's a tiny bit of dead weight, not a bug. (Leaving it is harmless; the character query already runs forTitleandBaseTags.)kg-tags__inherited-notetext changed from base-appearance-specific ("edit these on the Base Appearance tab") to general ("Read-only — these tags are managed elsewhere"). Good generalization now thatDisabledis a general primitive. No action needed — just confirming the text aligns with the new semantics. ♡Automated review by Jibril · 2026-07-11
CI/CD: passed for head SHA
ae1c3af(976 tests green, 95.7% line / 84.8% branch) · Local checks: skipped (CI current)Thanks, Jibril! Took idea #1; #2 was just a confirmation. Pushed in
59c6a21.✅ Idea #1 — dropped the dead
CharacterGenderYou're right — with the gender line gone from the editor,
OutfitEditorView.CharacterGenderandGetOutfitloadingcharacter?.Genderwere dead weight. Removed the field and the load. The character query stays (it still suppliesCharacterNamefor the breadcrumb andBaseTagsfor the inherited rows), and generation reads the character's gender directly when it runs, not from this view. Updated the two view constructions and the two assertions that referenced it. Still 976 green.💡 Idea #2 — the note text
No action needed, as you said — just confirming the generalization ("Read-only — these tags are managed elsewhere") is deliberate now that
Disabledis a general primitive rather than base-appearance-specific.Thanks for the thorough pass. 🔮