From 9bc4ed0bb2a0684dab1f279f769e0b5b2729835b Mon Sep 17 00:00:00 2001 From: Eli White Date: Tue, 28 Apr 2026 13:49:40 -0700 Subject: [PATCH] writeChartFile: re-quote extraChartSongFields per .chart spec types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parser strips enclosing quotes from `[Song]` values on read, so `extraChartSongFields` always holds bare strings. Emitting them verbatim produced `MusicStream = song.ogg` (unquoted) — Moonscraper rejects that shape and fails to load the audio. Re-apply quoting on write per the spec's field-type table: - `Player2` (the lone `bare string` field) stays bare. - Numeric and boolean primitives stay bare via value-shape check. - Everything else gets quoted. The default-quote-strings rule is verified across a 62K-chart corpus: every `string` / `file path` field is quoted ≥99.97% of the time, and the only de-facto boolean field (`OriginalArtist = false`) appears unquoted in 171/171 occurrences. --- src/__tests__/chart-round-trip-extras.test.ts | 3 +- .../round-trip-unrecognized-extras.test.ts | 67 +++++++++++++++++-- src/chart/chart-writer.ts | 40 +++++++++-- 3 files changed, 98 insertions(+), 12 deletions(-) diff --git a/src/__tests__/chart-round-trip-extras.test.ts b/src/__tests__/chart-round-trip-extras.test.ts index f9b9987..22e0f41 100644 --- a/src/__tests__/chart-round-trip-extras.test.ts +++ b/src/__tests__/chart-round-trip-extras.test.ts @@ -67,7 +67,8 @@ describe('.chart [Song] unknown-key preservation (metadata.extraChartSongFields) // Known typed fields still populate. expect(r.metadata.name).toBe('Test') expect(r.metadata.artist).toBe('Me') - // Everything else lands in the preservation bag, values unchanged. + // Everything else lands in the preservation bag, with quotes stripped. + // The writer re-applies quoting per the spec's field-type table on emit. expect(r.metadata.extraChartSongFields).toEqual({ Player2: 'bass', PreviewEnd: '0', diff --git a/src/__tests__/round-trip-unrecognized-extras.test.ts b/src/__tests__/round-trip-unrecognized-extras.test.ts index 0cff877..f70367a 100644 --- a/src/__tests__/round-trip-unrecognized-extras.test.ts +++ b/src/__tests__/round-trip-unrecognized-extras.test.ts @@ -63,22 +63,77 @@ describe('writeChartFile round-trip: metadata.extraChartSongFields', () => { }) }) - it('preserves quoted string values including inner spaces', () => { + it('round-trips bare values cleanly (writer re-quotes per spec)', () => { + // The parser strips enclosing quotes on read, so values stored in + // `extraChartSongFields` are always bare. The writer re-applies quoting + // from the spec's field-type table, so a bare-then-quoted-then-stripped + // trip is lossless for the value content. const chart = createEmptyChart({ format: 'chart' }) chart.metadata.extraChartSongFields = { - ArtistText: '"by"', - MusicStream: '"Some Song.ogg"', + ArtistText: 'by', + MusicStream: 'Some Song.ogg', } const re = roundTripChart(chart) expect(re.metadata.extraChartSongFields).toEqual({ - // The parser strips one layer of enclosing quotes on read. What matters - // is that the key survives and the value round-trips: re-writing the - // stripped form re-adds no quotes, so the next parse sees it unquoted. ArtistText: 'by', MusicStream: 'Some Song.ogg', }) }) + it('emits MusicStream with the quotes Moonscraper expects', () => { + // Pin the on-disk shape directly: re-writing must produce + // `MusicStream = "song.ogg"` (quoted), not `MusicStream = song.ogg`. + // Bare value in → quoted value out. + const chart = createEmptyChart({ format: 'chart' }) + chart.metadata.extraChartSongFields = { MusicStream: 'song.ogg' } + const text = writeChartFile(chart) + expect(text).toContain('MusicStream = "song.ogg"') + expect(text).not.toMatch(/MusicStream = song\.ogg[^"]/) + }) + + it('emits Player2 unquoted (bare-string field per spec)', () => { + // `Player2` is the lone `bare string` type in the [Song] section. The + // writer must NOT quote it, even though it's a string-y value. + const chart = createEmptyChart({ format: 'chart' }) + chart.metadata.extraChartSongFields = { Player2: 'bass' } + const text = writeChartFile(chart) + expect(text).toContain('Player2 = bass') + expect(text).not.toContain('Player2 = "bass"') + }) + + it('emits numeric-typed legacy fields unquoted', () => { + const chart = createEmptyChart({ format: 'chart' }) + chart.metadata.extraChartSongFields = { HoPo: '0', PreviewEnd: '180' } + const text = writeChartFile(chart) + expect(text).toContain('HoPo = 0') + expect(text).toContain('PreviewEnd = 180') + expect(text).not.toContain('HoPo = "0"') + }) + + it('quotes unknown string-shaped keys, leaves primitive-shaped keys bare', () => { + // Forward-compat heuristic for keys outside the spec table: if the + // value looks like a primitive literal (number / decimal / boolean), + // emit bare; otherwise quote. Erring on the side of quoting matches + // what readers (Moonscraper, scan-chart) accept and what authoring + // tools mostly emit. The boolean case mirrors `OriginalArtist = false` + // observed in the corpus. + const chart = createEmptyChart({ format: 'chart' }) + chart.metadata.extraChartSongFields = { + MysteryString: 'hello', + MysteryNumber: '42', + MysteryDecimal: '-1.5', + MysteryBool: 'false', + OriginalArtist: 'false', + } + const text = writeChartFile(chart) + expect(text).toContain('MysteryString = "hello"') + expect(text).toContain('MysteryNumber = 42') + expect(text).toContain('MysteryDecimal = -1.5') + expect(text).toContain('MysteryBool = false') + expect(text).toContain('OriginalArtist = false') + expect(text).not.toContain('OriginalArtist = "false"') + }) + it('preserves future / unknown keys the parser has never heard of', () => { const chart = createEmptyChart({ format: 'chart' }) chart.metadata.extraChartSongFields = { FutureField: 'future-value', Boss: '1' } diff --git a/src/chart/chart-writer.ts b/src/chart/chart-writer.ts index 3266a46..208b27c 100644 --- a/src/chart/chart-writer.ts +++ b/src/chart/chart-writer.ts @@ -84,13 +84,14 @@ function serializeSongSection(chart: ParsedChart): string[] { // Round-trip any `[Song]` keys the parser didn't claim (deprecated // Moonscraper / GHTCP fields — `Player2`, `HoPo`, `PreviewEnd`, `MediaType`, - // audio-stream filenames, etc.). Values are preserved verbatim: if the - // source didn't quote the value, we don't quote it here either. Consumers - // should treat these as opaque and never synthesize them — editors should - // discover audio via folder scan rather than trust `*Stream` values here. + // audio-stream filenames, etc.). The parser strips enclosing quotes on + // read, so values stored here are always bare; we re-quote per the .chart + // spec field-type table on write. Moonscraper rejects unquoted stream + // filenames (`MusicStream = song.ogg` fails to load audio), so getting the + // quoting right matters for game compatibility, not just round-tripping. if (m.extraChartSongFields) { for (const [key, value] of Object.entries(m.extraChartSongFields)) { - lines.push(` ${key} = ${value}`) + lines.push(` ${key} = ${formatExtraSongValue(key, value)}`) } } @@ -98,6 +99,35 @@ function serializeSongSection(chart: ParsedChart): string[] { return lines } +// `[Song]` keys whose values must stay unquoted even though the value is a +// non-primitive string. Per the .chart spec, `Player2` is the only `bare +// string` field — every other string-typed field is quoted. Numeric and +// boolean values are handled by `BARE_VALUE_RE` below, so they don't need an +// entry here. +const BARE_STRING_SONG_FIELDS = new Set(['Player2']) + +// Primitive literals (`number`, `decimal`, `boolean`) that the spec emits +// bare. The spec doesn't list any boolean-typed [Song] fields, but the corpus +// shows `OriginalArtist = false` written bare in every chart that uses it, and +// this leaves room for any future spec additions. Booleans are matched +// case-sensitively per the spec — `True`/`TRUE` would be malformed values and +// fall through to the quoting branch, which is defensible. +const BARE_VALUE_RE = /^(?:-?\d+(?:\.\d+)?|true|false)$/ + +/** + * Format a value for an `extraChartSongFields` key. The parser strips quotes + * on read, so values arrive bare; this re-applies them on write. The rule: + * primitive-literal values (numeric / boolean) stay bare, `Player2` stays + * bare per spec, and everything else gets quoted. Quoting strings by default + * matches the spec (every string / file-path field is quoted) and the + * 62K-chart corpus, and fixes the Moonscraper audio-load bug for unquoted + * stream filenames. + */ +function formatExtraSongValue(key: string, value: string): string { + if (BARE_STRING_SONG_FIELDS.has(key)) return value + return BARE_VALUE_RE.test(value) ? value : `"${value}"` +} + // --------------------------------------------------------------------------- // [SyncTrack] section // ---------------------------------------------------------------------------