search

xeruf / monsterutilities

Browse, stream and download Monstercat Songs
GNU General Public License v3.0
31 stars 2 forks source link

Add Viewer for large coverart on click #55

Closed defvs closed 5 years ago

defvs commented 5 years ago

Fixed the typo; Added caching for large file. Could do some improvements on the fuctions, but at this point other than codestyle it's ready to merge.

defvs commented 5 years ago

The already existing cover images has been renamed "thumbnail" as it is a very small 64x64 image, thus being a thumbnail. The "large" cover art is now the real cover.

defvs commented 5 years ago

Closing; Already merged in #15 because needed.

xeruf commented 5 years ago

Why needed? I don't like it if PRs grow unnecessarily big, and these seem to be two distinct features.

defvs commented 5 years ago

I needed a fix for my Playlist branch. You could still merge this before #15 if you want it now.

xeruf commented 5 years ago

I'd like to squash this PR since it's rather small with many trivial commits, but that would create conflicts with the other branch... That's why I don't like intertwining branches.

defvs commented 5 years ago

Which branch ? If it is Playlist, I'll just revert back before my merge.

defvs commented 5 years ago

Travis keeps failing, no idea why. semaphore works fine, local build too. Seems like the build cannot find "Release.Type.MIXES". Could be a conflict ? It finds all the other types fine.

xeruf commented 5 years ago

Travis keeps failing, no idea why. semaphore works fine, local build too. Seems like the build cannot find "Release.Type.MIXES". Could be a conflict ? It finds all the other types fine.

It is now called "MIX", I renamed it for consistency since everything else is in singular as well. I am actually confused why it works locally for you and on Semaphore ^^

defvs commented 5 years ago

Because it is still named "MIXES" on this branch ? We're not up to date on master. Probably because Travis merges this branch into master THEN compiles !

xeruf commented 5 years ago

Because it is still named "MIXES" on this branch ? We're not up to date on master. Probably because Travis merges this branch into master THEN compiles !

Ah right, yes, this is the PR build, which means it checks whether it works when merged.

defvs commented 5 years ago

I will merge master into it to resolve conflicts after we're done.

defvs commented 5 years ago

Merged master; ready to merge.

xeruf commented 5 years ago

Please remove the commented-out code-blocks and fix this little comment, other than that LGTM.

defvs commented 5 years ago

Yeah totally forgot about my commented block.

xeruf commented 5 years ago

There's still one ^^

defvs commented 5 years ago

Got no time till tonight. I'm still trying to figure out a way to change the filename to end with size while the extension is left untouched

xeruf commented 5 years ago

Kotlin has an extensions function to get the extension and the files's nameWithoutExtension ;)

defvs commented 5 years ago

DO NOT MERGE ! There is an issue where the program will freeze when playing a song (even when resolving conflicts)

defvs commented 5 years ago

Good lord found the issue; the merge conflict would get resolved with the Downloader getting all covers NOT AS THUMBNAILS but as full 2048x2048 covers :unamused:

defvs commented 5 years ago

Fixed it in cf9cc81b3ac8c11cc540821d41e1dada92c52c9b if you want to cherrypick into master

defvs commented 5 years ago

Ready to merge.

xeruf commented 5 years ago

I have solved the problems in "Covers" and incorporated your changes into master. It should be possible to resolve the conflicts by simply accepting my changes. please have a look at my comments nonetheless, and how I resolved them in my commit.

defvs commented 5 years ago

Good, thanks.

defvs commented 5 years ago

Wait, just understood, all the comments here, you fixed them on master ?

defvs commented 5 years ago

Merged master just fine. Thanks