fix: preserve Guitar Pro pick direction

CHANGELOG.md

@@ -633,6 +633,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Path traversal in archive extractors and library path resolution.** `lib/sloppak.py::_unpack_zip` and `server.py::_resolve_dlc_path` previously concatenated attacker-controlled entry names or filenames directly onto the extraction or library directory, so a crafted sloppak zip member or library filename with `..` segments, an absolute path, or backslash separators could write or read outside the intended directory. Any code path that unpacks a user-supplied archive (library upload, click-to-play, retune) or resolves a library path was reachable. Both locations now delegate to a new `lib/safepath.py::safe_join` helper that resolves each destination once and rejects entries that don't fall under the target directory; rejected entries are logged and skipped, the rest of the archive still extracts. The stem-split paths in `lib/sloppak_convert.py::split_stems` and `scripts/split_stems.py` previously called `ZipFile.extractall()` directly on user-supplied sloppaks; both now delegate to the same hardened `lib/sloppak.py::_unpack_zip` so every sloppak-unzip site in the codebase shares one containment guarantee. Tests in `tests/test_archive_traversal.py` and `tests/test_safepath.py` pin the contract for `../`, deep traversal, absolute paths, mixed `subdir/../../` forms, Windows-style separators, NUL bytes, names that resolve to the unpack root, and symlinked roots.
 
 ### Fixed
+- Guitar Pro imports now preserve explicitly authored up/down pick strokes for
+  both GP3/4/5 and GP6/7/8 files instead of leaving every note's
+  `pickDirection` unset (#1054).
 - E Standard retune now stays metadata-consistent across a chart's arrangement files (feedBack-plugin-notedetect#50). Previously the retune path could shift the audio and update manifests while leaving some arrangement metadata untouched, so `load_song()` later exposed the *original* tuning at runtime. `lib/retune.py` now updates every arrangement's tuning metadata consistently before applying the E Standard tuning, and raises on a partial update instead of silently packing split tuning metadata. EStd files generated before this fix should be re-converted so their metadata is consistent.
 - Keyboard shortcut help now opens from the Player/3D Highway context when Linux/Electron reports Shift+Slash as `key="/"`, including while player controls such as the visualization picker are focused (#598).
 - 3D Highway left-handed mode now has regression coverage for fret-axis mirroring, board rebuilds on runtime lefty changes, and mirrored camera state including the lookahead target and shoulder offset; the maintainer guide no longer claims the renderer ignores `bundle.lefty` (#321).

lib/gp2rs.py

@@ -72,6 +72,9 @@ class RsNote:
     tremolo: bool = False
     tap: bool = False
     link_next: bool = False
+    # Teaching mark (§6.2.2): 0 down-stroke, 1 up-stroke, -1 unset.
+    # Display only — never used for grading.
+    pick_direction: int = -1
     # Teaching mark (§6.2.2): fret-hand finger (-1 unset, 0 thumb..4 pinky).
     # Display only — never used for grading.
     fret_finger: int = -1
@@ -268,6 +271,30 @@ def _finger_xml_attrs(n: "RsNote") -> dict:
     return {}
 
 
+def _pick_direction_xml_attrs(n: "RsNote") -> dict:
+    """Optional ``pickDirection`` XML attribute for a converted note."""
+    if getattr(n, "pick_direction", -1) in (0, 1):
+        return {"pickDirection": str(int(n.pick_direction))}
+    return {}
+
+
+def _gp_beat_pick_direction(beat_effect) -> int:
+    """Map a PyGuitarPro beat's authored stroke to the feedBack convention.
+
+    PyGuitarPro represents a pick stroke directly as ``pickStroke`` and older
+    brush/strum effects as ``stroke.direction``. Both use ``up=1, down=2``;
+    feedBack/RS XML uses ``down=0, up=1``. Prefer the explicit pick stroke and
+    fall back to the legacy beat stroke. Missing/unknown values stay unset.
+    """
+    if beat_effect is None:
+        return -1
+    direction = getattr(beat_effect, "pickStroke", None)
+    if getattr(direction, "name", "none").lower() == "none":
+        direction = getattr(getattr(beat_effect, "stroke", None), "direction", None)
+    name = getattr(direction, "name", "").lower()
+    return {"down": 0, "up": 1}.get(name, -1)
+
+
 def _tempo_at_tick(tick: int, tempo_map: list[TempoEvent]) -> float:
     """Get the tempo at a given tick."""
     result = tempo_map[0].tempo
@@ -796,6 +823,7 @@ def convert_track(
                     + entry.output_start_secs + audio_offset
                 tempo = _tempo_at_tick(beat.start, tempo_map)
                 dur = _duration_to_seconds(beat.duration, tempo)
+                pick_direction = _gp_beat_pick_direction(beat.effect)
 
                 beat_notes = []
                 for note in beat.notes:
@@ -820,6 +848,7 @@ def convert_track(
                         fret=fret,
                         sustain=dur if dur > 0.2 else 0.0,
                         mute=note.type == guitarpro.NoteType.dead,
+                        pick_direction=pick_direction,
                     )
 
                     # Techniques
@@ -1213,6 +1242,7 @@ def _build_xml(
         }
         attrs.update(_bend_shape_xml_attrs(n))
         attrs.update(_finger_xml_attrs(n))
+        attrs.update(_pick_direction_xml_attrs(n))
         ET.SubElement(notes_el, "note", **attrs)
 
     # Chords
@@ -1246,6 +1276,7 @@ def _build_xml(
             }
             cn_attrs.update(_bend_shape_xml_attrs(cn))
             cn_attrs.update(_finger_xml_attrs(cn))
+            cn_attrs.update(_pick_direction_xml_attrs(cn))
             ET.SubElement(chord_el, "chordNote", **cn_attrs)
 
     # Anchors

lib/gp2rs_gpx.py

@@ -903,6 +903,17 @@ def _beat_has_tremolo(beat_el: ET.Element) -> bool:
     return beat_el.find('Tremolo') is not None
 
 
+def _gpif_pick_direction(beat_el: ET.Element) -> int:
+    """Map GPIF ``Stroke/Direction`` to feedBack's pick-direction values.
+
+    GPIF writes the direction as a case-insensitive ``Up`` or ``Down`` token.
+    feedBack/RS XML uses ``0`` for down and ``1`` for up; absent or unknown
+    tokens remain ``-1`` (unset) rather than fabricating a direction.
+    """
+    raw = (beat_el.findtext('Stroke/Direction') or '').strip().lower()
+    return {'down': 0, 'up': 1}.get(raw, -1)
+
+
 # ---------------------------------------------------------------------------
 # list_tracks — mirrors gp2rs.list_tracks interface
 # ---------------------------------------------------------------------------
@@ -1771,6 +1782,7 @@ def _auto_guitar_hint(track_idx: int):
 
                             dur = _beat_dur_secs(beat_el, rhythms_dict, _cur_tempo)
                             t = voice_time + audio_offset
+                            pick_direction = _gpif_pick_direction(beat_el)
 
                             notes_text = beat_el.findtext('Notes', '').strip()
                             if notes_text:
@@ -1856,6 +1868,9 @@ def _auto_guitar_hint(track_idx: int):
                                         string=rs_str,
                                         fret=rs_fret,
                                         sustain=sustain,
+                                        pick_direction=(
+                                            pick_direction if not (is_drum or is_keys) else -1
+                                        ),
                                     )
 
                                     # Techniques — GPIF stores these as <Property>

tests/test_gp2rs.py

@@ -25,6 +25,7 @@
     _compute_tuning,
     _extract_year,
     _gp_bend_shape,
+    _gp_beat_pick_direction,
     _gp_string_to_rs,
     _is_bass_track,
     _standard_tuning_for,
@@ -869,6 +870,64 @@ def test_tied_note_without_predecessor_is_silently_dropped():
     assert len(notes) == 0
 
 
+@pytest.mark.parametrize("direction, expected", [
+    (guitarpro.models.BeatStrokeDirection.down, 0),
+    (guitarpro.models.BeatStrokeDirection.up, 1),
+    (guitarpro.models.BeatStrokeDirection.none, -1),
+])
+def test_gp_pick_stroke_direction_mapping(direction, expected):
+    """Map each PyGuitarPro pick-stroke enum to the feedBack wire value."""
+    effect = SimpleNamespace(
+        pickStroke=direction,
+        stroke=SimpleNamespace(
+            direction=guitarpro.models.BeatStrokeDirection.none),
+    )
+    assert _gp_beat_pick_direction(effect) == expected
+
+
+def test_gp_legacy_beat_stroke_is_fallback():
+    """Use the legacy beat stroke when no explicit pick stroke is authored."""
+    effect = SimpleNamespace(
+        pickStroke=guitarpro.models.BeatStrokeDirection.none,
+        stroke=SimpleNamespace(
+            direction=guitarpro.models.BeatStrokeDirection.up),
+    )
+    assert _gp_beat_pick_direction(effect) == 1
+
+
+@pytest.mark.parametrize("direction, expected", [
+    (guitarpro.models.BeatStrokeDirection.down, "0"),
+    (guitarpro.models.BeatStrokeDirection.up, "1"),
+])
+def test_convert_track_preserves_pick_direction(direction, expected):
+    """Serialize an authored pick direction on a converted standalone note."""
+    beat = _ct_beat(
+        tick=0,
+        dur_value=4,
+        notes=[_ct_note(guitarpro.NoteType.normal, gp_string=1, fret=5)],
+    )
+    beat.effect.pickStroke = direction
+    root = ET.fromstring(convert_track(_ct_song([beat]), track_index=0))  # noqa: S314
+    assert root.find(".//notes/note").get("pickDirection") == expected
+
+
+def test_convert_track_preserves_pick_direction_on_every_chord_note():
+    """Serialize the beat's pick direction on every converted chord note."""
+    beat = _ct_beat(
+        tick=0,
+        dur_value=4,
+        notes=[
+            _ct_note(guitarpro.NoteType.normal, gp_string=1, fret=3),
+            _ct_note(guitarpro.NoteType.normal, gp_string=2, fret=2),
+        ],
+    )
+    beat.effect.pickStroke = guitarpro.models.BeatStrokeDirection.down
+    root = ET.fromstring(convert_track(_ct_song([beat]), track_index=0))  # noqa: S314
+    chord_notes = root.findall(".//chords/chord/chordNote")
+    assert len(chord_notes) == 2
+    assert {note.get("pickDirection") for note in chord_notes} == {"0"}
+
+
 # ── convert_track: bend shape (bn / bt / bnv, §6.2.1) ────────────────────────
 
 def _ct_bend(points):

tests/test_gp2rs_gpx.py

@@ -22,6 +22,7 @@
     _note_is_tie,
     _note_has_vibrato,
     _beat_has_tremolo,
+    _gpif_pick_direction,
     _note_midi,
     _gpx_percussion_midis,
     _gpx_tuning,
@@ -681,6 +682,26 @@ def test_convert_file_gp8_tremolo_beat_flags_note(tmp_path, monkeypatch):
     assert tremolo_by_string[5] == "0"
 
 
+def test_convert_file_gp8_preserves_stroke_direction(tmp_path, monkeypatch):
+    """Preserve GPIF stroke direction without assigning it to other beats."""
+    gpif = _GPIF_GUITAR_TREMOLO.replace(
+        '<Tremolo>1/8</Tremolo>',
+        '<Stroke><Direction>Up</Direction></Stroke>',
+    )
+    monkeypatch.setattr(gp2rs_gpx, "_load_gpif", lambda _p: ET.fromstring(gpif))
+    out_files = convert_file(
+        "dummy.gp", str(tmp_path),
+        track_indices=[0], arrangement_names={0: "Lead"},
+    )
+    root = ET.parse(out_files[0]).getroot()
+    direction_by_string = {
+        int(note.get("string")): note.get("pickDirection")
+        for note in root.iter() if note.tag == "note"
+    }
+    assert direction_by_string[0] == "1"
+    assert direction_by_string[5] is None
+
+
 def test_vocal_pitch_sidecar_sorts_multi_voice_by_time():
     # Two voices in one bar. Voice 0 (traversed first) emits its lyric note at
     # t=0.5 (a no-lyric quarter precedes it); voice 1 (traversed second) emits
@@ -889,6 +910,24 @@ def test_beat_tremolo_independent_of_whammy_trembar():
     assert _beat_has_tremolo(b) is False
 
 
+@pytest.mark.parametrize("token, expected", [
+    ("Down", 0),
+    ("Up", 1),
+    ("down", 0),
+    ("Sideways", -1),
+])
+def test_gpif_pick_direction_mapping(token, expected):
+    """Map supported GPIF direction tokens and reject unknown directions."""
+    beat = ET.fromstring(
+        f'<Beat id="1"><Stroke><Direction>{token}</Direction></Stroke></Beat>')
+    assert _gpif_pick_direction(beat) == expected
+
+
+def test_gpif_pick_direction_absent_is_unset():
+    """Leave pick direction unset when a GPIF beat has no stroke element."""
+    assert _gpif_pick_direction(ET.fromstring('<Beat id="1"/>')) == -1
+
+
 # ── _gpif_left_fingering (GP7/GP8 per-note fret-hand finger -> fg) ───────────
 # GPIF stores a single note's fret-hand finger as a direct <LeftFingering>
 # child of <Note> (NOT a <Property>), with classical p-i-m-a-c letter codes —