From 1b16a55ffced41b52ff41b592c72d6fe1efb0022 Mon Sep 17 00:00:00 2001 From: root Date: Wed, 9 Sep 2026 10:11:06 +0200 Subject: [PATCH] Round number fields to the precision they display MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `decimals` only reached `formatNumber`, so it changed the label and nothing else: typing 640.5 into a field with `decimals: 0` stored 640.5 and displayed 641. The two existing users got away with it because rounding happened outside the widget (`normalizeFrameRect`, the slice dialog), which their comments had to point out. `NumberField.clampValue` now rounds before it clamps, and every path that sets a value goes through it (typing, scrubbing, arrow keys, the constructor), so the stored number matches the shown one and `min`/`max` stay exact. The rounding is `roundTo` in math.ts, which `snapToStep` already did inline. With that safe, camera `View width`/`View height` and text `Font size` become integers – they were showing 640.000 and 32.000. Everything else that looked integral is not: positions, sprite and collider sizes and zoom are all scrubbed, and `z` is a summed coordinate (walk.ts), not a layer. Verified remotely against the RTX 4070 host: 270 checks, all passed (step 10 gained a check that the field text has no decimal point and survives a reopen). `npm run check` green, 407 tests. --- README.md | 4 ++-- src/core/properties/PropertySchema.ts | 12 ++++++----- src/editor/inspector/frameSection.ts | 1 - src/editor/texture/SliceDialog.ts | 1 - src/ui/math.ts | 10 ++++++--- src/ui/widgets/NumberField.ts | 13 +++++++----- tests/ui/math.test.ts | 29 ++++++++++++++++++++++++++- tools/e2e/10-text-and-enabled.mjs | 29 ++++++++++++++++++++++++++- 8 files changed, 80 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 16092e59..0d3a6b62 100644 --- a/README.md +++ b/README.md @@ -404,7 +404,7 @@ npm run format # Prettier over src, tests and tools npm run check # typecheck + lint + format:check + tests, what CI would run npm run build # writes dist/ and regenerates index.html export GM_REMOTE=user@host # where the browser runs; see below -node tools/seed-e2e.mjs && node tools/e2e.mjs # 267 editor checks, screenshots in /tmp/gm-e2e-*.png +node tools/seed-e2e.mjs && node tools/e2e.mjs # 270 editor checks, screenshots in /tmp/gm-e2e-*.png node tools/seed-jumprun.mjs && node tools/smoke-jumprun.mjs # 24 checks: the example game, played end to end ``` @@ -420,7 +420,7 @@ all, so the app never renders.) over SSH and forwards its CDP port back to `127.0.0.1`, so the harness itself is unchanged. The checks load the public base URL, so the remote side needs only a browser and internet: no repo, no node. Setup is an SSH key (`GM_REMOTE_KEY`, default `~/.ssh/gm-win.key`). Measured against a Windows -desktop with an RTX 4070: **267 checks in 128 s and 24 smoke checks in 16 s, with this machine idle** +desktop with an RTX 4070: **270 checks in 128 s and 24 smoke checks in 16 s, with this machine idle** (0.4 s of CPU for the whole smoke run, load average 0.04). **That is how the suite is meant to be run here.** A local run works and costs 124 s, but it pins the development box – set `GM_REMOTE` and leave it set; without a reachable host, do not fall back to local, fix the host. diff --git a/src/core/properties/PropertySchema.ts b/src/core/properties/PropertySchema.ts index 37a754e1..e82ef5af 100644 --- a/src/core/properties/PropertySchema.ts +++ b/src/core/properties/PropertySchema.ts @@ -5,8 +5,9 @@ import type { PropertyBag, PropertyValue } from '../model/Properties'; /** The inspector is generated from these entries, so a new property is one line in a schema below. */ export type PropertyDef = /** - * `decimals` belongs only on properties that are integral by meaning (counts, indices), not on - * geometry: dragging produces fractions, and a field that hid them would commit the rounded value. + * `decimals` belongs only on properties that are integral by meaning (counts, sizes in whole + * pixels), not on geometry: dragging produces fractions, and the field rounds the value away + * rather than hiding it. */ | { key: string; @@ -58,8 +59,9 @@ export function isCommonProp(key: string): boolean { const ASSET_SCHEMAS: Record = { scene: [{ key: 'backgroundColor', label: 'Background', kind: 'color', default: 0x2a2d34 }], camera: [ - { key: 'width', label: 'View width', kind: 'number', default: 640, min: 16, step: 1 }, - { key: 'height', label: 'View height', kind: 'number', default: 360, min: 16, step: 1 }, + // A view is a pixel raster, so half a pixel of it means nothing. + { key: 'width', label: 'View width', kind: 'number', default: 640, min: 16, step: 1, decimals: 0 }, + { key: 'height', label: 'View height', kind: 'number', default: 360, min: 16, step: 1, decimals: 0 }, { key: 'zoom', label: 'Zoom', kind: 'number', default: 1, min: 0.1, max: 10, step: 0.1 }, ], sprite: [ @@ -104,7 +106,7 @@ const ASSET_SCHEMAS: Record = { container: [], text: [ { key: 'text', label: 'Text', kind: 'text', default: 'Text' }, - { key: 'fontSize', label: 'Font size', kind: 'number', default: 32, min: 1, step: 1 }, + { key: 'fontSize', label: 'Font size', kind: 'number', default: 32, min: 1, step: 1, decimals: 0 }, { key: 'color', label: 'Color', kind: 'color', default: 0xffffff }, { key: 'align', diff --git a/src/editor/inspector/frameSection.ts b/src/editor/inspector/frameSection.ts index 9e7ae16d..fcf5e55b 100644 --- a/src/editor/inspector/frameSection.ts +++ b/src/editor/inspector/frameSection.ts @@ -76,7 +76,6 @@ function rectRow( ): void { const row = createFieldRow( host, - // `decimals: 0` is safe here: `normalizeFrameRect` rounds every value that reaches the model. { key, label, kind: 'number', default: 0, step: 1, decimals: 0 }, value, rectHandlers(host, assetId, frameId, key), diff --git a/src/editor/texture/SliceDialog.ts b/src/editor/texture/SliceDialog.ts index d38db097..d9cfe40e 100644 --- a/src/editor/texture/SliceDialog.ts +++ b/src/editor/texture/SliceDialog.ts @@ -84,7 +84,6 @@ export class SliceDialog extends Popup { const field = new NumberField(this.options[key], { min, step: 1, - // Counts and pixel offsets, rounded on change, so 0 decimals hides nothing. decimals: 0, onChange: (value) => { this.options[key] = Math.round(Number(value)); diff --git a/src/ui/math.ts b/src/ui/math.ts index c8a04ac4..9fb2c75a 100644 --- a/src/ui/math.ts +++ b/src/ui/math.ts @@ -21,14 +21,18 @@ export function formatNumber(value: number, decimals = NUMBER_DECIMALS): string return text === '-' + zero ? zero : text; } +/** Rounds to `decimals` places, which also removes binary-float noise (0.1 × 3 = 0.30000000000000004). */ +export function roundTo(value: number, decimals = NUMBER_DECIMALS): number { + return Number.isFinite(value) ? Number(value.toFixed(decimals)) : value; +} + /** * Rounds `value` onto the grid of `stepSize`, so scrubbing lands on clean values even though - * pointer deltas are fractional. The `toFixed` pass removes binary-float noise (0.1 × 3 = - * 0.30000000000000004). + * pointer deltas are fractional. */ export function snapToStep(value: number, stepSize: number): number { if (!(stepSize > 0) || !Number.isFinite(value)) return value; - return Number((Math.round(value / stepSize) * stepSize).toFixed(NUMBER_DECIMALS)); + return roundTo(Math.round(value / stepSize) * stepSize); } /** diff --git a/src/ui/widgets/NumberField.ts b/src/ui/widgets/NumberField.ts index ddc7a051..6a41d0c4 100644 --- a/src/ui/widgets/NumberField.ts +++ b/src/ui/widgets/NumberField.ts @@ -1,5 +1,5 @@ import type { FederatedPointerEvent } from 'pixi.js'; -import { clamp, formatNumber, NUMBER_DECIMALS, snapToStep } from '../math'; +import { clamp, formatNumber, NUMBER_DECIMALS, roundTo, snapToStep } from '../math'; import { ui } from '../UIContext'; import { TextField, type TextFieldOptions } from './TextField'; @@ -9,8 +9,8 @@ export interface NumberFieldOptions extends Omit { it('keeps a value inside the range', () => { @@ -60,6 +68,25 @@ describe('formatNumber', () => { }); }); +describe('roundTo', () => { + it('keeps a value at the precision its field shows', () => { + // Why the field rounds instead of only formatting: a `decimals: 0` field used to display + // "641" while the project still held 640.5. + expect(roundTo(640.5, 0)).toBe(641); + expect(roundTo(32.4, 0)).toBe(32); + expect(roundTo(1.2345)).toBe(1.234); + }); + + it('removes binary-float noise', () => { + expect(roundTo(0.30000000000000004)).toBe(0.3); + }); + + it('passes non-finite values through', () => { + expect(roundTo(NaN)).toBeNaN(); + expect(roundTo(Infinity)).toBe(Infinity); + }); +}); + describe('snapToStep', () => { it('lands on whole numbers for a step-1 field', () => { // The bug: a scrub added fractional device pixels to an already fractional value, so diff --git a/tools/e2e/10-text-and-enabled.mjs b/tools/e2e/10-text-and-enabled.mjs index ceec2d90..a4b807a7 100644 --- a/tools/e2e/10-text-and-enabled.mjs +++ b/tools/e2e/10-text-and-enabled.mjs @@ -1,6 +1,21 @@ /** Text assets rendered in the scene, and the enabled switch in the editor and in the running game. */ export async function textAndEnabled(t) { - const { check, sleep, evaluate, json, shot, click, clickText, mutate, object, play, stop } = t; + const { + check, + sleep, + evaluate, + json, + shot, + click, + clickText, + locate, + mutate, + object, + play, + press, + stop, + typeText, + } = t; const has = (text) => evaluate(`Boolean(__gm.locate(${JSON.stringify(text)}))`); // 17. Text assets: created through the + menu, placed, and rendered as real text in the scene. @@ -62,4 +77,16 @@ export async function textAndEnabled(t) { ); await stop(); check('stop leaves the play-mode switch behind', (await object('otext')).overrides.enabled === false); + + // 17c. A field that shows no decimals must not store any either. Font size is such a field, so + // typing a fraction has to round: showing "33" while the project holds 32.6 would be a lie. + const fontLabel = await locate('Font size'); + check('Font size reads as a whole number', Boolean(await has('32')) && !(await has('32.000'))); + await click(fontLabel.x + 160, fontLabel.y); + await typeText('32.6'); + await press('Enter'); + await sleep(200); + const fontSize = (await object('otext')).overrides.fontSize; + check('typing a fraction into it rounds the stored value', fontSize === 33, String(fontSize)); + check('and the field shows exactly what was stored', Boolean(await has('33'))); }