From d65fbfa216c70515769dc120469fa67459e0e087 Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Thu, 6 Aug 2026 23:59:21 -0400 Subject: [PATCH] fix(web)!: store the editor's settings as whole numbers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The atproto data model has no float. The editor's sliders are fractions, so every camo carrying settings failed to serialise inside atrium — before any request was built — and players got a 502 about a server that had never been asked. The blob upload before it had already succeeded, which is what made that message so misleading. Sliders are thousandths now: 0..1000 for a 0..1 control, which is the answer the lexicon linter already gives for schemas — no float, use an integer and say what the units are. The version goes to 2 so a v1 record, which could never actually have been written, is dropped rather than read as if 510 meant 0.51. The test now walks everything the record carries and asserts each number is whole. Its fixture used integers before, which is exactly why it passed while production failed. Co-Authored-By: Claude Opus 5 (1M context) --- web/scripts/camo-record.test.mjs | 42 +++++++++++++-- web/src/camo/record.ts | 88 +++++++++++++++++++++++++++----- 2 files changed, 113 insertions(+), 17 deletions(-) diff --git a/web/scripts/camo-record.test.mjs b/web/scripts/camo-record.test.mjs index 459eec8..e9b14a1 100644 --- a/web/scripts/camo-record.test.mjs +++ b/web/scripts/camo-record.test.mjs @@ -57,7 +57,16 @@ const PALETTE = { [104, 112, 78], ], }; -const PARAMS = { seed: 7, scale: 0.5, contrast: 0.4, detail: 0.3, angle: 0 }; +/** What the editor holds: fractions, straight off the sliders. */ +const PARAMS = { seed: 7, scale: 0.51, contrast: 0.4, detail: 0.3, angle: 0 }; +/** What those become on the record: thousandths, and whole numbers. */ +const STORED_PARAMS = { + seed: 7, + scale: 510, + contrast: 400, + detail: 300, + angle: 0, +}; /** * A record as it comes off the wire, carrying settings the schema never @@ -79,7 +88,7 @@ const wire = () => source: { v: SOURCE_VERSION, generator: "dpm", - params: PARAMS, + params: STORED_PARAMS, palette: PALETTE, }, }); @@ -117,6 +126,11 @@ test("settings this build cannot act on read as none", () => { (r.source.generator = "hologram"), "a parameter that is not a number": (r) => (r.source.params.seed = "seven"), "a parameter missing": (r) => delete r.source.params.angle, + // The bug this whole shape exists for: the data model has no float, so a + // fraction cannot be written and must not be read back as if it could. + "a fractional slider": (r) => (r.source.params.scale = 0.51), + "a slider past its range": (r) => (r.source.params.scale = 1001), + "a fractional colour": (r) => (r.source.palette.colors[0][0] = 74.5), "a colour outside 0-255": (r) => (r.source.palette.colors[0][1] = 300), "a colour that is not a triple": (r) => (r.source.palette.colors[0] = [1, 2]), @@ -129,9 +143,29 @@ test("settings this build cannot act on read as none", () => { } }); -test("settings this build wrote read back exactly", () => { +test("settings this build wrote are whole numbers, and read back", () => { const source = makeSource("dpm", PARAMS, PALETTE); - assert.deepEqual(readSource({ source }), source); + + // The check that would have caught the production failure: everything the + // record carries has to be an integer, because atproto's data model has no + // float and refuses the entire write when it meets one. + const numbers = []; + (function walk(v) { + if (typeof v === "number") numbers.push(v); + else if (Array.isArray(v)) v.forEach(walk); + else if (v && typeof v === "object") Object.values(v).forEach(walk); + })(source); + assert.ok(numbers.length > 0); + for (const n of numbers) { + assert.ok(Number.isInteger(n), `${n} is not a whole number`); + } + assert.deepEqual(source.params, STORED_PARAMS); + + // And the sliders come back where the player left them. + const back = readSource({ source }); + assert.equal(back.params.scale, 0.51); + assert.equal(back.params.contrast, 0.4); + assert.equal(back.generator, "dpm"); // A palette flag is carried; anything else on the palette is not invented. const flagged = makeSource("pride", PARAMS, { ...PALETTE, flag: true }); assert.equal(readSource({ source: flagged })?.palette.flag, true); diff --git a/web/src/camo/record.ts b/web/src/camo/record.ts index 5da3952..aaf831f 100644 --- a/web/src/camo/record.ts +++ b/web/src/camo/record.ts @@ -34,18 +34,47 @@ import type { Palette, RGB } from "./palette"; * Bumped when the shape below changes in a way that makes an older reader * wrong. A reader that does not recognise the version drops the whole thing, * which is why every field can be added to but none can change meaning. + * + * 2 stores the sliders as thousandths rather than fractions. Version 1 could + * never be written — the atproto data model has no float, so every record + * carrying one failed to serialise — so no reader will ever meet one, and + * this bump exists to make sure a v1 that somehow exists is dropped rather + * than read as if 510 meant 0.51. + */ +export const SOURCE_VERSION = 2; + +/** + * A slider position, as thousandths of its range: 0..1000 for a 0..1 control. + * + * The data model these end up in has integers and no float at all, which is + * the whole reason for the unit. Thousandths because the sliders step in + * hundredths and one spare digit costs nothing. */ -export const SOURCE_VERSION = 1; +export type Thousandths = number; /** How the editor drew a camo, enough to put the controls back where they were. */ export type CamoSource = { v: typeof SOURCE_VERSION; /** A generator id from patterns.ts. Unknown here means a newer editor wrote it. */ generator: string; - params: PatternParams; + params: { + /** Whole number already, and the one parameter that is not a fraction. */ + seed: number; + scale: Thousandths; + contrast: Thousandths; + detail: Thousandths; + angle: Thousandths; + }; palette: Palette; }; +/** A 0..1 slider as thousandths, clamped so a stray value cannot travel. */ +function toThousandths(v: number): Thousandths { + return Math.max(0, Math.min(1000, Math.round(v * 1000))); +} + +const fromThousandths = (v: Thousandths): number => v / 1000; + /** * A camo record as this client writes it: the lexicon's fields, plus the * editor's own. `source` is absent on a camo that came from an imported file, @@ -74,26 +103,37 @@ export type StoredCamo = { stale?: boolean; }; -const isFinite01 = (v: unknown): v is number => - typeof v === "number" && Number.isFinite(v); +/** + * A whole number, and nothing else. + * + * `Number.isInteger` is the check that matters rather than a range one: a + * fraction anywhere in here cannot be written at all, because the data model + * has no float, and a record that carries one is refused before it is sent. + */ +const isWhole = (v: unknown): v is number => + typeof v === "number" && Number.isInteger(v); + +const isSlider = (v: unknown): v is Thousandths => + isWhole(v) && v >= 0 && v <= 1000; function readParams(value: unknown): PatternParams | null { if (typeof value !== "object" || value === null) return null; const p = value as Record; const { seed, scale, contrast, detail, angle } = p; - if (![seed, scale, contrast, detail, angle].every(isFinite01)) return null; + if (!isWhole(seed)) return null; + if (![scale, contrast, detail, angle].every(isSlider)) return null; return { - seed: seed as number, - scale: scale as number, - contrast: contrast as number, - detail: detail as number, - angle: angle as number, + seed, + scale: fromThousandths(scale as Thousandths), + contrast: fromThousandths(contrast as Thousandths), + detail: fromThousandths(detail as Thousandths), + angle: fromThousandths(angle as Thousandths), }; } function readColor(value: unknown): RGB | null { if (!Array.isArray(value) || value.length !== 3) return null; - if (!value.every((c) => isFinite01(c) && c >= 0 && c <= 255)) return null; + if (!value.every((c) => isWhole(c) && c >= 0 && c <= 255)) return null; return [value[0], value[1], value[2]] as RGB; } @@ -131,11 +171,33 @@ export function readSource(record: unknown): CamoSource | null { return { v: SOURCE_VERSION, generator: s.generator, params, palette }; } -/** The settings to write for a camo the editor drew, in the shape above. */ +/** + * The settings to write for a camo the editor drew, in the shape above. + * + * This is where fractions stop. Everything below is whole numbers, because + * the record is serialised into a data model that has no float and refuses + * the whole write when it meets one. + */ export function makeSource( generator: string, params: PatternParams, palette: Palette, ): CamoSource { - return { v: SOURCE_VERSION, generator, params, palette }; + return { + v: SOURCE_VERSION, + generator, + params: { + seed: Math.trunc(params.seed), + scale: toThousandths(params.scale), + contrast: toThousandths(params.contrast), + detail: toThousandths(params.detail), + angle: toThousandths(params.angle), + }, + palette: { + ...palette, + colors: palette.colors.map( + (c) => c.map((v) => Math.round(v)) as unknown as RGB, + ), + }, + }; } -- 2.51.2