search

sailfishos-applications / flowplayer

Music Player for SailfishOS
https://openrepos.net/content/olf/flowplayer
Other
6 stars 7 forks source link

[Bug] Configured music directories are not scanned #64

Closed llewelld closed 9 months ago

llewelld commented 9 months ago

SailfishOS VERSION (Settings → About product → Build): 4.5.0.24
HARDWARE (Settings → About product → Manufacturer & Product name): Sony Xperia 10 II - Dual SIM
FlowPlayer VERSION (FlowPlayer → [Top pulley] Settings → [Top pulley] About): 0.3.3 (devel commit a5c5c228730c77b)

BUG DESCRIPTION

Although it's possible to configure folders in the "Manage folders" section, they're not actually scanned for music.

STEPS TO REPRODUCE

  1. Ensure that the following two files have been removed for a clean configuration: ~/.config/Unknown\ Organization.conf ~/.config/sailfishos-applications/flowplayer.conf
  2. Open the app, go to Settings > Manage folders
  3. Add a music folder to the list
  4. Pop the page back to Settings
  5. Select "Update collection"
  6. Either the app crashes, or the added music folder isn't scanned.

ADDITIONAL INFORMATION

<Please consider which other pieces of information may be relevant: Denote if this is not always reproducible, if this is a regression (then name to which older version), attach relevant data such as log files or the systemd journal, provide screenshots etc.>

The problem seems to stem from the fact that the QSettings is created statically in utils.cpp. As a consequence, the configuration file there is stored as ~/.config/Unknown\Organization.conf. However, the QSettings is created dynamically in datareader.cpp. There the configuration file is correctly read in from ~/.config/sailfishos-applications/flowplayer.conf. Since the two don't match the Folders config value is saved out to one file and read in from another, so is always empty when read in just prior to the music file search being performed.

llewelld commented 9 months ago

PR #65 demonstrates a quick fix for this (though perhaps not the most elegant solution).

dcaliste commented 9 months ago

Thank you @flypig , I was worried of static QSettings when migrating to org/app standard locations. It seems, I didn't test it enough :/ My bad. Thanks for your report and fix.

llewelld commented 9 months ago

Thanks for all of your work on this @dcaliste (alongside @Olf0, @smokku and others). This was my first time trying out the app for the newsletter and it's really nice, so I'm so happy you're rejuvenating it. But that also gave me an advantage with testing, because I think this bug is hard to sport unless it's a clean install.