From 4df90a4ed9b89ffc903c9eb6a0360bbd7bb416ef Mon Sep 17 00:00:00 2001
From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Sat, 30 Apr 2022 12:42:01 +0100
Subject: [PATCH] multigrab: Set the z coordinate

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---
 daemon/api.rs       | 11 ++++++++++-
 src/gamestate.rs    | 25 ++++++++++++++++++++++++-
 templates/script.ts |  6 ++++--
 3 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/daemon/api.rs b/daemon/api.rs
index 4a6f0eb3..24c4f94f 100644
--- a/daemon/api.rs
+++ b/daemon/api.rs
@@ -569,6 +569,7 @@ api_route!{
 api_route!{
   api_multigrab, "/_/api/multigrab",
   struct ApiPieceMultigrab {
+    z: ZCoord,
     n: MultigrabQty,
   }
 
@@ -581,7 +582,15 @@ api_route!{
       let y = pri.fully_visible().ok_or(Ia::Occultation)?;
       let gpc = a.gs.pieces.byid_mut(a.piece)?;
       if gpc.held != None { throw!(Ia::PieceHeld) }
-      a.ipc.show(y).op_multigrab(a, pri, self.n)?
+      if ! (self.z > gpc.zlevel.z) { throw!(Ia::BadPieceStateForOperation); }
+      op_do_set_z(gpc, a.gs.gen, &self.z)?;
+      a.ipc.show(y).op_multigrab(a, pri, self.n, &self.z).map_err(|e| match e {
+        // TODO: The error handling is wrong, here.  If op_multigrab
+        // returns a deferred thunk, the APOE::PartiallyProcessed will
+        // not be applked.
+        APOE::Inapplicable(ia) => APOE::PartiallyProcessed(ia, vec![]),
+        other => other,
+      })?
     }
   }
 }
diff --git a/src/gamestate.rs b/src/gamestate.rs
index 0e6f8751..4caeddec 100644
--- a/src/gamestate.rs
+++ b/src/gamestate.rs
@@ -245,8 +245,31 @@ pub trait PieceTrait: PieceBaseTrait + Send + Debug + 'static {
     default()
   }
 
+  // This is going to have to set the Z coordinate.  This is because
+  // when we split a banknote, the note the user grabbed must end up
+  // above the "change", the "new" note which remains.  So we need to
+  // select Z coordinates.
+  //
+  // We *could* solve this by simply giving the two pieces different Z
+  // generations, but the same ZCoord, but this is quite undesirable
+  // because it can lead to the client having to do piece restacking.
+  //
+  // We can't readily find a suitable Z ZCoord for the change (ie, the
+  // lower coordinate), because that's piece lowering which is very
+  // complicated, because it may have to restack due to clashing
+  // ZCoords.  (We have that in JS in the client.)
+  //
+  // So we want to set the ZCoord of the piece the client has just
+  // grasped.  We don't want to disrupt the client's drag operation,
+  // so we don't want to change anything server-side that's subject to
+  // the update concurrency protocool.  Or to put it another way,
+  // the ZCoord must be predictable (and predicted) by the client.
+  // And the client can easily select a suitable ZCoord: the top
+  // one will do.
+  //
+  // So the multigrab operation specifies a ZCoord.
   fn op_multigrab(&self, _a: ApiPieceOpArgs, _pri: PieceRenderInstructions,
-                  _qty: MultigrabQty)
+                  _qty: MultigrabQty, _new_z: &ZCoord)
                   -> Result<OpOutcomeThunk,ApiPieceOpError>  {
     Err(Ia::BadPieceStateForOperation)?
   }
diff --git a/templates/script.ts b/templates/script.ts
index 771443cc..1edca6e4 100644
--- a/templates/script.ts
+++ b/templates/script.ts
@@ -994,8 +994,10 @@ function grab_clicked(clicked: PieceId[], loose: boolean,
       api_piece_x(api_immediate, loose,
 		  wresting ? 'wrest' : 'grab', piece,p, { });
     } else {
-      api_piece_x(api_immediate, loose, 'multigrab',
-		  piece,p, { n: multigrab });
+      piece_raise(piece,p, 'Raised', function(piece,p,z) {
+	api_piece_x(api_immediate, loose, 'multigrab',
+		    piece,p, { n: multigrab, z: z });
+      })
     }
   }
 }
-- 
2.30.2