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, + ), + }, + }; }