Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nexus remove deprecated function calls #54

Open
wants to merge 8 commits into
base: nexus-staging
Choose a base branch
from

Conversation

Yavos
Copy link

@Yavos Yavos commented Mar 15, 2024

I removed deprecated function calls (ListItem.setInfo() and parts of ListItem.setProperty()) while trying to catch all possible errors with this. This makes the kodi log file for debugging a little cleaner.

For more info on deprecated functions see:

Also added a language block in front of seasons that can be enabled in settings. (Disabled by default. Probably not the cleanest way to do that, but it works.)

@smirgol
Copy link
Owner

smirgol commented Mar 15, 2024

Thank you for your contribution. The problem with removing the deprecated calls is, that the methods that do replace them are Kodi 20+ only, so it won't work for matrix. It makes it much more complicated to make changes to these two versions, because as of now, I can simply cherry-pick changes from one version to the other one, which I can't when I remove the deprecated methods for Nexus. It's already a mess backporting things to Leia, but having to juggle with three different versions, I'm not super excited about that. :)

I definitely have it on my list for Omega, but as of now, it just would make things more complicated to maintain and all it would do is reduce the amount of logging.

@Yavos
Copy link
Author

Yavos commented Mar 15, 2024

I understand that reasoning. I'll integrate it into mine anyway, since I only use Nexus upwards. Except for deprecation warnings it doesn't have an influence on the current versions, so it's fine to go without.

It'll still be helpful for Omega. ;)

@@ -370,7 +370,11 @@ def __init__(self, data: dict):
super().__init__()

self.id = data.get("id")
self.title: str = '[' + data.get("audio_locale") + '] ' + data.get("title")
settings = xbmcaddon.Addon(id=re.sub(r"^plugin://([^/]+)/.*$", r"\1", sys.argv[0])).getSettings()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I personally would move that logic outside of this DTO to utils.get_listables_from_response .
Also, I think sometimes there already is a language thingy in front of the season name, not often, but sometimes it is. It might be worth checking for that with a regex and only add this if it doesn't exist already.

If you don't mind I'll cherry-pick that functionality from your code and implement it as described.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I don't mind at all.
I don't really like that functionality in there either, but I didn't figure out how to detect seasons in get_listables and that was the easiest way for me.

@smirgol
Copy link
Owner

smirgol commented Mar 15, 2024

I understand that reasoning. I'll integrate it into mine anyway, since I only use Nexus upwards. Except for deprecation warnings it doesn't have an influence on the current versions, so it's fine to go without.

It'll still be helpful for Omega. ;)

It's definitely helpful, so I'd keep that PR open if you don't mind.

A wild idea that came to my mind to have both worlds, is to extend the ListItem from Kodi with a wrapper class and use that one instead. I could still use things like e.g. setProperty but I overload the original method in the wrapper class and decide which method to call on the original, parent, object, depending on the Kodi version. Not sure if python can do that and it's just a wild idea, but I found it an interesting idea and not too difficult to implement. Have to play around with that, when I find some free time. :)

@smirgol smirgol force-pushed the nexus-staging branch 2 times, most recently from 11faf27 to e652833 Compare November 26, 2024 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants