search

smirgol / plugin.video.crunchyroll

Watch videos from the anime platform Crunchyroll.com on Kodi
GNU Affero General Public License v3.0
49 stars 10 forks source link

Multi-profiles support #62

Closed rjousse18 closed 5 months ago

rjousse18 commented 5 months ago

Hi ! Crunchyroll just added the multi-profile support, just like Netflix. Is it possible to support this feature ? Atm, the addon work with the default profile but we can't change the profile manually.

rjousse18 commented 5 months ago

It seems there is an endpoint to get the profiles

image

325205260-82b8ec4a-0611-4455-9a85-4a5ede7f60ce

smirgol commented 5 months ago

When I saw that they have dropped that feature, I was asking how long till somebody requests that as a feature. :) I'll look into it, sooner or later. :)

rjousse18 commented 5 months ago

Okay, I'm working on the implementation, I'm not python-dev, but I'm on good progress, I will open PR soon

smirgol commented 5 months ago

Cool. To elaborate on why I don't jump on this new feature right away is that it requires some work to make it look nice.

When I think about profile selection, I think about like e.g. Netflix is doing it, showing you a "splash screen" with nice icons and stuff, where you select your profile (if your account has more than 1 profile, otherwise why bother?). That requires a custom window and I'm not very familiar with this stuff yet (plus it is awfully documented), so it requires quite some fiddling.

The rest, like fetching the profile data, storing it and use the stored profile to fetch the usual stuff, that's the easy part. :)

rjousse18 commented 5 months ago

Yup I noticed the same things, but, in my side, the feature dev is in the good way, probably not optimized because I'm not familiar with python dev and kodi addon dev but should be enough for now, I hope I will post PR today or tomorrow

smirgol commented 5 months ago

There's one more thing, I'm curious about your solution to that: Right now we completely ignore the subtitle language setting provided by crunchyroll. I actually don't know why, could be a legacy thing from the past. With the profiles, you now can have multiple profiles using different subtitle languages. Ignoring those would be bad in that case, but not ignoring them would make the subtitles setting in the addon obsolete (maybe the fallback language could be kept). I'm unsure what to do about that and am curious about how you solve that. :) Take your time, though, no need to hurry. :)

Meanwhile I did some refactoring to finally get rid of the API and Args being passed around everywhere, but I'll wait with merging, so I don't mess up your branch with conflicts. ;)

Yavos commented 5 months ago

Don't want to open a new issue but want to discuss 2 points in the current state of this implemented feature.

  1. Is it useful to use the wallpaper at all? When the plugin finds a wallpaper in the profile settings it apparently will be shown instead of the avatar in the profile selection dialog. That's not very useful as it's just a wide background picture. After commenting out all the wallpaper lines in model.py it showed the avatar for me, which seems way more plausible to me.

  2. After selecting the profile it will show the username (used for forums) instead of the profile_name. For me it appears more useful to use the profile name for the change profile menu item as these 2 can differ and profile names aren't globally unique while usernames are. What do you guys think?

rjousse18 commented 5 months ago

Mmmmmh that's right, the wallpaper is not useful, I used the wallpaper in the beginning of my work on this feature, but now, with the use of Dialog, the wallpaper is useless, we can remove it.

Yep, totally agree for profile_name, need to change that, I can make a PR tonight if no one does it before

smirgol commented 5 months ago

I'd rather keep the code for the wallpaper, it doesn't hurt and if I ever find the patience to fiddle with the XML Profile Selection, I'd need that.

As for the issue with the wallpaper showing instead of the avatar: I don't have that issue. for me it shows the Avatar picture. Not sure if this is skin related. I've commented out the fanart and poster arts for the listitem.

As for the username, good point. I've changed that.

Yavos commented 5 months ago

My main profile doesn't have a wallpaper set (doesn't show up in the json either), so it's working without issues there. But I set up a second profile with wallpaper and there I had that issue.

smirgol commented 5 months ago

closed as implemented in staging branches. thank you rjousse18 for your work!