Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
52 changes: 46 additions & 6 deletions routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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,
)

Expand Down Expand Up @@ -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
Expand All @@ -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():
Expand Down
91 changes: 91 additions & 0 deletions tests/test_gp_import_offset.py
Original file line number Diff line number Diff line change
@@ -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
Loading