diff --git a/lib/gp2rs.py b/lib/gp2rs.py index dbfe1bff..486d8337 100644 --- a/lib/gp2rs.py +++ b/lib/gp2rs.py @@ -491,6 +491,25 @@ def _chord_fingers(chord, frets: list[int], num_strings: int) -> list[int]: return fingers +def _chord_diagram_frets(chord, num_strings: int, width: int) -> list[int]: + """RS-string-ordered absolute frets of the chord DIAGRAM voicing, padded to + ``width`` with -1. + + Used to confirm the diagram describes the voicing actually played before + enriching a template — mirrors the GP8 exact fret-pattern guard. pyguitarpro + stores absolute frets in ``chord.strings`` (``firstFret`` is display-only), + so the result compares directly against the played ``frets``.""" + out = [-1] * width + strings = getattr(chord, "strings", None) or [] + for i, fret in enumerate(strings): + if fret is None or fret < 0: + continue + rs = _gp_string_to_rs(i + 1, num_strings) + if 0 <= rs < width: + out[rs] = fret + return out + + def _is_bass_track(track: guitarpro.Track) -> bool: """Detect whether a GP track is a bass. @@ -869,21 +888,34 @@ def convert_track( else: idx = chord_template_map[fret_key] - # Enrich the template with the GP chord diagram (name + - # per-string fingering) attached to this beat. Back-fill - # any still-blank template so the data attaches regardless - # of whether the annotated beat is the one that first - # created the template (a plain earlier strum of the same - # voicing must not shadow it). First annotation wins. + # Enrich the template from the GP chord diagram attached to + # this beat — but ONLY when the diagram describes the voicing + # actually played (same width-normalized fret pattern). A + # mismatched chord label/diagram would otherwise mis-name / + # finger the played template, and the back-fill would spread + # it to other strums of the same played pattern. Mirrors the + # GP8 exact fret-pattern guard. + # + # Name and fingers back-fill INDEPENDENTLY: a name-only first + # annotation must not block a later beat that carries fingers + # (and vice versa). Back-fill any still-blank field so the + # data attaches regardless of which strum carries it. if beat.effect and beat.effect.chord: - ct = chord_templates[idx] - if not ct.name and all(f < 0 for f in ct.fingers): - name = beat.effect.chord.name or "" - fingers = _chord_fingers( - beat.effect.chord, frets, num_strings) - if name or any(f >= 0 for f in fingers): - ct.name = name - ct.fingers = fingers + gpc = beat.effect.chord + # Compare over the FULL string span (played width vs the + # track's string count) so a diagram that frets an + # extended string the played voicing doesn't use counts + # as a mismatch instead of being silently trimmed. + _w = max(len(frets), num_strings) + _played = frets + [-1] * (_w - len(frets)) + if _chord_diagram_frets(gpc, num_strings, _w) == _played: + ct = chord_templates[idx] + if not ct.name and gpc.name: + ct.name = gpc.name + if all(f < 0 for f in ct.fingers): + fingers = _chord_fingers(gpc, frets, num_strings) + if any(f >= 0 for f in fingers): + ct.fingers = fingers rs_chords.append(RsChord( time=t, diff --git a/tests/test_gp2rs.py b/tests/test_gp2rs.py index 5e7e62ce..54ef1970 100644 --- a/tests/test_gp2rs.py +++ b/tests/test_gp2rs.py @@ -795,12 +795,14 @@ def _ct_note(note_type, gp_string, fret): ) -def _ct_song(beats): - """One-measure mock song for convert_track, standard 6-string guitar at 120 BPM.""" +def _ct_song(beats, string_values=None): + """One-measure mock song for convert_track, standard 6-string guitar at 120 BPM. + + `string_values` overrides the tuning/string count (e.g. a 7-string track).""" voice = SimpleNamespace(beats=beats) measure = SimpleNamespace(voices=[voice]) strings = [SimpleNamespace(number=i + 1, value=v) - for i, v in enumerate([64, 59, 55, 50, 45, 40])] + for i, v in enumerate(string_values or [64, 59, 55, 50, 45, 40])] track = SimpleNamespace( strings=strings, channel=SimpleNamespace(instrument=24), @@ -1181,3 +1183,102 @@ def test_chord_diagram_backfills_template_first_strummed_unannotated(): assert len(cts) == 1, "same voicing must dedup to one template" assert cts[0].get("chordName") == "Gtest" assert cts[0].get("finger5") == "2" and cts[0].get("finger4") == "1" + + +def test_chord_diagram_mismatch_not_applied(): + # The attached diagram describes a DIFFERENT voicing (frets 5/5) than the + # notes actually played (3/2). It must NOT enrich the played template — + # otherwise a mislabeled chord would name/finger the wrong voicing (and the + # back-fill would spread it). Name + fingers stay blank. + note_e = _ct_note(guitarpro.NoteType.normal, gp_string=1, fret=3) + note_b = _ct_note(guitarpro.NoteType.normal, gp_string=2, fret=2) + beat = _ct_beat(tick=0, dur_value=4, notes=[note_e, note_b]) + beat.effect.chord = _ct_chord( + "Wrong", strings=[5, 5, -1, -1, -1, -1], # != played 3/2 + fingerings=[guitarpro.Fingering.annular, guitarpro.Fingering.annular], + ) + + xml_str = convert_track(_ct_song([beat]), track_index=0) + root = ET.fromstring(xml_str) # noqa: S314 + ct = root.find(".//chordTemplates/chordTemplate") + assert ct is not None + assert ct.get("chordName") == "" + assert [ct.get(f"finger{i}") for i in range(6)] == ["-1"] * 6 + + +def test_chord_diagram_name_then_fingers_decoupled(): + # First annotated beat carries a NAME but no fingers (all open); a later beat + # of the same voicing carries the fingers. Both must land — a name-only first + # annotation must not block the later fingers (name/fingers back-fill + # independently). + def _beat(tick, name, fingerings): + b = _ct_beat( + tick=tick, dur_value=4, + notes=[_ct_note(guitarpro.NoteType.normal, gp_string=1, fret=3), + _ct_note(guitarpro.NoteType.normal, gp_string=2, fret=2)], + ) + b.effect.chord = _ct_chord(name, strings=[3, 2, -1, -1, -1, -1], + fingerings=fingerings) + return b + + first = _beat(0, "Gtest", + [guitarpro.Fingering.open, guitarpro.Fingering.open]) + second = _beat(GP_TICKS_PER_QUARTER, "", + [guitarpro.Fingering.middle, guitarpro.Fingering.index]) + + xml_str = convert_track(_ct_song([first, second]), track_index=0) + root = ET.fromstring(xml_str) # noqa: S314 + cts = root.findall(".//chordTemplates/chordTemplate") + assert len(cts) == 1 + assert cts[0].get("chordName") == "Gtest" # from the first (name-only) beat + # fingers from the second beat — not blocked by the first beat's name + assert cts[0].get("finger5") == "2" and cts[0].get("finger4") == "1" + + +def test_chord_diagram_barre_higher_position_matches(): + # A voicing high on the neck: diagram strings hold ABSOLUTE frets (firstFret + # is display-only), so they match the played absolute frets and the template + # enriches. Guards against an absolute-vs-relative matching regression. + notes = [_ct_note(guitarpro.NoteType.normal, gp_string=1, fret=5), + _ct_note(guitarpro.NoteType.normal, gp_string=2, fret=5), + _ct_note(guitarpro.NoteType.normal, gp_string=3, fret=6)] + beat = _ct_beat(tick=0, dur_value=4, notes=notes) + ch = _ct_chord("A", strings=[5, 5, 6, -1, -1, -1], + fingerings=[guitarpro.Fingering.index, guitarpro.Fingering.index, + guitarpro.Fingering.middle]) + ch.firstFret = 5 # display base — must not affect matching + beat.effect.chord = ch + + xml_str = convert_track(_ct_song([beat]), track_index=0) + root = ET.fromstring(xml_str) # noqa: S314 + ct = root.find(".//chordTemplates/chordTemplate") + assert ct is not None + assert ct.get("chordName") == "A" + assert ct.get("fret5") == "5" and ct.get("finger5") == "1" + assert ct.get("fret4") == "5" and ct.get("finger4") == "1" + assert ct.get("fret3") == "6" and ct.get("finger3") == "2" + + +def test_chord_diagram_extended_string_outside_played_width_not_applied(): + # 7-string track. Played voicing is on strings 2 & 3 only (width 6 — the + # high e / rs6 is unused), but the diagram ALSO frets string 1 (the extended + # rs6). The extra diagram note must make this a MISMATCH, not be silently + # trimmed to a false match — so the played template stays un-enriched. + seven = [64, 59, 55, 50, 45, 40, 35] # low-B 7-string + note_b = _ct_note(guitarpro.NoteType.normal, gp_string=2, fret=3) # rs5 + note_g = _ct_note(guitarpro.NoteType.normal, gp_string=3, fret=2) # rs4 + beat = _ct_beat(tick=0, dur_value=4, notes=[note_b, note_g]) + # diagram index0 = gp_string1 (rs6) frets 5 (NOT played); index1/2 match. + beat.effect.chord = _ct_chord( + "Bogus", strings=[5, 3, 2, -1, -1, -1, -1], + fingerings=[guitarpro.Fingering.index, guitarpro.Fingering.middle, + guitarpro.Fingering.index], + ) + + xml_str = convert_track(_ct_song([beat], string_values=seven), track_index=0) + root = ET.fromstring(xml_str) # noqa: S314 + ct = root.find(".//chordTemplates/chordTemplate") + assert ct is not None + assert ct.get("chordName") == "" + # played template is width 6 (rs6/high-e unused) -> finger0..finger5 + assert all(ct.get(f"finger{i}") == "-1" for i in range(6))