Skip to content

Commit 15bd418

Browse files
authored
fix: accept * N anywhere in the track header, not only the first slot (#6)
* fix: accept `* N` anywhere in the track header, not only the first slot `* N` was recognized only immediately after `gen <id>`. Anywhere else it fell through to the key/value loop and became a macro param literally named `*`, so track Pad id pad gen gameBoyDmg layer 0 * 16 silently lost its pattern length and played as ONE bar. No error — the length just became the default. Emit always writes `* N` first, which is why this survived: it only bites hand-written and generated files. tish-gba has 24 such track lines across 4 songs, and two of them bake differently because of it — found by diffing the baked ROM data before and after moving that repo onto this parser. Now `* N` and the trailing `key value` pairs are accepted in any order, and a `*` that names no valid length is an error rather than a dropped token — a length that quietly becomes 1 is the worst kind of wrong. Corpus case 002 gains both orderings; coverage holds at 100%. * fix(test): pin the new `* N` orderings from the Rust side too
1 parent ca0e7b3 commit 15bd418

6 files changed

Lines changed: 157 additions & 24 deletions

File tree

conformance/002-track-header.deck

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,8 @@ track Pad id c4 gen fm * inf
77
mix gain 0.5
88
track Lead id c5 gen mymacro cutoff 1200 wave saw
99
mix gain 0.7
10+
track Layered id c6 gen fm layer 0 * 16
11+
mix gain 0.6
12+
track Mixed id c7 gen fm layer 2 * 4 cutoff 900
13+
mix gain 0.6
1014
remove_track obsolete

conformance/002-track-header.expected.json

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,51 @@
8585
}
8686
],
8787
"genBlocks": []
88+
},
89+
{
90+
"name": "Layered",
91+
"id": "c6",
92+
"generatorId": "fm",
93+
"rawGenId": "fm",
94+
"genParams": {
95+
"layer": 0
96+
},
97+
"loopBars": 16,
98+
"body": [
99+
{
100+
"lineNo": 11,
101+
"tokens": [
102+
"mix",
103+
"gain",
104+
"0.6"
105+
],
106+
"raw": "mix gain 0.6"
107+
}
108+
],
109+
"genBlocks": []
110+
},
111+
{
112+
"name": "Mixed",
113+
"id": "c7",
114+
"generatorId": "fm",
115+
"rawGenId": "fm",
116+
"genParams": {
117+
"layer": 2,
118+
"cutoff": 900
119+
},
120+
"loopBars": 4,
121+
"body": [
122+
{
123+
"lineNo": 13,
124+
"tokens": [
125+
"mix",
126+
"gain",
127+
"0.6"
128+
],
129+
"raw": "mix gain 0.6"
130+
}
131+
],
132+
"genBlocks": []
88133
}
89134
],
90135
"removeTrackIds": [
@@ -179,6 +224,40 @@
179224
}
180225
],
181226
"errors": []
227+
},
228+
{
229+
"id": "c6",
230+
"rows": [
231+
{
232+
"kind": "mix",
233+
"gain": 0.6,
234+
"pan": null,
235+
"mute": null,
236+
"solo": null,
237+
"eqLo": null,
238+
"eqMid": null,
239+
"eqHi": null,
240+
"lineNo": 11
241+
}
242+
],
243+
"errors": []
244+
},
245+
{
246+
"id": "c7",
247+
"rows": [
248+
{
249+
"kind": "mix",
250+
"gain": 0.6,
251+
"pan": null,
252+
"mute": null,
253+
"solo": null,
254+
"eqLo": null,
255+
"eqMid": null,
256+
"eqHi": null,
257+
"lineNo": 13
258+
}
259+
],
260+
"errors": []
182261
}
183262
],
184263
"clipBodies": []

docs/DECK_GRAMMAR.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ track <displayName> id <channelId> gen <generatorId|macro> [ * <N|inf> ] [ <para
7272

7373
- `* N`**pattern length** in bars (default 1). Channel spans `N × 16` steps and repeats. `* inf` / `* infinite` clears an explicit finite length.
7474
- Trailing `key value` pairs — **macro parameter overrides** when `gen` is a macro name.
75+
- `* N` and the `key value` pairs may appear in **any order** after `gen <id>`. Emit writes `* N` first; a `*` that names no valid length is an error, never a silently dropped token.
7576
- `generatorId` spellings are host-registered (`registerGeneratorIdAliases`). Undeclared ids pass through as-is.
7677

7778
---

rust/conformance.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,20 @@ fn pinned_divergences() {
110110

111111
let p = deckfile::parse(&read("002-track-header", "deck"));
112112
let names: Vec<&str> = p.tracks.iter().map(|t| t.name.as_str()).collect();
113-
assert_eq!(names, ["MOS 6581", "Bass", "Pad", "Lead"], "multi-word names");
113+
assert_eq!(
114+
names,
115+
["MOS 6581", "Bass", "Pad", "Lead", "Layered", "Mixed"],
116+
"multi-word names"
117+
);
114118
assert_eq!(p.tracks[1].loop_bars, Some(2), "`* 2`");
115119
assert_eq!(p.tracks[2].loop_bars, None, "`* inf` clears the length, it is not an error");
116120
assert_eq!(p.tracks[3].gen_params.num("cutoff"), Some(1200.0), "macro param override");
121+
// `* N` after other params. This is the case that silently played at 1 bar instead of 16 and
122+
// made two of tish-gba's songs bake differently.
123+
assert_eq!(p.tracks[4].loop_bars, Some(16), "`* N` after a key/value pair");
124+
assert_eq!(p.tracks[4].gen_params.num("layer"), Some(0.0), "params survive alongside a late `* N`");
125+
assert_eq!(p.tracks[5].loop_bars, Some(4), "`* N` between key/value pairs");
126+
assert_eq!(p.tracks[5].gen_params.num("cutoff"), Some(900.0), "params on both sides of `* N`");
117127

118128
let p = deckfile::parse(&read("003-steps-and-locks", "deck"));
119129
let steps = p.tracks[0]

src/deckfile/Parser.tish

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -456,38 +456,48 @@ export fn parseProgram(source) {
456456
}
457457
}
458458
if (idIdx >= 2 && toks.length >= idIdx + 4) {
459+
// Everything after `gen <id>` is `* <N|inf>` (pattern length) plus `key value` pairs (macro
460+
// parameter overrides), in ANY order.
461+
//
462+
// `* N` used to be recognized only in the first slot. Anywhere else it fell through to the
463+
// key/value loop and became a param literally named `*` — so `track Pad id pad gen x layer 0
464+
// * 16` silently lost its length and played as one bar, with no error. Emit always writes
465+
// `* N` first, but hand-written and generated files do not, and a length that quietly
466+
// becomes 1 is the worst kind of wrong.
459467
let after = idIdx + 4
460468
let loopBars = null
461-
if (toks.length >= after + 2 && toks[after] === "*") {
462-
let lx = toks[after + 1]
463-
if (lx === "inf" || lx === "infinite") {
464-
loopBars = null
465-
} else {
466-
if (isNumberToken(lx)) {
467-
let nn = Math.floor(Number(lx))
468-
if (nn >= 1) {
469-
loopBars = nn
469+
let genParams = {}
470+
let gp = after
471+
while (gp < toks.length) {
472+
if (toks[gp] === "*") {
473+
if (gp + 1 < toks.length) {
474+
let lx = toks[gp + 1]
475+
if (lx === "inf" || lx === "infinite") {
476+
loopBars = null
477+
} else if (isNumberToken(lx)) {
478+
let nn = Math.floor(Number(lx))
479+
if (nn >= 1) {
480+
loopBars = nn
481+
} else {
482+
errors.push({ line: i, msg: "track * N: N must be a positive integer" })
483+
}
470484
} else {
471-
errors.push({ line: i, msg: "track * N: N must be a positive integer" })
485+
errors.push({ line: i, msg: "track * N: expected number or inf/infinite" })
472486
}
473487
} else {
474488
errors.push({ line: i, msg: "track * N: expected number or inf/infinite" })
475489
}
490+
gp = gp + 2
491+
} else if (gp + 1 < toks.length) {
492+
let k = toks[gp]
493+
let v = toks[gp + 1]
494+
let nv = Number(v)
495+
genParams[k] = (nv === nv && v.length > 0) ? nv : v
496+
gp = gp + 2
497+
} else {
498+
gp = gp + 1
476499
}
477500
}
478-
// Trailing `key value` pairs after the gen id (and optional `* N`) are macro parameter overrides.
479-
let genParams = {}
480-
let gp = after
481-
if (toks.length >= after + 2 && toks[after] === "*") {
482-
gp = after + 2
483-
}
484-
while (gp + 1 < toks.length) {
485-
let k = toks[gp]
486-
let v = toks[gp + 1]
487-
let nv = Number(v)
488-
genParams[k] = (nv === nv && v.length > 0) ? nv : v
489-
gp = gp + 2
490-
}
491501
currentTrack = {
492502
name: toks.slice(1, idIdx).join(" "),
493503
id: toks[idIdx + 1],

test/coverage.mjs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,35 @@ check("directives no errors", dir.errors.length === 0)
373373
check("directives do not block parse", dir.bpm === 120)
374374
check("bare @ errors", parseProgram("@\n").errors.some((e) => e.msg.includes("directive verb")))
375375

376+
// ── Track header: `* N` and key/value params in any order ────────────────────
377+
// `* N` was only recognized in the first slot; anywhere else it became a param literally named `*`,
378+
// so the pattern length silently became 1.
379+
const hdr = (s) => parseProgram(s).tracks[0]
380+
check("star first", hdr("track P id p gen g * 16 layer 0\n").loopBars === 16)
381+
check("star last", hdr("track P id p gen g layer 0 * 16\n").loopBars === 16)
382+
check("star middle", hdr("track P id p gen g layer 0 * 4 cutoff 900\n").loopBars === 4)
383+
check("star last keeps params", hdr("track P id p gen g layer 0 * 16\n").genParams.layer === 0)
384+
check("star middle keeps params", hdr("track P id p gen g layer 0 * 4 cutoff 900\n").genParams.cutoff === 900)
385+
check("star is not a param", hdr("track P id p gen g layer 0 * 16\n").genParams["*"] === undefined)
386+
check("inf last", hdr("track P id p gen g layer 2 * inf\n").loopBars === null)
387+
check("infinite last", hdr("track P id p gen g layer 2 * infinite\n").loopBars === null)
388+
check("no star", hdr("track P id p gen g layer 0\n").loopBars === null)
389+
// A `*` that names no length is an error, not a silently dropped token.
390+
check(
391+
"star zero errors",
392+
parseProgram("track P id p gen g layer 0 * 0\n").errors.some((e) => e.msg.includes("positive integer"))
393+
)
394+
check(
395+
"star non-numeric errors",
396+
parseProgram("track P id p gen g layer 0 * zz\n").errors.some((e) => e.msg.includes("inf/infinite"))
397+
)
398+
check(
399+
"bare trailing star errors",
400+
parseProgram("track P id p gen g layer 0 *\n").errors.some((e) => e.msg.includes("inf/infinite"))
401+
)
402+
// An odd trailing token (no value) is skipped rather than looping forever.
403+
check("odd trailing token", hdr("track P id p gen g layer 0 dangling\n").genParams.layer === 0)
404+
376405
// ── Body lines ────────────────────────────────────────────────────────────────
377406
const body = (s) => parseBodyLine(tokenize(s))
378407

0 commit comments

Comments
 (0)