Appearance settings rewrite: shared style editor, resolve model, honest previews, skin-referenced colors

[sproctor]

Summary

Reworks the Appearance settings end to end: a new style model, one shared style editor reused across every section, the base "default text" style promoted out of the presets, honest previews, and skin-referenced colors that follow the skin. Desktop (Jewel) and mobile (Material3) both covered.

The renderer was kept behavior-preserving throughout: a golden characterization suite pins how a style becomes a Compose SpanStyle, and it stays green across every model swap.

The style model (core)

• StyleLayer — a sparse per-scope override set; every attribute is optional. resolve(layers) takes the first layer that sets each attribute independently, reproducing the old boolean-OR / earliest-color merge for existing data.
• Tri-state Background — Fill(color) / None (explicitly transparent) / Unset (inherit). None and unset look identical in game but resolve differently, so they're kept distinct end to end.
• Weight, not bold — bold is weight == 700; an explicit weight and "bold" can't contradict through the cascade.
• SourcedStyle / resolveSourced — the source-tracking twin of resolve, so each editor control can show set here / from Global / from skin and gate a per-attribute reset.
• ColorValue + ref metadata — a color is a frozen literal or a SkinRef(slot) that re-resolves against the skin palette. StyleLayer carries the ref beside the resolved color and resolveRefs(palette) refreshes it at the compose boundary, so the render path stays plain-WarlockColor and untouched.

Shared editor + master-detail UI

• One TextStyleEditor (Jewel + M3 twins) drives all four sections. Per attribute: control + source tag + reset; a live sample; text color, tri-state background, weight, italic, underline, and per-item fonts.
• Text styles page (was "Presets"): a Global/character scope selector over the base Default text plus the named presets, each row an honest chip in its own resolved style, drilling in to the editor. Per-preset fonts; the base and monospace font sit above the presets list.
• Windows / Highlights / Names all adopt the shared editor and scope selector; highlights keep their per-capture-group model, names gain B/I/U they never exposed.
• A global (all-characters) preset/base layer, mirroring the existing highlight/name idiom.

Base "default text" style

• Promoted out of the preset map into first-class character settings (color + fonts + italic/underline), cascaded skin → global → character like everything else.
• Migration: on config load, any legacy presets["default"] is folded into the base and dropped (base values win; the legacy preset fills the rest), per scope.

Honest previews

• StyleChip — a fixed-width swatch composited on the effective game window background (never the settings panel), full-bleed, with an inset ring only when the background is explicitly set (luminance-derived so it reads on any skin). The item name is always normal, legible UI text — never styled with its own style.
• Contrast — WCAG ratio against the effective background; warns below ~3:1 with the actual ratio, never blocks.
• Background picker — explicit Inherit / None / Color rows; Inherit is annotated with its fallback.

Skin-referenced colors

• Pick from the skin palette → stored as a SkinRef that tracks the skin (no dark-on-light trap on skin switch); type/custom → a frozen literal. The UI labels which is which ("tracks <slot>").
• Persisted backward-compatibly (nullable, defaulted) for presets, base, names, highlights, and windows; resolved at the render/editor boundary so the golden render tests are unaffected.

Storage / export

• Per-section human-editable TOML (bold↔weight canonicalized so existing files stay byte-stable).
• Export now carries the base style + default/mono fonts and the new per-preset weight/font + window flags, all defaulted so older export files still import.

Testing

• Golden SpanStyle / AnnotatedString characterization + ComposeTextStream re-render tests (unchanged across the rewrite).
• Unit tests for resolve, resolveSourced, the editor reducer, ColorValue/resolveRefs, contrast, and TOML/mapper round-trips (incl. skin refs and legacy-file load).

Known gaps / follow-ups

• Export/import carries no color refs — an exported ref degrades to its resolved literal (StyleExport has no ref fields).
• Windows at render refresh a ref color on save rather than instantly on a skin switch (presets/names/highlights update live); threading the palette through GameViewModel's hot path wasn't worth it.
• The StyleDefinition façade is retained where it's still load-bearing (render entire-line path, some chrome) rather than fully retired.

Notes

• Requires a ./gradlew run human smoke pass for the editors/pickers (the visual gate).
• This branch also carries the previously-merged Harden Compose Desktop crash workarounds (macOS a11y + context-menu NPE) #228 / Remove gradle-daemon-jvm.properties daemon JVM pin #229 / Group settings pages into sections with icons #230 and the gradle-daemon change; those diffs drop out once they land on master.

🤖 Generated with Claude Code

[sproctor]

Automated deep review (xhigh effort)

Ten finder angles swept the ~9k-line diff (62 raw candidates → 31 distinct mechanisms), each surviving candidate got an independent verification pass against the PR head, plus a final gap sweep. 24 mechanisms confirmed, 6 refuted, 4 plausible-but-unproven. The 15 inline comments below are the confirmed findings, ranked most severe first (numbered in each comment).

The dominant theme: the new shared editor and resolve model teach per-attribute cascade semantics (sparse layers, explicit-false overrides, 9-step weights, skin refs) that the persistence layer (non-nullable schema booleans, missing weight fields, whole-map preset merges) and several render paths (names, highlights, window scaffold, window refs) don't implement yet. Most findings are instances of that one gap — closing it at the schema/registry level would fix several at once.

Confirmed but cut by the 15-finding cap (lower severity):

• Ref-only backgrounds silently dropped on skin switch: SetBackgroundRef's Fill(Unspecified) placeholder round-trips to Unset through the mappers, so the documented "missing slot keeps last resolved color" fallback in resolveRefs is unreachable.
• Font picker pins inherited family/size/weight as explicit edits on Save (seeded from the resolved sample, saved unconditionally) — flips source tags to "set here" and stops skin tracking.
• Every Presets-page row shows the explicit-background ring + hex because chipStack includes baseLayers, so resolved.background is never Unset; the list and the drill-in editor disagree for the same preset.
• Custom skin without a default preset + no user base style → black entry/status text on dark chrome (the old ?: SAFE_DEFAULT_STYLE net survives only as a CompositionLocal default the views bypass).
• Hot-path allocation regressions (confirmed vs master): toSpanStyle bridge routes through resolve(listOf(toLayer())) per match span; appendStyledStringLeaf concats unconditionally; resolve() allocates a list for background; getEntireLineStyles adds a per-leaf list + re-projection. RenderingBenchmark covers three of these, so a before/after run will show it.
• resolveSourced() duplicates resolve()'s cascade attribute-for-attribute (drift between editor and renderer would be silent — a toResolved() projection would unify them); ~88% verbatim desktop/mobile duplication across the new settings views; dead API (ColorValue, sourceOf, StyleLayer.toStyleDefinition, PresetRepository.observeForCharacter/saveGlobal); StyleChip uses naive luminance() > 0.5 twice instead of CONTRAST_CROSSOVER_LUMINANCE.

Investigated and refuted (so you don't have to chase them): settings-dialog navigation while open (dialog is document-modal, trigger unreachable); LaunchedEffect empty-map classification race (titles StateFlow present on frame 1, self-heals); entry-style save wiping new fields ('entry' isn't editable via any new path; byte-identical to master); highlight regex recompiles on palette changes (rare events; old code already full-re-rendered); Fill(Unspecified) beating real backgrounds in the cascade (never durable); per-keystroke entry re-resolution (only ~1-2 Hz during roundtime).

🤖 Generated with Claude Code

[sproctor]

1. Preset scope merge is whole-map replacement, but the editor teaches per-attribute cascade [correctness]

skinDefaults + savedPresets here (and global.presets + own.presets in PresetRepository.observeLayersForCharacter) replaces a preset's layer wholesale per name, while the editor saves sparse scope layers and previews a per-attribute cascade (styleEditorModel stacking character→global→skin).

Repro: skin gives speech a color; check only Italic at character scope → the persisted {italic=true} layer replaces the skin layer entirely → speech renders italic in the base color in game, while the editor sample shows colored italic tagged "from skin". The renderer consumes one layer per name (StyledStringHelpers.kt:97), so the merge here must cascade per attribute (or the save must persist the merged layer). Note PresetRepositoryTest.kt:67 currently codifies the replacement behavior the editor contradicts.

[sproctor]

2. Window skin-referenced colors never take effect in game [correctness]

All five GameViewModel call sites (535/566/583/1231/1290) call getStyle() bare, so the emptyMap() default means textColorRef/backgroundColorRef never resolve on the render path — unlike names/highlights/presets, which WindowRegistryImpl resolves against colorPalette.

Repro: pick a palette slot for a window's text color → SetTextColorRef sets only the ref, setStyleLayer persists textColor=Unspecified + ref → the game window renders unstyled (or the stale pre-pick literal) while the settings preview shows the picked color; skin/dark-mode switches never re-tint windows. Also: the in-game context-menu dialog saves via setStyle (WindowSettingsRepository.kt:134), which nulls both refs outright. Suggest resolving window refs at the WindowRegistryImpl boundary and deleting the empty-palette default.

[sproctor]

3. Explicit boolean-false overrides cannot persist — the checkbox bounces back [correctness]

Write side collapses Boolean? to a plain flag (italic = italic == true), read side maps false → null (if (italic) true else null), so "explicitly off" round-trips to "unset".

Repro: skin sets speech italic; uncheck Italic at character scope → SetItalic(false) → persisted as false → re-read as null → resolve() falls through to the skin's true → the checkbox re-checks itself as soon as the flow re-emits, and the italic keeps rendering. Same collapse in every config type incl. base style (applyBaseStyle) and windows. Root cause: the schema booleans are non-nullable (ConfigSchema.kt), so the TOML can't express the tri-state that StyleLayer/applyEdit/resolve support — StyleEditorTest.setItalicFalseOverridesRatherThanClears pins exactly the semantics persistence breaks. Suggest Boolean? = null schema fields.

[sproctor]

4. Any weight other than 700 is silently discarded for windows (and names) [correctness]

bold = layer.weight == 700 with no weight field on WindowStyleConfig discards every other option the always-visible Weight dropdown offers (100–600, 800, 900). Because the window editor's editLayer is rebuilt from the persisted flow, the dropdown visibly snaps back to Default right after picking e.g. Semi Bold; 800/900 lose boldness entirely (== 700, not >=).

NameConfig.withStyle (StyleEditing.kt:37) has the identical unpaired collapse — and the weight/fontFamily/fontSize fields added to NameConfig in this PR are dead (no reader or writer anywhere). Related fork: read side treats >=600 as bold (ResolvedStyle.kt:48, ConfigMappers.kt:154) vs ==700 writes. Presets/highlights survive only because they carry a separate weight field. Suggest persisting weight for windows, wiring NameConfig.weight, and one shared bold↔weight helper.

[sproctor]

5. Window/base bold, italic, underline are render-dead for stream text [correctness] (anchored to nearest diff line — the issue is at lines 172 and ~262)

style = uiState.style.mergeWith(defaultStyle) (line 172) carries the newly editable window/base bold/italic/underline, but WindowViewContent consumes the merged style only for textColor/backgroundColor; rowTextStyle (~line 262) is TextStyle(color, fontFamily, fontSize, fontWeight) with no fontStyle/textDecoration, and font attributes come only from effectiveFont.

So per-window Italic/Underline (new WindowStyleConfig booleans) and base-style Italic/Underline are persisted and previewed in the editor but never affect game text. Base weight renders only via toFontConfig→LocalDefaultFont, and is dropped in any window with a per-window font override (font ?: replaces the whole FontConfig). The rowTextStyle block is byte-identical to master — new settings surface wired into a render path never extended to consume it.

[sproctor]

11. toSortedMap() is JVM-only — this breaks the iOS build [build-break]

kotlin.collections.toSortedMap exists only in the JVM stdlib, and mobileMain is a parent source set of iosMain (group("mobile") { withAndroidTarget(); group("ios") { withIos() } }). Same call in ColorRefPickerDialog.kt:60. Latent because CI and release pass -PiosSkip=true, so nothing catches it until someone builds the documented iOS path (-PiosSkip=false) and hits an unresolved reference. Fix: palette.entries.sortedBy { it.key }.

[sproctor]

12. Windows page computes its "default text" from the folded-away presets["default"] [correctness]

After this PR's own load-time migration, savedPresets can never contain "default", and it's single-scope (global skipped), so defaultStyle degenerates to the skin default. The game's real base is WindowRegistryImpl.baseStyle (character + global base via observeBaseStyle, + skin). A user's custom base text style therefore never appears in this page's window samples and inherited values — the honest-preview goal is defeated on exactly the page built for it. Mobile WindowsView.kt:53 is identical. Suggest reusing the base-style cascade here (both views currently lack characterSettingsRepository).

[sproctor]

13. Unvalidated weight from TOML crashes the settings UI and silently swallows stream lines [correctness]

FontWeight(it) requires 1..1000. The new per-style weight schema fields accept any Int from hand-edited TOML (an explicitly supported flow — the config watcher exists for it, and the value parses so the reload-error catch never fires), and nothing clamps between decode and construction (FontPickerState clamps size but not weight).

With weight = 0/1200 on a preset: opening Text styles/Highlights crashes composition via StyleChip.kt:83 (the uncaught-exception workaround in Main.kt only swallows known Compose bugs), and on the render path StreamWorkQueue's catch turns every line using the style into a logged error that never appears in the window. Suggest clamping/validating at the mapper (weight?.coerceIn(1, 1000) or reject on load).

[sproctor]

14. Highlights/names previews and contrast warnings composite against a hardcoded dark background [correctness]

The windowBackground = Color(0xFF1E1F22) default feeds both the preview composite and the low-contrast warning, and the highlights/names call sites on both platforms never pass the real base background (list rows hardcode SAFE_DEFAULT_STYLE's too). On a light skin, a dark-navy highlight previews nearly invisible with a spurious "Low contrast" warning while sitting at ~12:1 in game; light colors preview fine yet are unreadable in game with no warning. The Presets page resolves and passes the real base — the other two pages should get the same value.

[sproctor]

15. "Inherit" can never show as selected when any lower layer sets a background [correctness]

selected = current == Background.Unset where callers pass current = sample.background — the resolved cascade value, which is a Fill whenever the skin/base sets a background. Round-trip proof: pick Inherit (writes Unset to the edit layer), reopen the dialog → Color is highlighted with the inherited hex, contradicting the choice just made. The editor already has editLayer.background in hand (used for the tracks-slot tag) — the selection state should be driven from the sparse layer. Mobile TextStyleEditor/BackgroundPickerDialog identical.