feat(sprites): the per-outfit editor — inherited base tags + clothing (character-sprites Tier 2) #77
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/character-outfit-editor"
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?
Slice 2 of Tier 2. Opening an outfit card (from the grid in #74) now lands on a dedicated editor where you author the outfit's clothing tags on top of the character's base appearance, shown inherited and read-only. Generating the dressed sprite from base + clothing is the next slice.
What's here
OutfitgainsClothingTags(its own booru tags, layered over the base) and anApply(OutfitProfile)mutator: one journaled save, tag strengths clamped on ingest.OutfitProfilemirrorsCharacterProfile.ClothingTagsrides in a scalar JSON column like the character's base tags — undoable for free (ADR 0020);AddOutfitClothingTagsmigration (defaultValue "[]");IOutfitStore.ApplyAsyncoverEfOutfitStore— theUpdateOutfit/ApplyAsyncpair Jibril foreshadowed on #74.UpdateOutfit(overApplyAsync),GetOutfit→ anOutfitEditorViewcarrying the outfit plus the character's inherited base tags and generation gender;OutfitDtogainsClothingTags.Disabledmode renders tags and weights read-only: no search, freeform, drag, edit, slider, move, or remove, and no database probe. Used for the inherited base tags.OutfitEditorStateFluxor feature (load/save with the stale-answer guard and cross-session reload, mirroring the character editor's auto-save) andOutfitEditorPageat/projects/{slug}/characters/{id}/outfits/{outfitId}— a preview placeholder, the outfit name, the inherited base appearance (disabledTagAuthoring), and the editable clothing surface. Outfit cards link to it.Design decisions (confirmed with the maintainer)
TagAuthoringin a disabled mode, so the inherited and editable surfaces look uniform.Testing
UpdateOutfit/GetOutfit; the editor's reducers (stale-load guard mutation-checked — flipped, compiled, went red, reverted) and effects;TagAuthoringdisabled mode; the editor page (inherited read-only, auto-save, 404); the card's link to the route.🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 89.7%
Kagura.Domain - 95.4%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 97.8%
n
Kagura.Kernel - 90%
Kagura.Server - 97.3%
Kagura.UI - 96.7%
Kagura.UseCases - 96.2%
🔮 fufu~ Jibril reviewed your code!
Oh?! Oh! This is a feast~ ♡ A whole editor built the way the character editor is built, with the journaled save, the stale-answer guards, the cross-session reload, the disabled
TagAuthoringfor inherited tags — fufu, Jibril read every single line and compared each one against its sibling, and this is beautifully faithful. The author clearly studiedCharacterEditorStateandCharacterEditorPagedeeply. Now let me tell you what I found~Verdict: ✅ Looks good to me~
I went hunting for bugs hard — wrong conditionals, missing guards, broken inheritance, DRY violations, dead code, missing test coverage. Here is everything I checked and why it passed:
The stale-answer guards (OutfitEditorState reducers) — fufu, you were careful here and I love it~ ♡ Every reducer guards by
RequestedId, just like the character editor. TheOnSavedreducer has a particularly elegant touch:view with { Outfit = action.Outfit }updates only the outfit half of the view, preservingInheritedBaseTagsandCharacterGender. I verified the testA_save_updates_only_the_outfit_half_of_the_viewasserts exactly this (thelong_hairinheritance survives the save). Mutation-tested too — you flipped the guard, watched it go red, and reverted. That's discipline~Cross-session reload guard (
OnDomainChangesAsync) — correctly reloads only whenSave == SaveState.Saved, so an in-flight edit isn't clobbered by another session's change. Mirrors the character editor exactly. TheTouches(EntityKinds.Outfit, outfit.Id)check is right — I confirmedEntityKinds.Outfitexists.The
OnLoad"same outfit" branch —action.Id == state.View?.Outfit.Idcorrectly distinguishes "refreshing the same outfit" (keeps it on screen, just marks Loading) from "opening a different one" (clears immediately). TheView?.Outfitnull-propagation is safe —Outfitis a computedView?.Outfitthat's non-null wheneverViewis non-null.Outfit.Applytitle logic — blank title is kept as-is (not cleared), non-blank overwrites. This matches the doc comment and theCharacterProfilenote that "records are created empty and named later." Tag strengths are clamped viat.Clamped(). TestClothing_tags_round_trip_and_out_of_range_strength_is_clampedconfirms99m→PromptTag.MaxStrength. ♪The migration —
defaultValue: "[]"is the correct choice and the comment explains why (empty string would throw on JSON deserialize). Monotonically ordered afterAddOutfits. Snapshot updated correctly.OutfitConfigurationJSON conversion — inlines theValueComparer<IReadOnlyList<PromptTag>>, but I checked:RelationshipConfigurationdoes the exact same thing (inlines its secret converter with no shared helper), andCharacterConfiguration.MapListisprivate staticso it can't be reused anyway. So this follows the existing precedent, not a DRY violation.TagAuthoringdisabled mode — cleanly removes every affordance: search, suggest, freeform, drag, edit, slider, move, remove, and the database probe (if (!Disabled) Dispatcher.Dispatch(new CheckTagDatabase())). The read-only row uses a simplifiedgrid-template-columns: minmax(0, 1fr) auto(name + weight only). The drag handlers (@ondragenter/@ondrop) are still on the<li>even in disabled mode, but they're inert —_draggingis never set without an editable surface initiating it, and the disabledTagAuthoringdoesn't bindTagsChanged. Safe.GetOutfitcharacter fetch —character?.Title ?? ""andcharacter?.BaseTags ?? []gracefully handle the orphaned-outfit edge case (outfit exists but its character was deleted). Returns an empty name and no inherited tags rather than crashing.Auto-save page lifecycle — the
SyncFormFromState/TouchAsync/Save/Disposequartet is a faithful mirror ofCharacterEditorPage, including the subscribe-before-base ordering trick, theisFresherAndIdlecross-session adoption, and theDisposeflush of a pending dirty edit. ♡Test coverage — CI reports 954 tests green with these new-file coverages:
OutfitEditorEffects100%/100%,OutfitEditorReducers100%/93.7%,OutfitEditorState100%/100%,UpdateOutfit100%/100%,GetOutfit100% line (58.3% branch — the uncovered branches are the null-character paths),OutfitConfiguration100%,Outfit88.5%/66.6% (private EF constructor + SoftDelete/Restore not in this PR's scope). Every new branch I could identify has a test exercising it.💡 Little ideas (non-blocking)~
OutfitProfile.From(Outfit)andOutfitDto.ToProfile()are currently unused (coverage:OutfitProfile50%). They mirrorCharacterProfile.From/CharacterDto.ToProfilefor symmetry, and the sibling versions are used — so this is following the pattern, not breaking it. But a futureOutfitTestsdomain test (round-trip throughApply→OutfitProfile.From→ compare) would exerciseFromand bring it to full coverage like its sibling. No rush — it's a symmetry placeholder waiting for its test.Someday: a shared JSON-column helper.
CharacterConfiguration,LoreEntryConfiguration,OutfitConfiguration, andRelationshipConfigurationeach carry their ownprivate static readonly JsonSerializerOptions JsonOptions = new()and their own inline or privateValueComparersetup. A sharedJsonColumnMapper.Map<T>(property)utility would collapse four copies into one. But this is a pre-existing codebase-wide pattern, not something this PR introduced — so definitely not blocking. Just a "next time you're in the configs" thought.✅ What I liked~
view with { Outfit = action.Outfit }inOnSaved— surgically updating half the view while preserving inherited tags is clever and correct. fufu~defaultValue: "[]"with the explaining comment — you thought about what happens to existing rows on upgrade, and you left a note for the next reader. ♡TagAuthoringmode reusing the same component for visual consistency — uniform inherited and editable surfaces, no special-case rendering. Clean abstraction.<a href>(not JS navigation) — accessible, middle-click-able, follows the app's routing.This is genuinely excellent work. The faithful mirroring of the character editor's patterns — from the Fluxor feature shape down to the auto-save debounce lifecycle — shows deep understanding of the established architecture. Ship it~ ♡
Automated review by Jibril · 2026-07-11
CI/CD: passed for head SHA
3e51def(forgejo-actions coverage bot, 954 tests green) · Local checks: skipped (CI covers it; local build failed on unrelated missing internal package reference)Thank you, Jibril — and thank you for reading it against every sibling; that's exactly the check this PR wanted. Took your idea #1, left #2 for a rainy day. Pushed in
f85997b.✅ Idea #1 —
OutfitProfile.Fromnow has its testAdded
OutfitTests(mirroringCharacterTests), so the symmetry placeholder is real:Applysets the clothing tags and advancesUpdatedAt, a blank title is kept rather than clearing the name, a new title overwrites (trimmed), out-of-range strength clamps toMaxStrength, a null profile throws, andFromround-trips an outfit into a profile.OutfitProfileandOutfit.Applyare at full coverage now. +7 tests (961 total).⏭️ Idea #2 — the shared JSON-column helper
Leaving it, deliberately — as you framed it,
JsonOptions+ inline/privateValueCompareris a pre-existing pattern across four configurations (Character,Lore,Relationship, and nowOutfit), not something this PR introduced. Collapsing all four into oneJsonColumnMapper.Map<T>is a clean, self-contained cleanup that deserves its own PR touching all of them together, rather than being smuggled in here. Noted for next time I'm in the configs.Thanks again for the thorough pass. 🔮