search

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.03k stars 73 forks source link

MVVM #160

Open esibruti opened 2 years ago

esibruti commented 2 years ago

What's the Problem?

We have to do a lot of work on the code part. UWP is meant to be programmed with the MVVM pattern. RiseMP, from what I have seen, is very poor on this. For maintenance it becomes much better and much easier. Many UWP applications use MVVM.

It has to be done during Alpha Preview / Alpha. A step in this direction is to from now on program / add new features using the MVVM pattern.

Solution/Idea

Ex: We have to pass the code in NowPlayingBar.xaml.cs to a ViewModel.

Alternatives

N/A

Rise Media Player Version

No response

Windows Version

No response

Comments

No response

itsWindows11 commented 2 years ago

This is an important issue to work on so I'll pin this.

We have to pass the code in NowPlayingBar.xaml.cs to a ViewModel.

That's not the only thing we should do tbh, there's a LOT more than this.

We have to do a lot of work on the code part. UWP is meant to be programmed with the MVVM pattern. RiseMP, from what I have seen, is very poor on this.

Not being rude, but @josephbeattie and @rounk-ctrl messed up on that part a bit, assuming they're both beginners.

Also btw, not everything needs to be in the MVVM pattern.

esibruti commented 2 years ago

Also btw, not everything needs to be in the MVVM pattern.

I agree with you, not everything will need to move to a ViewModel but we have to start prioritizing that because to do maintenance and add functionality, right now, it gets confusing and complicated since it is "all spread out".

This is one reason why there are so few contributors to the Rise Media Player.

YourOrdinaryCat commented 2 years ago

I also think we should debloat some of the ViewModels and outright remove a few, some do stuff that they shouldn't be responsible for and others just wrap a model without property change notifications.

There's also a lot of singletons in App.xaml.cs, some of which I think would work better as static classes.

esibruti commented 2 years ago

Wouldn't it be better to start working on these changes for the next version?

YourOrdinaryCat commented 2 years ago

I was working on improving that internally, but there's lots of code and I don't have much time. Certainly will be improved by the next release though.

esibruti commented 2 years ago

I was working on improving that internally, but there's lots of code and I don't have much time. Certainly will be improved by the next release though.

The best way to start is when we mess with parts of the code that is not in MVVM, is to do like this or even, new features and things like that is to do it right the first time