search

Rachel030219 / Poweramp-LRC-Plugin

An app to implement local .lrc file support for Poweramp.
Other
80 stars 4 forks source link

Bugfix: crash when embedded lyrics is enabled, but no embedded lyrics or lrc file #30

Closed fei0316 closed 3 years ago

fei0316 commented 3 years ago

I fixed the bug I mentioned in my comment yesterday. Again it really is just a fix to make the app at least working.

If I understand correctly, most of the file-finding features in your code revolves around the embedded boolean variable. If embedded is enabled, then it finds music files; otherwise it finds lrc files. Since we now want to find music files with embedded enabled, I can't reuse the code designed to do exactly that.

To implement the lrc fallback feature in a cleaner way, I think a better way would be to add two helper methods to readFile, one for lrc and one for tag. Then when you call readFile (with embedded enabled), it would first call the tag helper method, and if nothing is found, it calls the lrc helper method; if embedded is disabled, it always looks for lrc files.

What do you think about this?

Rachel030219 commented 3 years ago

Correct, but actually I'm rearranging the codes locally (which has not been committed yet), so… thanks for your devotion, but sorry that I may have to close this PR, and that I doesn't have much time recently, making the rearrangement process a little long.