Piece internal API overhaul: pass PieceState, fallible

Magic pieces are going to need to look at their state to decide how
to format themselves, and that is fallible.

Make formatting methods on PieceState fallible.  They return
InternalError.  Pass these methods &PieceState.

Textually extremely intrusive, but no functional change with existing
actually-infallible call sites.

We provide infallible versions of the describe_html functions, which
return a placeholder if there is an error.  This is because typically
at their call sites, returning an error is very inconvenient.  Eg, it
happens after a game update has actually occurred.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>

diff --git a/daemon/api.rs b/daemon/api.rs
index 18492240e1a1b60748f66b8bae296b03b9591dcd..dd2f04fa702f20e403661ab9fdaee45b2058f177 100644 (file)
--- a/daemon/api.rs
+++ b/daemon/api.rs
@@ -242,7 +242,7 @@ api_route!{
 
     let gpl = players.byid_mut(player)?;
     let pri = piece_pri(&gs.occults, player, gpl, piece, pc);
-    let pcs = p.describe_pri(&pri).0;
+    let pcs = p.describe_pri(pc, &pri).0;
 
     pc.held = Some(player);
 

diff --git a/daemon/cmdlistener.rs b/daemon/cmdlistener.rs
index 67aa47c6cef0c8c3ba75290d8fe68bd84f6433ba..9c1a7ee56aa184a30994e35557b784f88ae24803 100644 (file)
--- a/daemon/cmdlistener.rs
+++ b/daemon/cmdlistener.rs
@@ -376,7 +376,7 @@ fn execute_game_insn<'cs, 'igr, 'ig: 'igr>(
       let pieces = ig.gs.pieces.iter().map(|(piece,p)|{
         let &PieceState { pos, face, .. } = p;
         let pinfo = ig.ipieces.get(piece)?;
-        let desc_html = pinfo.describe_html(None);
+        let desc_html = pinfo.describe_html_infallible(None, p);
         let itemname = pinfo.itemname().to_string();
         let bbox = pinfo.bbox_approx();
         #[allow(irrefutable_let_patterns)]
@@ -551,7 +551,11 @@ fn execute_game_insn<'cs, 'igr, 'ig: 'igr>(
         .remove(piece).ok_or(ME::PieceNotFound)?;
       let gs = &mut ig.gs;
       let pc = gs.pieces.as_mut(modperm).remove(piece);
-      let desc_html = p.describe_html(Some(default()));
+      let desc_html = if let Some(pc) = &pc {
+        p.describe_html_infallible(Some(default()), pc)
+      } else {
+        Html::lit("<piece partially missing from game state!>")
+      };
       if let Some(pc) = pc { p.delete_hook(&pc, gs); }
       (U{ pcs: vec![(piece, PieceUpdateOp::Delete())],
           log: vec![ LogEntry {

diff --git a/daemon/session.rs b/daemon/session.rs
index bd9381d46fc475f0bc1382c8c9eae89f19690673..ad6faa65a3f914454233cc2229e53aa2be5fe368 100644 (file)
--- a/daemon/session.rs
+++ b/daemon/session.rs
@@ -111,7 +111,7 @@ fn session_inner(form: Json<SessionForm>,
       let pri = piece_pri(&ig.gs.occults, player, gpl, gpid, pr);
       let p = if let Some(p) = ig.ipieces.get(gpid) { p }
       else { continue /* was deleted */ };
-      let defs = p.make_defs(&pri)?;
+      let defs = p.make_defs(pr, &pri)?;
       alldefs.push((pri.id, defs));
 
       let vangle = match pri.angle {

diff --git a/src/bin/otterlib.rs b/src/bin/otterlib.rs
index 88e0bde391992e18fb9cf59fa3ab981397fff2bb..77d4b053b1dfd95032e9798dcd8b4f65c5781219 100644 (file)
--- a/src/bin/otterlib.rs
+++ b/src/bin/otterlib.rs
@@ -93,20 +93,7 @@ fn preview(items: Vec<ItemForOutput>) {
     (||{
       let pc = spec.clone().load().context("load")?;
       let mut uos = vec![];
-      let gen_dummy = Generation(1);
-      let gpc_dummy = PieceState {
-        pos: PosC([0,0]),
-        face: default(),
-        held: None,
-        zlevel: ZLevel { z: default(), zg: gen_dummy },
-        pinned: false,
-        occult: default(),
-        angle: default(),
-        gen: gen_dummy,
-        lastclient: ClientId(default()),
-        gen_before_lastclient: gen_dummy,
-        xdata: None,
-      };
+      let gpc_dummy = PieceState::dummy();
       pc.add_ui_operations(&mut uos, &gpc_dummy).context("add uos")?;
       let uos = uos.into_iter().map(|uo| uo.opname).collect::<Vec<_>>();
       let spec = spec.clone();
@@ -133,6 +120,8 @@ fn preview(items: Vec<ItemForOutput>) {
   let max_facecols = pieces.iter().map(|s| s.face_cols()).max().unwrap_or(1);
   let max_uos = pieces.iter().map(|s| s.uos.len()).max().unwrap_or(0);
 
+  let gpc_dummy = PieceState::dummy();
+
   println!("{}", &HTML_PRELUDE);
   println!(r#"<table rules="all">"#);
   for s in &pieces {
@@ -142,7 +131,8 @@ fn preview(items: Vec<ItemForOutput>) {
              Html::from_txt(&spec.lib).0);
     println!(r#"<th align="left"><kbd>{}</kbd></th>"#,
              Html::from_txt(&spec.item).0);
-    println!(r#"<th align="left">{}</th>"#, pc.describe_html(None).0);
+    println!(r#"<th align="left">{}</th>"#,
+             pc.describe_html(None, &gpc_dummy)?.0);
     let only1 = s.face_cols() == 1;
     let getpri = |face: FaceId| PieceRenderInstructions {
       id: default(),
@@ -189,7 +179,7 @@ fn preview(items: Vec<ItemForOutput>) {
                  &surround.0, &dasharray.0, HELD_SURROUND_COLOUR);
         }
         let mut html = Html("".into());
-        pc.svg_piece(&mut html, &pri)?;
+        pc.svg_piece(&mut html, &gpc_dummy, &pri)?;
         println!("{}</svg>", html.0);
       }
       println!("</td>");

diff --git a/src/gamestate.rs b/src/gamestate.rs
index d7a872c021c261c13eee7e12a7116dbb3ea0faa5..905df11384a0b39534353b43e136ec243d829306 100644 (file)
--- a/src/gamestate.rs
+++ b/src/gamestate.rs
@@ -149,10 +149,11 @@ pub trait Piece: Outline + Send + Debug {
   }
 
   // #[throws] doesn't work here - fehler #todo
-  fn svg_piece(&self, f: &mut Html, pri: &PieceRenderInstructions)
-               -> Result<(),IE>;
+  fn svg_piece(&self, f: &mut Html, gpc: &PieceState,
+               pri: &PieceRenderInstructions) -> Result<(),IE>;
 
-  fn describe_html(&self, face: Option<FaceId>) -> Html;
+  fn describe_html(&self, face: Option<FaceId>, gpc: &PieceState)
+                   -> Result<Html,IE>;
 
   fn delete_hook(&self, _p: &PieceState, _gs: &mut GameState)
                  -> ExecuteGameChangeUpdates { 
@@ -269,7 +270,7 @@ impl PieceState {
     PreparedPieceState {
       pos        : self.pos,
       held       : self.held,
-      svg        : p.make_defs(pri)?,
+      svg        : p.make_defs(self, pri)?,
       z          : self.zlevel.z.clone(),
       zg         : self.zlevel.zg,
       pinned     : self.pinned,
@@ -286,6 +287,23 @@ impl PieceState {
   pub fn xdata_mut<T:PieceXData+Default>(&mut self) -> &mut T {
     self.xdata.get_mut()?
   }
+
+  pub fn dummy() -> Self {
+    let gen_dummy = Generation(1);
+    PieceState {
+      pos: PosC([0,0]),
+      face: default(),
+      held: None,
+      zlevel: ZLevel { z: default(), zg: gen_dummy },
+      pinned: false,
+      occult: default(),
+      angle: default(),
+      gen: gen_dummy,
+      lastclient: ClientId(default()),
+      gen_before_lastclient: gen_dummy,
+      xdata: None,
+    }
+  }
 }
 
 pub trait PieceXDataExt {
@@ -330,14 +348,19 @@ impl PieceXDataExt for PieceXDataState {
 }
 
 pub trait PieceExt {
-  fn make_defs(&self, pri: &PieceRenderInstructions) -> Result<Html, IE>;
-  fn describe_pri(&self, pri: &PieceRenderInstructions) -> Html;
+  fn make_defs(&self, gpc: &PieceState, pri: &PieceRenderInstructions)
+               -> Result<Html, IE>;
+  fn describe_html_infallible(&self, face: Option<FaceId>, gpc: &PieceState)
+                              -> Html;
+  fn describe_pri(&self, gpc: &PieceState, pri: &PieceRenderInstructions)
+                  -> Html;
   fn ui_operations(&self, gpc: &PieceState) -> Result<Vec<UoDescription>, IE>;
 }
 
 impl<T> PieceExt for T where T: Piece + ?Sized {
   #[throws(IE)]
-  fn make_defs(&self, pri: &PieceRenderInstructions) -> Html {
+  fn make_defs(&self,  gpc: &PieceState, pri: &PieceRenderInstructions)
+               -> Html {
     let mut defs = Html(String::new());
     let dragraise = match self.thresh_dragraise(pri)? {
       Some(n) if n < 0 => throw!(SvgE::NegativeDragraise),
@@ -348,7 +371,7 @@ impl<T> PieceExt for T where T: Piece + ?Sized {
     write!(&mut defs.0,
            r##"<g id="piece{}" transform="{}" data-dragraise="{}">"##,
            pri.id, &transform.0, dragraise)?;
-    self.svg_piece(&mut defs, &pri)?;
+    self.svg_piece(&mut defs, gpc, &pri)?;
     write!(&mut defs.0, r##"</g>"##)?;
     write!(&mut defs.0,
            r##"<path id="surround{}" d="{}"/>"##,
@@ -356,8 +379,18 @@ impl<T> PieceExt for T where T: Piece + ?Sized {
     defs
   }
 
-  fn describe_pri(&self, pri: &PieceRenderInstructions) -> Html {
-    self.describe_html(Some(pri.face))
+  fn describe_html_infallible(&self, face: Option<FaceId>, gpc: &PieceState)
+                              -> Html {
+    self.describe_html(face, gpc)
+      .unwrap_or_else(|e| {
+        error!("error describing piece: {:?}", e);
+        Html::lit("<internal error describing piece>")
+      })
+  }
+
+  fn describe_pri(&self, gpc: &PieceState, pri: &PieceRenderInstructions)
+                  -> Html {
+    self.describe_html_infallible(Some(pri.face), gpc)
   }
 
   #[throws(InternalError)]

diff --git a/src/hidden.rs b/src/hidden.rs
index 86cae4b4f8c61af9d5b3d7da9f1765225108d611..9164976afb405baec43ca49b23b9124337515255 100644 (file)
--- a/src/hidden.rs
+++ b/src/hidden.rs
@@ -310,7 +310,8 @@ pub fn recalculate_occultation(
           format!("missing occulter piece {:?} for occid {:?}",
                   opiece, h.occid));
       let oipc = ipieces.get(opiece).ok_or_else(bad)?;
-      Ok::<_,IE>(oipc.describe_html(None))
+      let ogpc = gs.pieces.get(opiece).ok_or_else(bad)?;
+      Ok::<_,IE>(oipc.describe_html(None, ogpc)?)
     };
 
     let most_obscure = most_obscure.unwrap_or(&OccK::Visible); // no players!
@@ -321,7 +322,7 @@ pub fn recalculate_occultation(
       }
       OccK::Scrambled | OccK::Displaced{..} => {
         let face = ipc.nfaces() - 1;
-        let show = ipc.describe_html(Some(face.into()));
+        let show = ipc.describe_html(Some(face.into()), gpc)?;
         vec![ LogEntry { html: Html(format!(
           "{} moved {} from {} to {}",
           who_by.0, &show.0,

diff --git a/src/pieces.rs b/src/pieces.rs
index 86edd198d269d86bc82327d99178c39b417258ea..3854aadeedb0fe300d52950ad7730a223ea02f66 100644 (file)
--- a/src/pieces.rs
+++ b/src/pieces.rs
@@ -128,10 +128,12 @@ impl Outline for SimpleShape {
 #[typetag::serde]
 impl Piece for SimpleShape {
   #[throws(IE)]
-  fn svg_piece(&self, f: &mut Html, pri: &PieceRenderInstructions) {
+  fn svg_piece(&self, f: &mut Html, _gpc: &PieceState,
+               pri: &PieceRenderInstructions) {
     self.svg_piece_raw(f, pri, &mut |_|Ok(()))?;
   }
-  fn describe_html(&self, face: Option<FaceId>) -> Html {
+  #[throws(IE)]
+  fn describe_html(&self, face: Option<FaceId>, _gpc: &PieceState) -> Html {
     Html(if_chain! {
       if let Some(face) = face;
       if let Some(colour) = self.colours.get(face);
@@ -303,9 +305,10 @@ impl Outline for Hand {
 impl Piece for Hand {
   delegate!{
     to self.shape {
-      fn svg_piece(&self, f: &mut Html, pri: &PieceRenderInstructions)
-                   -> Result<(),IE>;
-      fn describe_html(&self, face: Option<FaceId>) -> Html;
+      fn svg_piece(&self, f: &mut Html, gpc: &PieceState,
+                   pri: &PieceRenderInstructions) -> Result<(),IE>;
+      fn describe_html(&self, face: Option<FaceId>,  _gpc: &PieceState)
+                       -> Result<Html,IE>;
       fn itemname(&self) -> &str;
     }
   }

diff --git a/src/shapelib.rs b/src/shapelib.rs
index 671e4549c8c172c9e3c1fb4fdb59a4ed5cf812be..38d0ceffb189cc8b81ed9a50beeec36f1d789658 100644 (file)
--- a/src/shapelib.rs
+++ b/src/shapelib.rs
@@ -155,7 +155,8 @@ impl Piece for Item {
   fn nfaces(&self) -> RawFaceId { self.faces.len().try_into().unwrap() }
 
   #[throws(IE)]
-  fn svg_piece(&self, f: &mut Html, pri: &PieceRenderInstructions) {
+  fn svg_piece(&self, f: &mut Html, _gpc: &PieceState,
+               pri: &PieceRenderInstructions) {
     let face = &self.faces[pri.face];
     let svgd = &self.svgs[face.svg];
     write!(&mut f.0,
@@ -163,7 +164,8 @@ impl Piece for Item {
            face.scale[0], face.scale[1], -face.centre[0], -face.centre[1],
            svgd.0)?;
   }
-  fn describe_html(&self, face: Option<FaceId>) -> Html {
+  #[throws(IE)]
+  fn describe_html(&self, face: Option<FaceId>, _gpc: &PieceState) -> Html {
     self.descs[ match face {
       Some(face) => self.faces[face].desc,
       None => self.desc_hidden,
@@ -264,7 +266,7 @@ impl Contents {
       let ier = ItemEnquiryData {
         itemname: k.clone(),
         f0bbox,
-        f0desc: loaded.describe_html(Some(default())),
+        f0desc: loaded.describe_html(Some(default()), &PieceState::dummy())?,
       };
       out.push(ier);
     }

diff --git a/src/updates.rs b/src/updates.rs
index 016ab3d6ac75906b2e4e3ff6df54f2f6f452b846..84896e636d03791dde145e0817f0c1cd16ff60c2 100644 (file)
--- a/src/updates.rs
+++ b/src/updates.rs
@@ -203,7 +203,7 @@ pub fn log_did_to_piece_whoby(
     "{} {} {}",
     &who_by.0,
     did,
-    p.describe_pri(&pri).0
+    p.describe_pri(pc, &pri).0,
   ))}];
   (log, who_by)
 }