Skip to content

feat(player): drum-part picker for multiple drum charts (feedpak 1.17.0)#1021

Closed
ChrisBeWithYou wants to merge 4 commits into
feat/drum-parts-loaderfrom
feat/drum-part-picker
Closed

feat(player): drum-part picker for multiple drum charts (feedpak 1.17.0)#1021
ChrisBeWithYou wants to merge 4 commits into
feat/drum-parts-loaderfrom
feat/drum-part-picker

Conversation

@ChrisBeWithYou

Copy link
Copy Markdown
Contributor

The last mile of the multiple-drum-parts feature — stacked on #1020 (base feat/drum-parts-loader). #1020 taught the loader + highway WS to carry several drum charts (song_info.drum_parts, ?drum_part=<id>, a part_id echo on drum_tab); this adds the host-chrome selector that lets a player choose which drummer plays.

What

A Drum part <select> sits beside the arrangement switcher (advanced settings), shown only when a song has 2+ drum charts. Selecting one re-streams that part's tab over the highway WS — exactly like an arrangement switch. Single-drum / no-drum songs hide the row (nothing changes for them).

  • static/highway.jsreconnect() gains a drumPart arg → ?drum_part=<id> on the WS URL (mirrors the arrangement param); the song_info handler populates #drum-part-select + shows/hides the row on length > 1 (parallel to the #arr-select block); the drum_tab handler carries msg.part_id onto hwState.drumTab (plugins can read bundle.drumTab.part_id) and reflects it as the picker's selected value (stays honest when the server falls an unknown selection back to the primary).
  • static/app.jschangeArrangement() gains an optional drumPart (forwarded, else the current picker selection is preserved so an arrangement switch keeps the chosen part — parts are song-level); new changeDrumPart(id) delegates to it (a part switch is the same re-stream, reusing the transition ceremony). Exported on window.
  • static/v3/index.html — the #drum-part-select row (hidden by default).

No plugin change — the drum renderers just draw whatever drum_tab streams. Since the WS reconnect is host-owned, the picker lives in host chrome (like the arrangement switcher), so it works regardless of which drum renderer is active.

Verification

  • Runtime-verified, 10/10 (Playwright, the core player, a real 2-drum pack + a no-drum pack): the picker populates with both parts and shows for the multi-drum song; song_info.drum_parts reaches getSongInfo(); the primary is pre-selected; selecting the 2nd part drives highway.reconnect with the id and the WS URL carries ?drum_part=drums-2; the picker then reflects the server's part_id echo; a no-drum song hides the row; no page errors.
  • ESLint 0 errors (2 pre-existing max-lines warnings on these already-large files). JS-only; no pytest touched.

⚠️ Stacked on #1020 (feat/drum-parts-loader) — merge that first; CI is main-only until this retargets. And the editor + spec sides of the arc (got-feedBack/feedpak-spec#63, got-feedBack/feedBack-plugin-editor #335#340) complete the authoring end.

🤖 Generated with Claude Code

The last mile of the multiple-drum-parts feature: let a player CHOOSE which
drum chart plays. #1020 taught the loader + highway WS to carry several drum
parts (song_info.drum_parts + ?drum_part=<id> + a part_id echo on drum_tab);
this adds the host-chrome selector that drives it.

A "Drum part" <select> sits beside the arrangement switcher in the advanced
settings popover, shown only when a song has 2+ drum charts (drum_parts is
always present — empty for non-drum songs — so single-drum / no-drum songs
hide the row and nothing changes for them). Selecting a part re-streams that
part's tab over the highway WS, exactly like an arrangement switch.

- static/highway.js:
  - reconnect() gains a third `drumPart` arg → sets `?drum_part=<id>` on the WS
    URL (mirrors the existing `arrangement` param one line up). Empty/undefined
    → the primary part, i.e. byte-identical to today for any pack untouched.
  - song_info handler populates #drum-part-select from msg.drum_parts and
    shows/hides #v3-drum-part-row on `length > 1` (parallel to the #arr-select
    block right above it).
  - drum_tab handler carries msg.part_id onto hwState.drumTab (plugins can read
    bundle.drumTab.part_id) and reflects it as the picker's selected value, so
    the dropdown stays honest even when the server resolves an unknown/absent
    selection to the primary.
- static/app.js:
  - changeArrangement() gains an optional `drumPart`; at reconnect it forwards
    the explicit part, else preserves the current picker selection — so an
    ARRANGEMENT switch keeps the chosen drum part (parts are song-level).
  - new changeDrumPart(id) delegates to changeArrangement with the current
    arrangement held + the new part applied (a part switch is the same
    re-stream, so it reuses all the transition ceremony). Exported on window.
- static/v3/index.html: the #drum-part-select row (hidden by default).

No plugin change: the drum renderers just draw whatever drum_tab streams.

RUNTIME-VERIFIED (Playwright, the core player, a 2-drum pack + a no-drum pack):
10/10 — the picker populates with both parts and shows for the multi-drum song;
song_info.drum_parts reaches getSongInfo(); the primary is pre-selected;
selecting the 2nd part drives highway.reconnect with the id and the WS URL
carries `?drum_part=drums-2`; the picker then reflects the server's part_id
echo; a no-drum song hides the row; no page errors. ESLint 0 errors (the two
max-lines warnings are pre-existing on these files). No pytest touched (JS-only).
Stacked on #1020.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix
Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com>
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5cabf4f6-8444-489f-8a7b-2fdaa6216fe7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/drum-part-picker

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ChrisBeWithYou

Copy link
Copy Markdown
Contributor Author

Auto-closed, not landed. This PR was closed automatically when its base branch (feat/drum-parts-loader / #1020) was squash-merged to main this morning — the picker never actually landed. Verified: drum-part-select / changeDrumPart / drum_part are all absent from main, even though #1020's loader is in.

Re-landed unchanged as #1028 (cherry-picked onto current main, runtime-verification carried over). Please review there — this thread is dead.


Reconciliation with #1017 (for review): the two are complementary, not redundant. This picker only renders for songs with 2+ drum charts, so a single drum chart bundled with a melodic instrument (e.g. drums + bass) gets no picker row — and, without #1017's broadened placeholder, no way to reach the drums at all, because #1020 kept the placeholder trigger drum-only. So #1017 fills the gap #1020/#1021 leave open. It merges clean on the new main, but should pick up a CHANGELOG entry + a loader test before it lands.

byrongamatos pushed a commit that referenced this pull request Jul 21, 2026
…1021) (#1028)

* feat(player): drum-part picker for multiple drum charts (feedpak 1.17.0)

The last mile of the multiple-drum-parts feature: let a player CHOOSE which
drum chart plays. #1020 taught the loader + highway WS to carry several drum
parts (song_info.drum_parts + ?drum_part=<id> + a part_id echo on drum_tab);
this adds the host-chrome selector that drives it.

A "Drum part" <select> sits beside the arrangement switcher in the advanced
settings popover, shown only when a song has 2+ drum charts (drum_parts is
always present — empty for non-drum songs — so single-drum / no-drum songs
hide the row and nothing changes for them). Selecting a part re-streams that
part's tab over the highway WS, exactly like an arrangement switch.

- static/highway.js:
  - reconnect() gains a third `drumPart` arg → sets `?drum_part=<id>` on the WS
    URL (mirrors the existing `arrangement` param one line up). Empty/undefined
    → the primary part, i.e. byte-identical to today for any pack untouched.
  - song_info handler populates #drum-part-select from msg.drum_parts and
    shows/hides #v3-drum-part-row on `length > 1` (parallel to the #arr-select
    block right above it).
  - drum_tab handler carries msg.part_id onto hwState.drumTab (plugins can read
    bundle.drumTab.part_id) and reflects it as the picker's selected value, so
    the dropdown stays honest even when the server resolves an unknown/absent
    selection to the primary.
- static/app.js:
  - changeArrangement() gains an optional `drumPart`; at reconnect it forwards
    the explicit part, else preserves the current picker selection — so an
    ARRANGEMENT switch keeps the chosen drum part (parts are song-level).
  - new changeDrumPart(id) delegates to changeArrangement with the current
    arrangement held + the new part applied (a part switch is the same
    re-stream, so it reuses all the transition ceremony). Exported on window.
- static/v3/index.html: the #drum-part-select row (hidden by default).

No plugin change: the drum renderers just draw whatever drum_tab streams.

RUNTIME-VERIFIED (Playwright, the core player, a 2-drum pack + a no-drum pack):
10/10 — the picker populates with both parts and shows for the multi-drum song;
song_info.drum_parts reaches getSongInfo(); the primary is pre-selected;
selecting the 2nd part drives highway.reconnect with the id and the WS URL
carries `?drum_part=drums-2`; the picker then reflects the server's part_id
echo; a no-drum song hides the row; no page errors. ESLint 0 errors (the two
max-lines warnings are pre-existing on these files). No pytest touched (JS-only).
Stacked on #1020.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017xGPjDBF8NTwTK7VQvizix
Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com>

* Update reconnect source contract test

---------

Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants