cleanup

kiln-core/CLAUDE.md

@@ -22,6 +22,7 @@ The core types that were formerly monolithic in `game.rs` are now split into foc
 
 **`kiln-core/src/builtin_scripts.rs`** — archetypes that are really scripted objects (`pub(crate)`):
 - `archetype_script(arch) -> Option<&'static str>` — the dispatch (one arm per script-backed archetype) returning the embedded Rhai source (`include_str!`); currently `Pusher(_) -> scripts/pusher.rhai`, `Spinner(_) -> scripts/spinner.rhai`, and `Gem -> scripts/gem.rhai` (just `fn grab() { add_gems(1); die(); }`). Such an archetype loads as a plain terrain cell (`layer::resolve_entry`); `Board::expand_builtin_archetypes` (called from `TryFrom<MapFile>` at load and again before a playtest) is the **single** place that turns it into an `ObjectDef` carrying that source in `builtin_script` plus a `BUILTIN_<archetype>` tag. The spinner reads its `BUILTIN_spinner_cw`/`_ccw` tag for spin direction (defaulting clockwise) and stores per-instance animation frame state in the board `Registry`.
+- `archetype_script_key(arch) -> Option<&'static str>` — the synthetic `BUILTIN_*` compile-key for a script-backed archetype (`"BUILTIN_pusher"`/`"BUILTIN_spinner"`/`"BUILTIN_gem"`). `expand_builtin_archetypes` stores it as the expanded object's `script_name` so all instances share one compiled AST (the *source* comes from `archetype_script`, keyed by this name). Distinct from `builtin_tag`, which is per-direction (`BUILTIN_pusher_east`) and drives save round-tripping.
 - `BUILTIN_TAG_PREFIX` (`"BUILTIN_"`), `builtin_tag(arch) -> String`, `archetype_from_builtin_tag(tag) -> Option<Archetype>` — the tag convention. The script reads its tag for per-instance params (the pusher reads it for its direction, since `Me` isn't visible inside Rhai helper functions); `map_file::save` uses it to collapse the object back into its archetype keyword so maps round-trip. `scripts/pusher.rhai` self-propels via `move(dir)` (which already shoves chains via `step_object`/`push`), pacing itself with `delay` to the old ~0.5 s cadence.
 
 **`kiln-core/src/utils.rs`** — shared primitive types (`pub(crate)`):
@@ -33,7 +34,7 @@ The core types that were formerly monolithic in `game.rs` are now split into foc
 - `PortalDef { x, y, target_map, target_entry }` — parsed from map files, not yet runtime-wired.
 
 **`kiln-core/src/object_def.rs`** — scripted objects (`pub(crate)`):
-- `ObjectDef` — a scripted tile: `x`, `y`, `z: usize` (layer index, drives draw order), `glyph: Glyph`, `solid: bool` (default `true`), `opaque: bool` (default `true`), `pushable: bool` (default `false`), `grab: bool` (default `false`; set when a grab archetype like a gem is expanded — walking onto it fires `grab()`), `script_name: Option<String>`, `builtin_script: Option<&'static str>` (embedded source set when a script-backed archetype like a pusher is expanded at load; takes precedence over `script_name`; not serialized — see [`builtin_scripts`]), `tags: HashSet<String>`, `name: Option<String>`. The optional `name` is validated for board-wide uniqueness at load time; the first claimant keeps the name and any later duplicate has its name cleared to `None` (nonfatal). `ObjectDef::new(x, y)` constructs with defaults (`z = 0`). `ObjectDef::default_glyph()` returns tile 63 (`?`) yellow on black.
+- `ObjectDef` — a scripted tile: `x`, `y`, `z: usize` (layer index, drives draw order), `glyph: Glyph`, `solid: bool` (default `true`), `opaque: bool` (default `true`), `pushable: bool` (default `false`), `grab: bool` (default `false`; set when a grab archetype like a gem is expanded — walking onto it fires `grab()`), `script_name: Option<String>`, `builtin_script: Option<&'static str>` (embedded source set when a script-backed archetype like a pusher is expanded at load; the expansion also sets `script_name` to a synthetic `BUILTIN_*` compile-key, so `builtin_script` supplies the *source* while `script_name` is the *key*; not serialized — see [`builtin_scripts`]), `tags: HashSet<String>`, `name: Option<String>`. The optional `name` is validated for board-wide uniqueness at load time; the first claimant keeps the name and any later duplicate has its name cleared to `None` (nonfatal). `ObjectDef::new(x, y)` constructs with defaults (`z = 0`). `ObjectDef::default_glyph()` returns tile 63 (`?`) yellow on black.
 
 **`kiln-core/src/layer.rs`** — palette layers (the map-file/draw-stack unit):
 - `Layer { cells: Vec<(Glyph, Archetype)> }` (`pub(crate)`) — one row-major draw layer. A cell whose `glyph.tile == 0` (`Glyph::transparent()`) draws nothing, so the layer beneath shows through; solidity comes from the archetype, independent of transparency.
@@ -78,7 +79,7 @@ The core types that were formerly monolithic in `game.rs` are now split into foc
 - `LogSpan { text, fg: Option<Rgba8>, bg: Option<Rgba8> }` and `LogLine { spans: Vec<LogSpan> }` — a UI-agnostic styled message (colors are core `Rgba8`, not a front-end type). `LogLine::raw()`, a chainable `push()`, `append()`, and `LogLine::error()` (a red-on-black single-span constructor used for nonfatal load errors) build messages; each front-end converts a `LogLine` to its own styled text at render time.
 
 **`kiln-core/src/script.rs`** — Rhai scripting runtime:
-- `ScriptHost` — owns the Rhai `Engine`, the compiled scripts referenced by a board's objects, a per-object persistent `Scope` + output queue + ready timer, and the shared board queue. Built with `ScriptHost::new(&Rc<RefCell<Board>>, &HashMap<String, String>)`: the first arg is a shared ref to the active board (used by read-API closures), the second is the world-level script pool. Each object resolves to a `(key, source)` compiled once per key: a named script keys by its pool name; an object's embedded `builtin_script` keys by its source text (so identical built-ins, e.g. all pushers, share one AST). Reports compile/unknown-script failures onto the error sink; runs nothing.
+- `ScriptHost` — owns the Rhai `Engine`, the compiled scripts referenced by a board's objects, a per-object persistent `Scope` + output queue + ready timer, and the shared board queue. Built with `ScriptHost::new(&Rc<RefCell<Board>>, &HashMap<String, String>)`: the first arg is a shared ref to the active board (used by read-API closures), the second is the world-level script pool. Each object resolves to a `(key, source)` compiled once per key: the key is the object's `script_name` (a world-pool name for a named script, or a synthetic `BUILTIN_*` name assigned by `expand_builtin_archetypes` so identical built-ins, e.g. all pushers, share one AST); the source is the pool entry for that name, or the object's embedded `builtin_script` when present. Reports compile/unknown-script failures onto the error sink; runs nothing.
 - Lifecycle hooks per object: `init()` (zero-arg), `tick(dt)` (elapsed seconds as `f64`), `bump(id)` (the bumper's `ObjectId`, or `-1` for the player), and `grab()` (zero-arg; fired when the player walks onto / pushes a `grab` thing into themselves), all optional — detected via `AST::iter_functions()` by name and arity. Driven by `run_init()` / `run_tick(dt)` / `run_bump(object_id, bumper)` / `run_grab(object_id)`; runtime errors go to the error sink (drained to the log), not fatal.
 - **Reads (direct):** read getters (`player_x`, `player_y`, `width`, `height`) are registered on `Rc<RefCell<Board>>` (the `BoardRef` alias, exposed to Rhai under the type name `Board`) — getters only, so read-only by construction. A clone of the handle is pushed into each scope as the constant `Board`. `blocked(dir) -> bool` is also a read fn: true if the caller's target cell is off-board, already solid, or the destination of a pending move on the board queue (an object never sees its *own* move, which is pumped only after its script returns). `can_push(x, y, dir) -> bool` is a read fn mirroring `Board::can_push`: true if `(x, y)` holds a pushable whose chain can be shoved in `dir` (false off-board). `can_shift(x, y, dir) -> bool` mirrors `Board::can_shift`: like `can_push` but only checks the single cell ahead (pushable source + an empty-or-pushable next cell), the right gate for a simultaneous shift/rotation via `swap`. `passable(x, y) -> bool` mirrors `Board::is_passable`: true if `(x, y)` is on-board and holds no solid (off-board → false), letting a script tell a hole apart from a blocked solid (which `can_shift`/`can_push` alone cannot). `has_tag(s) -> bool` and `get_tags() -> Array` read the calling object's tag set; `objects_with_tag(s) -> Array` returns all object ids carrying that tag. `my_name() -> String` returns the calling object's name (or `""` if unnamed); `object_id_for_name(s) -> i64` looks up an object by name and returns its id, or `0` if not found. Each getter briefly borrows the shared `Board`.
 - **Actions & rate limiting (two-tier queues):** host fns `move(dir)`, `set_tile(n)`, `log(s)`, `say(s)`, `scroll(lines)`, `push(x, y, dir)`, `swap(pairs)`, `add_gems(n)` (adjust the player's gem count), `die()` (remove the calling object) append an `Action` (`Move`/`SetTile`/`Log`/`Say`/`Scroll`/`Push`/`Swap`; `MOVE_COST = 0.25` s for `Move`, else 0 — `push`/`swap` add no delay) to the *issuing object's* output queue (routed by the per-call tag via a `HashMap<usize, ObjQueue>`). `scroll(lines)` takes a Rhai array where each element is a string (text line) or a two-element array `[choice_key, display_text]` (selectable choice). `push(x, y, dir)` shoves the pushable chain at arbitrary coords `(x, y)`. `swap(pairs)` takes an array of four-int `[src_x, src_y, dst_x, dst_y]` arrays (a malformed entry is skipped + logged) and moves all the named solids simultaneously (see `Board::apply_swap`). `ScriptHost::pump(i)` promotes ready actions onto the shared **board queue** (`Vec<BoardAction { source, action }>`): the leading run of zero-cost actions plus at most one timed action, which arms that object's **ready timer** and ends the pump. While a ready timer is `> 0` nothing is pulled (this caps object speed); `advance_timers(dt)` counts them down each frame. `GameState` drains the board queue with `take_board_queue()` and applies it after the batch — so scripts mutate without a `&mut GameState` borrow. Errors (compile/runtime) bypass the queues via the error sink (`take_errors()`).

kiln-core/src/board.rs

@@ -235,11 +235,13 @@ impl Board {
         self.solid_at(x, y).is_none()
     }
 
-    /// Returns `true` if the mover as `(x1, y1)` can enter `(x2, y2)` — i.e. it would not
-    /// break the rules of "one solid per cell, except for player + grab".
+    /// Returns `true` if the solids at `(x1, y1)` and `(x2, y2)` may share a cell —
+    /// i.e. moving one onto the other wouldn't break the "one solid per cell"
+    /// invariant. That holds when at least one cell is empty, or one holds the
+    /// player and the other a **grab** thing (the player collects it).
     ///
-    /// Convenience inverse of [`solid_at`](Board::solid_at).
-    /// Panics if `x` or `y` are out of bounds.
+    /// Off-board coordinates are never combinable (returns `false` rather than
+    /// panicking). Backs the script-facing `combinable(x1, y1, x2, y2)` fn.
     pub fn is_combinable(&self, x1: usize, y1: usize, x2: usize, y2: usize) -> bool {
         // Are either out of bounds?
         if !self.in_bounds((x1 as i32, y1 as i32)) || !self.in_bounds((x2 as i32, y2 as i32)) {

kiln-core/src/builtin_scripts.rs

@@ -55,9 +55,11 @@ pub(crate) fn archetype_script(arch: Archetype) -> Option<&'static str> {
     }
 }
 
-/// Returns the embedded script implementing `arch` as an object, or `None` if the
-/// archetype is a plain terrain cell. The one place script-backed archetypes are
-/// declared.
+/// Returns the synthetic `BUILTIN_*` compile-key for `arch`'s embedded script, or
+/// `None` if the archetype is a plain terrain cell. [`Board::expand_builtin_archetypes`](crate::board::Board::expand_builtin_archetypes)
+/// stores this as the expanded object's `script_name` so every instance of a
+/// script-backed archetype shares one compiled AST (the source comes from
+/// [`archetype_script`]).
 pub(crate) fn archetype_script_key(arch: Archetype) -> Option<&'static str> {
     match arch {
         Archetype::Pusher(_) => Some("BUILTIN_pusher"),

kiln-core/src/object_def.rs

@@ -54,13 +54,18 @@ pub struct ObjectDef {
     /// remove itself via `die()`). Set when a grab archetype (e.g. a gem) is
     /// expanded into an object. Defaults to `false`.
     pub grab: bool,
-    /// Name of the Rhai script in [`Board::scripts`] that drives this object.
-    /// `None` means this object has no script yet.
+    /// Compile-key of the Rhai script that drives this object: a name in
+    /// [`World::scripts`](crate::world::World::scripts) for a hand-authored object,
+    /// or a synthetic `BUILTIN_*` name set when a script-backed archetype is
+    /// expanded (see [`builtin_script`](ObjectDef::builtin_script)). `None` means
+    /// this object has no script yet.
     pub script_name: Option<String>,
     /// Embedded built-in script source, set when a script-backed archetype (e.g. a
-    /// `pusher_*`) is expanded into an object at load time (see
-    /// [`crate::builtin_scripts`]). Takes precedence over [`script_name`](ObjectDef::script_name)
-    /// and is not part of the map file (it is regenerated from the archetype on load).
+    /// `pusher_*` or `gem`) is expanded into an object at load time (see
+    /// [`crate::builtin_scripts`]). When set, this is the object's script *source*
+    /// (its compile-key is the synthetic `BUILTIN_*` [`script_name`](ObjectDef::script_name)
+    /// the same expansion assigns). Not part of the map file — it is regenerated
+    /// from the archetype on load.
     pub builtin_script: Option<&'static str>,
     /// Open-ended string labels for this object. Serialized as a TOML array;
     /// not subject to any rate limit — mutations take effect immediately after

kiln-core/src/script.rs

@@ -115,15 +115,19 @@ struct CompiledScript {
 struct ObjectRuntime {
     object_id: ObjectId,
     /// Key into [`ScriptHost::scripts`] for this object's compiled script — the
-    /// pool name for a named script, or the source text for an embedded built-in.
+    /// world-pool name for a named script, or the synthetic `BUILTIN_*` name for an
+    /// expanded built-in.
     script_key: String,
     scope: Scope<'static>,
     output_queue: ObjQueue,
 }
 
-/// The compile-key for an object's script: the embedded source itself for a
-/// built-in (so identical built-ins share one compiled AST), or the world-pool
-/// name for a named script. `None` if the object has no script.
+/// The compile-key for an object's script: the object's `script_name` — a
+/// world-pool name for a named script, or a synthetic `BUILTIN_*` name set by
+/// [`Board::expand_builtin_archetypes`](crate::board::Board::expand_builtin_archetypes)
+/// for an expanded built-in (so identical built-ins share one compiled AST,
+/// while the source still comes from `builtin_script`). `None` if the object has
+/// no script.
 fn script_key(obj: &ObjectDef) -> Option<String> {
     obj.script_name.clone()
 }
@@ -226,9 +230,10 @@ impl ScriptHost {
 
         let board = board_cell.borrow();
 
-        // Compile each referenced script once, keyed by `script_key` (pool name for
-        // named scripts, source text for embedded built-ins so identical ones share
-        // one AST).
+        // Compile each referenced script once, keyed by `script_key` (the object's
+        // `script_name`: a world-pool name for named scripts, or a synthetic
+        // `BUILTIN_*` name for built-ins so identical ones share one AST). The
+        // source for a built-in still comes from its embedded `builtin_script`.
         let mut scripts: HashMap<String, CompiledScript> = HashMap::new();
         let mut failed: HashSet<String> = HashSet::new();
         for obj in board.objects.values() {

kiln-core/src/tests/map_file/pushers.rs

@@ -36,7 +36,9 @@ fn pusher_loads_as_a_tagged_scripted_solid_object() {
         p.builtin_script.is_some(),
         "carries the embedded pusher script"
     );
-    assert!(p.script_name.is_none());
+    // Expansion keys all pushers under one synthetic script name so they share a
+    // compiled AST; the source still comes from `builtin_script`.
+    assert_eq!(p.script_name.as_deref(), Some("BUILTIN_pusher"));
     assert!(!board.is_passable(0, 0), "pusher is solid");
 }
 

kiln-core/src/tests/map_file/spinners.rs

@@ -25,7 +25,9 @@ fn spinner_loads_as_a_tagged_scripted_solid_object() {
         obj.builtin_script.is_some(),
         "carries the embedded spinner script"
     );
-    assert!(obj.script_name.is_none());
+    // Expansion keys all spinners under one synthetic script name so they share a
+    // compiled AST; the source still comes from `builtin_script`.
+    assert_eq!(obj.script_name.as_deref(), Some("BUILTIN_spinner"));
     assert!(!board.is_passable(0, 0), "spinner is solid");
 }
 

kiln-core/src/utils.rs

@@ -84,17 +84,19 @@ pub enum Solid {
 }
 
 impl Solid {
+    /// Whether this occupant is the player.
     pub fn player(&self) -> bool {
         matches!(self, Solid::Player)
     }
 
+    /// Whether this occupant is a **grab** thing (a gem-like solid the player
+    /// collects on contact). The player itself is never grabbable; a terrain cell
+    /// reports its archetype's `grab` flag, and an object reports its own.
     pub fn grab(&self, board: &Board) -> bool {
         match self {
             Solid::Player => false,
             Solid::Cell(arch) => arch.behavior().grab,
-            Solid::Object(id) => {
-                board.objects.get(id).map_or(false, |obj| obj.grab)
-            }
+            Solid::Object(id) => board.objects.get(id).is_some_and(|obj| obj.grab),
         }
     }
 }