-
Notifications
You must be signed in to change notification settings - Fork 25
feat: instruments as plugins — extract hardcoded instrument knowledge into plugin-based registry #997
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
j0b333
wants to merge
76
commits into
got-feedBack:main
Choose a base branch
from
j0b333:feat/instruments-as-plugins
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
feat: instruments as plugins — extract hardcoded instrument knowledge into plugin-based registry #997
Changes from all commits
Commits
Show all changes
76 commits
Select commit
Hold shift + click to select a range
1a5b49f
feat: extract instruments from hardcoded logic into plugins
j0b333 4a13a70
fix: rename Standard tuning to E Standard
j0b333 d4ca98c
fix: properly rename Standard to E Standard for 6-string only
j0b333 2e0542e
fix: name standard tunings properly for all string counts
j0b333 161a16a
fix: route utility functions through registry rather than hardcoded data
j0b333 4cf5b54
fix: tuning pill updates immediately; remember last tuning per instru…
j0b333 f119d0d
feat: role selector in instrument card, used for arrangement routing
j0b333 9c9e093
feat: show tuning name on tuner card, role on instrument pill
j0b333 bf44806
fix: role row above strings, pill shows role not tuning, tuner no tru…
j0b333 1ab3aac
fix: remove orphaned : '' from old role selector position that broke …
j0b333 50007ad
fix: show role on pill for all instruments; tuner text matches pill size
j0b333 ad15233
fix: abbreviate 'Standard' as 'Std' on tuner badge
j0b333 17f2f28
feat: per-role mastery %, hover overlay with all arrangement accuraci…
j0b333 7c9d04f
fix: use bg-black/75 (in prebuilt CSS), show hover overlay for all so…
j0b333 0c82b45
fix: badge shows for unplayed songs, inline styles for hover overlay,…
j0b333 1a2e538
fix: remove instrument abbreviation from badge, keep just the %
j0b333 b64afa4
fix: SQL syntax error in arrangement_accuracy_map when _existing_song…
j0b333 81b8563
temp: disable hover overlay to check if it interferes with audio preview
j0b333 b2a1322
fix: SQL error in arrangement_accuracy_map, re-enable hover overlay
j0b333 b524971
feat: hover overlay shows role - tuning - %, hides badge/fmt/tuning o…
j0b333 a1f99ff
ui: tuning chip in top-left corner, mastery badge with colored border…
j0b333 f358c2e
fix: omit tuning from hover overlay for non-pitched instruments (drum…
j0b333 fa412be
fix: remove double AND in arrangement_accuracy_map SQL
j0b333 ad0ea79
fix: don't strip existing_song_filter - it starts with a space
j0b333 fac28d1
fix: per-role mastery uses active role, not first matching arrangement
j0b333 c00002b
fix: stats-recorder reads resolved arrangement from song:ready
j0b333 d317259
feat: preferred highway per instrument in settings, applied on instru…
j0b333 01edf33
debug: add console.error logging to instruments-settings load function
j0b333 4c2a3b1
debug: add sync test panel update, console.log in load()
j0b333 c789e37
debug: verbose logging in load(), safe wrapper for highway select
j0b333 8a5cdf7
fix: highway wiring was inserted inside _renderTunings — moved to _wi…
j0b333 6d02cd7
fix: remove orphaned closing brace left from misplaced highway code
j0b333 7429e37
fix: restore instruments-settings.js from working version, re-add hig…
j0b333 e564286
fix: preferred highway now updates DOM picker, dropdown includes venu…
j0b333 e090f96
fix: call window.setViz() directly instead of just updating picker DOM
j0b333 3a6a45f
fix: song:ready applies non-auto viz, preferred highway sets picker v…
j0b333 0cdfe11
fix: don't fall back to song-wide max % when auto-filter is active
j0b333 a1d9c84
fix: restore missing }); closing for song:ready handler
j0b333 29c5ad0
fix: show 'n/a' instead of '— %' for no-matching-arrangement badge
j0b333 420d807
fix: show 'n/a' when no matching arrangement, '— %' when unplayed
j0b333 3a3ecc2
fix: expose active instrument profile on initial loadSettings too
j0b333 eac9f8e
fix: return null from _matchingArrangementIndex when active role has …
j0b333 082db90
fix: expand arrangement filter allowlist with registry-derived names
j0b333 3e6bc31
docs: add module/class docstrings, fix file headers to match conventions
j0b333 4ec49c1
docs: add why-comments to all changed sections across the codebase
j0b333 ff7afd2
docs: add instruments-as-plugins to CHANGELOG
j0b333 322f725
fix: use arr_has.length instead of _arrAutoInstrument for 'n/a' detec…
j0b333 7f49e7e
fix: show 'n/a' when filter is inactive but song lacks selected role'…
j0b333 d0e7ab7
fix: smart-mode filter uses simple name matching for non-standard arr…
j0b333 cd3d803
fix: remove song-wide max fallback — never attribute wrong role's score
j0b333 97f3516
feat: add 'Auto-filter by instrument' toggle in Gameplay settings
j0b333 4249b08
fix: remove unused Default arrangement setting, fix orphaned HTML clo…
j0b333 2f6cd82
debug: add console.log to instrument:changed handler
j0b333 40b3f01
debug: add console.log to initial render auto-filter
j0b333 180633b
fix: always read auto-filter setting, not just on first render
j0b333 3fb69d8
fix: read auto-filter setting in onV3SongsScreenEnter, not render()
j0b333 edeaf4d
docs: add docstrings to all new/modified functions to meet 80% coverage
j0b333 38c4624
docs: add docstrings to our new/modified functions across all changed…
j0b333 316d733
merge: integrate upstream/main (drum parts, tuning perspectives, bass…
j0b333 3f98715
fix: tuning chip flush top-left, hover fade, and reliable offset dete…
j0b333 f68f7da
fix: missing renderDrawerIfOpen, bass chip inferred offset truncation
j0b333 131498f
fix: restore name-based tuning offset fallbacks for older songs
j0b333 c5428f2
fix: shownTuningOffsets gate on offsets not name; chipIsBass always t…
j0b333 f59b1e5
fix: registry-driven instrument chip logic, prevent double-reload race
j0b333 5f95437
fix: encoding corruption in instruments-settings.js, add pick/fingere…
j0b333 f2d37ca
feat: add Vocals instrument plugin and fret_count for stringed instru…
j0b333 026a608
chore: add CC0 SVG source comment to instrument plugins, fix duplicat…
j0b333 486e350
Revert "chore: add CC0 SVG source comment to instrument plugins, fix …
j0b333 1657dec
docs: add instrument-plugins.md reference, fix duplicate bass icon
j0b333 e0946e3
docs: update changelog with vocals plugin, fret count, bass pick styl…
j0b333 e997d27
chore: rename instrument_piano to instrument_keys, drop instrument_vo…
j0b333 6f0ebc1
feat: add arrangement_types key alongside arrangement_names in instru…
j0b333 2b95f4c
feat: display arrangement_types in settings role cards (non-editable)
j0b333 5a6772a
chore: normalize indentation in songs.js (no functional change)
j0b333 dd1e183
feat: instrument-aware library — per-role mastery, hover overlay, tun…
j0b333 dd927fa
Merge pull request #2 from j0b333/fix/songs-proper
j0b333 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,181 @@ | ||
| # Instrument Plugins | ||
|
|
||
| Instruments in fee[dB]ack are defined by bundled **instrument plugins** — each is a | ||
| small `plugins/instrument_<name>/` directory with a `plugin.json` manifest and an | ||
| SVG icon. They declare the instrument's tuning presets, string/key counts, | ||
| arrangement role mappings, and detection strategy, replacing hardcoded checks | ||
| across the codebase. | ||
|
|
||
| ## Interface | ||
|
|
||
| Each instrument plugin sets `"type": "instrument"` in its manifest and carries an | ||
| `"instrument"` block with the following schema: | ||
|
|
||
| ```jsonc | ||
| { | ||
| "id": "instrument_guitar", // unique plugin id (snake_case, prefixed "instrument_") | ||
| "name": "Guitar", // human-readable name | ||
| "version": "1.0.0", | ||
| "type": "instrument", // ⬅ required | ||
| "bundled": true, // a bundled plugin (not user-installed) | ||
| "icon": "assets/icon.svg", // path relative to the plugin directory | ||
| "instrument": { | ||
| "id": "guitar", // instrument id (snake_case, used in settings & profiles) | ||
| "label": "Guitar", // display label for the instrument selector | ||
| "kind": "stringed", // "stringed" | "keyboard" | "percussion" | "vocal" | "custom" | ||
|
|
||
| // ── Stringed instruments ────────────────────────────────────────────── | ||
| "string_counts": [6, 7, 8], // (required for stringed) array of ints | ||
| "default_string_count": 6, // (required for stringed) must be in string_counts | ||
| "fret_counts": [21, 22, 24], // (required for stringed) array of ints | ||
| "default_fret_count": 22, // (required for stringed) must be in fret_counts | ||
| "reference_pitch": 440.0, // A4 reference (default 440) | ||
| "standard_tunings": { // (required for stringed) open MIDI pitches per string count | ||
| "6": [40, 45, 50, 55, 59, 64], | ||
| "7": [35, 40, 45, 50, 55, 59, 64], | ||
| "8": [30, 35, 40, 45, 50, 55, 59, 64] | ||
| }, | ||
| "tunings": { // (required for stringed) named tuning presets per string count | ||
| "6": { | ||
| "E Standard": [ 0, 0, 0, 0, 0, 0], | ||
| "Drop D": [-2, 0, 0, 0, 0, 0], | ||
| "Eb Standard": [-1, -1, -1, -1, -1, -1] | ||
| } | ||
| }, | ||
|
|
||
| // ── Keyboard instruments ─────────────────────────────────────────────── | ||
| "key_counts": [25, 49, 61, 88], // (required for keyboard) array of ints | ||
| "default_key_count": 88, // (required for keyboard) must be in key_counts | ||
|
|
||
| // ── All instruments ──────────────────────────────────────────────────── | ||
| "detect_strategy": "pitch", // "pitch" (uses tuner) | "onset" (drums) | null | ||
| "roles": [ // one or more arrangement roles | ||
| { | ||
| "id": "lead", // role id (snake_case), used in profile ids | ||
| "label": "Lead", // display label for the role selector | ||
| "arrangement_flags": ["path_lead"], // XML flags that identify this role | ||
| "arrangement_types": ["lead"], // spec `type` values that identify this role (normative, exact match — preferred over names) | ||
| "arrangement_names": ["Lead", "Lead Guitar"], // display names that identify this role (fuzzy/fallback) | ||
| "default": true // the default role for this instrument | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### `kind` values | ||
|
|
||
| | Kind | Example | Has tunings? | Has string counts? | Notes | | ||
| |------|---------|:---:|:---:|-------| | ||
| | `"stringed"` | guitar, bass | Yes | Yes | Shows tunings, frets, handedness in the selector | | ||
| | `"keyboard"` | piano, keys | Yes | No (key counts instead) | Shows key count selector | | ||
| | `"percussion"` | drums | No | No | No tuner, hit detection | | ||
| | `"vocal"` | vocals | No | No | Pitched, uses tuner, no instrument-specific controls | | ||
| | `"custom"` | any future type | Depends | Depends | Free-form | | ||
|
|
||
| ### Role mapping | ||
|
|
||
| When a player selects a role (e.g. Guitar → Rhythm), the highway opens the | ||
| arrangement matching that role. Roles match against: | ||
|
|
||
| 1. **Arrangement types** — spec `type` values (e.g. `"bass"`, `"drums"`, `"lead"`). | ||
| Matched exactly against the arrangement's `type` field. This is the primary, | ||
| normative routing key and is stable across display-name changes. | ||
| 2. **Arrangement flags** — XML `<arrangementProperties>` attributes (e.g. `path_bass`, `path_lead`). | ||
| Flagged arrangements match immediately, regardless of name. | ||
| 3. **Arrangement names** — matched case-insensitively against the arrangement's | ||
| smart name (from XML parsing) and raw name. List common GP/RS2014 naming | ||
| variants for each role to maximise coverage (e.g. `["Bass", "Pick Bass", "Fingered Bass"]`). | ||
|
|
||
| The library auto-filters to arrangements matching the current instrument's roles. | ||
|
|
||
| ## Creating a new instrument plugin | ||
|
|
||
| 1. Create `plugins/instrument_<id>/plugin.json` (copy the skeleton below). | ||
| 2. Create `plugins/instrument_<id>/assets/icon.svg` — use a | ||
| **[CC0-licensed SVG from SVGrepo's music instrument collection](https://www.svgrepo.com/collection/music-instrument)** (all free for any use, no attribution required). | ||
| 3. Fill in the `instrument` block with tunings, string/fret/key counts, and role | ||
| mappings for your instrument. | ||
| 4. Restart fee[dB]ack — the loader discovers the plugin automatically. | ||
|
|
||
| ### Minimal skeleton | ||
|
|
||
| ```jsonc | ||
| { | ||
| "id": "instrument_<id>", | ||
| "name": "<Display Name>", | ||
| "version": "1.0.0", | ||
| "type": "instrument", | ||
| "bundled": true, | ||
| "icon": "assets/icon.svg", | ||
| "instrument": { | ||
| "id": "<id>", | ||
| "label": "<Display Name>", | ||
| "kind": "stringed", | ||
| "string_counts": [6], | ||
| "default_string_count": 6, | ||
| "fret_counts": [22, 24], | ||
| "default_fret_count": 22, | ||
| "reference_pitch": 440.0, | ||
| "standard_tunings": { | ||
| "6": [40, 45, 50, 55, 59, 64] | ||
| }, | ||
| "tunings": { | ||
| "6": { "E Standard": [0, 0, 0, 0, 0, 0] } | ||
| }, | ||
| "detect_strategy": "pitch", | ||
| "roles": [ | ||
| { | ||
| "id": "<role>", | ||
| "label": "<Role Label>", | ||
| "arrangement_names": ["<Arr Name>"], | ||
| "default": true | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### Non-stringed skeleton | ||
|
|
||
| ```jsonc | ||
| { | ||
| "id": "instrument_<id>", | ||
| "name": "<Display Name>", | ||
| "version": "1.0.0", | ||
| "type": "instrument", | ||
| "bundled": true, | ||
| "icon": "assets/icon.svg", | ||
| "instrument": { | ||
| "id": "<id>", | ||
| "label": "<Display Name>", | ||
| "kind": "vocal", | ||
| "string_counts": [], | ||
| "default_string_count": 0, | ||
| "key_counts": [], | ||
| "default_key_count": 0, | ||
| "reference_pitch": 440.0, | ||
| "detect_strategy": "pitch", | ||
| "roles": [ | ||
| { | ||
| "id": "<role>", | ||
| "label": "<Role Label>", | ||
| "arrangement_names": ["<Arr Name>"], | ||
| "default": true | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ## Bundled instruments | ||
|
|
||
| These ship with fee[dB]ack and are in the `plugins/` directory tree by default: | ||
|
|
||
| | Plugin directory | Instrument id | Kind | | ||
| |------------------|:---:|---| | ||
| | `instrument_guitar` | `guitar` | stringed | | ||
| | `instrument_bass` | `bass` | stringed | | ||
| | `instrument_drums` | `drums` | percussion | | ||
| | `instrument_keys` | `keys` | keyboard | | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Do not mark community-plugin templates as bundled.
The guide defines
bundled: trueas “not user-installed,” but both new-plugin templates set it totrue. Omit it or set it tofalsefor community plugins.Proposed fix
Also applies to: 137-144
🤖 Prompt for AI Agents