From 435e27392778f1b5e3eab5b6192c1d3acb49a101 Mon Sep 17 00:00:00 2001 From: gionnibgud Date: Thu, 23 Jul 2026 00:33:50 +0200 Subject: [PATCH] Carry rig bindings through the sloppak tone payload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sloppak_tone_changes` emitted `{t, name}` only, so a chart's declared sound never reached the client: `base_rig` was never read and each change's `rig` was dropped at the wire boundary. Both survive load intact (`Arrangement.tones` is an opaque passthrough) — the strip happened here, at the last step before send. That left the rig model (feedpak-spec 1.18.0 §6.9/§7.9) unreachable from core: a pack could declare which rig voices a part, and nothing downstream could ever see it. First step of the core reader for source rigs; the rig library itself and the manifest precedence cascade follow. Return `(base, base_rig, changes)` and keep `rig` on each change. Both ids are validated as non-blank strings and stripped — anything else is dropped rather than forwarded, so presence of the key means the change binds a rig. Resolution against `rigs.json` deliberately does NOT happen here: this builder preserves the declared binding, while realization selection and the `intent.gm` fallback belong to whatever voices the part. On the wire `base_rig` is omitted entirely when empty, so packs that bind no rig produce the byte-identical `tone_changes` message they always did. Signed-off-by: gionnibgud --- lib/routers/ws_highway.py | 21 +++++++++---- lib/tones.py | 34 ++++++++++++++------ tests/test_tones.py | 65 ++++++++++++++++++++++++++++++++++----- 3 files changed, 97 insertions(+), 23 deletions(-) diff --git a/lib/routers/ws_highway.py b/lib/routers/ws_highway.py index 61cb65b..02b2756 100644 --- a/lib/routers/ws_highway.py +++ b/lib/routers/ws_highway.py @@ -774,20 +774,29 @@ async def highway_ws(websocket: WebSocket, filename: str, arrangement: int = -1, # (Arrangement.tones, populated by the converter), so read it straight # off `arr` rather than walking for XML that doesn't exist. if is_slop: - # `sloppak_tone_changes` builds the (base, sorted changes) pair - # from `Arrangement.tones`, skipping non-string names and - # non-finite/non-numeric times — unit-tested in test_tones.py. + # `sloppak_tone_changes` builds the (base, base_rig, sorted + # changes) triple from `Arrangement.tones`, skipping non-string + # names, non-finite/non-numeric times, and unusable rig ids — + # unit-tested in test_tones.py. from tones import sloppak_tone_changes - base_name, tone_changes = sloppak_tone_changes(getattr(arr, "tones", None)) + base_name, base_rig, tone_changes = sloppak_tone_changes( + getattr(arr, "tones", None) + ) # Send when there's a base tone OR timed changes — a single-tone # arrangement has a base but no switches, and the highway should # still be able to show the initial tone. if tone_changes or base_name: - await websocket.send_json({ + payload = { "type": "tone_changes", "base": base_name, "data": tone_changes, - }) + } + # `base_rig` is additive (feedpak-spec §6.9) — omitted entirely + # when the chart binds no rig, so consumers that predate the rig + # model see the exact payload they always did. + if base_rig: + payload["base_rig"] = base_rig + await websocket.send_json(payload) else: xml_paths = sorted(_xml_walk("*.xml")) diff --git a/lib/tones.py b/lib/tones.py index 3d56134..9137163 100644 --- a/lib/tones.py +++ b/lib/tones.py @@ -32,20 +32,29 @@ def tokens(s: str) -> set[str]: return {t for t in re.split(r"[^a-z0-9]+", (s or "").lower()) if t} -def sloppak_tone_changes(arr_tones) -> tuple[str, list[dict]]: +def sloppak_tone_changes(arr_tones) -> tuple[str, str, list[dict]]: """Build the highway tone-change payload from an arrangement's tone block. Given ``Arrangement.tones`` (the dict embedded in the sloppak, or ``None``), - returns ``(base, changes)`` where ``base`` is the initial tone name and - ``changes`` is a time-sorted ``[{"t", "name"}]`` list. Non-string names, - non-dict entries, and non-numeric / non-finite times are skipped — a - hand-edited or third-party sloppak must not crash the highway WebSocket - or emit NaN/inf (which the client's ``JSON.parse`` rejects). + returns ``(base, base_rig, changes)`` where ``base`` is the initial tone + name, ``base_rig`` is the ``rigs.json`` rig id bound to it (feedpak-spec + §6.9; ``""`` when absent), and ``changes`` is a time-sorted + ``[{"t", "name", "rig"?}]`` list. Non-string names, non-dict entries, and + non-numeric / non-finite times are skipped — a hand-edited or third-party + sloppak must not crash the highway WebSocket or emit NaN/inf (which the + client's ``JSON.parse`` rejects). + + ``rig`` / ``base_rig`` are carried through but NOT resolved against + ``rigs.json`` here: this builder only preserves the binding the chart + declared. Realization selection and the ``intent.gm`` fallback (§7.9) belong + to the consumer that actually voices the part. """ if not isinstance(arr_tones, dict): - return "", [] + return "", "", [] base_val = arr_tones.get("base", "") base = base_val.strip() if isinstance(base_val, str) else "" + base_rig_val = arr_tones.get("base_rig", "") + base_rig = base_rig_val.strip() if isinstance(base_rig_val, str) else "" changes: list[dict] = [] raw_changes = arr_tones.get("changes") @@ -65,6 +74,13 @@ def sloppak_tone_changes(arr_tones) -> tuple[str, list[dict]]: continue if not math.isfinite(t): continue - changes.append({"t": round(t, 3), "name": name}) + change = {"t": round(t, 3), "name": name} + # ponytail: `rig` only when it's a usable id — a non-string or blank + # value is dropped rather than forwarded, so a consumer can treat + # presence of the key as "this change binds a rig". + rig = c.get("rig") + if isinstance(rig, str) and rig.strip(): + change["rig"] = rig.strip() + changes.append(change) changes.sort(key=lambda x: x["t"]) - return base, changes + return base, base_rig, changes diff --git a/tests/test_tones.py b/tests/test_tones.py index a5b83a8..3eb8db5 100644 --- a/tests/test_tones.py +++ b/tests/test_tones.py @@ -6,16 +6,17 @@ from tones import sloppak_tone_changes # ── sloppak_tone_changes (highway payload builder) ─────────────────────────── def test_sloppak_tone_changes_sorts_and_returns_base(): - base, changes = sloppak_tone_changes({ + base, base_rig, changes = sloppak_tone_changes({ "base": "Clean", "changes": [{"t": 12.5, "name": "Drive"}, {"t": 3.0, "name": "Clean"}], }) assert base == "Clean" + assert base_rig == "" assert changes == [{"t": 3.0, "name": "Clean"}, {"t": 12.5, "name": "Drive"}] def test_sloppak_tone_changes_skips_malformed_markers(): - _, changes = sloppak_tone_changes({ + _, _, changes = sloppak_tone_changes({ "changes": [ {"t": "nan", "name": "BadStr"}, {"t": float("inf"), "name": "Inf"}, @@ -29,18 +30,66 @@ def test_sloppak_tone_changes_skips_malformed_markers(): def test_sloppak_tone_changes_handles_none_and_bad_base(): - assert sloppak_tone_changes(None) == ("", []) - base, changes = sloppak_tone_changes({"base": 123, "changes": []}) - assert base == "" and changes == [] + assert sloppak_tone_changes(None) == ("", "", []) + base, base_rig, changes = sloppak_tone_changes({"base": 123, "changes": []}) + assert base == "" and base_rig == "" and changes == [] def test_sloppak_tone_changes_non_dict_input(): """A truthy non-dict payload must not crash.""" - assert sloppak_tone_changes(["not", "a", "dict"]) == ("", []) - assert sloppak_tone_changes("nope") == ("", []) + assert sloppak_tone_changes(["not", "a", "dict"]) == ("", "", []) + assert sloppak_tone_changes("nope") == ("", "", []) def test_sloppak_tone_changes_non_list_changes(): """A truthy non-list `changes` value must not raise on iteration.""" - base, changes = sloppak_tone_changes({"base": "Clean", "changes": 1}) + base, _, changes = sloppak_tone_changes({"base": "Clean", "changes": 1}) assert base == "Clean" and changes == [] + + +# ── rig bindings (feedpak-spec 1.18.0 §6.9) ────────────────────────────────── + +def test_sloppak_tone_changes_carries_rig_bindings(): + """`base_rig` and per-change `rig` reach the wire — the binding a chart + declares is what core must hand the consumer that voices the part.""" + base, base_rig, changes = sloppak_tone_changes({ + "base": "Clean Rhythm", + "base_rig": "clean-rhythm", + "changes": [ + {"t": 12.5, "name": "Lead Drive", "rig": "lead-drive"}, + {"t": 48.0, "name": "Clean Rhythm", "rig": "clean-rhythm"}, + ], + }) + assert base == "Clean Rhythm" + assert base_rig == "clean-rhythm" + assert changes == [ + {"t": 12.5, "name": "Lead Drive", "rig": "lead-drive"}, + {"t": 48.0, "name": "Clean Rhythm", "rig": "clean-rhythm"}, + ] + + +def test_sloppak_tone_changes_omits_unusable_rig_ids(): + """A non-string or blank `rig` is dropped rather than forwarded, so a + consumer can treat presence of the key as "this change binds a rig".""" + _, base_rig, changes = sloppak_tone_changes({ + "base_rig": " ", + "changes": [ + {"t": 1.0, "name": "A", "rig": 7}, + {"t": 2.0, "name": "B", "rig": ""}, + {"t": 3.0, "name": "C", "rig": None}, + {"t": 4.0, "name": "D", "rig": " padded-id "}, + ], + }) + assert base_rig == "" + assert changes == [ + {"t": 1.0, "name": "A"}, + {"t": 2.0, "name": "B"}, + {"t": 3.0, "name": "C"}, + {"t": 4.0, "name": "D", "rig": "padded-id"}, + ] + + +def test_sloppak_tone_changes_non_string_base_rig(): + """A non-string `base_rig` must not crash or leak a non-id onto the wire.""" + _, base_rig, _ = sloppak_tone_changes({"base": "Clean", "base_rig": 42}) + assert base_rig == ""