diff --git a/kiln-core/src/colors.rs b/kiln-core/src/colors.rs index 9f210f8..dd93705 100644 --- a/kiln-core/src/colors.rs +++ b/kiln-core/src/colors.rs @@ -32,20 +32,5 @@ pub const NAMED_COLORS: [(&str, Rgba8); 16] = [ ("White", rgb(0xFF, 0xFF, 0xFF)), ]; -/// Parses an `"#RRGGBB"` hex color string into an [`Rgba8`]. -/// Returns opaque black on any parse failure. -pub(crate) fn parse_color(hex: &str) -> Rgba8 { - let hex = hex.trim_start_matches('#'); - if hex.len() != 6 { - return Rgba8 { - r: 0, - g: 0, - b: 0, - a: 255, - }; - } - let r = u8::from_str_radix(&hex[0..2], 16).unwrap_or(0); - let g = u8::from_str_radix(&hex[2..4], 16).unwrap_or(0); - let b = u8::from_str_radix(&hex[4..6], 16).unwrap_or(0); - Rgba8 { r, g, b, a: 255 } -} \ No newline at end of file +// Hex color parsing/formatting lives in `glyph.rs` alongside the `Glyph` serde +// adapter that uses it — see `glyph::parse_color` / `glyph::color_to_hex`. \ No newline at end of file diff --git a/kiln-core/src/glyph.rs b/kiln-core/src/glyph.rs index 3aef67c..1b702e1 100644 --- a/kiln-core/src/glyph.rs +++ b/kiln-core/src/glyph.rs @@ -22,8 +22,14 @@ pub struct Glyph { /// Which tile to draw pub tile: u32, /// Foreground color, applied to non-background pixels of the tile. + /// + /// Serialized as an `"#RRGGBB"` hex string — see [`parse_color`]. + #[serde(with = "hex_color")] pub fg: Rgba8, /// Background color, drawn as a filled rectangle behind the tile. + /// + /// Serialized as an `"#RRGGBB"` hex string — see [`parse_color`]. + #[serde(with = "hex_color")] pub bg: Rgba8, } @@ -93,43 +99,149 @@ impl Glyph { } } -// TODO make TileIndex work again -/* -/// A tile index in a palette entry: either a plain integer or a character literal. +// TODO make TileIndex work again: `tile` should accept either a single-character +// string (used directly as the display char) or a `u8` (looked up in CP437). That +// change turns `Glyph::tile` into a `char`, with `'\0'` as the transparent +// sentinel and `CP437[0]` remapped to `'\0'` to match. + +/// Parses an `"#RRGGBB"` hex color into an opaque [`Rgba8`]. /// -/// Accepting both forms lets map files write `tile = 35` or `tile = "#"` (the char -/// is converted to its Unicode scalar) interchangeably. -#[derive(Deserialize, Serialize, Eq, Clone, Copy, Debug)] -#[serde(untagged)] -pub enum TileIndex { - /// A direct tile index (e.g. `tile = 35`). - Num(u32), - /// A single-character shorthand (e.g. `tile = "#"`); converted to its Unicode scalar. - Chr(char), +/// The leading `#` is optional. Returns a human-readable `Err` describing the +/// problem — map loading surfaces it with the file position, and the script color +/// API logs it. +/// +/// **Alpha is deliberately not part of the format.** A cell draws exactly one +/// glyph, chosen by the fixed precedence in +/// [`Board::glyph_at`](crate::board::Board::glyph_at) — there is no compositing +/// pass, so there is nothing for a translucent color to blend against. (A +/// see-through cell is expressed by tile `0`, which falls through to whatever the +/// next precedence level draws; that is a different mechanism entirely.) Every +/// front-end drops alpha at the render boundary, so accepting an `"#RRGGBBAA"` +/// here would silently do nothing — an `Err` is more honest. +pub fn parse_color(hex: &str) -> Result { + let digits = hex.strip_prefix('#').unwrap_or(hex); + if digits.len() != 6 { + return Err(format!( + "expected a hex color like \"#RRGGBB\", got {hex:?}" + )); + } + // Two hex digits at `offset`, or an error naming the offending channel. + let byte = |offset: usize, channel: &str| -> Result { + u8::from_str_radix(&digits[offset..offset + 2], 16) + .map_err(|_| format!("{channel} channel of {hex:?} is not a hex byte")) + }; + Ok(Rgba8 { + r: byte(0, "red")?, + g: byte(2, "green")?, + b: byte(4, "blue")?, + a: 255, + }) } -impl TileIndex { - /// Returns the tile index as a `u32`, converting a char to its scalar value. - pub(crate) fn into_u32(self) -> u32 { - match self { - TileIndex::Num(n) => n, - TileIndex::Chr(c) => c as u32, - } +/// Formats an [`Rgba8`] as `"#RRGGBB"`. The inverse of [`parse_color`]. +/// +/// Alpha is not emitted — see [`parse_color`] for why it is not part of the +/// format. Colors in a board are always opaque, so nothing is lost. +pub fn color_to_hex(c: Rgba8) -> String { + format!("#{:02X}{:02X}{:02X}", c.r, c.g, c.b) +} + +/// `serde` adapter letting [`Glyph`]'s color fields round-trip as hex strings. +/// +/// `Rgba8` comes from the `color` crate, so it cannot implement `Serialize` / +/// `Deserialize` for our format directly; `#[serde(with = "hex_color")]` on each +/// field routes through [`parse_color`] / [`color_to_hex`] instead of the +/// structural `{ r, g, b, a }` form. +mod hex_color { + use super::{color_to_hex, parse_color}; + use color::Rgba8; + use serde::{Deserialize, Deserializer, Serializer}; + + pub fn serialize(color: &Rgba8, s: S) -> Result { + s.serialize_str(&color_to_hex(*color)) + } + + pub fn deserialize<'de, D: Deserializer<'de>>(d: D) -> Result { + let text = String::deserialize(d)?; + parse_color(&text).map_err(serde::de::Error::custom) } } -impl PartialEq for TileIndex { - fn eq(&self, other: &Self) -> bool { - self.into_u32() == other.into_u32() - } -} +#[cfg(test)] +mod tests { + use super::{color_to_hex, parse_color, Glyph}; + use color::Rgba8; -impl Into for TileIndex { - fn into(self) -> u32 { - match self { - TileIndex::Num(n) => n, - TileIndex::Chr(c) => c as u32, - } + #[test] + fn parse_color_accepts_six_digits_with_optional_hash() { + let expected = Rgba8 { r: 0x11, g: 0x22, b: 0x33, a: 255 }; + assert_eq!(parse_color("#112233").unwrap(), expected); + assert_eq!(parse_color("112233").unwrap(), expected, "leading # is optional"); + assert_eq!(parse_color("#AABBCC").unwrap(), parse_color("#aabbcc").unwrap()); } -} - */ \ No newline at end of file + + #[test] + fn parse_color_rejects_bad_input() { + // Wrong length, non-hex digits, and empty all report rather than + // silently yielding black (which would be an invisible authoring bug). + assert!(parse_color("#12345").is_err()); + assert!(parse_color("#1122334").is_err()); + assert!(parse_color("#gg2233").is_err()); + assert!(parse_color("").is_err()); + } + + #[test] + fn parse_color_rejects_an_alpha_channel() { + // A cell draws one glyph by precedence with no compositing pass, and every + // front-end drops alpha at the render boundary — so "#RRGGBBAA" would be + // silently ignored. Rejecting it tells the author instead. + assert!(parse_color("#11223380").is_err()); + } + + #[test] + fn color_to_hex_never_emits_alpha() { + assert_eq!(color_to_hex(Rgba8 { r: 1, g: 2, b: 3, a: 255 }), "#010203"); + // Even a non-opaque value (which a board never holds) stays six digits, + // so output always round-trips back through parse_color. + assert_eq!(color_to_hex(Rgba8 { r: 1, g: 2, b: 3, a: 4 }), "#010203"); + } + + #[test] + fn glyph_colors_round_trip_through_toml_as_hex() { + let glyph = Glyph { + tile: 35, + fg: Rgba8 { r: 0x80, g: 0x80, b: 0x80, a: 255 }, + bg: Rgba8 { r: 0x60, g: 0x60, b: 0x60, a: 255 }, + }; + let text = toml::to_string(&glyph).unwrap(); + assert!(text.contains(r##"fg = "#808080""##), "fg emitted as hex: {text}"); + assert_eq!(toml::from_str::(&text).unwrap(), glyph); + } + + #[test] + fn glyph_deserializes_from_hand_written_hex() { + let glyph: Glyph = toml::from_str( + r##" + tile = 35 + fg = "#808080" + bg = "#606060" + "##, + ) + .unwrap(); + assert_eq!(glyph.fg, Rgba8 { r: 0x80, g: 0x80, b: 0x80, a: 255 }); + assert_eq!(glyph.bg, Rgba8 { r: 0x60, g: 0x60, b: 0x60, a: 255 }); + } + + #[test] + fn glyph_reports_a_malformed_color_instead_of_defaulting() { + let err = toml::from_str::( + r##" + tile = 35 + fg = "#nothex" + bg = "#606060" + "##, + ) + .unwrap_err(); + assert!(err.to_string().contains("hex"), "error mentions hex: {err}"); + } +} \ No newline at end of file diff --git a/kiln-core/src/script.rs b/kiln-core/src/script.rs index eb46a38..eebc46e 100644 --- a/kiln-core/src/script.rs +++ b/kiln-core/src/script.rs @@ -49,7 +49,8 @@ use crate::api::player::PlayerWithPos; use crate::api::queue::ObjQueue; use crate::api::registry::Registry; use crate::builtin::BUILTIN_SOURCES; -use crate::colors::parse_color; +use color::Rgba8; +use crate::glyph::parse_color; use crate::glyph::Glyph; use crate::keys::Keyring; use crate::player::PlayerRef; @@ -506,8 +507,22 @@ fn register_write_api(engine: &mut Engine, board: BoardRef, log_sink: LogSink) { } ); + // Parses a script-supplied "#RRGGBB" color, logging and dropping a malformed + // one (a `None` channel leaves that color unchanged) rather than silently + // painting it black, which is invisible to debug. + fn color_arg(text: &str, fn_name: &str, log_sink: &LogSink) -> Option { + match parse_color(text) { + Ok(color) => Some(color), + Err(err) => { + log_sink.error(format!("{fn_name}: {err}")); + None + } + } + } + // set_fg(fg): change foreground color only. let b = board.clone(); + let sink = log_sink.clone(); engine.register_fn( "set_fg", move |ctx: NativeCallContext, fg: ImmutableString| { @@ -515,7 +530,7 @@ fn register_write_api(engine: &mut Engine, board: BoardRef, log_sink: LogSink) { &b, source_of(&ctx), Action::SetColor { - fg: Some(parse_color(fg.as_str())), + fg: color_arg(fg.as_str(), "set_fg", &sink), bg: None, }, ); @@ -524,6 +539,7 @@ fn register_write_api(engine: &mut Engine, board: BoardRef, log_sink: LogSink) { // set_bg(bg): change background color only. let b = board.clone(); + let sink = log_sink.clone(); engine.register_fn( "set_bg", move |ctx: NativeCallContext, bg: ImmutableString| { @@ -532,7 +548,7 @@ fn register_write_api(engine: &mut Engine, board: BoardRef, log_sink: LogSink) { source_of(&ctx), Action::SetColor { fg: None, - bg: Some(parse_color(bg.as_str())), + bg: color_arg(bg.as_str(), "set_bg", &sink), }, ); }, @@ -540,6 +556,7 @@ fn register_write_api(engine: &mut Engine, board: BoardRef, log_sink: LogSink) { // set_color(fg, bg): change both colors. let b = board.clone(); + let sink = log_sink.clone(); engine.register_fn( "set_color", move |ctx: NativeCallContext, fg: ImmutableString, bg: ImmutableString| { @@ -547,8 +564,8 @@ fn register_write_api(engine: &mut Engine, board: BoardRef, log_sink: LogSink) { &b, source_of(&ctx), Action::SetColor { - fg: Some(parse_color(fg.as_str())), - bg: Some(parse_color(bg.as_str())), + fg: color_arg(fg.as_str(), "set_color", &sink), + bg: color_arg(bg.as_str(), "set_color", &sink), }, ); }, diff --git a/kiln-tui/src/render.rs b/kiln-tui/src/render.rs index 643c9aa..005f603 100644 --- a/kiln-tui/src/render.rs +++ b/kiln-tui/src/render.rs @@ -197,13 +197,6 @@ mod tests { spec.into_board(&HashSet::new()).expect("board spec converts") } - /// An `{ r, g, b, a }` color table. `Glyph` derives serde straight onto - /// `color::Rgba8`, so map TOML spells colors out structurally rather than as - /// `"#rrggbb"` strings. - fn rgb(r: u8, g: u8, b: u8) -> String { - format!("{{ r = {r}, g = {g}, b = {b}, a = 255 }}") - } - /// Builds a tiny dark board: a 5×1 corridor with the player at the left end /// and a wall at x=2 occluding the two cells behind it. fn dark_corridor() -> Board { @@ -267,7 +260,7 @@ mod tests { // and a lamp you can walk through belongs off-grid. The player sits at x=0 // (Board::lighting reads player_pos for the sightline) but carries no torch, // so the sensor is the only light. - let board = board_from(&format!( + let board = board_from( r##" name = "lit" width = 6 @@ -284,11 +277,9 @@ mod tests { draw_layer = "Above" opaque = false glow = 2 - glyph = {{ tile = 1, fg = {fg}, bg = {bg} }} + glyph = { tile = 1, fg = "#FF0000", bg = "#000000" } "##, - fg = rgb(255, 0, 0), - bg = rgb(0, 0, 0), - )); + ); let fov = board.lighting(0); // no player torch — only the sensor lights let area = Rect::new(0, 0, 6, 1); diff --git a/maps/tiny.toml b/maps/tiny.toml index 57253a3..8faa0cb 100644 --- a/maps/tiny.toml +++ b/maps/tiny.toml @@ -31,7 +31,7 @@ grid = """ ##################### # # # G @ # -# oo # +# oo hh # # # ##################### """ @@ -40,4 +40,5 @@ grid = """ "o" = { type = "builtin", kind = "crate", glyph = { tile = 254, fg = "#aaaaaa", bg = "#000000" } } "@" = { type = "player" } # "G" = { type = "object", tile = 35, fg = "#aa3333", bg = "#000000", enter = "block", script_name = "greeter" } +"h" = { type = "builtin", kind = "heart" } "G" = { type = "builtin", kind = "gem" }