Describe the bug
AlbumCoverManager::SaveCoverToFile() seems not to be working as intended. It relies on the following loop that iterates over the cover_types_ list which is initialized as cover_types_ = AlbumCoverLoaderOptions::LoadTypes();
Song song = GetSingleSelectionAsSong();
if (!song.is_valid() || song.art_unset()) return; // <-- checking song.art_unset()) here causes undesired behavior in the loop below
for (const AlbumCoverLoaderOptions::Type cover_type : std::as_const(cover_types_)) {
switch (cover_type) {
case AlbumCoverLoaderOptions::Type::Unset:
return; // <-- song.art_unset() should be checked here instead to avoid early exit
The problem is that AlbumCoverLoaderOptions::LoadTypes() always loads the default full list of types, in this order:
AlbumCoverLoaderOptions::Type::Unset
AlbumCoverLoaderOptions::Type::Embedded
AlbumCoverLoaderOptions::Type::Manual
AlbumCoverLoaderOptions::Type::Automatic
This list is the default when CoversSettings::kSettingsGroup\CoversSettings::kTypes is checked. However, this setting never
seems to be set anywhere in the Strawberry code, and therefore always the same default list is returned.
In the above switch in the for loop the Type::Unset case is always met first, causing the function as a whole to always exit early, doing nothing else.
Incidently, the same kind of logic seems to be applied in AlbumCoverChoiceController::ShowCover():
for (const AlbumCoverLoaderOptions::Type type : std::as_const(cover_types_)) {
switch (type) {
case AlbumCoverLoaderOptions::Type::Unset:{
if (song.art_unset()) {
return;
}
...
}
...
}
The difference between both loop implementations is that in the latter song.art_unset() is checked within the case label, while in the former,
this is done at the top of the function if (!song.is_valid() || song.art_unset()) return;, which seems to be what causes the unintended behavior:
the song.art_unset() check should also be performed within the case section, to avoid early exit.
To Reproduce
Open the album cover manager, select a cover and choose Save cover to file. Nothing happens.
Expected behavior
The selected cover should be saved to file.
Screenshots:
If applicable, add screenshots to help explain your problem.
System Information:
- Operating system: Debian 13 (Trixie)
- Strawberry Version: Version 1.2.18-3-g7bf1bc0c
Additional context
Add any other context about the problem here.
Describe the bug
AlbumCoverManager::SaveCoverToFile() seems not to be working as intended. It relies on the following loop that iterates over the
cover_types_list which is initialized ascover_types_ = AlbumCoverLoaderOptions::LoadTypes();The problem is that
AlbumCoverLoaderOptions::LoadTypes()always loads the default full list of types, in this order:This list is the default when
CoversSettings::kSettingsGroup\CoversSettings::kTypesis checked. However, this setting neverseems to be set anywhere in the Strawberry code, and therefore always the same default list is returned.
In the above
switchin theforloop theType::Unsetcase is always met first, causing the function as a whole to always exit early, doing nothing else.Incidently, the same kind of logic seems to be applied in
AlbumCoverChoiceController::ShowCover():The difference between both loop implementations is that in the latter
song.art_unset()is checked within thecaselabel, while in the former,this is done at the top of the function
if (!song.is_valid() || song.art_unset()) return;, which seems to be what causes the unintended behavior:the
song.art_unset()check should also be performed within thecasesection, to avoid early exit.To Reproduce
Open the album cover manager, select a cover and choose
Save cover to file. Nothing happens.Expected behavior
The selected cover should be saved to file.
Screenshots:
If applicable, add screenshots to help explain your problem.
System Information:
Additional context
Add any other context about the problem here.