Markers: editable, manual x/y/z popup placement, drop heightmap auto-derive

Markers now carry a caller-supplied x/y/z instead of an auto-resolved
terrain height: clicking the map opens a real Leaflet popup pre-filled
with the clicked x/z, a default y of 60, and a random color, all
editable before saving. The same popup now also opens for editing an
existing marker (new updateMarker/PATCH /api/markers/:markerId, TDD'd
in markers.test.ts/index.test.ts). Removes the now-unused
resolveHeight/GET /api/height machinery. Linked-account markers
already synced server-side via the markers table, so cross-device
sync falls out of the existing loadMarkers()-on-init flow.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015tKdPZt78zbPUZMXWzKEKt

api/src/db/schema.ts

@@ -60,9 +60,9 @@ export const chatMessages = pgTable("chat_messages", {
 
 // A web-placed marker, owned by a linked account (anonymous visitors keep their marker list in
 // browser localStorage only — see the plan's "Feature: web markers..." section — so there's
-// nothing to persist here for them). Placement is 2D-only (x/z picked on the Leaflet map); `y` is
-// resolved once at creation time from `chunkColumns`' heightmap and stored, not recomputed later,
-// so a marker doesn't silently move if the terrain above it changes afterward.
+// nothing to persist here for them). x/y/z are all set directly by the player in the web UI's
+// click-to-place/edit popup (x/z pre-filled from the click, y defaulting to 60), not derived
+// server-side, and can be changed later via the same popup (see markers.ts's updateMarker).
 export const markers = pgTable("markers", {
   id: uuid("id").defaultRandom().primaryKey(),
   accountId: uuid("account_id")

api/src/index.test.ts

@@ -5,7 +5,6 @@ import { meshPointers, tilePointers } from "./db/schema";
 import { minio, TILE_BUCKET } from "./minio";
 import { createTestServer, deleteTestServer, createTestSession } from "./test-helpers";
 import { storeLinkCode } from "./link";
-import { chunkColumns } from "./db/schema";
 
 // Elysia's `.handle()` drives the app in-process against a plain Request/Response, without
 // binding a real port — avoids racing a real running instance for the port (see MCMapper's
@@ -28,6 +27,16 @@ function del(path: string, headers?: Record<string, string>) {
   return app.handle(new Request(`http://localhost${path}`, { method: "DELETE", headers }));
 }
 
+function patch(path: string, body?: unknown, headers?: Record<string, string>) {
+  return app.handle(
+    new Request(`http://localhost${path}`, {
+      method: "PATCH",
+      headers: { "content-type": "application/json", ...headers },
+      body: body !== undefined ? JSON.stringify(body) : undefined,
+    }),
+  );
+}
+
 describe("GET /health", () => {
   test("reports ok", async () => {
     const res = await get("/health");
@@ -215,15 +224,6 @@ describe("marker routes", () => {
   beforeAll(async () => {
     server = await createTestServer("markers-route");
     ({ sessionToken } = await createTestSession("MarkerRouteTester"));
-    await db.insert(chunkColumns).values({
-      serverId: server.id,
-      dimension: 0,
-      x: 42,
-      z: 43,
-      blockId: 2,
-      blockMeta: 0,
-      height: 75,
-    });
   });
 
   afterAll(async () => {
@@ -231,19 +231,29 @@ describe("marker routes", () => {
   });
 
   test("POST /api/markers requires a session", async () => {
-    const res = await post("/api/markers", { serverId: server.id, dimension: 0, x: 42, z: 43, name: "N", color: "#fff" });
+    const res = await post("/api/markers", {
+      serverId: server.id,
+      dimension: 0,
+      x: 42,
+      y: 60,
+      z: 43,
+      name: "N",
+      color: "#fff",
+    });
     expect(res.status).toBe(401);
   });
 
-  test("POST /api/markers creates a marker with y resolved from the heightmap, then it's listable and deletable", async () => {
+  test("POST /api/markers creates a marker with the caller-supplied x/y/z, then it's listable, editable and deletable", async () => {
     const createRes = await post(
       "/api/markers",
-      { serverId: server.id, dimension: 0, x: 42, z: 43, name: "RouteMarker", color: "#ABCDEF" },
+      { serverId: server.id, dimension: 0, x: 42, y: 60, z: 43, name: "RouteMarker", color: "#ABCDEF" },
       { "X-MCMapper-Session": sessionToken },
     );
     expect(createRes.status).toBe(200);
     const created = (await createRes.json()) as any;
-    expect(created.marker.y).toBe(75);
+    expect(created.marker.x).toBe(42);
+    expect(created.marker.y).toBe(60);
+    expect(created.marker.z).toBe(43);
 
     const listRes = await get(`/api/markers/${server.id}`);
     expect(listRes.status).toBe(401);
@@ -256,6 +266,27 @@ describe("marker routes", () => {
     const list = (await authedListRes.json()) as any[];
     expect(list.some((m) => m.id === created.marker.id)).toBe(true);
 
+    const patchRes = await patch(
+      `/api/markers/${created.marker.id}`,
+      { name: "Renamed", x: 1, y: 2, z: 3, color: "#000000" },
+      { "X-MCMapper-Session": sessionToken },
+    );
+    expect(patchRes.status).toBe(200);
+    const patched = (await patchRes.json()) as any;
+    expect(patched.marker.name).toBe("Renamed");
+    expect(patched.marker.x).toBe(1);
+    expect(patched.marker.y).toBe(2);
+    expect(patched.marker.z).toBe(3);
+
+    const patchNoSessionRes = await patch(`/api/markers/${created.marker.id}`, {
+      name: "X",
+      x: 0,
+      y: 0,
+      z: 0,
+      color: "#fff",
+    });
+    expect(patchNoSessionRes.status).toBe(401);
+
     const deleteRes = await del(`/api/markers/${created.marker.id}`, { "X-MCMapper-Session": sessionToken });
     expect(deleteRes.status).toBe(200);
 
@@ -268,21 +299,10 @@ describe("marker routes", () => {
     expect(listAfter.some((m) => m.id === created.marker.id)).toBe(false);
   });
 
-  test("GET /api/height/:serverId/:dimension/:x/:z needs no session (anonymous markers need a real y too)", async () => {
-    const res = await get(`/api/height/${server.id}/0/42/43`);
-    expect(res.status).toBe(200);
-    expect(await res.json()).toEqual({ height: 75 });
-  });
-
-  test("GET /api/height/... 404s for an unmapped column", async () => {
-    const res = await get(`/api/height/${server.id}/0/999999/999999`);
-    expect(res.status).toBe(404);
-  });
-
   test("POST /api/markers/:id/share reports server_not_connected when the mod isn't online", async () => {
     const createRes = await post(
       "/api/markers",
-      { serverId: server.id, dimension: 0, x: 42, z: 43, name: "ShareMe", color: "#111111" },
+      { serverId: server.id, dimension: 0, x: 42, y: 60, z: 43, name: "ShareMe", color: "#111111" },
       { "X-MCMapper-Session": sessionToken },
     );
     const created = (await createRes.json()) as any;

api/src/index.ts

@@ -6,7 +6,7 @@ import { wsGateway } from "./ws-gateway";
 import { chatGateway } from "./chat-gateway";
 import { minio, TILE_BUCKET, ensureTileBucket } from "./minio";
 import { redeemLinkCode, getAccountForSession, revokeSession } from "./link";
-import { createMarker, listMarkers, deleteMarker, shareMarkerToChat, resolveHeight } from "./markers";
+import { createMarker, listMarkers, deleteMarker, updateMarker, shareMarkerToChat } from "./markers";
 
 const MARKER_SHARE_ERROR_STATUS: Record<string, number> = {
   not_found: 404,
@@ -118,20 +118,10 @@ export const app = new Elysia()
     if (token) await revokeSession(token);
     return { ok: true };
   })
-  // Unauthenticated on purpose: anonymous visitors placing a localStorage-only marker still need
-  // a real y (see markers.ts's resolveHeight doc comment), and this is read-only, scoped to
-  // already-public tile data.
-  .get("/api/height/:serverId/:dimension/:x/:z", async ({ params, set }) => {
-    const height = await resolveHeight(params.serverId, Number(params.dimension), Number(params.x), Number(params.z));
-    if (height === null) {
-      set.status = 404;
-      return { error: "column_not_mapped" };
-    }
-    return { height };
-  })
   // Markers are linked-account-only — anonymous visitors keep their marker list in browser
   // localStorage instead (see markers.ts's doc comment and the plan's marker feature section),
-  // so every route here requires a valid session.
+  // so every route here requires a valid session. x/y/z are all caller-supplied (the web UI's
+  // click-to-place popup lets the player set them directly), not resolved server-side.
   .post("/api/markers", async ({ body, headers, set }) => {
     const token = headers["x-mcmapper-session"];
     const account = token ? await getAccountForSession(token) : null;
@@ -139,19 +129,28 @@ export const app = new Elysia()
       set.status = 401;
       return { error: "unauthenticated" };
     }
-    const { serverId, dimension, x, z, name, color } = body as {
+    const { serverId, dimension, x, y, z, name, color } = body as {
       serverId?: string;
       dimension?: number;
       x?: number;
+      y?: number;
       z?: number;
       name?: string;
       color?: string;
     };
-    if (!serverId || dimension === undefined || x === undefined || z === undefined || !name || !color) {
+    if (
+      !serverId ||
+      dimension === undefined ||
+      x === undefined ||
+      y === undefined ||
+      z === undefined ||
+      !name ||
+      !color
+    ) {
       set.status = 400;
       return { error: "missing_fields" };
     }
-    const result = await createMarker({ accountId: account.id, serverId, dimension, x, z, name, color });
+    const result = await createMarker({ accountId: account.id, serverId, dimension, x, y, z, name, color });
     if (!result.ok) {
       set.status = 400;
       return result;
@@ -167,6 +166,28 @@ export const app = new Elysia()
     }
     return listMarkers(account.id, params.serverId);
   })
+  .patch("/api/markers/:markerId", async ({ params, body, headers, set }) => {
+    const token = headers["x-mcmapper-session"];
+    const account = token ? await getAccountForSession(token) : null;
+    if (!account) {
+      set.status = 401;
+      return { error: "unauthenticated" };
+    }
+    const { name, x, y, z, color } = body as {
+      name?: string;
+      x?: number;
+      y?: number;
+      z?: number;
+      color?: string;
+    };
+    if (x === undefined || y === undefined || z === undefined || !name || !color) {
+      set.status = 400;
+      return { error: "missing_fields" };
+    }
+    const result = await updateMarker(account.id, params.markerId, { name, x, y, z, color });
+    if (!result.ok) set.status = 404;
+    return result;
+  })
   .delete("/api/markers/:markerId", async ({ params, headers, set }) => {
     const token = headers["x-mcmapper-session"];
     const account = token ? await getAccountForSession(token) : null;

api/src/markers.test.ts

@@ -1,8 +1,8 @@
 import { describe, test, expect, beforeAll, afterAll } from "bun:test";
 import { eq } from "drizzle-orm";
 import { db } from "./db/client";
-import { chunkColumns, servers } from "./db/schema";
-import { createMarker, listMarkers, deleteMarker, shareMarkerToChat, resolveHeight } from "./markers";
+import { servers } from "./db/schema";
+import { createMarker, listMarkers, deleteMarker, updateMarker, shareMarkerToChat } from "./markers";
 import { createTestServer, deleteTestServer, createTestSession, FakeSocket } from "./test-helpers";
 import { wsGateway } from "./ws-gateway";
 
@@ -13,83 +13,33 @@ describe("createMarker", () => {
   beforeAll(async () => {
     server = await createTestServer("markers-create");
     ({ accountId } = await createTestSession("MarkerPlacer"));
-    await db.insert(chunkColumns).values({
-      serverId: server.id,
-      dimension: 0,
-      x: 100,
-      z: 200,
-      blockId: 2,
-      blockMeta: 0,
-      height: 71,
-    });
   });
 
   afterAll(async () => {
     await deleteTestServer(server.id);
   });
 
-  test("resolves y from the chunk store's heightmap at that column", async () => {
+  test("stores the caller-supplied x/y/z as-is, no server-side height lookup", async () => {
     const result = await createMarker({
       accountId,
       serverId: server.id,
       dimension: 0,
       x: 100,
+      y: 60,
       z: 200,
       name: "Base",
-      color: "#3391FF",
+      color: "#3391ff",
     });
     expect(result.ok).toBe(true);
     if (!result.ok) return;
-    expect(result.marker.y).toBe(71);
     expect(result.marker.x).toBe(100);
+    expect(result.marker.y).toBe(60);
     expect(result.marker.z).toBe(200);
     expect(result.marker.name).toBe("Base");
   });
-
-  test("fails rather than guessing a y for an unmapped column", async () => {
-    const result = await createMarker({
-      accountId,
-      serverId: server.id,
-      dimension: 0,
-      x: 999999,
-      z: 999999,
-      name: "Nowhere",
-      color: "#FFFFFF",
-    });
-    expect(result).toEqual({ ok: false, error: "column_not_mapped" });
-  });
 });
 
-describe("resolveHeight", () => {
-  let server: { id: string };
-
-  beforeAll(async () => {
-    server = await createTestServer("markers-resolve-height");
-    await db.insert(chunkColumns).values({
-      serverId: server.id,
-      dimension: 0,
-      x: 7,
-      z: 8,
-      blockId: 2,
-      blockMeta: 0,
-      height: 90,
-    });
-  });
-
-  afterAll(async () => {
-    await deleteTestServer(server.id);
-  });
-
-  test("returns the mapped column's height", async () => {
-    expect(await resolveHeight(server.id, 0, 7, 8)).toBe(90);
-  });
-
-  test("returns null for an unmapped column, rather than guessing", async () => {
-    expect(await resolveHeight(server.id, 0, 12345, 12345)).toBeNull();
-  });
-});
-
-describe("listMarkers / deleteMarker ownership", () => {
+describe("listMarkers / deleteMarker / updateMarker ownership", () => {
   let server: { id: string };
   let ownerAccountId: string;
   let otherAccountId: string;
@@ -99,23 +49,15 @@ describe("listMarkers / deleteMarker ownership", () => {
     server = await createTestServer("markers-owner");
     ({ accountId: ownerAccountId } = await createTestSession("Owner"));
     ({ accountId: otherAccountId } = await createTestSession("Other"));
-    await db.insert(chunkColumns).values({
-      serverId: server.id,
-      dimension: 0,
-      x: 5,
-      z: 5,
-      blockId: 2,
-      blockMeta: 0,
-      height: 64,
-    });
     const result = await createMarker({
       accountId: ownerAccountId,
       serverId: server.id,
       dimension: 0,
       x: 5,
+      y: 64,
       z: 5,
       name: "Mine",
-      color: "#00FF00",
+      color: "#00ff00",
     });
     if (!result.ok) throw new Error("setup failed");
     ownedMarkerId = result.marker.id;
@@ -134,6 +76,40 @@ describe("listMarkers / deleteMarker ownership", () => {
     expect(otherMarkers).toHaveLength(0);
   });
 
+  test("updateMarker refuses to update another account's marker", async () => {
+    const result = await updateMarker(otherAccountId, ownedMarkerId, {
+      name: "Hijacked",
+      x: 0,
+      y: 0,
+      z: 0,
+      color: "#000000",
+    });
+    expect(result).toEqual({ ok: false, error: "not_found" });
+
+    const stillMine = await listMarkers(ownerAccountId, server.id);
+    expect(stillMine[0]!.name).toBe("Mine");
+  });
+
+  test("updateMarker succeeds for the owning account and persists the change", async () => {
+    const result = await updateMarker(ownerAccountId, ownedMarkerId, {
+      name: "Renamed",
+      x: 10,
+      y: 70,
+      z: 20,
+      color: "#123456",
+    });
+    expect(result.ok).toBe(true);
+    if (!result.ok) return;
+    expect(result.marker.name).toBe("Renamed");
+    expect(result.marker.x).toBe(10);
+    expect(result.marker.y).toBe(70);
+    expect(result.marker.z).toBe(20);
+    expect(result.marker.color).toBe("#123456");
+
+    const listed = await listMarkers(ownerAccountId, server.id);
+    expect(listed[0]!.name).toBe("Renamed");
+  });
+
   test("deleteMarker refuses to delete another account's marker", async () => {
     const result = await deleteMarker(otherAccountId, ownedMarkerId);
     expect(result).toEqual({ ok: false, error: "not_found" });
@@ -159,20 +135,12 @@ describe("shareMarkerToChat", () => {
   beforeAll(async () => {
     server = await createTestServer("markers-share");
     ({ accountId } = await createTestSession("Sharer"));
-    await db.insert(chunkColumns).values({
-      serverId: server.id,
-      dimension: 0,
-      x: 10,
-      z: 20,
-      blockId: 2,
-      blockMeta: 0,
-      height: 80,
-    });
     const result = await createMarker({
       accountId,
       serverId: server.id,
       dimension: 0,
       x: 10,
+      y: 80,
       z: 20,
       name: "Shareable",
       color: "#B311CF",

api/src/markers.ts

@@ -1,6 +1,6 @@
 import { and, eq } from "drizzle-orm";
 import { db } from "./db/client";
-import { chunkColumns, markers, servers } from "./db/schema";
+import { markers, servers } from "./db/schema";
 import { getModSocket } from "./ws-gateway";
 
 interface NewMarkerInput {
@@ -8,6 +8,7 @@ interface NewMarkerInput {
   serverId: string;
   dimension: number;
   x: number;
+  y: number;
   z: number;
   name: string;
   color: string;
@@ -16,39 +17,11 @@ interface NewMarkerInput {
 export type CreateMarkerResult = { ok: true; marker: typeof markers.$inferSelect } | { ok: false; error: string };
 
 /**
- * Topmost non-air block's height at a column, straight from the chunk store's heightmap — `null`
- * if that column hasn't been mapped by the mod yet. Exposed on its own (see index.ts's
- * unauthenticated `/api/height` route) because anonymous visitors also need a real `y` for their
- * localStorage-only marker list (see the plan's marker feature section: "either way the record
- * includes x, y, z"), even though they never reach `createMarker` below.
- */
-export async function resolveHeight(serverId: string, dimension: number, x: number, z: number): Promise<number | null> {
-  const [column] = await db
-    .select({ height: chunkColumns.height })
-    .from(chunkColumns)
-    .where(
-      and(
-        eq(chunkColumns.serverId, serverId),
-        eq(chunkColumns.dimension, dimension),
-        eq(chunkColumns.x, x),
-        eq(chunkColumns.z, z),
-      ),
-    )
-    .limit(1);
-  return column ? column.height : null;
-}
-
-/**
- * Placement is 2D-only (see the plan's marker feature section) — the caller only supplies x/z,
- * and `y` is auto-derived here from the chunk store's heightmap rather than picked by the player,
- * so the marker is a genuine 3D point usable by both the 3D viewer and the in-game waypoint
- * formats (both require a real y). Fails rather than guessing a y if that column hasn't been
- * mapped by the mod yet.
+ * Placement is fully manual (the web UI's click-to-place popup lets the player set x/y/z
+ * directly — x/z pre-filled from the click, y defaulting to 60 but editable — rather than y
+ * being auto-derived from the chunk heightmap), so there's nothing to resolve server-side here.
  */
 export async function createMarker(input: NewMarkerInput): Promise<CreateMarkerResult> {
-  const height = await resolveHeight(input.serverId, input.dimension, input.x, input.z);
-  if (height === null) return { ok: false, error: "column_not_mapped" };
-
   const [marker] = await db
     .insert(markers)
     .values({
@@ -56,7 +29,7 @@ export async function createMarker(input: NewMarkerInput): Promise<CreateMarkerR
       serverId: input.serverId,
       dimension: input.dimension,
       x: input.x,
-      y: height,
+      y: input.y,
       z: input.z,
       name: input.name,
       color: input.color,
@@ -85,6 +58,31 @@ export async function deleteMarker(accountId: string, markerId: string): Promise
   return { ok: true };
 }
 
+interface MarkerUpdateInput {
+  name: string;
+  x: number;
+  y: number;
+  z: number;
+  color: string;
+}
+
+export type UpdateMarkerResult = { ok: true; marker: typeof markers.$inferSelect } | { ok: false; error: string };
+
+/** Scopes the update to the owning account in the WHERE clause, same pattern as deleteMarker. */
+export async function updateMarker(
+  accountId: string,
+  markerId: string,
+  update: MarkerUpdateInput,
+): Promise<UpdateMarkerResult> {
+  const [marker] = await db
+    .update(markers)
+    .set({ name: update.name, x: update.x, y: update.y, z: update.z, color: update.color })
+    .where(and(eq(markers.id, markerId), eq(markers.accountId, accountId)))
+    .returning();
+  if (!marker) return { ok: false, error: "not_found" };
+  return { ok: true, marker };
+}
+
 export type ShareMarkerResult = { ok: true } | { ok: false; error: string };
 
 /**