feat: Load episode data on playback since we recreate a list item before starting a video

[lumiru]

In some conditions, episode data is lost on video start and video show "Title not provided".

This is based on PR #19, so everything is in commit 764c68fe3de6bd3027514ab889e073ee7cbb676a and next ones.

[smirgol]

What conditions? Just curious and would like to understand the underlying issue. Never happened to me, yet.

[lumiru]

What conditions? Just curious and would like to understand the underlying issue. Never happened to me, yet.

As far as I know, all you need is to expose the video URL outside of a list item (e.g., in favorites or in an external plugin, as I do with "resume" view I added to my kodi home with another extension) and all the video metadata will be lost when URL will be opened. It will probably cause the same issue for upnext plugin integration too.

[smirgol]

Makes sense, thank you for the context information. I thought you were talking about losing that information within the app. Never tried passing the urls to the outside of the app, so it wasn't a problem for me, yet.

[smirgol]

It's a chore to merge these, as they are all out of sync with each other and staging and reference things that are in none of the PRs. Just saying. :)

[smirgol]

It will take me a while to fix this up, there are just too many things that are broken.

I'm curious, does this work for you? Like "watched" status, soft-subs, inputstream-adaptive playback, goto episodes/series context menu, update playhead to server to crunchy, ...

Sorry if I'm sounding rude, I'm just a bit frustrated that everything is falling apart what has been working pretty much flawlessly before and it's really a lot of work to figure out why and patch things up. :|

[lumiru]

I never enabled soft-sub, so I do not know. The other features are working for me. I will take a look at this later (tonight or tomorrow). Sorry if I miss something, it is a bit tricky to work on 4 branches at the same time but I am probably faulty.

[smirgol]

I think I've patched it all up yesterday. It's the same for me, I'm juggling with branches, merges here, cherry-picks there, trying to keep it all in sync and working.

It was just that there were so many core changes that were not done by me and things got spread out to so many functions, thus when things broke I had to get into all of them in detail to find out what's wrong and why. Also, I need to test everything over and over again to make sure everything is still working as expected. That is increasingly time-consuming, thus my frustration. :)

Overall it's a good practice to test all functionality first before submitting a PR and if time goes on, try to merge important changes back into the PRs, either by merge or cherry-picking. That saves me a lot of work and time. And put a movie on your playlist, as they can easily break things because they are different than regular shows and that's easy to overlook otherwise. Thank you! :)

[smirgol]

I have rolled back the recent changes on staging, as I need more time to sort things out and make it work smoothly for everyone. Thank you for your work, though! Really appreciate it. It's just that it needs more work and until then it's better to revert the changes. I have moved the old staging to a dev branch to continue working on it.

[lumiru]

I have rolled back the recent changes on staging, as I need more time to sort things out and make it work smoothly for everyone. Thank you for your work, though! Really appreciate it. It's just that it needs more work and until then it's better to revert the changes. I have moved the old staging to a dev branch to continue working on it.

Can you say me what is not smooth? I could work on it.

[smirgol]

It's difficult to explain, but one of the major issues is with movies, which you completely ignore in your code. Once you start to add them, things just blow up literally everywhere. :D Not to speak that I would like to add the music videos at a later time, too.

Just as a single example, the Objects endpoint thinks it is a good idea to return a movie_listing object, instead of the movie object it returns everywhere else, and these are not compatible with the MovieData object. But that's just one of many issues.

Then I'm somewhat unhappy with having a mix of the old way of adding items to the views - which, while it duplicates code, is easy to read and maintain - and the new one, which are fairly abstract and hard to follow and debug. Trying to make the new ones handle everything - in an elegant way - turned out to be a big headache.

With the router I still think it is problematic to completely omit the args from the urls, as this is the only way in Kodi to pass things across pages. I did encounter issues with that, but I can't remember right now what these were. It's just, sometimes you need to pass data across pages and you can't or don't want to fetch it from the API (again). I know that this is ugly, but this is how Kodi works, as there is no persistent data storage, unfortunately.

Some of the issues I could have worked around, but workarounds makes things just ugly, so, after many hours, I decided to stop and plan out how to rewrite stuff in a proper way. But that takes time and sometimes I need to do something else than debugging. :)

Finally we have to face that we did change so much of the core-functionality and it does nothing new but to complicate things - at least for me and I'm trying to maintain it at the moment, until MrKrabat hopefully takes over again. I spent so much time on debugging and trying to find out which new function is now breaking things and why, it just wasn't very fun.

I understand that most of it is a foundation for the PlayNext and Bookmark functionality, but I think that could have been integrated in a more straight-forward way and it's not something that I personally consider high-priority.

Nevertheless your code is a good starting point to build upon it somehow, I just don't know how, yet. So I cannot tell you exactly what needs to be improved in which way exactly, I'm sorry.

Again, I really appreciate your work and don't want to discourage you, it's just that I'm not feeling happy with it at the moment and I'm trying to come up with a solution to this, based on your work.

[smirgol]

There is a new PR on the main repository for another rewrite of the addon. It makes use of a library called script.module.codequick to make things a whole lot easier, the code is pretty slick and easy to read as most parts are abstracted by the library, I really like it. I'm usually not a big fan of pulling in dependencies, but that one probably is worth a look or two. Now, if my holidays wouldn't be over next week and if the trial license of my IDE would last longer... xD

It also makes use of this @foobar annotation of python I've stumbled across a couple of days before, which I already thought could be used for setting up routing more easily.

[lumiru]

I agree with you, a full rewrite is probably a better way to do for this kind of change, and his way to do is really simpler to understand and update. My proposal was like this not to lose all your work since I am not sure what is mandatory.

[smirgol]

At this point I'm definitely not going for a full rewrite, I just wanted to mention it because I found his approach very interesting and clean and it's always nice to have a look how others are doing things. I think it has the same issue with movies, though and still lacks some functionality, but he's progressing. He is also using a lot of API calls plus the dependency, which I personally dislike, but it makes things easier, though. :)

I did a rather large rewrite today to the controller and the models, to simplify the rendering a lot. There is still much room for improvement, but it's getting better I think. Have to test a bit more before I push it to staging, though.

[smirgol]

I've merged staging into your work, as there were some conflicts due to recent changes to staging, and did a few changes. As far as I can tell from a few tests it should work flawlessly.

Two things I've changed that are worth mentioning:

• I've moved your code for loading the obj cms data to VideoStream, since this class already collects all the data needed for playback. I disabled fetching the series obj data for now, as its data isn't used at the moment.
• I've introduced the new object type PlayableItem as a common base class for the EpisodeData and MovieData
• In _prepare_and_start_playback I've copied over the playhead data from the VideoStream object to the PlayableItem that is then used to create the ListItem for the player. This is needed for resume capability: There is some "magic" going on when we only set a few things on the ListItem, like I did before your changes, as it worked before without explicitly copying over that data. But as we now set much more properties by using PlayableItem.to_item(), some info got lost that kodi somehow filled in on it's own, if not set explicitly, like the playhead data.

I think that's it. It still suffers from loading all that data when starting playback - the cms obj data tends to be slow most of the time and didn't make it any better :o) - and my current approach for async fetching isn't optimal at best. Still need to figure out how to have proper async http in kodi.

I'll test it a little bit more - please feel free to also test - and if it's okay I think we can merge it.

[smirgol]

So far, so good. I've found one thing with the routing in the GotoSeries and GotoSeasons not working and fixed that.

The latest changes also include a commit where I've cleaned up the language settings - it's required to reboot Kodi after update and re-set the language settings.

Totally unrelated, but it really bugs me that when you are opening the context menu on a playable item and select show info, while it will display the info page for the item, it will also start playing it after a few seconds. I have no idea how to stop Kodi from doing that and it really annoys me. Any idea? :)

[smirgol]

FYI: I'm removing my latest changes that are unrelated to your PR before merging and then will add them to staging again.