Multiprofile feature

[rjousse18]

Hey !

This is the very beginning of the multiprofile feature.

Todo : [x] Profile selection [x] Local storage with profile id [x] Profile selection at first login [] Specific-profile subtitle support

[smirgol]

Looks very promising. I'm not sure yet how the persistence of the selected profile works, though.

As for the translations, you'd need them to add them to the other languages as well - you can leave the "msgstr" empty and only have the english text on the msgid. Here's the translation for german: [COLOR orange]%s[/COLOR] (Profil wechseln)

Also, why increase the version number that much, we still have 55 numbers left in the range before we'd need to bump up the 0 to 1 :D

[rjousse18]

Yup I just changed the persistence of selected profile. Much better now.

Ok for the rest ;)

[Yavos]

The variable session_restart in api.py -> start() is obsolete now with your new code. Is the check in line 260 sufficient to replace the one in line 109? (It probably is. I'm just asking to confirm.)

[smirgol]

Hm I don't think so. You'd need to revert this line, too: https://github.com/smirgol/plugin.video.crunchyroll/blob/d77063d3b64bcf83c7b9aba33422657d6d05319a/resources/lib/api.py#L114

To explain: When you first start the app (or cleared the session cache), you need a regular login with email and password, since you don't have any session token yet that you can use or refresh.

With your change in line 114, you always force a refresh, rendering line 126 to 135 useless, since it's no longer called anywhere - besides 171, but that's a fallback, which probably could catch that, but it is a safety net for a different possible issue (refresh token no longer valid) and if we do know already that our previous call (line 155) to the endpoint could be invalid (with self.account_data.refresh_token being None as we don't have any token/account data), we should not send it in the first place

The check in 260 only checks if we do have a session already and if it has expired, for whatever reason, and it does so for the further api calls, not the initial to set up the session, which does not use this function (it's a raw self.http.request in 155). So, as said, when you login for the first time or don't have any cached account_data, you need a full login (type="access"). This is what the start() method is supposed to figure out and then call start_session with the proper type.

Thus, you need to revert 114 and we need to keep line 109 to call start_session with type="refresh" to refresh our expired token, otherwise do a full login type="access".

[rjousse18]

Fuck ! I just saw my error, I read the code wrong, sorry !

[rjousse18]

Probably better

[rjousse18]

And now it's muuuuuch better, I use Dialog for profile selection now

[rjousse18]

Okay, I added the profile selection at first login, now I think it's ready for production, the settings for preferred audio / subtitle can be released later I think

[smirgol]

Cool, thank you very much. I'll give it a spin this weekend and see how it goes. :)

[Yavos]

Making api.start() return a string instead of a bool seems kinda hacky/sloppy.

The clean way would probably be to switch to int as return type (-1 on failure, 0 or 1 on success, maybe 2 for first login).

But since the return value was always bound to be True from the code before, you could just use the False value for first login.

[rjousse18]

Yep, you're right, I changed it

[smirgol]

I took the liberty to push some smaller changes. Overall it's looking quite good. Needs a little bit more work on the details, but for now that will do it I think. :) Will test some more, but I don't think there any more showstoppers. Good job!

[smirgol]

Okay, that finally should do it.

I basically split the account data and the profile data. Both are cached in their individual files, so both can be updated individually without bothering to lose data from the other.

For the caching I've extracted the caching methods from API and added it to a common base class for ProfileData and AccountData. I'm not super happy with the multiple inheritance on the ProfileData object, but it will do the trick for now.

Finally I've renamed the properties of the ProfileData class to match the API property names, so it's easier to read.

From my point of view it can be merged now.

---

In other notes: Inheritance in python sucks. :D

[rjousse18]

Thanks for refactored my code :p

In other notes: Inheritance in python sucks. :D

Yup totally agree