Rise-Software / Rise-Media-Player

One media player for everything you own or stream; whether it's music or videos, online or offline Rise Media Player does it all. And it's beautiful and native with the latest version of WinUI.
GNU General Public License v3.0
1.07k stars 76 forks source link

Massive backend/indexing improvements #159

Closed itsWindows11 closed 2 years ago

itsWindows11 commented 2 years ago

Songs shouldn't duplicate anymore, and the new backend will be much easier to maintain (its just one, ONE file!).

itsWindows11 commented 2 years ago

(Proper review needed before merge, so please don't merge)

YourOrdinaryCat commented 2 years ago

Code looks good to me, although I'm unable to test it as of now. Will share any updates here as soon as I'm able to.

josephbeattie commented 2 years ago

Seems to work okay. Images for props, setup, playlists arent showing up though. Might be on my end :/

itsWindows11 commented 2 years ago

Seems to work okay. Images for props, setup, playlists arent showing up though. Might be on my end :/

I'll investigate it tomorrow, can't do anything anymore rn.

itsWindows11 commented 2 years ago

@josephbeattie every image works for me, not sure why it wouldn't work for you, perhaps you're on an old branch or you accidentally deleted the files?

image image image

(note: this result was from the indexing improvements branch, which is what this PR uses right now)

josephbeattie commented 2 years ago

That’s really strange. Haven’t changed branches and all the images are there in the files. Certain images (like the main titlebar icon and widgets icon) show up. This is weird I’m going to try to reclone

YourOrdinaryCat commented 2 years ago

In the Repository.cs file you could just use a common base class (e.g DbObject) me thinks, for upsert, delete, etc

esibruti commented 2 years ago

For my part, everything is working. Just one thing: is it possible to change the name of the Rise Media Player Dev folder to Rise.Uwp? What about Rise Media Player Dev.sln to RiseMediaPlayer.sln? I think it would be better because files and folders shouldn't contain spaces.

contributing.md -> CONTRIBUTING.md readme.md -> README.md

YourOrdinaryCat commented 2 years ago

@esibruti I do agree with you, but that'd break existing clones, so maybe at a later date.

esibruti commented 2 years ago

@esibruti I do agree with you, but that'd break existing clones, so maybe at a later date.

@YourOrdinaryCat I'm doing this test and I was able to change the names without causing any problems. I have a draft pull request that shows this

itsWindows11 commented 2 years ago

@esibruti I do agree with you, but that'd break existing clones, so maybe at a later date.

@YourOrdinaryCat I'm doing this test and I was able to change the names without causing any problems. I have a draft pull request that shows this

Well you did too many breaking changes so I'll have to close the PR to ensure compatibility with existing branches/forks.

esibruti commented 2 years ago

No problem, I was just testing to see what would happen if I did such a thing. As it says there, just for testing

YourOrdinaryCat commented 2 years ago

@itsWindows11 was trying to improve GetListsAsync, but I'm running into an exception. It's in the songs page code and related to AdvancedCollectionView, so it'll take a while to get fixed. Should we merge this now? None of the commits up until this point are problematic I think, and most of the new stuff seems to be finished.

itsWindows11 commented 2 years ago

@itsWindows11 was trying to improve GetListsAsync, but I'm running into an exception. It's in the songs page code and related to AdvancedCollectionView, so it'll take a while to get fixed. Should we merge this now? None of the commits up until this point are problematic I think, and most of the new stuff seems to be finished.

Okies, I could merge.