Skip to content

gp2rs.py drops Guitar Pro arpeggio/handshape flag during import — chord templates never get arp #57

Closed

Description

@cchacs

Summary

lib/gp2rs.py (the binary Guitar Pro → arrangement-XML converter, used for GP3/GP4/GP5 import) never reads or propagates the arpeggio flag that Guitar Pro authors attach to a beat/chord. As a result, every GP-sourced chart imports its arpeggiated passages as ordinary strummed chords, and the downstream .feedpak handshape data (handshapes[].arp) is always false for GP imports — even when the source GP file explicitly marks the chord as an arpeggio.

This is not a request for a new wire-format concept — the arp field already exists and is spec-legal (see "Where this fits" below). It's a bug/gap in one converter: the flag exists in pyguitarpro's beat data, and there's a place downstream ready to receive it, but gp2rs.py never wires the two together.

Where this fits (already spec-legal, no FEP needed)

  • feedpak-spec/spec/feedpak-v1.md §6.5/§6.6 already define handshape.arp (boolean — "treat as an arpeggio span rather than a strummed hold") and chord-template arp.
  • feedback-plugin-feedpakr's feedpakr_handshapes.py (derive_handshapes()) already reads arp straight off the chord template the wire arrangement provides — i.e. the downstream consumer is correctly implemented and just waiting for the flag to actually be set.
  • The break is entirely upstream, in this repo.

What's happening in lib/gp2rs.py

  • ChordTemplate (around line 111) is defined as:

    @dataclass
    class ChordTemplate:
        name: str
        frets: list[int]   # per string, -1 = unused
        fingers: list[int] # per string, -1 = unused

    There is no arp/arpeggio field at all.

  • Chord templates are built and appended around line 1027 (chord_templates.append(ChordTemplate(name="", frets=list(frets), fingers=[-1] * width))) with no arpeggio detection anywhere in that code path.

  • gp2rs.py does already read other per-beat effect data off pyguitarpro's guitarpro.BeatEffect / guitarpro.NoteEffect objects in this same function — e.g.:

    • _gp_pick_direction() (line ~274) reads effect.stroke.direction / effect.pickStroke for strum direction, with detailed comments on which pyguitarpro field GP writes for authored chord strums vs. individual note picks.
    • getattr(eff, "letRing", False) (line ~911) is read and mapped to link_next.

    So the plumbing pattern for "read a pyguitarpro beat/note effect flag → set it on the wire model" is already established in this file; arpeggio just isn't one of the flags being read.

  • The only place in the codebase that currently sets arp/arpeggio on a chord template or handshape is the legacy arrangement-XML path (RS2014/EOF <handShape> elements with a displayName ending in -arp or an explicit arp attribute) — i.e. the loose-folder importer, not the binary GP parser. GP-authored arpeggio data specifically is what's being lost.

What needs to happen

  1. Identify the pyguitarpro field(s) that carry arpeggio/"let ring as arpeggio" semantics for a chord beat. (pyguitarpro exposes GP's BeatStroke with a "brush"/arpeggio-style stroke distinct from a simple down/up strum — worth checking guitarpro.BeatStrokeDirection and any arpeggio-specific enum/flag alongside it, and cross-checking against how EOF/RS2014 authoring tools decide when to emit arp="1" on a <handShape>, since that's the semantics we're trying to match.)
  2. Add an arp: bool = False field to ChordTemplate in gp2rs.py.
  3. In the chord-template construction/enrichment code (~line 1000-1045), set arp=True when the source GP beat carries that flag — following the existing "enrich from the GP chord diagram, only when it matches the actually-played voicing" guard pattern already used for name/fingers back-fill.
  4. Thread arp through to the arrangement XML output (wherever ChordTemplate fields are currently serialized — the fret/finger writing near the XML-emission code, mirroring how linkNext/pickDirection attrs are written per-note around line 1277/1316).
  5. Verify feedpakr_handshapes.py's derive_handshapes() picks it up correctly with no changes needed on the feedpakr side (per the earlier investigation, it already reads templates[cid].get('arp') — this should "just work" once gp2rs.py sets the flag).
  6. Add a regression test in tests/ (this repo uses pytest, tests/test_*.py, config in pyproject.toml) importing a small fixture GP file with a known authored arpeggio and asserting the resulting ChordTemplate.arp / arrangement-XML output.

Image

Context / mental model

For anyone unfamiliar with the RS/GP arpeggio convention: an arpeggio marker in Rocksmith-style notation is a "hold this chord shape" indicator, not an instruction to strike all strings at once. The shape shown establishes the fretting-hand handshape; only the individually highlighted notes crossing the strike line are actually picked at that moment, in sequence. Rocksmith 2014 arrangements generally represent this via the handshape mechanism (arp flag) rather than a dedicated arpeggio note-type — which is exactly the mechanism feedpak's handshape.arp already models. The gap here is purely that GP-sourced imports never populate it.

Repos involved (for reference, no action needed elsewhere)

  • get-flashbacks/feedpak-spec — format already supports this (§6.5/§6.6), no changes needed.
  • get-flashbacks/feedback-plugin-feedpakr — feedpakr_handshapes.py already correctly consumes arp from chord templates, no changes needed.
  • get-flashbacks/feedback (this repo) — lib/gp2rs.py is where the fix belongs.

Activity

  1. pullfrog commented on Aug 8, 2026

    @pullfrog

    Implementation Plan — #57: GP import drops arpeggio flag (chord templates never get arp)

    Root cause (confirmed)

    lib/gp2rs.py (GP3/4/5 path) has no arpeggio plumbing anywhere. ChordTemplate (gp2rs.py:111) lacks an arp field, chord templates are constructed at gp2rs.py:1027 with no arpeggio detection, and _build_xml emits <chordTemplate> with only chordName/fretN/fingerN (gp2rs.py:1244-1251). The downstream chain already supports it end-to-end:

    • lib/song.py:76 — ChordTemplate.arpeggio: bool = False already exists.
    • lib/song.py:342 — chord_template_to_wire() already emits "arp": ct.arpeggio on the wire.
    • lib/song.py:1053 — _chord_template_arpeggio_flag() already parses arp/Arpeggio/arpeggio/Arp (or a -arp displayName) off a <chordTemplate> element.
    • ws_highway.py:728 ships templates via chord_template_to_wire; feedpakr's derive_handshapes() consumes templates[cid].get('arp') (verified by the issue author; the plugin isn't cloned here, so step 9 re-checks it).

    So the whole fix is upstream: set arp in gp2rs, emit it on <chordTemplate>, and the flag flows to the wire untouched.

    Key finding — what pyguitarpro actually exposes (issue step 1)

    pyguitarpro's GP3/4/5 beat model has no binary arpeggio flag. The relevant fields on BeatEffect are stroke (BeatStroke(direction, value)), pickStroke (bare BeatStrokeDirection), and hasRasgueado (flamenco strumming — exclude, not an arpeggio):

    • effect.stroke is the brush — the arpeggio-style chord mark GP's own UI writes when you drag across a chord. Its value is the roll speed (Duration 1=hundredTwentyEighth … 6=quarter).
    • effect.pickStroke is a rare bare up/down strum mark with no brush semantics.
    • The existing _gp_pick_direction() (gp2rs.py:274) already keys on exactly this distinction (.stroke first, .pickStroke fallback), and its comment documents the brush as "arpeggio chord-strum".

    Detection decision: treat a present effect.stroke with direction != BeatStrokeDirection.none as the arpeggio marker (sets arp), and a bare .pickStroke as a plain strum (does not set arp). This matches the issue's framing and the existing _gp_pick_direction precedent — one helper, one signal. Open refinement to validate against real GP files: gate on stroke.value (roll speed) so only slow brushes (≥ eighth, value >= 5) count as arpeggio; presence-based is the recommended first cut, with the speed gate as a follow-up knob if a validation pass shows GP-authored strums being over-tagged.

    Scope

    In scope: lib/gp2rs.py (GP3/4/5) + lib/song.py round-trip + tests. The GPX path (lib/gp2rs_gpx.py) is a related-but-separate module; GPIF exposes a genuinely distinct <Arpeggio> <Beat> child (see gp2rs_gpx.py:654-686), which is a cleaner signal than GP4/5. Kept as an optional follow-up (Milestone 5) rather than folded into this issue — its detection differs and it needs its own tests.

    Milestones

    M1. Add the field and a detector helper (lib/gp2rs.py)

    • Add arp: bool = False to ChordTemplate (gp2rs.py:110-114). Mirrors lib/song.py's ChordTemplate.arpeggio. Default False keeps the piano path (convert_piano_track, gp2rs.py:1663) and drum path (convert_drum_track, gp2rs.py:1844) byte-for-byte unchanged.
    • Add _gp_beat_arpeggio(effect) -> bool next to _gp_pick_direction (gp2rs.py:274), same defensive getattr style: returns True when effect.stroke exists with direction present and != BeatStrokeDirection.none; False otherwise. Do not fall back to .pickStroke (plain strum ≠ arpeggio). One short docstring noting the brush = GP's arpeggio notation and why hasRasgueado/pickStroke are excluded.

    M2. Set arp during chord-template construction (convert_track, gp2rs.py:1006-1034)

    • After idx = chord_template_map[fret_key] resolves (both the create and reuse paths), apply ct = chord_templates[idx]; if _gp_beat_arpeggio(beat.effect): ct.arp = True.
    • Do not gate on the chord-diagram match (if beat.effect and beat.effect.chord: at gp2rs.py:1048): the brush is a per-beat articulation, not diagram data, so arp must attach even for beats with no diagram. Name/fingers enrichment stays as-is behind the existing guard.
    • Accepted model limitation (note in a comment): templates dedupe by fret_key, so one arpeggiated beat marks every strum sharing that voicing — the same template-level model feedpakr consumes, consistent with name/fingers back-fill.

    M3. Emit arp="1" in _build_xml (gp2rs.py:1244-1251)

    • In the <chordTemplate> loop, add if ct.arp: attrs["arp"] = "1" alongside chordName. lib/song.py:1053 reads it back; absent arp → attribute omitted (matches the default-omit convention used for bendIntent/fretFinger/pickDirection). No change to song.py needed.

    M4. Regression tests (tests/test_gp2rs.py)
    Reuse the existing fixture-free _ct_song/_ct_beat/_ct_note/_ct_chord helpers (see test_chord_diagram_fingers_extracted at test_gp2rs.py:1206 and the stroke tests at :1259). Cover:

    • Chord beat (2+ notes) with effect.stroke = SimpleNamespace(direction=BeatStrokeDirection.down, value=64) → output XML's <chordTemplate> has arp="1", and the chord notes still carry pickDirection="0" (both signals coexist).
    • Chord beat with no stroke → no arp attribute (plain strum).
    • Chord beat with only pickStroke (up) → no arp attribute, pickDirection="1" still set (brush-vs-strum distinction).
    • stroke with direction=none → no arp.
    • Round-trip: feed the produced XML into song.parse (or the equivalent used by the fg round-trip test at :1235) and assert chord_template_to_wire(...)["arp"] is True, proving the flag survives XML → dataclass → wire.
    • Confirm the existing test_chord_beat_stroke_field_imports_as_pkd and _gp_pick_direction tests still pass (their mocked effect shapes must keep working — the helper reads stroke defensively).

    M5 (optional, separate follow-up). GPX path (lib/gp2rs_gpx.py)
    GPIF has a real <Arpeggio> direct <Beat> child — a distinct arpeggio articulation vs the <Property name="Brush"> strum. _gpif_pick_direction currently folds both into direction. A follow-up could set arp=True on the GPX chord template (built at gp2rs_gpx.py:2029) when <Arpeggio> is present, with its own GPIF fixture tests in the test_gp2rs_gpx.py style (test_gpif_pick_direction_falls_back_to_arpeggio already shows the fixture shape).

    Verification (M4 + manual)

    • python -m pytest tests/test_gp2rs.py -v (sandbox: pip install pytest first; skip TestClient files, per repo learnings).
    • Sanity-check no <chordTemplate> in existing output gains spurious arp (default-omit keeps current golden output identical).
    • Optionally grep the feedpakr plugin's feedpakr_handshapes.py at review time to reconfirm templates[cid].get('arp') consumption (issue author states it's already correct).

    Delivery notes

    • All changes are lib/ + tests/. Per the fork's core-commit-labeling gate, the PR's commit subjects touching lib/** need a ^(core|hook|sync|fix)(\([^)]*\))?: prefix — fix: gp2rs import arp... or core: both pass; a bare "Add arp flag…" subject will fail CI.
    • Add an [Unreleased] entry under CHANGELOG.md per repo convention.
    • Branch pattern: pullfrog/57-gp2rs-arpeggio-flag; do not push to main.

    Open questions for the implementer

    1. Presence-based vs speed-threshold arp (M1 decision above) — recommended presence-based first; validate against a real GP file with an authored arpeggio if one is handy.
    2. Whether to also do the GPX follow-up (M5) in the same PR or defer — recommend deferring to keep the PR scoped to the issue.
    Task list (5/6 completed)

    Pullfrog  | Implement plan ➔ | View workflow run | via Pullfrog | Using Big Pickle (free) | 𝕏

  2. added theissue type on Aug 9, 2026
  3. changed the issue type fromtoon Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

enhancementNew feature or request

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions