diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f6f2d2..7ff4ea5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -119,6 +119,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `_gp_drum_arrs_to_parts` (covered by `tests/test_gp_multidrum_split.py`); the frontend adopts the extras via the existing `adoptDrumParts` load path. This brings the GP create-import to parity with the MIDI drum auto-split. +- **An auto-sync import no longer deletes the front of the chart.** A Guitar + Pro import aligned to a recording with a lead-in (the audio starts before the + tab's bar 1) used to lose its opening notes: the scalar-offset path baked the + offset into the converted XML, pushing that pre-roll to a negative time that + core's `parse_arrangement` then sliced off at `t ≥ 0` — silently dropping, in + one report, the first 16 notes. Guitar Pro is now always converted at offset 0 + and the shift is applied to the parsed arrangement **in memory** (the same + in-place retime the per-bar warp path already used), so a lead-in's notes, + beats, sections and keys notation all survive. The per-bar warp path was + already immune; this repairs the scalar-offset fallback it hands off to (GP5 + with repeats, degenerate or absent sync points, or an older core). Tests: + `tests/test_gp_import_offset.py`. - **A drum edit no longer strips another tool's authored keys from drum-part manifest entries.** Saving with drum changes rebuilds the `type: drums` diff --git a/routes.py b/routes.py index 2df781e..02cc82f 100644 --- a/routes.py +++ b/routes.py @@ -2260,6 +2260,34 @@ def _tones(t): song.offset = 0.0 +def _apply_gp_import_offset(song, xml_paths, provided_offset: float) -> None: + """Apply a scalar GP-import audio offset to an ALREADY-PARSED ``song`` (and + its on-disk notation sidecars) in memory — never baked into the converted + XML. + + ``convert_file`` adds ``audio_offset`` to every note time, so any lead-in + (a negative resolved offset) lands the front of the chart at a NEGATIVE XML + time. Core's ``parse_arrangement`` then slices each level at ``t >= 0`` and + silently drops those notes — the reported "auto-sync deleted the front 16 + notes" data loss. Converting at ``audio_offset=0.0`` and shifting the parsed + Song here keeps the pre-roll: ``_apply_chart_offset`` rewrites absolute + times only and preserves negatives (the same in-memory retime the per-bar + warp path already relies on). + + ``convert_file`` adds the offset, whereas ``_apply_chart_offset`` subtracts + its delta, so the delta is ``-provided_offset``. Zero offset is a no-op. + Module-level so pytest can reach it. + """ + if not provided_offset: + return + _apply_chart_offset(song, -provided_offset) + # Sidecars were written by convert_file on the unshifted (0.0) timeline; + # move them by the same amount the notes moved so _attach_gp_notation reads + # times consistent with the shifted notes. + for _xp in xml_paths: + _warp_notation_sidecar(_xp, lambda t: round(t + provided_offset, 6)) + + def _arrangement_xml_candidates(tmp_dir): """Return the source XMLs that represent playable arrangements. @@ -7740,11 +7768,15 @@ def _convert(): "offset-only sync" ) - # Convert GP to XMLs + # Convert GP to XMLs. Always at offset 0.0 — the per-bar warp and + # the scalar-offset fallback both retime the PARSED Song below + # instead of baking the shift into the XML, whose negative lead-in + # times core's parse_arrangement would slice off at t>=0 (dropping + # the front of the chart). xml_paths = convert_file( gp_path, tmp, track_indices=indices, - audio_offset=0.0 if _warp_anchors else _provided_offset, + audio_offset=0.0, arrangement_names=names_map, ) @@ -7789,11 +7821,14 @@ def _convert(): start_time=float(s.get("startTime", "0")), )) - # Apply the per-bar warp: retime every note/beat/section from the - # tab's authored timeline onto the recording's (Songsterr-style - # piecewise mapping — the recording's tempo drift is followed - # instead of accumulating). + # Retime the parsed Song onto the recording's timeline. Both paths + # converted at offset 0.0 (above) and apply the shift here, in + # memory, so a lead-in's negative pre-roll survives parse. if _warp_anchors: + # Per-bar warp: retime every note/beat/section from the tab's + # authored timeline onto the recording's (Songsterr-style + # piecewise mapping — the recording's tempo drift is followed + # instead of accumulating). from lib.gp_autosync import warp_song_times, warp_time warp_song_times(song, lambda t: warp_time(t, _warp_anchors)) # The GP notation sidecars (keys tracks) were written by @@ -7803,6 +7838,11 @@ def _convert(): for _xp in xml_paths: _warp_notation_sidecar( _xp, lambda t: warp_time(t, _warp_anchors)) + else: + # Scalar-offset fallback (per-bar warp unavailable: GP5 repeats, + # degenerate/absent sync points, or an older core). Applied to + # the parsed Song so the front of the chart is never dropped. + _apply_gp_import_offset(song, xml_paths, _provided_offset) # If we have a local audio file path, copy to static if audio_path and Path(audio_path).exists(): diff --git a/tests/test_gp_import_offset.py b/tests/test_gp_import_offset.py new file mode 100644 index 0000000..cb05e54 --- /dev/null +++ b/tests/test_gp_import_offset.py @@ -0,0 +1,91 @@ +"""Regression: a GP import with an audio-sync lead-in must NOT drop the front +of the chart. + +The scalar-offset import path used to bake the offset into the converted XML +(``convert_file(audio_offset=...)``). For any lead-in the resolved offset is +negative, so the front notes land at a NEGATIVE XML time — and core's +``parse_arrangement`` slices each level at ``t >= 0``, silently discarding +them (the reported "auto-sync deleted the front 16 notes" bug). + +The fix converts at ``audio_offset=0.0`` and applies the offset to the PARSED +Song in memory via ``_apply_gp_import_offset`` (built on ``_apply_chart_offset``, +which preserves negative times). These assert the front notes survive and the +notation sidecars are retimed by the same amount. They fail on main, where +``_apply_gp_import_offset`` does not exist. +""" + +from __future__ import annotations + +import sys +from pathlib import Path +from types import SimpleNamespace as NS + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import routes # noqa: E402 +from routes import _apply_gp_import_offset # noqa: E402 + + +def _note(t): + return NS(time=t) + + +def _song(note_times): + """A minimal but structurally complete Song for _apply_chart_offset.""" + arr = NS( + notes=[_note(t) for t in note_times], + chords=[], + anchors=[], + hand_shapes=[], + phrases=[], + tones=None, + ) + return NS(arrangements=[arr], beats=[_note(0.0)], + sections=[NS(start_time=0.0)], offset=0.0, song_length=10.0) + + +def test_leadin_offset_keeps_the_front_notes_at_negative_time(): + # A 1.0s lead-in: GP-sign negation resolves to provided_offset = -1.0. + # convert_file would ADD that, so notes near t=0 go negative and were + # dropped. The in-memory shift keeps every one. + song = _song([0.0, 0.2, 1.5, 3.0]) + _apply_gp_import_offset(song, [], provided_offset=-1.0) + times = [n.time for n in song.arrangements[0].notes] + # Nothing dropped, and the front notes are preserved at negative time. + assert len(times) == 4 + assert times == [-1.0, -0.8, 0.5, 2.0] + # Beats and sections ride along; the residual chart offset is cleared. + assert song.beats[0].time == -1.0 + assert song.sections[0].start_time == -1.0 + assert song.offset == 0.0 + + +def test_positive_offset_shifts_forward_and_drops_nothing(): + song = _song([0.0, 1.0]) + _apply_gp_import_offset(song, [], provided_offset=0.5) + assert [n.time for n in song.arrangements[0].notes] == [0.5, 1.5] + + +def test_zero_offset_is_a_noop(): + song = _song([0.0, 1.0]) + _apply_gp_import_offset(song, [], provided_offset=0.0) + assert [n.time for n in song.arrangements[0].notes] == [0.0, 1.0] + # song_length and offset untouched on the no-op path. + assert song.song_length == 10.0 + + +def test_sidecars_are_retimed_by_the_same_offset(monkeypatch): + # Prove the sidecar path actually uses provided_offset (not just the notes): + # capture the mapping callable handed to _warp_notation_sidecar. + captured = {} + + def _fake_sidecar(xml_path, warp): + captured[xml_path] = warp + + monkeypatch.setattr(routes, "_warp_notation_sidecar", _fake_sidecar) + song = _song([0.0]) + _apply_gp_import_offset(song, ["a.xml", "b.xml"], provided_offset=-1.0) + assert set(captured) == {"a.xml", "b.xml"} + # convert_file ADDS the offset, so the sidecar remap is t -> t + offset. + assert captured["a.xml"](5.0) == 4.0 + assert captured["b.xml"](0.0) == -1.0