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

Fix #539 "cast not available" #731

Merged
merged 27 commits into from Mar 24, 2024
Merged

Conversation

GregoireDruant
Copy link
Contributor

@GregoireDruant GregoireDruant commented May 6, 2023

Closes #539 "cast not available" message when viewing tvshow or episode info in kodi.
Works with add-on mode and native mode as well.

The fix only consist in setting the mediatype and scrapper for each path entry in kodi database.
Normally only parent paths have this information, but jellyfin-kodi does not set parentPathIds.
Also I tryed setting the parentPathIds but this does not fix the "cast not available" message if the parent path is not a regular folder (does not work with plugin://).

@oddstr13
Copy link
Member

oddstr13 commented May 6, 2023 via email

@GregoireDruant
Copy link
Contributor Author

Sorry there is an issue with this fix, regarding the information pannel for a tvshow episode.
If you put the cursor on an episode item, long press enter and chose information, it works.
However the information panel won't show up in these cases:

  • click on the episode when the default action is show information
  • if the default action on video item is "chose", chosing information from the menu that shows up when clicking on an episode
  • on a pc, type "i" when the cursor is on an episode.

If I remove the value "metadata.local" from the scrapper column, the info panel works again but without cast information.

I know how to fix this in native mode but not sure I can fix it in add-on mode.

Anyway I will commit a proper fix for native mode, that does not break anything in add-on mode.

I will do so by setting the path trail from episode to top parent path and set the mediatype and scrapper to the top parent path only, as kodi does.

Also do not set content and scrapper from path not being top paths
Does not seem to harm anything even if it does not fix the infos
@GregoireDruant
Copy link
Contributor Author

GregoireDruant commented May 7, 2023

This is the proper fix, works perfectly in native mode, but does not fix add-on mode.

Regarding add-on mode,
PlexKodiConnect seems to have a fix also for add-on mode, here: https://github.com/croneter/PlexKodiConnect/blob/python3-beta/resources/lib/itemtypes/tvshows.py#L428

I neither use Plex nor PlexKodiConnect but I think the same approach could be used, especially since I found out that in add-on mode context menus act strange, even with the latest release of jellyfin-kodi.

Not sure if I'll be able to check that this week, I will try when I have some time.

Context menus are working again
Still no cast information
Issue with idParentPath to fix
tvshow path to check
Hacky, to be tested
@GregoireDruant
Copy link
Contributor Author

GregoireDruant commented May 13, 2023

After a lot of time spent to search how to make every kodi menu work in addon mode - for tvshows, which are the only videos that was not working ok - , the only way I cloud achieve it is by creating a top path entry with strPath = "plugin://plugin.video.jellyfin/" and strContent = tvshows, scrapper = metadata.local.

I found in the commits history that it used to be done like that but it was changed by Matt in in 2019, when he added the libraryId in the paths. I could not figure out why.

I did not remove his changes but I re added the top path with the plugin address and it seems to be working fine now.

So this PR includes fixes for both the add-on mode and native mode.

@GregoireDruant
Copy link
Contributor Author

I have been using this for 3 weeks both in native mode on a nvidia shield and in add-on mode on a windows pc, it has been working without issue.

Beside @oddstr13, can anyone review this PR ?

INSERT OR REPLACE does not work when null values are provided as part of the unique index
Which caused new videos not to be added to library on automatic update
@GregoireDruant
Copy link
Contributor Author

GregoireDruant commented Jun 13, 2023

I just added two new bugfixes to the music video library.
These are not related to bug 539, but since I already have this PR pending I figured I might as well commit here.

  • Artist link to a music video was inserted using "insert or update", passing values that did not constitute an unique key, resulting in duplicate links (and appearing like duplicate videos in db, at least with my kodi settings).
  • In musicvideos.py, obj['LibraryId'] and obj['LibraryName'] was incorrectly set, when they were correctly set a few lines before.
    I removed the problematic lines of code (see diff).
    This fixes automatic updates of the music video library, new videos were not added before this change.

@oddstr13
Copy link
Member

I (and the Jellyfin project) has a strong preference for smaller, easier to review PRs, so please create another PR for the unrelated changes 🙂

I'm done with school for the summer, so I'll hopefully have more time for this project going forward

@@ -71,6 +71,15 @@
except TypeError:
return

def get_episode_kodi_parent_path_id(self, *args):

Check notice

Code scanning / CodeQL

Explicit returns mixed with implicit (fall through) returns Note

Mixing implicit and explicit returns may indicate an error as implicit returns always return None.
@GregoireDruant
Copy link
Contributor Author

I (and the Jellyfin project) has a strong preference for smaller, easier to review PRs, so please create another PR for the unrelated changes 🙂

I'm done with school for the summer, so I'll hopefully have more time for this project going forward

I just reverted these commits and created PR #742

@sonarcloud
Copy link

sonarcloud bot commented Jun 13, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@codecov
Copy link

codecov bot commented Aug 27, 2023

Codecov Report

Attention: 29 lines in your changes are missing coverage. Please review.

Comparison is base (e95e8d6) 21.51% compared to head (68b3e8d) 21.50%.
Report is 24 commits behind head on master.

Files Patch % Lines
jellyfin_kodi/objects/tvshows.py 0.00% 23 Missing ⚠️
jellyfin_kodi/database/jellyfin_db.py 16.66% 5 Missing ⚠️
jellyfin_kodi/objects/kodi/kodi.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #731      +/-   ##
==========================================
- Coverage   21.51%   21.50%   -0.02%     
==========================================
  Files          63       63              
  Lines        8542     8575      +33     
  Branches     1572     1575       +3     
==========================================
+ Hits         1838     1844       +6     
- Misses       6680     6707      +27     
  Partials       24       24              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sonarcloud
Copy link

sonarcloud bot commented Sep 23, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link

sonarcloud bot commented Dec 16, 2023

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@oddstr13 oddstr13 added enhancement New feature or request ux User experience related issues labels Feb 6, 2024
@oddstr13
Copy link
Member

@mcarlton00 said in chat that this PR functionally is good to go, and I think the styling lints are fine as is, considering the fact that that's how the old code base has been doing it, and as such is present all over these files.

@oddstr13 oddstr13 merged commit 890d54c into jellyfin:master Mar 24, 2024
11 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ux User experience related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Actor information and thumbnails not displayed on video Information Pages for Jellyfin library
3 participants