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

Feature: Show current episode in season list and mark current episode in episode list #1667

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Falke-Design
Copy link
Contributor

Check if this PR fulfills these requirements:

Types of changes

  • New feature (non-breaking change which adds functionality)
  • Feature change (non-breaking change which changes behaviour of an existing functionality)
  • Improvement (non-breaking change which improves functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactor (non-breaking performance or readability improvements)

Description

Motivation: It would be very nice if it would be possible to resume the current episode or start the next episode of a tv show from the start screen / tv show overview. Like on the Netflix website. Currently I always need to remember at which season I'm and then I need to scroll down to select the episode after the last watched one.

In case of Feature change / Breaking change:

Describe the current behavior

  1. select tv show
  2. season list is visible
  3. select season in season list
  4. episode list is visible
  5. scroll manually down to the last watched episode

Describe the new behavior

  1. select tv show
  2. season list and the current episode is visible
  3. current epsiode can be started directly or season can be selected
  4. episode list is visible
  5. scroll manually down to the episode of which the title has a color
  • Showing the current epsiode in the season list can be disabled via settings. Default: true
  • The color of the current episode in the episode list can be changed / disabled via settings. Default: Yellow
  • For tv shows with only one season the current episode will be not added in the season list (because there is no season list, opens directly the episode list)
  • The current episode will be marked in each season, because Netflix has for each season a last watched episode.
  • If the current episode is not playable, then it will be not added to the season list
  • If there is no current episode, Netflix automatically returns the first episode of the first season.

Screenshots (if appropriate):

Current episode in season list:
grafik

Colorized title of the current episode:
grafik

Settings:
grafik

Netflix:
grafik

grafik

Open tasks:

@CastagnaIT I need some help with the following points and the commented code sections:

  • For the current episode in the season list, I added hard-coded the season and episode number. Kodi adds from it self the number in the episode list. 4x08. is that ok?
  • Translation: How does it work with the translation? I took the unused numbers 30217 and 30218 and only added the text in the en_gb translation. Is that ok or does every translation file need the new entries?
  • In __init__ of SeasonList I added new lines. But I don't know if there is a cleaner way.
  • I have not added a cache annotation to req_current_episode. I'm not sure if this is needed

Comment on lines +178 to +181
if 'current' in source:
path = reference_path(source['current'])
if path is not None:
yield ('current', path)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added current to iterate_references

@@ -255,6 +255,8 @@ def __init__(self, videoid, path_response):
self.tvshow = self.data['videos'][self.videoid.tvshowid]
self.seasons = OrderedDict(
resolve_refs(self.tvshow['seasonList'], self.data))
if 'current' in self.tvshow['seasonList']:
self.current_seasonid = videoid.derive_season(self.tvshow['seasonList']['current']['value'][1])
Copy link
Contributor Author

@Falke-Design Falke-Design Jan 3, 2024

Choose a reason for hiding this comment

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

Is getting the value like self.tvshow['seasonList']['current']['value'][1] ok?

"seasonList": {
                    "current": {
                        "$type": "ref",
                        "value": [
                            "seasons",
                            "81370447"
                        ]
                    }
                }

@CastagnaIT
Copy link
Owner

I had thought about this in the past and intentionally avoided doing anything like this,
because this require relying on a specific API path of the website and you need also that the request you are doing to website will be answered with updated data immediately, and this is unreliable

1° big problem:
(maybe today website have better data sync between servers but...i wouldn't rely too much for experience... and api path changes by website over the time are vicious beasts that may break easily things)
Here you are doing a pathEvaluator request to know the current episode, but the pathEvaluator "server" may not provide always instantly updated data, in the past to do this was required to do a "callPath" request to a specific list in use, this was a problem because the request did not always end successfully (and i could never get it to work correctly, now has been removed from addon code).
This particular callPath must be done after the STOP request of MSL events, and with this use case must that happens also before that kodi update the GUI and request to the addon to provide the season list, but MSL events are asynchronous and i dont think this will be possible, not easily, so will always be a race condition that lead to inconsistent data.

2° big problem:
Netflix dont have a real watched status that means "full watched" use a point timestamp for credits,
yes let's say most of the time it works, and mark as "completed" and it goes to the next episode correctly, but,
not few times it happens that this does not happen, and it marks as an unfinished episode.
This will cause a nice problem for this feature, because the episode in the season list will be stuck to show always the same episode, and so you are forced to open the specific season list and find/play manually to the next episode each time this happens..

in the past i have tried also an alternative way by using the metadata api to do something like this, but its the same problem no fresh data and the problem 2 is still here

3°:
this addon works even without synchronization with the netflix server, and has not been considered

these problems will not make this feature so reliable
users will report the above problems that we cannot fix without introducing some kind of workarounds
i appreciate the attempt but i am not convinced of this PR,
good to see that you are making progress in understanding how it works

other side notes despite all:

  • I would not introduce new color settings, if needed exists "select" listitem property to highlight a listitem
  • as you suspected i confirm hard-coded things are usually bad (not always), but the hardcoded season and episode number may not like users, atm idr if may be customized by some kodi setting or skin

@Falke-Design
Copy link
Contributor Author

Thank you for taking the time to answer. I understand that this will add some complexity but I really miss this feature. Add added my thoughts:

because this require relying on a specific API path of the website

.. and api path changes by website over the time are vicious beasts that may break easily things

Yes this could be a problem, but is it not the same with all the other api paths? All the paths are hard coded and only the video_id / list_id are changed for the request.

1° problem:

This particular callPath must be done after the STOP request of MSL events, and with this use case must that happens also before that kodi update the GUI and request to the addon to provide the season list, but MSL events are asynchronous and i dont think this will be possible, not easily, so will always be a race condition that lead to inconsistent data.

Okay I understand that refreshing the GUI with the new episode can be a problem. A (not perfect) solution could be that we remove the current episode data after starting the video. If we are to late with loading the current episode, because of asynchronous, nothing will be shown and if we get the current data fast enough, it will be displayed correctly.

instantly updated data, in the past to do this was required to do a "callPath" request to a specific list in use

For my tests i never had a problem with pathEvaluator but I understand that it could me a problem over time. Could it work if we are only showing and requesting the current episode for the "Continue Watching" list? Each tv show which is in the continue watching list will be updated.

2° problem:

Netflix dont have a real watched status that means "full watched" use a point timestamp for credits,
yes let's say most of the time it works, and mark as "completed" and it goes to the next episode correctly, but,
not few times it happens that this does not happen, and it marks as an unfinished episode.

But this will be the same problem on the official netflix website. It shows the same video on the netflix website and on kodi (watched but maybe not completed), so we can argue that this are the provided data from netflix and the same behavior.

This will cause a nice problem for this feature, because the episode in the season list will be stuck to show always the same episode, and so you are forced to open the specific season list and find/play manually to the next episode each time this happens..

Currently the user always don't know in which season and at which episode he is. So it will be still better to see the watched episode with the season / episode number in the title then having no information. And after starting a new episode it will work again.

3° problem:

this addon works even without synchronization with the netflix server, and has not been considered

Okay I didn't know that. Is synchronization state detectable? If yes, we could hide the current episode if synchronization is not working.

I would not introduce new color settings, if needed exists "select" listitem property to highlight a listitem

Yes this is much better. But then it is not possible to disable it.

hardcoded season and episode number may not like users

Yes that was my thought too, but I added it because it is much more helpful to have the season and episode if something is wrong or the episode is already watched, then if you need to start searching in which season the next episode will be.

i appreciate the attempt but i am not convinced of this PR

Thank you for the response. My first goal was to understand the code better and I will use the feature anyway for my self, so it not a problem if it will be not merged. But maybe with some modifications it will work for the offical addon too 😄

@CastagnaIT
Copy link
Owner

Yes this could be a problem, but is it not the same with all the other api paths? All the paths are hard coded and only the video_id / list_id are changed for the request.

to get a list data you can re-adapt the code if changed, here you strictly depend on "current" value of seasonList,
so if website dont provide anymore the "current" value, will break this feat permanently, that's what i meant

Okay I didn't know that. Is synchronization state detectable? If yes, we could hide the current episode if synchronization is not working.

There is the addon setting to enable/disable the watched status sync with website
and this change also the behaviour in how will be set the watched status on each video listitem

I'm blocking the PR for now, not closing it because when i will have time i want to understand and test better

@CastagnaIT CastagnaIT added the Don't merge PR that should not be merged (yet?) label Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Don't merge PR that should not be merged (yet?)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants