The core rig reader landed its reading half but not its exposing half:
the bindings shipped (`base_rig`, per-change `rig`) while the library
they are references INTO stayed server-side on LoadedSloppak.rigs with
no consumer at all. A client could see that a change binds
"grand-piano" and had no way to learn what that was.
Add a `rigs` WS message carrying the library whole and verbatim, and put
each drum part's resolved `tones` binding on its `song_info.drum_parts`
entry — the drum half had the same gap, since the loader resolved the
binding in P3 and the wire payload was still `{id, name}`.
Sent whenever the pack ships a library, deliberately NOT gated on this
arrangement having tone changes: a pack can bind sound to its drum parts
alone via `drum_tones` and still need the library.
Verbatim matters here. Core selects no realization and applies no
`intent.gm` floor, so the consumer that voices the part needs the whole
block to make that choice — including the unknown `role`/`engine`/`kind`
values and `ext` namespaces §7.9 requires a Reader to preserve.
Client side: `hwState.rigs` / `toneBaseRig`, `getRigs()` /
`getToneBaseRig()`, and `bundle.rigs` / `bundle.toneBaseRig` so
renderers reach them the same way they reach every other chart array,
with both reset paths clearing them between songs.
Packs that bind no rig are byte-identical on the wire — no `rigs`
message, no `base_rig`, no `rig`, no `tones` on a drum part.
Signed-off-by: gionnibgud <gionnibgud@gmail.com>
* Carry rig bindings through the sloppak tone payload
`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 <gionnibgud@gmail.com>
* Load the pack's rig library from the manifest
feedpak 1.18.0 lets a chart declare what a MIDI part should sound like
by binding a rig id, but core had nothing to bind to: `rigs`, `base_rig`
and `drum_tones` appeared nowhere in lib/, server.py or static/. The
preceding commit carries the reference onto the wire; this adds the
library it references.
Read the manifest `rigs:` key into a new `LoadedSloppak.rigs`, alongside
the other side-files rather than on Song — every side-file (drum_tab,
song_timeline, keys, notation) hangs off the load result, and rigs is
pack-level, not per-arrangement. Same permissive posture as its
neighbours: missing, unreadable, malformed or traversing disables rigs
with a warning and never fails the pack, which §7.9 requires outright.
Rig objects pass through VERBATIM. §7.9 obliges a Reader to preserve
unknown role/engine/kind values and `ext` namespaces, so validating
block structure here would be wrong as well as premature — realization
selection and the `intent.gm` floor belong to whatever voices the part.
The only entries dropped are ones unreachable by construction: a rig is
addressable solely by `id`, so a non-dict entry or one without a usable
string id can never be referenced. Ids are stripped to match the
reference side, and a duplicate id resolves first-wins with a warning,
since ambiguity there would surface as the wrong sound rather than an
error.
Signed-off-by: gionnibgud <gionnibgud@gmail.com>
* Resolve which tones block binds a part
feedpak 1.18.0 lets a sound binding arrive from three places, and core
honoured none of them: the manifest arrangement entry, the arrangement
JSON, and the top-level drum_tones. Reading them needs a precedence
rule, because two of the three can be present at once.
Arrangement entries: the entry's `tones` replaces the arrangement JSON's
WHOLESALE (spec 5.2), unlike name/tuning/capo/centOffset beside it,
which override field by field. A merge would produce a sound nobody
authored -- one source's base under the other's changes -- which is
worse than either block alone. This is also what makes a notation-only
keys entry bindable at all, since it has no arrangement JSON to carry
tones in the first place.
Drums: the top-level drum_tones binds the song-level primary part, and
a `type: drums` entry's own tones takes precedence, with a Reader
forbidden from applying both to the same part (5.1). That is the same
shape as the drum_tab alias rule, so it lives inside
_resolve_drum_parts next to it rather than beside it -- one precedence
resolver, not two that drift. drum_tones is the PRIMARY's fallback
only: a second drummer with no binding gets None, never the primary's
kit.
An empty `tones: {}` reads as absent rather than as an override to
silence, matching how arrangement_from_wire already normalizes the
in-JSON empty dict, so a stray empty object cannot quietly unbind a
part.
Spec-conformance gate passes with drum_tones added to the keys core
reads (22 of the spec's 32, all declared).
Signed-off-by: gionnibgud <gionnibgud@gmail.com>
* Document the rig bindings on the tone_changes wire message
CHANGELOG entry for the core rig reader, plus the WS protocol table in
CLAUDE.md, which described `tone_changes` as carrying only base + name.
While in that row: its time key was documented as `time`, but every
producer emits `t` — both the sloppak builder and the legacy XML path.
The 3D highway already carries a comment warning readers about exactly
this discrepancy. Corrected here rather than left sitting next to the
newly-added keys, where a reader would reasonably assume both were
equally reliable.
Signed-off-by: gionnibgud <gionnibgud@gmail.com>
---------
Signed-off-by: gionnibgud <gionnibgud@gmail.com>
Design change, at the maintainer's direction: the gate now checks out
feedpak-spec at HEAD instead of a pinned SHA. .feedpak-spec-ref, its
40-char validation step, and the pin-bump machinery are gone.
Rationale: it is vital that the app conforms to the spec — the current
spec, not a snapshot. The pin bought determinism at the cost of a
maintenance loop (bump PRs, a PAT, weekly latency) and a window where the
gate verified against a stale spec. Tracking HEAD makes the dev flow fully
self-serve with zero upkeep: gated PR -> FEP -> spec merge -> re-run
checks -> green. Nothing to bump.
The trade-off is accepted with eyes open, and the docs state it: the
normal FEP is additive and can only loosen the gate, so it cannot redden
anyone's PR. Only a breaking spec change (rare, deliberate, MAJOR per the
spec's compatibility policy) turns PRs red repo-wide — which is the
correct org-wide signal that the app is out of conformance. The CI job
logs the spec SHA each run verified against, so any red run is
reproducible.
Failure messages now also say why it matters beyond the one PR (also at
the maintainer's direction): non-conformance that lands shows up as red CI
on every teammate's PR until it is resolved, and only its author can clear
it — the FEP route keeps everyone else unblocked. Tone softened throughout
(the exceptions-file header now explains rather than shouts).
Signed-off-by: topkoa <topkoa@gmail.com>
The CI gate catches spec drift at merge time; these two additions catch it
at write time, which is where "developer didn't read the spec first"
actually happens.
CLAUDE.md (Song Formats): a spec-is-sacrosanct paragraph next to the spec
pointer — the spec defines the format, the app implements it, any new
manifest key/file/directory lands in the spec first via the FEP process,
and the gate has no in-repo bypass. AI agents and contributors both hit
this while writing feedpak-touching code, not after CI reddens.
.github/pull_request_template.md (new — the repo had only issue templates):
a feedpak-surface section requiring either "doesn't touch pack I/O" or a
link to the landed FEP + the .feedpak-spec-ref bump, plus the standing
changelog/tests/DCO checklist.
Signed-off-by: topkoa <topkoa@gmail.com>
Every rule here is something that has already gone wrong on this feature —
mostly in core's own code, twice in the two plugins that adopted it first.
They are cheap to get right up front and miserable to diagnose later,
because a broken pane almost always LOOKS perfect.
The traps, and why each one is easy to walk into:
- Your code still runs in the main window. That is exactly why moving the
element works at all — and exactly why `document.body.appendChild(tooltip)`
inside a popped-out panel lands in the window the user is NOT looking at.
- Don't hide your own panel when it pops out. Core hides it and leaves a
stub. A plugin that also hides it hides the node that just moved — which is
precisely how core's own chip shipped a blank pop-out window.
- Use `hidden` or a class, not inline `display`, for show/hide. `.fb-paned`
forces the panel visible while it is out; when it docks and that class is
removed, an inline `display:none` reasserts itself and the panel returns
invisible.
- `isConnected` does not mean "docked". A panel in a pane window IS connected,
just not to this document. The test you meant is
`el.ownerDocument === document`.
- `element` is a function so it can be resolved late: return the LIVE node, and
re-attach the chip if you rebuild your panel (Camera Director rebuilds on
every mode change).
- rAF is throttled while the main window is backgrounded — which it is, whenever
the user is looking at your pane. Event-driven panels don't care; continuously
animating ones will stutter exactly when they are the only thing on screen.
- Don't synchronise anything. One realm, one panel. Writing sync code means
you have misunderstood the model.
Also states what core guarantees back, including the one that cost the most to
learn: the element is evacuated BEFORE the pane window's document is destroyed,
so it comes home alive rather than as a photograph of a panel with every
listener in its subtree silently gone.
Signed-off-by: topkoa <topkoa@gmail.com>
* refactor(ui)!: remove the classic v2 shell — v3 is the only UI (R3a)
Deletes `static/index.html`, the `/v2` route, and the `FEEDBACK_UI` v2/legacy
opt-out. `/` and `/v3` both serve `static/v3/index.html`, which has been the
default since 0.3.0.
This is step 0 of the core-frontend ES-module migration (R3a). Both shells load
the same `static/app.js`, so every later step of that migration — exposing the
window contract, the `defer` ordering fix, the `type="module"` flips — would
otherwise have to be made and verified twice. Removing the fallback now halves
that surface before any of it is touched.
Incidentally fixes a latent bug in `index()`: its guard read
`if getenv_compat("FEEDBACK_UI") or getenv_compat("FEEDBACK_UI") in ("v2", "legacy")`,
whose left operand is truthy for *any* non-empty value — so `FEEDBACK_UI=v3`
actually served the **v2** shell.
- `static/tailwind.min.css` regenerated: the content globs scanned the deleted
file, so v2-only utility classes are now purged (CI's tailwind-fresh job
rebuilds and diffs it).
- Constitution amended to 1.3.0 — Principle II's frontend file list now names
`static/v3/index.html`.
- Tests: 4 suites read the v2 shell (3 via a constructed `path.join` that a
literal grep misses). Their v2 halves are paired duplicates of v3 tests that
stay, so they are dropped; `alpha_warning_banner` and the capability-registry
script-order test retarget to `static/v3/index.html`.
BREAKING CHANGE: `FEEDBACK_UI=v2` / `=legacy` and the `/v2` route are gone.
Unset the variable and use `/`. No chart, settings, or plugin data changes, and
no plugin API changes — v3 reuses the same engine.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs: drop the stale '/ is v2' plugin-verification guidance (CodeRabbit)
The v3-only rewrite updated the intro paragraphs but left three lines that
still instructed plugin authors to verify in 'both / (v2) and /v3' — now the
same shell. Historical 'in v2 it was X' contrasts are kept: they still orient
authors whose plugins also ship to users on older cores.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Static-analysis follow-ups to the trace-backed fixes; each is cheap
insurance on machines where the profiled headroom doesn't exist.
- highway.js: _makeBundle now mutates one persistent per-instance
object instead of allocating a fresh ~35-field bundle every rAF
frame (xN under splitscreen). Object identity is stable and
meaningless; array fields still swap reference on chart changes,
which field-identity caches rely on. Contract documented in both
CLAUDE.mds.
- highway.js: new bsearchTime (lower-bound on .time) windows the
default 2D renderer's beat-line scan (was O(all beats) per frame);
bundle.lowerBoundT / bundle.lowerBoundTime expose the searches to
custom viz so they stop reimplementing visible-window culling.
- highway_3d: localStorage 'h3d_full_sus' polled at ~1 Hz instead of
every frame (synchronous storage read on the hot path).
- highway_3d: drawLyrics caches the measureText row layout keyed on
(lyrics ref, line index, shown count, font size, width) — per-frame
work is now just drawing over cached widths.
- tests/js: bundle source-shape assertions widened to accept the
assignment form ([:=]) alongside the old object-literal form.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(v3): reject accidental text-selection of UI chrome (user-select policy)
Dragging/double-clicking across the v3 UI marquee-highlighted buttons, labels,
the sidebar, transport, and the note-highway HUD — looks broken (reported Mac +
Windows). Default the v3 shell to user-select:none on html, then opt CONTENT
back in. Decided by a 4-lens panel (UX / a11y / dev-ops / plugin-ecosystem);
their guardrails are baked in:
- Form fields ALWAYS re-enabled (input/textarea/select/[contenteditable]) so the
caret + IME composition never break. No `* { user-select:none }` (WebKit input
bug 82692).
- Plugin screens (.screen[id^="plugin-"]) stay selectable BY INHERITANCE (no `*`,
so a plugin's own non-select chrome still wins) — a plugin's copyable text
(lyrics, chords, results), including community/out-of-tree plugins that never
adopt the class, isn't silently locked.
- Core read-only content opts back in by CONTAINER via a hand-authored
`.fb-selectable` (not a Tailwind utility — so runtime-installed plugins get it
too): the whole Settings panel (paths, device names, version, diagnostics,
About) and the now-playing song metadata. Answers the open "keep settings
copyable?" question: yes, at the container.
Cosmetic only — never used to lock copy-worthy text (errors/IDs/paths/versions/
metadata stay selectable; WCAG 2.2 allows copy-paste as a mechanism). v3-only
(v2 unchanged; v3.css loads only on /v3); plain CSS, no Tailwind rebuild; no
desktop/Electron changes (standard OS-framed window). `.fb-selectable` is
documented in CLAUDE.md for plugin authors.
Tests: tests/js/v3_user_select_policy.test.js (html default, form-field
re-enable, plugin-screen carve without `*`, .fb-selectable, container opt-ins,
and the no-`*`-rule guardrail).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF
* fix(v3): address review of the user-select policy (#637)
Review (manual + Codex) of the v3 text-selection policy:
- P1 (real bug): the now-playing HUD metadata opted into `.fb-selectable` but
its `#player-hud` parent is `pointer-events: none`, so the mouse could never
reach the text to select it — the opt-in was inert. Add `pointer-events-auto`
to the metadata block (verified in-browser: user-select:text + pointer-
events:auto, while the HUD parent stays pointer-events:none).
- Coverage: the PR's a11y guardrail promised copyable text stays selectable
"incl. in modals/toasts", but only Settings + the HUD were opted in. Blanket-
opt the focused copyable surfaces back in by selector — `.feedBack-modal`,
`[role="dialog"]`, `#fb-notify-stack`, `#v3-fb-toast`, `#scan-banner` — so
errors / IDs / paths / file names in dialogs, toasts, and the scan banner stay
copyable. These are focused panels, not dense card lists, so re-enabling
selection there can't recreate the across-cards marquee mess.
(Deliberately NOT opting in the library grid / dashboard / profile card lists:
making dense card text selectable would reintroduce exactly that marquee mess
on a drag — copy song metadata from the now-playing HUD / Settings instead.)
- Test (P3): assert the selectable rule's selectors order-independently, cover
the new modal/toast/banner surfaces, and check the HUD block carries BOTH
fb-selectable and pointer-events-auto (class-order independent).
Verified in a real browser (chromium): html=none, sidebar chrome=none, input=
text, Settings=text, HUD meta=text+pointer-events:auto, dialog/modal=text.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
Cosmetic follow-up to #537 (doc-only).
- Virtuoso: README + CHANGELOG used lowercase
`got-feedback/feedback-plugin-virtuoso`; the canonical repo (like every
other got-feedback repo) is capital-B `feedBack-plugin-virtuoso`. Brought
it in line with the sibling rows.
- Community plugin references in CLAUDE.md, TODO.md, docs/, and the bundled
tuner README were over-renamed to `feedBack-*` by the rename, but those
repos are owned by community members who never renamed them
(topkoa/stems+notedetect, OmikronApex/tuner, masc0t/update-manager).
Restored their real `slopsmith-*` names. got-feedback's own `feedBack-*`
references in the same files are left untouched.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Update GitHub repo references from feedback* to feedBack*
* rename: slopsmith -> feedBack, byron -> got-feedBack
Renames across the entire codebase:
- slopsmith/Slopsmith/SLOPSMITH/SlopSmith -> feedBack/FeedBack/FEEDBACK/FeedBack
- byron/Byron/Byrongamatos -> got-feedBack/got-feedBack/got-feedBack
- /home/byron/ -> /opt/got-feedBack/
- byron@ougsoft.com -> hi@got-feedBack.org
- github.com/byrongamatos/ -> github.com/got-feedback/
- com.byron. -> com.got-feedback.
- SLOPSMITH_ env vars -> FEEDBACK_ with backward-compat fallback
- Protocol/storage strings migrated with read-old/write-new pattern
- window.slopsmith JS API -> window.feedBack (canonical) + backward-compat alias
Refs: #rename-slopsmith
* rename: complete regen against current main + fix backward-compat alias
Regenerated the slopsmith->feedBack / byron->got-feedBack rename on top of
current main (3 commits had landed since the branch: #572/#554/#574),
resolving the four content conflicts in favour of main's newer content
(autoplay/auto-exit, accuracy-badge, Virtuoso re-home, feedpak badge).
Completion fixes on top of the mechanical rename:
- Re-apply rename to post-branch content the original rename never saw:
window.slopsmith(.Tour) consumers in lessons.js / notifications.js /
onboarding-tour.js, and the matching JS + python tests (autoplay_exit,
progression_*, test_feedpak_extension FEEDBACK_* env vars). The test env
vars now match server.py (which reads FEEDBACK_SYNC_STARTUP /
FEEDBACK_SKIP_STARTUP_TASKS), so the sync-startup test exercises the real
path again.
- Restore the window.slopsmith backward-compat alias dropped during conflict
resolution, and move the bus aliases to AFTER the _feedBackExisting merge
block so they reference the fully-assembled object (also fixes the
loop_api.test.js API-surface regex, which the original PR latently broke).
- Drop the stray empty data/web_library.db (runtime DB lives in CONFIG_DIR)
and gitignore it.
- Fix stale tone-source test: feed[dB]ack -> fee[dB]ack to match shipped
source labels.
Verified locally (org CI billing-blocked): JS 819/819 pass; pytest 1669
passed / 1683 collected with 0 import errors; zero residual slopsmith/byron
except the two intentional window.slopsmith aliases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* rename: implement advertised backward-compat + prune dead community plugins
Address gaps where PR #537's "Backward compatibility" section was advertised
but not implemented, and clean up the community plugin list.
Env vars (FEEDBACK_* canonical, legacy SLOPSMITH_* honoured):
- New lib/env_compat.py (getenv_compat / env_flag_compat) + tests. server.py
(_env_flag + all FEEDBACK_* reads), diagnostics_hardware, gp2midi and
tailwind_rebuild now resolve the legacy alias, so existing SLOPSMITH_UI /
SLOPSMITH_PLUGINS_DIR / etc. deployments keep working.
- Fix the rename collapsing plugins/__init__.py and minigames/routes.py from
`FEEDBACK_PLUGINS_DIR or SLOPSMITH_PLUGINS_DIR` into a redundant
`FEEDBACK_ or FEEDBACK_` (the fallback was silently lost).
Storage (app.js update-channel):
- Read feedBack-update-channel, fall back to legacy slopsmith-update-channel,
and clear the legacy key on write — so a user's update-channel preference
survives the rename instead of resetting to "stable".
Community plugin list (README): the rename rewrote third-party repo URLs we
don't own. Probed every one; their owners never renamed, so:
- Restore the 13 live community plugins to their real slopsmith-* names.
- Prune 6 that are 404 to the public (topkoa splitscreen/stems, OmikronApex
tuner, Jafz2001 nam-rig-builder, DeathlySin song-preview, Erikcb91 shuffle).
- Fix a pre-existing Guitar Theory clone-command typo (nam-tone -> guitar-theory).
Verified: env_compat 7/7, JS 819/819, pytest 1690 collected / 0 import errors,
rename-sensitive + startup suites green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: byrongamatos <xasiklas@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The 940-line format developer guide has moved to its own published repo,
got-feedback/feedback-feedpak-spec (released as feedpak v1.0.0). Replace
docs/sloppak-spec.md with a thin pointer at the same path so existing
references keep resolving; it links to the authoritative spec, bridges the
sloppak/feedpak naming, and keeps the feedback-specific "where it lives in
lib/" implementation map.
Also repoint the human-facing references — CLAUDE.md's developer-reference
link, the constitution's format reference, and the hand-editing guide's
cross-links (to the spec's renumbered §6/§8/§9.5). The hand-editing guide
itself stays: its practical "edit your own pack" content is not in the spec
repo. Inline code comments that cite old "sloppak-spec §X.Y" section numbers
are left for a follow-up (the path still resolves; numbers are approximate).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Kris Anderson <topkoa@gmail.com>
Reword comments/docstrings/strings and rename identifiers that referenced
the external game and its file formats:
- format-id "psarc" -> "archive"; local vars psarc_path -> song_path,
psarc_base -> tone_base
- lyrics provenance value "sng" -> "notechart" (legacy "sng" still accepted)
- highway_3d fret-ghost scope value "rocksmith" -> "chords" (invalid/legacy
values fall back to the default, preserving behaviour)
- neutralise references in prose, test names/data, .gitattributes and docs
No functional change beyond the renamed identifiers; all Python compiles.
gionnibgud