From 0f9de9f244a9b49a3a88dcba6d1a2624bf985c68 Mon Sep 17 00:00:00 2001 From: byrongamatos Date: Sun, 21 Jun 2026 01:50:24 +0200 Subject: [PATCH] =?UTF-8?q?feat(editor):=20author=20teaching=20marks=20fg/?= =?UTF-8?q?ch/sd=20in=20the=20note=20inspector=20(=C2=A76.2.2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Author the three optional per-note teaching marks, mirroring the bend-shape authoring (#13). Display only — these never affect grading. - screen.js: a @pure:teaching-marks block (FRET_FINGER_OPTIONS + nextUnusedStrumGroup) and SetTeachingMarkCmd (one undoable batch edit per field, snapshot/rollback per note). Inspector gains a fret-hand-finger picker (fg, -1..4), a scale-degree override field (sd, -1..11; blank/-1 = auto), and Group/Ungroup-as-strum buttons (ch — Group assigns the next unused key across the selection, Ungroup clears it). All routed through S.history.exec. - routes.py: fret_finger/strum_group/scale_degree join _NOTE_TECH_FIELDS so they load (_tech_dict) and feed the content signature; the wire serializer emits fg/ch/sd default-omitted (matching core), and the chart-XML export writes fretFinger (core's _parse_note reads it). getattr-over-fields sites default to -1 so a core build predating the marks doesn't break load/alignment. Tests: tests/teaching_marks.test.js (@pure extract — FRET_FINGER_OPTIONS + nextUnusedStrumGroup edge cases); test_xml_export.py round-trip for fg/ch/sd through the wire + fretFinger through chart XML, plus default-omit assertions. Part of got-feedback/feedback#334 Co-Authored-By: Claude Opus 4.8 (1M context) --- routes.py | 24 +++++- screen.js | 140 +++++++++++++++++++++++++++++++++++ tests/teaching_marks.test.js | 61 +++++++++++++++ tests/test_xml_export.py | 28 +++++++ 4 files changed, 251 insertions(+), 2 deletions(-) create mode 100644 tests/teaching_marks.test.js diff --git a/routes.py b/routes.py index 9f53c8f5..7a2e5258 100644 --- a/routes.py +++ b/routes.py @@ -53,6 +53,8 @@ "pull_off", "harmonic", "harmonic_pinch", "palm_mute", "mute", "vibrato", "tremolo", "accent", "tap", "link_next", "fret_hand_mute", "pluck", "slap", "right_hand", "pick_direction", "ignore", + # Teaching marks (§6.2.2) — display only, never grading. + "fret_finger", "strum_group", "scale_degree", ) # `bend_values` (the §6.2.1 bend curve) is deliberately NOT in the tuple above: # it's a list, and the tuple feeds hashable content-signature tuples @@ -174,7 +176,10 @@ def _align_xml_files_to_arrangements(tmp_dir, result): def _obj_note_sig(n): return ( round(n.time, 3), n.string, n.fret, round(n.sustain or 0.0, 3), - tuple(getattr(n, f) for f in _NOTE_TECH_FIELDS), + # Default -1 so the signature stays stable against a core build that + # predates a field (e.g. the teaching marks before #534 lands) — + # parse_arrangement notes from older core simply lack the attribute. + tuple(getattr(n, f, -1) for f in _NOTE_TECH_FIELDS), ) def _obj_chord_sig(c): @@ -598,6 +603,17 @@ def _note(n): _bnv = _safe_bend_curve(tech.get("bend_values")) if _bnv: out["bnv"] = _bnv + # Teaching marks (§6.2.2) — default-omitted, matching core's note_to_wire. + # Display only; never used for grading. + _fg = _safe_int(tech.get("fret_finger"), -1) + if _fg != -1: + out["fg"] = _fg + _ch = _safe_int(tech.get("strum_group"), -1) + if _ch != -1: + out["ch"] = _ch + _sd = _safe_int(tech.get("scale_degree"), -1) + if _sd != -1: + out["sd"] = _sd return out def _note_in_chord(n): @@ -894,6 +910,10 @@ def _flag(key): "slap": _flag("slap"), "rightHand": str(_safe_int(techs.get("right_hand"), -1)), "pickDirection": str(_safe_int(techs.get("pick_direction"), -1)), + # Teaching mark (§6.2.2): fret-hand finger. core's _parse_note reads + # `fretFinger` back; strum_group/scale_degree have no chart-XML attribute + # (they round-trip through the sloppak wire instead). Display only. + "fretFinger": str(_safe_int(techs.get("fret_finger"), -1)), "ignore": _flag("ignore"), }) return attrs @@ -4721,7 +4741,7 @@ def _tech_dict(n): # round-trips so the editor can render and re-emit them. Field # set lives in `_NOTE_TECH_FIELDS` so the content signature # stays in sync (attr name == wire key for each). - d = {f: getattr(n, f) for f in _NOTE_TECH_FIELDS} + d = {f: getattr(n, f, -1) for f in _NOTE_TECH_FIELDS} # `bend_values` (§6.2.1 curve) is a list, kept out of the signature # tuple — carry it explicitly so an authored/imported curve loads. d["bend_values"] = getattr(n, "bend_values", None) diff --git a/screen.js b/screen.js index 521ea9c2..92db8d33 100644 --- a/screen.js +++ b/screen.js @@ -632,6 +632,36 @@ function rescaleBendCurveToPeak(raw, peak) { } /* @pure:bend-shape:end */ +/* @pure:teaching-marks:start — pure, no browser deps; node-tested by + * tests/teaching_marks.test.js. Helpers for authoring the §6.2.2 teaching + * marks (fg fret-hand finger, ch strum group, sd scale degree). Display only — + * the editor authors them; nothing here feeds grading. */ + +// Fret-hand-finger (`fg`) picker options, in spec order (-1 unset … 4 pinky). +const FRET_FINGER_OPTIONS = [ + { v: -1, label: 'Unset' }, + { v: 0, label: 'Thumb' }, + { v: 1, label: 'Index' }, + { v: 2, label: 'Middle' }, + { v: 3, label: 'Ring' }, + { v: 4, label: 'Pinky' }, +]; + +// Next free strum-group key (`ch`) across a note list: max used (>= 0) + 1, or +// 0 when none is grouped yet. Used by "Group as strum" so a new gesture never +// collides with an existing one. +function nextUnusedStrumGroup(noteList) { + let max = -1; + if (Array.isArray(noteList)) { + for (const n of noteList) { + const ch = n && n.techniques ? n.techniques.strum_group : undefined; + if (Number.isInteger(ch) && ch > max) max = ch; + } + } + return max + 1; +} +/* @pure:teaching-marks:end */ + // Reconstruct chords from notes at the same time before saving function reconstructChords() { if (!S.arrangements.length) return; @@ -1381,6 +1411,36 @@ class SetBendIntentCmd { } } +// Teaching marks (§6.2.2) — set one integer technique field (fret_finger / +// scale_degree / strum_group) across a set of notes as one undoable edit, +// snapshotting the prior per-note value. -1 is the unset sentinel (the save +// path omits it from the wire). Display only — never feeds grading. +class SetTeachingMarkCmd { + constructor(indices, key, value) { + this.indices = indices.slice(); + this.key = key; + this.value = Number.isInteger(value) ? value : -1; + this.old = this.indices.map(i => { + const t = notes()[i].techniques || {}; + return t[key]; + }); + } + exec() { + for (const i of this.indices) { + const n = notes()[i]; + if (!n.techniques) n.techniques = {}; + n.techniques[this.key] = this.value; + } + } + rollback() { + this.indices.forEach((i, k) => { + const n = notes()[i]; + if (!n.techniques) n.techniques = {}; + n.techniques[this.key] = this.old[k]; + }); + } +} + // ── Move-to-string helpers ────────────────────────────────────────── // Standard open-string MIDI pitches (low → high, string index order). // Guitar E2=40 A2=45 D3=50 G3=55 B3=59 e4=64; extended low strings @@ -3699,6 +3759,21 @@ function _renderInspector() { const v = n.techniques && n.techniques.slide_unpitch_to; return v === undefined ? -1 : v; }); + // Teaching marks (§6.2.2): fret-hand finger, scale-degree override, strum + // group. Default to -1 (unset) so a note that never authored them reads as + // unset rather than "mixed" against an authored sibling. + const sharedFinger = _selSharedValue(sel, n => { + const v = n.techniques && n.techniques.fret_finger; + return Number.isInteger(v) ? v : -1; + }); + const sharedScaleDeg = _selSharedValue(sel, n => { + const v = n.techniques && n.techniques.scale_degree; + return Number.isInteger(v) ? v : -1; + }); + const sharedStrum = _selSharedValue(sel, n => { + const v = n.techniques && n.techniques.strum_group; + return Number.isInteger(v) ? v : -1; + }); const inputVal = v => v === null ? '' : String(v); // Chord inspector (E1): when the selection is a chord (>=2 notes sharing a @@ -3759,6 +3834,31 @@ function _renderInspector() { class="flex-1 bg-dark-700 border border-gray-700 rounded px-1 py-0.5 text-xs"> +
+
Teaching marks
+ + +
+ Strum grp ${sharedStrum === null ? '(mixed)' : (sharedStrum >= 0 ? '#' + sharedStrum : '—')} + + +
+
`; for (const f of _INSPECTOR_FLAGS) { @@ -3920,6 +4020,46 @@ window.editorInspectorSetFlag = (key, on) => { updateStatus(); }; +// ─── Teaching marks (§6.2.2) ──────────────────────────────────────── +// Author fg (fret-hand finger), sd (scale-degree override) and ch (strum +// group) on the current selection. Each is one undoable batch edit +// (SetTeachingMarkCmd). Display only — these never affect grading. +function _applyTeachingMark(key, value) { + const idxs = [...(S.sel || [])]; + if (!idxs.length) return; + S.history.exec(new SetTeachingMarkCmd(idxs, key, value)); + draw(); + updateStatus(); + _renderInspector(); +} + +window.editorInspectorSetFretFinger = (raw) => { + const v = Math.trunc(Number(raw)); + if (!Number.isFinite(v)) return; + _applyTeachingMark('fret_finger', Math.max(-1, Math.min(4, v))); +}; + +window.editorInspectorSetScaleDegree = (raw) => { + const s = String(raw).trim(); + // Empty input clears the override back to -1 (auto/unset). + const v = s === '' ? -1 : Math.trunc(Number(s)); + if (!Number.isFinite(v)) { _renderInspector(); return; } + _applyTeachingMark('scale_degree', Math.max(-1, Math.min(11, v))); +}; + +// "Group as strum": assign every selected note a shared, unused ch key so the +// highway renders them as one strum/rake gesture (pkd gives direction). +window.editorGroupAsStrum = () => { + if (!(S.sel && S.sel.size)) return; + _applyTeachingMark('strum_group', nextUnusedStrumGroup(notes())); +}; + +// "Ungroup": clear the strum-group key on the selection (-1 = not grouped). +window.editorUngroupStrum = () => { + if (!(S.sel && S.sel.size)) return; + _applyTeachingMark('strum_group', -1); +}; + // ─── Chord inspector (E1) ─────────────────────────────────────────── // Resolve the current selection to a chord and its width-L fret pattern + // matching chord template, or null when the selection isn't a chord. diff --git a/tests/teaching_marks.test.js b/tests/teaching_marks.test.js new file mode 100644 index 00000000..33b0d030 --- /dev/null +++ b/tests/teaching_marks.test.js @@ -0,0 +1,61 @@ +'use strict'; +/* + * Tests for the §6.2.2 teaching-marks authoring helpers in screen.js. screen.js + * is a single browser IIFE, so this extracts the `@pure:teaching-marks` marked + * block (browser-free) and eval's it in isolation — real source, no drift. + * + * Run: node tests/teaching_marks.test.js + */ +const fs = require('fs'); +const path = require('path'); +const assert = require('assert'); + +const src = fs.readFileSync(path.join(__dirname, '..', 'screen.js'), 'utf8'); +const m = src.match(/\/\* @pure:teaching-marks:start[\s\S]*?@pure:teaching-marks:end \*\//); +if (!m) { + console.error('FAIL: @pure:teaching-marks block not found in screen.js'); + process.exit(1); +} +const { FRET_FINGER_OPTIONS, nextUnusedStrumGroup } = + new Function( + '"use strict";' + m[0] + + '\nreturn { FRET_FINGER_OPTIONS, nextUnusedStrumGroup };' + )(); + +let pass = 0, fail = 0; +function t(name, fn) { + try { fn(); pass++; console.log(' ok ' + name); } + catch (e) { fail++; console.error(' FAIL ' + name + ': ' + e.message); } +} + +// ── FRET_FINGER_OPTIONS ────────────────────────────────────────────────────── +t('FRET_FINGER_OPTIONS covers -1 (unset) + 0..4 (thumb..pinky) in order', () => { + assert.deepStrictEqual(FRET_FINGER_OPTIONS.map(o => o.v), [-1, 0, 1, 2, 3, 4]); + assert.strictEqual(FRET_FINGER_OPTIONS[1].label, 'Thumb'); + assert.strictEqual(FRET_FINGER_OPTIONS[5].label, 'Pinky'); +}); + +// ── nextUnusedStrumGroup ───────────────────────────────────────────────────── +const tech = (strum_group) => ({ techniques: { strum_group } }); + +t('first group is 0 when nothing is grouped', () => { + assert.strictEqual(nextUnusedStrumGroup([]), 0); + assert.strictEqual(nextUnusedStrumGroup([{ techniques: {} }, { techniques: { strum_group: -1 } }]), 0); +}); + +t('returns max used + 1 across the note list', () => { + assert.strictEqual(nextUnusedStrumGroup([tech(0), tech(2), tech(1)]), 3); + assert.strictEqual(nextUnusedStrumGroup([tech(5), tech(-1), tech(2)]), 6); +}); + +t('ignores non-integer / missing strum_group values', () => { + assert.strictEqual(nextUnusedStrumGroup([tech(1.5), tech('3'), { }, null, tech(2)]), 3); +}); + +t('tolerates bad input', () => { + assert.strictEqual(nextUnusedStrumGroup(null), 0); + assert.strictEqual(nextUnusedStrumGroup(undefined), 0); +}); + +console.log(`\n${pass} passed, ${fail} failed`); +if (fail) process.exit(1); diff --git a/tests/test_xml_export.py b/tests/test_xml_export.py index b3df03c1..d6da37b8 100644 --- a/tests/test_xml_export.py +++ b/tests/test_xml_export.py @@ -611,6 +611,34 @@ def test_arr_dict_to_wire_sanitizes_bend_curve(): assert "bnv" not in wn2 +# ---- teaching marks fg/ch/sd (§6.2.2) -------------------------------------- + +def test_arr_dict_to_wire_emits_teaching_marks(): + """fg/ch/sd ride the wire under their literal short codes.""" + notes = [_note(1.0, fret_finger=2, strum_group=5, scale_degree=7)] + wn = _arr_dict_to_wire("Lead", [0]*6, 0, notes, [], [])["notes"][0] + assert wn["fg"] == 2 + assert wn["ch"] == 5 + assert wn["sd"] == 7 + + +def test_arr_dict_to_wire_default_omits_teaching_marks(): + """fg/ch/sd are default-omitted when unset (-1) so payloads stay tight.""" + wn = _arr_dict_to_wire("Lead", [0]*6, 0, [_note(1.0)], [], [])["notes"][0] + for k in ("fg", "ch", "sd"): + assert k not in wn + + +def test_fret_finger_round_trips_through_xml(): + """fg exports as the `fretFinger` chart-XML attr (core's _parse_note reads + it back); strum_group/scale_degree have no chart-XML attribute.""" + note_el = _parse(_build(notes=[_note(1.0, fret_finger=3)])).find(".//notes/note") + assert note_el.get("fretFinger") == "3" + # Default unset. + plain = _parse(_build(notes=[_note(1.0)])).find(".//notes/note") + assert plain.get("fretFinger") == "-1" + + def test_arr_dict_to_wire_chord_note_carries_bend_shape(): """Chord member notes inherit bt/bnv through _note_in_chord -> _note.""" chord = {