fix: module 10 review round 1 (module-local SpellGrid cards, cast target check, CLI without panic hook)

- SpellGrid cards move to course/10-spellgrid/cards; start-module and verify-references stage course/cards plus module-local cards up to the module, so M5-M9 workspaces get exactly the pre-M10 card set. - cast() rejects a chosen-target spell without a target (checkCast/checkPlay); new test_10_targeted_spell_needs_target. - Rust CLI pre-validates instead of swapping the global panic hook. - Catalog: CRIER noun; DEATHRATTLE/BATTLECRY note on lexicon vs catalog role. - Lesson: design note on mid-movement reparse. Co-Authored-By: Claude Opus 5.5 <[email protected]>
2026-10-29 05:11:09 +01:00
parent a0cf9be214
commit 6f576940b7
34 changed files with 295 additions and 79 deletions
@@ -14,7 +14,6 @@

 use std::cell::RefCell;
 use std::io::{BufRead, Write};
-use std::panic::{self, AssertUnwindSafe};
 use std::rc::Rc;

 use crate::harness::{new_spell_grid, Dir, LevelDef, SpellGrid};
@@ -138,13 +137,7 @@ pub fn run(level_text: &str, input: Box<dyn BufRead>, output: Box<dyn Write>) ->
            .and_then(|n| options.get(n.wrapping_sub(1)));
        picked.unwrap_or(&options[0]).clone()
    });
-    // The harness panics on a move the rules don't allow (a card not in the hand, a
-    // full tile). The CLI reports the message instead of stopping.
-    let hook = panic::take_hook();
-    panic::set_hook(Box::new(|_| {}));
-    let outcome = play(&mut g, &level, &io);
-    panic::set_hook(hook);
-    outcome
+    play(&mut g, &level, &io)
 }

 /// Draw, read a command, do it; until the level is won or lost, or the player quits.
@@ -172,27 +165,26 @@ fn play(g: &mut SpellGrid, level: &LevelDef, io: &Rc<RefCell<Io>>) -> Outcome {
            Some(&"d") => Some(Dir::Right),
            _ => None,
        };
-        let result = panic::catch_unwind(AssertUnwindSafe(|| match (dir, words.as_slice()) {
-            (Some(dir), _) => g.step(dir),
+        // The harness panics on a move the rules don't allow (a card not in the hand, a
+        // full tile). The CLI asks first, and reports the reason instead.
+        let done = match (dir, words.as_slice()) {
+            (Some(dir), _) => Ok(g.step(dir)),
            (None, ["c", args @ ..]) if !args.is_empty() => match parse_cast(args) {
-                (card, None) => g.cast(&card, None),
+                (card, None) => g.check_cast(&card, None).map(|()| g.cast(&card, None)),
                (card, Some((x, y))) => match g.minion_at(x, y) {
-                    Some(target) => g.cast(&card, Some(target)),
-                    None => {
+                    Some(target) => g
+                        .check_cast(&card, Some(target))
+                        .map(|()| g.cast(&card, Some(target))),
+                    None => g.check_play(&card, x, y).map(|()| {
                        g.play(&card, x, y);
-                    }
+                    }),
                },
            },
-            _ => io
-                .borrow_mut()
-                .write("Commands: w a s d (move), c <card> [x y] (cast or play), q (quit)\n"),
-        }));
-        if let Err(payload) = result {
-            let message = payload
-                .downcast_ref::<String>()
-                .cloned()
-                .or_else(|| payload.downcast_ref::<&str>().map(|s| s.to_string()))
-                .unwrap_or_else(|| "that can't be done".to_string());
+            _ => {
+                Err("Commands: w a s d (move), c <card> [x y] (cast or play), q (quit)".to_string())
+            }
+        };
+        if let Err(message) = done {
            io.borrow_mut().write(&format!("{message}\n"));
        }
    }
@@ -15,6 +15,7 @@
  { "name": "MOTH", "role": "noun" },
  { "name": "SCRIBE", "role": "noun" },
  { "name": "KNIGHT", "role": "noun" },
+  { "name": "CRIER", "role": "noun" },
  { "name": "DEATHRATTLE", "role": "noun" },
  { "name": "YOU", "role": "property" },
  { "name": "WIN", "role": "property" },
@@ -24,7 +24,7 @@ use std::collections::{HashMap, HashSet};
 use serde::Deserialize;

 use super::aura::aura_update_other;
-use super::card::{Aura, CardLibrary, CardType, Stacking, TriggerOn};
+use super::card::{Aura, CardLibrary, CardType, Effect, Stacking, Target, TriggerOn};
 use super::event::Event;
 use super::puzzle::{Dir, Hazard, Kind, Level, LevelDef, ObjectId, Response, Rule, WordDef};
 use super::queue::{run_sequence, Action};
@@ -180,9 +180,62 @@ impl SpellGrid {
        self.side.grid_changed(&mut self.level);
    }

+    /// Why `cast(card, target)` can't be done now, or `Ok` if it can: the card must be a
+    /// spell in player 0's hand, and a spell that hits a chosen minion needs a target
+    /// minion on the grid. Nothing changes either way.
+    pub fn check_cast(&self, card: &str, target: Option<ObjectId>) -> Result<(), String> {
+        self.side.check_hand(card, CardType::Spell)?;
+        let chooses = self.side.world.card(card).on_cast.iter().any(|e| {
+            matches!(
+                e,
+                Effect::DealDamage {
+                    target: Target::Chosen,
+                    ..
+                } | Effect::GainAttack {
+                    target: Target::Chosen,
+                    ..
+                } | Effect::Destroy {
+                    target: Target::Chosen
+                }
+            )
+        });
+        match target {
+            None if chooses => Err(format!("{card:?} needs a target minion")),
+            None => Ok(()),
+            Some(id) => {
+                // Every id in `entities` was issued by the board, so `find` may ask about it.
+                let card = self.side.entities.contains_key(&id);
+                if card && self.level.board().find(id).is_some() {
+                    Ok(())
+                } else {
+                    Err(format!("{id} is not a minion on the grid"))
+                }
+            }
+        }
+    }
+
+    /// Why `play(card, x, y)` can't be done now, or `Ok` if it can.
+    pub fn check_play(&self, card: &str, x: i32, y: i32) -> Result<(), String> {
+        self.side.check_hand(card, CardType::Minion)?;
+        if self.side.world.card(card).def.noun.is_none() {
+            return Err(format!("{card:?} has no noun: it can't stand on the grid"));
+        }
+        if !self.level.board().in_bounds(x, y) {
+            return Err(format!("({x}, {y}) is off the grid"));
+        }
+        if self.level.board().at(x, y).next().is_some() {
+            return Err(format!("({x}, {y}) is not empty"));
+        }
+        Ok(())
+    }
+
    /// Cast a spell from player 0's hand, as one Sequence. `target` is a minion's id.
+    /// Panics if `check_cast` says no.
    pub fn cast(&mut self, card: &str, target: Option<ObjectId>) {
-        let in_hand = self.side.from_hand(card, CardType::Spell);
+        if let Err(problem) = self.check_cast(card, target) {
+            panic!("{problem}");
+        }
+        let in_hand = self.side.in_hand(card).expect("checked");
        self.side.world.move_to(in_hand, Zone::Graveyard);
        let target = target.map(|id| self.side.entity(id));
        let action = Action::Cast {
@@ -194,13 +247,12 @@ impl SpellGrid {
    }

    /// Play a minion from player 0's hand onto the empty tile (x, y), as one Sequence.
-    /// Returns its new id.
+    /// Returns its new id. Panics if `check_play` says no.
    pub fn play(&mut self, card: &str, x: i32, y: i32) -> ObjectId {
-        let in_hand = self.side.from_hand(card, CardType::Minion);
-        assert!(
-            self.level.board().at(x, y).next().is_none(),
-            "({x}, {y}) is not empty"
-        );
+        if let Err(problem) = self.check_play(card, x, y) {
+            panic!("{problem}");
+        }
+        let in_hand = self.side.in_hand(card).expect("checked");
        let kind = self.side.noun_of(card);
        let id = self.level.board_mut().place(kind, x, y);
        self.side.link(in_hand, id);
@@ -241,19 +293,26 @@ impl CardSide {
        }
    }

-    fn from_hand(&self, card: &str, card_type: CardType) -> EntityId {
-        let in_hand = self
-            .world
+    fn in_hand(&self, card: &str) -> Option<EntityId> {
+        self.world
            .zone(0, Zone::Hand)
            .iter()
            .copied()
            .find(|&id| self.world.card_name(id) == card)
-            .unwrap_or_else(|| panic!("{card:?} is not in the hand"));
-        assert!(
-            self.world.card(card).def.card_type == card_type,
-            "{card:?} is not a {card_type:?}"
-        );
-        in_hand
+    }
+
+    fn check_hand(&self, card: &str, card_type: CardType) -> Result<(), String> {
+        if self.in_hand(card).is_none() {
+            return Err(format!("{card:?} is not in the hand"));
+        }
+        if self.world.card(card).def.card_type != card_type {
+            let kind = match card_type {
+                CardType::Minion => "minion",
+                CardType::Spell => "spell",
+            };
+            return Err(format!("{card:?} is not a {kind}"));
+        }
+        Ok(())
    }

    /// Seam 2. Objects that aren't cards leave the board. Minions are marked pending
@@ -436,6 +436,14 @@ impl SpellGrid {
    pub fn set_chooser(&mut self, chooser: impl FnMut(&str, &[String]) -> String + 'static) {
        self.grid.world_mut().set_chooser(Box::new(chooser));
    }
+    /// For the command line: why `cast` would panic, or `Ok` if it wouldn't.
+    pub fn check_cast(&self, card: &str, target: Option<Id>) -> Result<(), String> {
+        self.grid.check_cast(card, target)
+    }
+    /// For the command line: why `play` would panic, or `Ok` if it wouldn't.
+    pub fn check_play(&self, card: &str, x: i32, y: i32) -> Result<(), String> {
+        self.grid.check_play(card, x, y)
+    }
    /// Test hook: the naive engine reads the auras before the dead leave play, the
    /// same end-of-Phase steps reversed.
    pub fn with_naive_aura_timing(&mut self) {