(fix) prevent deletion of current History playlist after purging tracks - #15991
Conversation
This is still the thing that frustrates me the most - another instance of looking all the cleanup and prep work is a crash before the end of the session... Would be nice to look as #13001 at some point! |
| // Else the following session log is lost -- or rather not recorded in the | ||
| // first place since the id passed to appendTrackToPlaylist() does not exist | ||
| // anymore. | ||
| // skipCurrHistory prevents that. |
There was a problem hiding this comment.
Is there a case where we would want to call this method with skipCurrHistory = false? To me, it sounds like, as long as m_currentHistoryPlaylist != kInvalidPlaylistId, we wouldn't never want to delete it as itwould prevent further recording. I am missing something?
There was a problem hiding this comment.
this is also run on shutdown in order to remove history with less than N tracks. And m_currentHistoryPlaylist is still valid then.
ywwg
left a comment
ywwg
left a comment
There was a problem hiding this comment.
would it also be possible to assert / throw an error if a track is attempted to be added to the setlog and it fails? I'm not sure what to do in a release build in that case, because crashing probably wouldn't be nice.
|
|
||
| if (idStringList.isEmpty()) { | ||
| return false; | ||
| } else { |
Sure we can do that. We could create the playlist, but IIRC it requires some back'n'forth between SetlogFeature and PlaylistDAO. Will take a look. Though, this should be safe now. We have the guard in the DAO delete method and, before this is called, we also already prevent manual deletion of the current history in SetlogFeature. |
agree, but at the very least I'd like to add a debug assert if an insert into a playlist fails |
|
I added |
2262ea9 to
cb30f0c
Compare
|
Ready for merge? |
|
Oh, I targeted 2.6 only because I assumed there wouldn't be a 2.5.5 release anymore. |
|
Oh, I already worked on top of 2.5 locally |
|
hmm why did I see "2.6" up there 🤔 |
3 participants
Some time I played a practice session, with lots of cleanup and prep work.
Next time I started Mixxx that session was gone 🤔
Turns out this happens when the first track in a session is removed from disk (-> purged from library).
Fix: add a safeguard to keep the current history if the playlist cleanup method is called after purge.
Usual cleanup during shutdown and startup is not affected.