Fix song position clamping when song length decreases

Author: juzzlin

CHANGELOG

@@ -29,6 +29,8 @@ New features:
 
 Bug fixes:
 
+* Fix song position clamping when song length decreases
+
 Other:
 
 * Update user manual with pan column documentation

src/application/service/editor_service.cpp

@@ -2726,7 +2726,7 @@ void EditorService::setSongLength(quint64 songLength)
         emit songLengthChanged();
         updateDuration();
         if (songPosition() >= m_song->length()) {
-            setSongPosition(songPosition() - 1);
+            setSongPosition(m_song->length() > 0 ? m_song->length() - 1 : 0);
         }
         setIsModified(true);
     }

src/unit_tests/editor_service_test/editor_service_test.cpp

@@ -1835,6 +1835,31 @@ void EditorServiceTest::test_midiNotesAtPosition_shouldReturnCorrectNotes()
     }
 }
 
+void EditorServiceTest::test_setSongLength_clampingPosition_shouldClampCorrectly()
+{
+    EditorService editorService { std::make_shared<SelectionService>(), std::make_shared<SettingsService>(), std::make_shared<AutomationService>(std::make_shared<PropertyService>()), std::make_shared<DataService>() };
+
+    editorService.setSongPosition(0);
+    editorService.insertPatternToPlayOrder(); // Now length is 2
+    editorService.insertPatternToPlayOrder(); // Now length is 3
+    editorService.insertPatternToPlayOrder(); // Now length is 4
+
+    QCOMPARE(editorService.songLength(), 4);
+
+    editorService.setSongPosition(3); // Go to last position (3)
+    QCOMPARE(editorService.songPosition(), 3);
+
+    // Set song length to 2.
+    // If the clamping bug exists, this will set song length to 2, then set position to 2,
+    // which triggers extension back to 3!
+    editorService.setSongLength(2);
+
+    // The song length should be 2.
+    QCOMPARE(editorService.songLength(), 2);
+    // The song position should be clamped to 1.
+    QCOMPARE(editorService.songPosition(), 1);
+}
+
 } // namespace noteahead
 
 QTEST_GUILESS_MAIN(noteahead::EditorServiceTest)

src/unit_tests/editor_service_test/editor_service_test.hpp

@@ -119,6 +119,7 @@ private slots:
 
     void test_velocityAtPosition_shouldReturnCorrectVelocity();
     void test_midiNotesAtPosition_shouldReturnCorrectNotes();
+    void test_setSongLength_clampingPosition_shouldClampCorrectly();
 };
 
 } // namespace noteahead

src/unit_tests/song_test/song_test.cpp

@@ -1208,6 +1208,35 @@ void SongTest::test_removePatternFromPlayOrder_shouldDecreaseLengthByOne()
     QCOMPARE(song.patternAtSongPosition(0), 1);
 }
 
+void SongTest::test_deleteUnusedPatterns_recreatedPatternHasNoNoteData()
+{
+    Song song;
+    song.createPattern(1);
+    song.createPattern(2);
+    QCOMPARE(song.patternCount(), 3);
+
+    Position pos { 2, 0, 0, 0 }; // pattern 2, track 0, column 0, line 0
+    NoteData note;
+    note.setAsNoteOn(60, 100);
+    song.pattern(2)->setNoteDataAtPosition(note, pos);
+    QVERIFY(song.pattern(2)->hasData());
+
+    song.setLength(2);
+    song.setPatternAtSongPosition(0, 0);
+    song.setPatternAtSongPosition(1, 1);
+
+    // Pattern 2 is unused, 0 and 1 are used
+    song.deleteUnusedPatterns();
+
+    QCOMPARE(song.patternCount(), 2);
+    QVERIFY(!song.hasPattern(2));
+
+    // Recreate pattern 2
+    song.createPattern(2);
+    QVERIFY(song.hasPattern(2));
+    QVERIFY(!song.pattern(2)->hasData()); // This should be empty, without the note data!
+}
+
 } // namespace noteahead
 
 QTEST_GUILESS_MAIN(noteahead::SongTest)

src/unit_tests/song_test/song_test.hpp

@@ -78,6 +78,7 @@ private slots:
 
     void test_deleteUnusedPatterns_shouldRemoveUnusedPatterns();
     void test_deleteUnusedPatterns_skippedButUsed_shouldNotBeRemoved();
+    void test_deleteUnusedPatterns_recreatedPatternHasNoNoteData();
     void test_hasPatternInPlayOrder_shouldReturnCorrectValue();
     void test_duration_allPatternsSkipped_shouldReturnZero();