From 99c5d2ad788cc379cc9e8a51bce8e58273779a00 Mon Sep 17 00:00:00 2001 From: Developer Date: Sun, 13 Sep 2026 20:32:45 +0200 Subject: [PATCH] Uploads carry the name the file came with (#83) An upload sends its file name next to the raw bytes, and the server keeps it in a sidecar next to the content-addressed file. Both pickers caption every tile and row with it, and an asset made there is named after the file instead of "Texture" / "Font". Last upload of the same content wins. The API's own set_error_handler ignores error_reporting(), so @-silenced warnings still throw: a missing sidecar took the whole listing down with a 500. Existence is checked instead of suppressed. Co-Authored-By: Claude Opus 5 --- api/index.php | 20 ++++--- api/lib/FontStore.php | 20 +++++-- api/lib/OriginalName.php | 93 +++++++++++++++++++++++++++++ api/lib/TextureStore.php | 28 +++++++-- src/core/commands/assetCommands.ts | 19 +++++- src/core/store/FontStore.ts | 6 +- src/core/store/HttpFontStore.ts | 7 ++- src/core/store/HttpTextureStore.ts | 7 ++- src/core/store/TextureStore.ts | 11 +++- src/core/store/http.ts | 13 +++- src/editor/font/FontPicker.ts | 14 ++++- src/editor/fontImport.ts | 14 ++--- src/editor/texture/ImagePicker.ts | 17 ++++-- src/editor/textureImport.ts | 16 +++-- tests/core/http-store.test.ts | 14 +++++ tests/core/texture-commands.test.ts | 13 ++++ tests/editor/font-import.test.ts | 15 ++++- tests/editor/texture-import.test.ts | 25 +++++++- tools/e2e/12-textures.mjs | 31 ++++++++++ tools/e2e/30-font.mjs | 17 +++++- tools/lib/projects.mjs | 8 ++- 21 files changed, 347 insertions(+), 61 deletions(-) create mode 100644 api/lib/OriginalName.php diff --git a/api/index.php b/api/index.php index 9bef4370..58513288 100644 --- a/api/index.php +++ b/api/index.php @@ -13,19 +13,23 @@ declare(strict_types=1); * PUT ?r=projects/{id} {revision, project} → {revision,updatedAt} | 409 {error,revision} * DELETE ?r=projects/{id} → 204 * - * GET ?r=textures → [{file,width,height,bytes,updatedAt}] - * POST ?r=textures raw image bytes → {file,width,height,bytes} - * DELETE ?r=textures/{file} → 204 + * GET ?r=textures → [{file,name,width,height,bytes,updatedAt}] + * POST ?r=textures&name=x.png raw image bytes → {file,name,width,height,bytes} + * DELETE ?r=textures/{file} → 204 * - * GET ?r=fonts → [{file,bytes,updatedAt}] - * POST ?r=fonts raw font bytes → {file,bytes} - * DELETE ?r=fonts/{file} → 204 + * GET ?r=fonts → [{file,name,bytes,updatedAt}] + * POST ?r=fonts&name=x.ttf raw font bytes → {file,name,bytes} + * DELETE ?r=fonts/{file} → 204 + * + * `name` is the file's original name, kept beside it as `{file}.json` (see OriginalName) and + * empty when none is known. It is a caption, never a path: the stored file is always `{hash}.{ext}`. * * Stored images and fonts are served straight from `data/textures/` and `data/fonts/` as * static files; the API only writes them. */ require __DIR__ . '/lib/FontStore.php'; +require __DIR__ . '/lib/OriginalName.php'; require __DIR__ . '/lib/Response.php'; require __DIR__ . '/lib/Store.php'; require __DIR__ . '/lib/TextureStore.php'; @@ -116,7 +120,7 @@ function routeTextures(array $parts, string $method): never Response::error('Image must be at most ' . (MAX_TEXTURE_BYTES / 1024 / 1024) . ' MB', 413); } try { - Response::json($store->store($bytes), 201); + Response::json($store->store($bytes, (string) ($_GET['name'] ?? '')), 201); } catch (InvalidImageException $e) { Response::error($e->getMessage(), 422); } @@ -149,7 +153,7 @@ function routeFonts(array $parts, string $method): never Response::error('Font must be at most ' . (MAX_FONT_BYTES / 1024 / 1024) . ' MB', 413); } try { - Response::json($store->store($bytes), 201); + Response::json($store->store($bytes, (string) ($_GET['name'] ?? '')), 201); } catch (InvalidFontException $e) { Response::error($e->getMessage(), 422); } diff --git a/api/lib/FontStore.php b/api/lib/FontStore.php index 41698c99..e27ba92a 100644 --- a/api/lib/FontStore.php +++ b/api/lib/FontStore.php @@ -46,10 +46,13 @@ final class FontStore * libmagic versions (`font/sfnt`, `application/vnd.ms-opentype`, plain `application/octet-stream`), * so a second opinion from it would reject valid files rather than catch bad ones. * - * @return array{file:string,bytes:int} + * `$originalName` is kept beside the file as a caption (see OriginalName): the hash says + * nothing, and the name inside the font is deliberately never read (see `gm-font-` families). + * + * @return array{file:string,name:string,bytes:int} * @throws InvalidFontException when the bytes are not a supported font */ - public function store(string $bytes): array + public function store(string $bytes, string $originalName = ''): array { $extension = self::SIGNATURES[substr($bytes, 0, 4)] ?? ''; if ($extension === '') { @@ -65,26 +68,29 @@ final class FontStore if (!is_file($path)) { $this->writeFile($path, $bytes); } + OriginalName::write($this->dir, $file, $originalName); - return ['file' => $file, 'bytes' => strlen($bytes)]; + return ['file' => $file, 'name' => OriginalName::read($this->dir, $file), 'bytes' => strlen($bytes)]; } /** * Every stored file, newest first – what the font picker offers. Unlike an image there is * nothing to read out of the header here: the picker shows each face by rendering with it. * - * @return list + * @return list */ public function list(): array { $items = []; foreach (glob($this->dir . '/*.*') ?: [] as $path) { $file = basename($path); + // Sidecars share the directory and are skipped: `{hash}.ttf.json` is not a valid name. if (!self::isValidName($file)) { continue; } $items[] = [ 'file' => $file, + 'name' => OriginalName::read($this->dir, $file), 'bytes' => (int) filesize($path), 'updatedAt' => gmdate('c', (int) filemtime($path)), ]; @@ -99,7 +105,11 @@ final class FontStore return false; } $path = $this->dir . '/' . $file; - return is_file($path) && unlink($path); + if (!is_file($path) || !unlink($path)) { + return false; + } + OriginalName::delete($this->dir, $file); + return true; } /** Temp file plus rename, so a reader never sees a half-written font (see TextureStore). */ diff --git a/api/lib/OriginalName.php b/api/lib/OriginalName.php new file mode 100644 index 00000000..b4f52ebb --- /dev/null +++ b/api/lib/OriginalName.php @@ -0,0 +1,93 @@ + self::MAX_LENGTH ? mb_substr($name, 0, self::MAX_LENGTH) : $name; + } + + /** + * The stored name, or '' when nothing is known about this file – which is the normal case for + * everything uploaded before sidecars existed. + * + * The absence is checked rather than suppressed with `@`: index.php installs an error handler + * that turns every warning into an exception, and it does not consult `error_reporting()`, so + * `@` does not silence it. A missing sidecar would otherwise fail the whole listing. + */ + public static function read(string $dir, string $file): string + { + $path = $dir . '/' . $file . '.json'; + if (!is_file($path)) { + return ''; + } + $raw = file_get_contents($path); + if ($raw === false) { + return ''; + } + $data = json_decode($raw, true); + return is_array($data) && is_string($data['name'] ?? null) ? self::sanitize($data['name']) : ''; + } + + /** Writes the sidecar; an empty name writes nothing and removes nothing. */ + public static function write(string $dir, string $file, string $name): void + { + $clean = self::sanitize($name); + if ($clean === '') { + return; + } + $path = $dir . '/' . $file . '.json'; + $tmp = $path . '.' . bin2hex(random_bytes(4)) . '.tmp'; + $json = json_encode(['name' => $clean], JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES); + if ($json === false || file_put_contents($tmp, $json, LOCK_EX) === false || !rename($tmp, $path)) { + if (is_file($tmp)) { + unlink($tmp); + } + return; // A nameless file is a small loss; failing the upload over it would be worse. + } + chmod($path, 0664); + } + + /** Removing a file that has no sidecar is the normal case, not a failure. */ + public static function delete(string $dir, string $file): void + { + $path = $dir . '/' . $file . '.json'; + if (is_file($path)) { + unlink($path); + } + } +} diff --git a/api/lib/TextureStore.php b/api/lib/TextureStore.php index 1c96c57b..9e5e7e0b 100644 --- a/api/lib/TextureStore.php +++ b/api/lib/TextureStore.php @@ -57,10 +57,14 @@ final class TextureStore * The extension is derived from the sniffed content, never from anything the client * says, so no request can choose the name or the type of the file it creates. * - * @return array{file:string,width:int,height:int,bytes:int} + * `$originalName` is kept beside the file as a caption (see OriginalName) – the picture may be + * re-encoded on the way, so it can name a `.png` that is stored as `.webp`. That is the point: + * it records where the file came from, not what it now is. + * + * @return array{file:string,name:string,width:int,height:int,bytes:int} * @throws InvalidImageException when the bytes are not a supported image */ - public function store(string $bytes): array + public function store(string $bytes, string $originalName = ''): array { $size = @getimagesizefromstring($bytes); if ($size === false) { @@ -88,8 +92,15 @@ final class TextureStore if (!is_file($path)) { $this->writeFile($path, $bytes); } + OriginalName::write($this->dir, $file, $originalName); - return ['file' => $file, 'width' => $width, 'height' => $height, 'bytes' => strlen($bytes)]; + return [ + 'file' => $file, + 'name' => OriginalName::read($this->dir, $file), + 'width' => $width, + 'height' => $height, + 'bytes' => strlen($bytes), + ]; } /** @@ -99,19 +110,22 @@ final class TextureStore * records the size, so a picker that only knew the file name could not create one without * decoding the picture first. An unreadable file reports 0 × 0 and the picker skips it. * - * @return list + * @return list */ public function list(): array { $items = []; foreach (glob($this->dir . '/*.*') ?: [] as $path) { $file = basename($path); + // Sidecars live in the same directory and are skipped here: `{hash}.png.json` is not + // a valid stored name. if (!self::isValidName($file)) { continue; } $size = @getimagesize($path); $items[] = [ 'file' => $file, + 'name' => OriginalName::read($this->dir, $file), 'width' => (int) ($size[0] ?? 0), 'height' => (int) ($size[1] ?? 0), 'bytes' => (int) filesize($path), @@ -128,7 +142,11 @@ final class TextureStore return false; } $path = $this->dir . '/' . $file; - return is_file($path) && unlink($path); + if (!is_file($path) || !unlink($path)) { + return false; + } + OriginalName::delete($this->dir, $file); + return true; } /** diff --git a/src/core/commands/assetCommands.ts b/src/core/commands/assetCommands.ts index dee6c8e6..82121ffa 100644 --- a/src/core/commands/assetCommands.ts +++ b/src/core/commands/assetCommands.ts @@ -81,9 +81,24 @@ export interface TextureImage { height: number; } -/** Apart from `buildAsset` so callers get a `TextureAsset` rather than the `Asset` union. */ +/** + * Apart from `buildAsset` so callers get a `TextureAsset` rather than the `Asset` union. + * + * The three fields are copied out one by one rather than spread: callers pass the whole upload + * result, which carries `bytes` and – since original names are kept – a `name` of its own. A + * spread would put the *file's* name where the asset's belongs and silently overrule the caller. + */ export function buildTextureAsset(name: string, image: TextureImage, id = newId()): TextureAsset { - return { id, name, props: defaultsFor('texture'), type: 'texture', ...image, frames: [] }; + return { + id, + name, + props: defaultsFor('texture'), + type: 'texture', + file: image.file, + width: image.width, + height: image.height, + frames: [], + }; } /** diff --git a/src/core/store/FontStore.ts b/src/core/store/FontStore.ts index 151d9ea9..5975fbd3 100644 --- a/src/core/store/FontStore.ts +++ b/src/core/store/FontStore.ts @@ -1,12 +1,15 @@ export interface UploadedFont { /** Content-addressed name in `data/fonts/`: `{sha256}.{ttf|otf|woff|woff2}` (see `isFontFile`). */ file: string; + /** The name the file was uploaded under, '' when the server knows none. Never a path. */ + name: string; bytes: number; } /** A typeface the server holds, whether any project references it or not. */ export interface StoredFont { file: string; + name: string; bytes: number; updatedAt: string; } @@ -16,7 +19,8 @@ export interface StoredFont { * an image a font is never re-encoded on the way, so what the server stores is what was picked. */ export interface FontStore { - upload(bytes: Blob | ArrayBuffer, type: string): Promise; + /** `name` is kept beside the file as its caption; the stored file is always the hash. */ + upload(bytes: Blob | ArrayBuffer, type: string, name?: string): Promise; list(): Promise; delete(file: string): Promise; } diff --git a/src/core/store/HttpFontStore.ts b/src/core/store/HttpFontStore.ts index ec99bdb8..1d12feb5 100644 --- a/src/core/store/HttpFontStore.ts +++ b/src/core/store/HttpFontStore.ts @@ -4,8 +4,8 @@ import { apiRequest } from './http'; export class HttpFontStore implements FontStore { constructor(private readonly endpoint: string) {} - async upload(bytes: Blob | ArrayBuffer, type: string): Promise { - return this.request('POST', 'fonts', { bytes, type }); + async upload(bytes: Blob | ArrayBuffer, type: string, name = ''): Promise { + return this.request('POST', 'fonts', { bytes, type, name }); } async list(): Promise { @@ -19,11 +19,12 @@ export class HttpFontStore implements FontStore { private async request( method: string, route: string, - payload?: { bytes: Blob | ArrayBuffer; type: string }, + payload?: { bytes: Blob | ArrayBuffer; type: string; name: string }, ): Promise { return apiRequest(this.endpoint, method, route, { headers: payload ? { 'Content-Type': payload.type } : {}, body: payload?.bytes, + query: payload ? { name: payload.name } : undefined, }); } } diff --git a/src/core/store/HttpTextureStore.ts b/src/core/store/HttpTextureStore.ts index 568dffb1..15907957 100644 --- a/src/core/store/HttpTextureStore.ts +++ b/src/core/store/HttpTextureStore.ts @@ -4,8 +4,8 @@ import type { StoredTexture, TextureStore, UploadedTexture } from './TextureStor export class HttpTextureStore implements TextureStore { constructor(private readonly endpoint: string) {} - async upload(bytes: Blob | ArrayBuffer, type: string): Promise { - return this.request('POST', 'textures', { bytes, type }); + async upload(bytes: Blob | ArrayBuffer, type: string, name = ''): Promise { + return this.request('POST', 'textures', { bytes, type, name }); } async list(): Promise { @@ -19,11 +19,12 @@ export class HttpTextureStore implements TextureStore { private async request( method: string, route: string, - payload?: { bytes: Blob | ArrayBuffer; type: string }, + payload?: { bytes: Blob | ArrayBuffer; type: string; name: string }, ): Promise { return apiRequest(this.endpoint, method, route, { headers: payload ? { 'Content-Type': payload.type } : {}, body: payload?.bytes, + query: payload ? { name: payload.name } : undefined, }); } } diff --git a/src/core/store/TextureStore.ts b/src/core/store/TextureStore.ts index d62e3168..da94325e 100644 --- a/src/core/store/TextureStore.ts +++ b/src/core/store/TextureStore.ts @@ -1,6 +1,11 @@ export interface UploadedTexture { /** Content-addressed name in `data/textures/`: `{sha256}.{png|jpg|webp|gif}` (see `isTextureFile`). */ file: string; + /** + * The name the file was uploaded under, '' when the server knows none. A picture may be + * re-encoded on the way, so this can be a `.png` name on a file stored as `.webp`. + */ + name: string; width: number; height: number; bytes: number; @@ -9,6 +14,7 @@ export interface UploadedTexture { /** A file the server holds, whether any project references it or not. */ export interface StoredTexture { file: string; + name: string; /** 0 × 0 when the server could not read the header; such a file cannot become an asset. */ width: number; height: number; @@ -23,9 +29,10 @@ export interface StoredTexture { export interface TextureStore { /** * `type` only fills the Content-Type header – the server trusts the bytes, not the label – and - * uploading the same picture twice yields the same file, the hash of its content. + * uploading the same picture twice yields the same file, the hash of its content. `name` is the + * caption kept beside it; uploading the same bytes under a new name renames what pickers show. */ - upload(bytes: Blob | ArrayBuffer, type: string): Promise; + upload(bytes: Blob | ArrayBuffer, type: string, name?: string): Promise; list(): Promise; delete(file: string): Promise; } diff --git a/src/core/store/http.ts b/src/core/store/http.ts index 5212d7c3..ba708284 100644 --- a/src/core/store/http.ts +++ b/src/core/store/http.ts @@ -20,7 +20,9 @@ export function statusMessage(status: number, statusText: string): string { /** * One call against the PHP API. Routing goes through the `r` query parameter because the site's - * nginx config does not pass PATH_INFO to PHP. + * nginx config does not pass PATH_INFO to PHP; `query` adds further parameters beside it, which is + * how an upload sends its file name while the body stays the raw bytes. An empty value is dropped + * rather than sent as `&name=`. * * `mapError` lets a store turn a particular status into its own error type before the generic * one is thrown. @@ -29,10 +31,15 @@ export async function apiRequest( endpoint: string, method: string, route: string, - init: Pick = {}, + init: Pick & { query?: Record } = {}, mapError?: (status: number, data: unknown) => Error | undefined, ): Promise { - const response = await fetch(`${endpoint}?r=${encodeURIComponent(route)}`, { method, ...init }); + const { query, ...request } = init; + let url = `${endpoint}?r=${encodeURIComponent(route)}`; + for (const [key, value] of Object.entries(query ?? {})) { + if (value !== '') url += `&${key}=${encodeURIComponent(value)}`; + } + const response = await fetch(url, { method, ...request }); if (response.status === 204) return undefined as T; const text = await response.text(); diff --git a/src/editor/font/FontPicker.ts b/src/editor/font/FontPicker.ts index 7801e38e..fac7e62c 100644 --- a/src/editor/font/FontPicker.ts +++ b/src/editor/font/FontPicker.ts @@ -90,6 +90,11 @@ class FontList extends Widget { return this.items.filter((item) => item.assetName !== null).map((item) => item.file); } + /** The captions as drawn, read off the rows themselves rather than recomputed (test hook). */ + get captions(): string[] { + return this.rows.map((row) => row.caption.text); + } + select(file: string | null): void { if (file === this.selectedFile) return; this.selectedFile = file; @@ -134,9 +139,11 @@ class FontList extends Widget { node.sample.text = truncate(SAMPLE_TEXT, node.sample.style, this.w - ROW_PAD * 2); node.sample.position.set(ROW_PAD, ROW_PAD - 2); + // The asset name wins where there is one – it is what the project calls this face. Otherwise + // the name it was uploaded under, which is all a bare hash leaves to go on. node.caption.style = textStyle(item.assetName ? 'accent' : 'muted'); node.caption.text = truncate( - item.assetName ?? formatBytes(item.bytes), + item.assetName ?? (item.name || formatBytes(item.bytes)), node.caption.style, this.w - ROW_PAD * 2, ); @@ -210,10 +217,11 @@ export class FontPicker extends Popup { } /** What the dialog offers, which of it the project already wears, and what is picked (test hook). */ - debugInfo(): { files: string[]; used: string[]; selected: string | null } { + debugInfo(): { files: string[]; used: string[]; captions: string[]; selected: string | null } { return { files: this.list.fileList, used: this.list.usedFiles, + captions: this.list.captions, selected: this.list.selected?.file ?? null, }; } @@ -266,7 +274,7 @@ export class FontPicker extends Popup { } function describe(item: PickerItem): string { - const size = formatBytes(item.bytes); + const size = item.name ? `${item.name} – ${formatBytes(item.bytes)}` : formatBytes(item.bytes); return item.assetName ? `${size} – already in the project as "${item.assetName}"` : size; } diff --git a/src/editor/fontImport.ts b/src/editor/fontImport.ts index 0439b940..5af83918 100644 --- a/src/editor/fontImport.ts +++ b/src/editor/fontImport.ts @@ -3,7 +3,7 @@ import { nextAssetName } from '../core/commands/naming'; import type { FontAsset } from '../core/model/Asset'; import type { FontStore, UploadedFont } from '../core/store/FontStore'; import type { EditorState } from './state'; -import { type ImportOutcome, importNotice } from './textureImport'; +import { type ImportOutcome, assetNameForFile, importNotice } from './textureImport'; /** What the file dialog offers and a drop accepts; the server rejects everything else anyway. */ export const FONT_ACCEPT = '.ttf,.otf,.woff,.woff2'; @@ -39,8 +39,8 @@ async function importFont(file: File, deps: FontImportDeps): Promise { return new Promise((resolve) => { diff --git a/src/editor/texture/ImagePicker.ts b/src/editor/texture/ImagePicker.ts index 4b7a1d4a..ada5f0fa 100644 --- a/src/editor/texture/ImagePicker.ts +++ b/src/editor/texture/ImagePicker.ts @@ -81,6 +81,11 @@ class ImageGrid extends Widget { return this.items.filter((item) => item.assetName !== null).map((item) => item.file); } + /** The captions as drawn, read off the tiles themselves rather than recomputed (test hook). */ + get captions(): string[] { + return this.tiles.map((tile) => tile.caption.text); + } + select(file: string | null): void { if (file === this.selectedFile) return; this.selectedFile = file; @@ -139,8 +144,10 @@ class ImageGrid extends Widget { node.sprite.height = fitted.h; } - // The asset name is the useful caption where there is one; a bare hash tells nobody anything. - const text = item.width > 0 ? (item.assetName ?? `${item.width} × ${item.height}`) : 'unreadable'; + // The asset name is the useful caption where there is one, then the name the file was + // uploaded under; a bare hash tells nobody anything, and the size is the last resort. + const text = + item.width > 0 ? (item.assetName ?? (item.name || `${item.width} × ${item.height}`)) : 'unreadable'; node.caption.style = textStyle(item.assetName ? 'accent' : 'muted'); node.caption.text = truncate(text, node.caption.style, side); node.caption.position.set(0, side + 1); @@ -210,10 +217,11 @@ export class ImagePicker extends Popup { } /** What the dialog offers, which of it the project already wears, and what is picked (test hook). */ - debugInfo(): { files: string[]; used: string[]; selected: string | null } { + debugInfo(): { files: string[]; used: string[]; captions: string[]; selected: string | null } { return { files: this.grid.fileList, used: this.grid.usedFiles, + captions: this.grid.captions, selected: this.grid.selected?.file ?? null, }; } @@ -270,7 +278,8 @@ export class ImagePicker extends Popup { function describe(item: PickerItem): string { if (item.width <= 0) return 'The server cannot read this file as an image.'; - const size = `${item.width} × ${item.height} px, ${formatBytes(item.bytes)}`; + const measurements = `${item.width} × ${item.height} px, ${formatBytes(item.bytes)}`; + const size = item.name ? `${item.name} – ${measurements}` : measurements; return item.assetName ? `${size} – already in the project as "${item.assetName}"` : size; } diff --git a/src/editor/textureImport.ts b/src/editor/textureImport.ts index e56faddb..2315cf92 100644 --- a/src/editor/textureImport.ts +++ b/src/editor/textureImport.ts @@ -119,8 +119,8 @@ export async function importImages( async function importImage(file: File, deps: TextureImportDeps): Promise { try { const image = await prepareImage(file); - const uploaded = await deps.textures.upload(image.blob, image.type); - return adopt(uploaded, assetNameFor(file), file.name, deps); + const uploaded = await deps.textures.upload(image.blob, image.type, file.name); + return adopt(uploaded, assetNameForFile(file.name, 'Texture'), file.name, deps); } catch (error) { return { name: file.name, assetId: null, error: error instanceof Error ? error.message : String(error) }; } @@ -155,7 +155,9 @@ function adopt( * unreferenced files, so a picture one project dropped is still there for the next to pick up. */ export function importStoredImage(image: UploadedTexture, deps: TextureImportDeps): ImportOutcome { - const outcome = adopt(image, 'Texture', image.file, deps); + // The name it was uploaded under is what the asset is called, the same as for a dropped file; + // a file from before names were kept has none, and falls back to "Texture" as it always did. + const outcome = adopt(image, assetNameForFile(image.name, 'Texture'), image.name || image.file, deps); // Chosen in the picker, so this is "+ → Texture": the tab opens like for any created asset. A // pasted or dropped picture (`importImages`) only selects – it usually goes into the scene. if (outcome.assetId) deps.state.openAsset(outcome.assetId); @@ -198,8 +200,12 @@ export function importNotice(outcomes: readonly ImportOutcome[], noun = 'image') return `Imported ${added + reused} of ${outcomes.length}.${known} ${problem}`; } -function assetNameFor(file: File): string { - return file.name.replace(/\.[^.]+$/, '').trim() || 'Texture'; +/** + * A file name without its extension, as an asset is called: "hero-idle.png" reads better than + * "Texture 3". Shared with the picker, which gets the same name back from the server. + */ +export function assetNameForFile(fileName: string, fallback: string): string { + return fileName.replace(/\.[^.]+$/, '').trim() || fallback; } /** Opens the browser's file dialog; resolves empty when the dialog was cancelled. */ diff --git a/tests/core/http-store.test.ts b/tests/core/http-store.test.ts index 4d25c0ed..539f9347 100644 --- a/tests/core/http-store.test.ts +++ b/tests/core/http-store.test.ts @@ -61,6 +61,20 @@ describe('HttpTextureStore', () => { expect(init?.headers).toEqual({ 'Content-Type': 'image/png' }); }); + it('sends the file name beside the route, and omits it when there is none', async () => { + const answer = JSON.stringify({ file: FILE, name: 'hero idle.png', width: 2, height: 3, bytes: 4 }); + const bytes = new Blob([new Uint8Array(4)]); + + let mock = stubFetch(answer, { status: 201 }); + await new HttpTextureStore('/api').upload(bytes, 'image/png', 'hero idle.png'); + // The body stays the raw image, so the name travels in the query – encoded, not as a path. + expect(mock.mock.calls[0]![0]).toBe('/api?r=textures&name=hero%20idle.png'); + + mock = stubFetch(answer, { status: 201 }); + await new HttpTextureStore('/api').upload(bytes, 'image/png'); + expect(mock.mock.calls[0]![0]).toBe('/api?r=textures'); + }); + it('reports the nginx size limit as a size problem', async () => { stubFetch(NGINX_413, { status: 413, statusText: 'Request Entity Too Large' }); const bytes = new Blob([new Uint8Array(8)]); diff --git a/tests/core/texture-commands.test.ts b/tests/core/texture-commands.test.ts index 2c8ded4b..13c01918 100644 --- a/tests/core/texture-commands.test.ts +++ b/tests/core/texture-commands.test.ts @@ -24,6 +24,19 @@ describe('CreateTextureCommand', () => { expect(findAsset(project, command.asset.id)?.name).toBe('Hero'); }); + it('keeps the caller’s name even though the upload result carries one of its own', () => { + const { project, stack } = makeProject(); + // What `TextureStore.upload` actually answers: the file's original name and its byte count + // ride along. Copied wholesale they would land in the asset and rename it behind the caller. + const upload = { file: FILE, name: 'hero-idle.png', width: 64, height: 32, bytes: 900 }; + const command = new CreateTextureCommand('Hero', upload); + stack.execute(command); + + const asset = findAsset(project, command.asset.id); + expect(asset?.name).toBe('Hero'); + expect(asset).not.toHaveProperty('bytes'); + }); + it('rejects a file name the server cannot have produced – in execute, not in the constructor', () => { const { project, stack } = makeProject(); const command = new CreateTextureCommand('Hero', { file: '../secrets.png', width: 1, height: 1 }); diff --git a/tests/editor/font-import.test.ts b/tests/editor/font-import.test.ts index 3b2a90d6..ddf193d0 100644 --- a/tests/editor/font-import.test.ts +++ b/tests/editor/font-import.test.ts @@ -11,7 +11,7 @@ import { EditorState } from '../../src/editor/state'; import { importNotice } from '../../src/editor/textureImport'; const FILE = `${'a'.repeat(64)}.ttf`; -const font = { file: FILE, bytes: 380_660 }; +const font = { file: FILE, name: '', bytes: 380_660 }; function setup(): EditorState { const state = new EditorState(); @@ -44,6 +44,16 @@ describe('importStoredFont', () => { expect(state.project.assets.filter((a) => a.type === 'font')).toEqual([]); }); + it('names the asset after the file it was uploaded as, not "Font"', () => { + const state = setup(); + importStoredFont({ ...font, name: 'DejaVuSerif.ttf' }, { state, fonts }); + + expect(state.project.assets.find((a): a is FontAsset => a.type === 'font')).toMatchObject({ + file: FILE, + name: 'DejaVuSerif', + }); + }); + it('reuses the asset that already names the file instead of making a second one', () => { const state = setup(); const first = importStoredFont(font, { state, fonts }); @@ -66,7 +76,8 @@ describe('importFonts', () => { const [outcome] = await importFonts([file], { state, fonts }); // The blob goes up as it came in: a typeface cannot be re-encoded smaller on the way. - expect(upload).toHaveBeenCalledWith(file, 'font/ttf'); + // The name travels with the bytes: it is all the server can caption the hash with. + expect(upload).toHaveBeenCalledWith(file, 'font/ttf', 'DejaVuSerif.ttf'); expect(outcome).toMatchObject({ error: null, reused: false }); expect(state.project.assets.find((a): a is FontAsset => a.type === 'font')).toMatchObject({ file: FILE, diff --git a/tests/editor/texture-import.test.ts b/tests/editor/texture-import.test.ts index f5d35a16..7b37fa54 100644 --- a/tests/editor/texture-import.test.ts +++ b/tests/editor/texture-import.test.ts @@ -4,6 +4,7 @@ import { createEmptyProject } from '../../src/core/model/Project'; import type { TextureStore } from '../../src/core/store/TextureStore'; import { EditorState } from '../../src/editor/state'; import { + assetNameForFile, encodeWithinBudget, type ImportOutcome, importNotice, @@ -27,6 +28,18 @@ function reused(name: string): ImportOutcome { return { name, assetId: `id-${name}`, error: null, reused: true }; } +describe('assetNameForFile', () => { + it('drops the extension and falls back when nothing readable is left', () => { + expect(assetNameForFile('hero-idle.png', 'Texture')).toBe('hero-idle'); + // Only the last extension goes: a name with dots in it keeps them. + expect(assetNameForFile('walk.cycle.left.png', 'Texture')).toBe('walk.cycle.left'); + expect(assetNameForFile('README', 'Texture')).toBe('README'); + expect(assetNameForFile('.png', 'Texture')).toBe('Texture'); + // What a file the server knows no name for looks like. + expect(assetNameForFile('', 'Font')).toBe('Font'); + }); +}); + describe('planImageFit', () => { it('leaves an image alone that fits in both senses', () => { expect(planImageFit(32, 32, 900)).toEqual({ kind: 'keep' }); @@ -114,7 +127,7 @@ describe('importNotice', () => { describe('importStoredImage', () => { const FILE = `${'a'.repeat(64)}.png`; - const image = { file: FILE, width: 64, height: 48, bytes: 900 }; + const image = { file: FILE, name: '', width: 64, height: 48, bytes: 900 }; /** The picker takes the image from the list; nothing here ever uploads. */ const textures = { upload: vi.fn(), list: vi.fn(), delete: vi.fn() } as unknown as TextureStore; @@ -138,6 +151,16 @@ describe('importStoredImage', () => { expect(state.project.assets.filter((a) => a.type === 'texture')).toEqual([]); }); + it('names the asset after the file the picture was uploaded as, not "Texture"', () => { + const state = setup(); + importStoredImage({ ...image, name: 'hero-idle.png' }, { state, textures }); + + expect(state.project.assets.find((a): a is TextureAsset => a.type === 'texture')).toMatchObject({ + file: FILE, + name: 'hero-idle', + }); + }); + it('reuses the asset that already names the file instead of making a second one', () => { const state = setup(); const first = importStoredImage(image, { state, textures }); diff --git a/tools/e2e/12-textures.mjs b/tools/e2e/12-textures.mjs index 52c94bdb..aa140dd9 100644 --- a/tools/e2e/12-textures.mjs +++ b/tools/e2e/12-textures.mjs @@ -315,6 +315,14 @@ export async function textures(t) { (picker?.used ?? []).includes(texture.file) && !(picker?.used ?? []).includes(spare), JSON.stringify(picker?.used), ); + // The caption: what the project calls a picture it wears, otherwise the name the file was + // uploaded under (#83). Before that a file nobody wore read as its pixel size and nothing else. + const captionOf = (file) => (picker?.captions ?? [])[(picker?.files ?? []).indexOf(file)]; + check( + 'an unused file is captioned by its original name, a used one by its asset name', + captionOf(spare) === 'spare.png' && captionOf(texture.file) === texture.name, + JSON.stringify({ spare: captionOf(spare), used: captionOf(texture.file), asset: texture.name }), + ); await shot('34-image-picker'); const tile = await json(`__gm.imagePickerTile(${JSON.stringify(spare)})`); @@ -337,6 +345,29 @@ export async function textures(t) { adopted.length === 2 && adopted[1]?.file === spare && adopted[1]?.w === 24 && adopted[1]?.h === 24, JSON.stringify(adopted), ); + check( + 'and it is named after the file it was uploaded as, not "Texture"', + adopted[1]?.name === 'spare', + JSON.stringify(adopted.map((a) => a.name)), + ); + + // 12g. The name beside the file, straight against the API: uploading the same bytes again under + // a new name renames it (the user's rule), a path is stripped to its last part, and an + // upload carrying no name at all leaves the known one alone rather than erasing it. + const renamed = await evaluate(`(async () => { + const bytes = await fetch('/game-maker/data/textures/${spare}').then((r) => r.blob()); + const post = (query) => fetch('/game-maker/api/index.php?r=textures' + query, + { method: 'POST', headers: { 'Content-Type': 'image/png' }, body: bytes }).then((r) => r.json()); + const evil = await post('&name=' + encodeURIComponent('../../evil/renamed.png')); + const nameless = await post(''); + await post('&name=spare.png'); + return { evil: evil.name, file: evil.file, nameless: nameless.name }; + })()`); + check( + 'a second upload renames the file, a path is stripped, and a nameless upload keeps the name', + renamed.evil === 'renamed.png' && renamed.file === spare && renamed.nameless === 'renamed.png', + JSON.stringify(renamed), + ); check( 'and the notice says it was added rather than imported', (await json('__gm.state.notice')) === 'Added the image to the project.', diff --git a/tools/e2e/30-font.mjs b/tools/e2e/30-font.mjs index e9eada31..3036cf3f 100644 --- a/tools/e2e/30-font.mjs +++ b/tools/e2e/30-font.mjs @@ -56,6 +56,12 @@ export async function font(t) { JSON.stringify({ serif: serif?.file, mono: mono?.file, count: stored.length }), ); if (!serif || !mono) return; + // The hash says nothing, so the name the file was uploaded under is kept beside it (#83). + check( + 'the listing carries the name each file was uploaded under', + serif.name === 'DejaVuSerif.ttf' && mono.name === 'DejaVuSansMono.ttf', + JSON.stringify({ serif: serif.name, mono: mono.name }), + ); // 30b. "+ → Font" offers them; a font is picked rather than created empty, like a texture. await evaluate('__gm.console.clear()'); @@ -69,6 +75,13 @@ export async function font(t) { Boolean(picker) && picker.files.includes(serif.file) && picker.used.length === 0, JSON.stringify(picker), ); + // A row the project does not wear yet is captioned by the file's own name (#83); without it + // there was nothing to read but its byte count. + check( + 'a row nobody wears yet is captioned with the name the file was uploaded under', + picker.captions[picker.files.indexOf(serif.file)] === 'DejaVuSerif.ttf', + JSON.stringify(picker.captions), + ); await shot('30-font-picker'); // A row is a container holding its background, not a Graphics with children: Pixi v8 warns // about the latter, and the warning is the only sign that the rows are built the wrong way. @@ -85,8 +98,8 @@ export async function font(t) { await sleep(400); const fonts = await assetsOfType('font'); check( - 'picking a row makes a font asset of that file, and nothing else', - fonts.length === 1 && fonts[0].file === serif.file && fonts[0].name === 'Font', + 'picking a row makes a font asset of that file, named after the file rather than "Font"', + fonts.length === 1 && fonts[0].file === serif.file && fonts[0].name === 'DejaVuSerif', JSON.stringify(fonts), ); if (fonts.length !== 1) return; diff --git a/tools/lib/projects.mjs b/tools/lib/projects.mjs index 6aa6006f..74e7be88 100644 --- a/tools/lib/projects.mjs +++ b/tools/lib/projects.mjs @@ -11,7 +11,7 @@ * All ids exist on the server (created via POST ?r=projects); PUT does not create projects. */ import { readFile } from 'node:fs/promises'; -import { extname } from 'node:path'; +import { basename, extname } from 'node:path'; export const PROJECTS = { playground: '649188c15414', @@ -91,7 +91,9 @@ export async function uploadFont(baseUrl, filePath) { console.error(`unsupported font type: ${filePath}`); process.exit(1); } - const res = await fetch(`${baseUrl}api/index.php?r=fonts`, { + // The name rides along like a browser upload's: the picker captions the hash with it. + const name = encodeURIComponent(basename(filePath)); + const res = await fetch(`${baseUrl}api/index.php?r=fonts&name=${name}`, { method: 'POST', headers: { 'Content-Type': type }, body: bytes, @@ -102,7 +104,7 @@ export async function uploadFont(baseUrl, filePath) { console.error('font upload failed', res.status, result); process.exit(1); } - return { file: result.file }; + return { file: result.file, name: result.name }; } /** Overwrites a project, keeping the name it already has on the server. */