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

[video] Fix playback of multi-version movies. Do not prompt for versi… #24327

Merged
merged 2 commits into from
Dec 26, 2023

Conversation

ksooo
Copy link
Member

@ksooo ksooo commented Dec 26, 2023

…on to play, as we already know it from the context of the info dialog.

Fixes #24322

In the context of the info dialog, we always display information for a certain version. No need to ask which one to play.

@jurialmunkey fyi

Runtime-tested on macOS, latest Kodi master.

@enen92 should be simple to review. We have an item property to control whether version chooser will be used.

…on to play, as we already know it from the context of the info dialog.
@ksooo ksooo added this to the Omega 21.0 Beta 3 milestone Dec 26, 2023
@ksooo ksooo requested a review from enen92 December 26, 2023 11:54
Copy link
Member

@enen92 enen92 left a comment

Choose a reason for hiding this comment

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

Thanks much

@ksooo ksooo merged commit 0adbbbc into xbmc:master Dec 26, 2023
2 checks passed
@ksooo ksooo deleted the video-info-dialog-fix-video-version-playback branch December 26, 2023 21:36
@CrystalP
Copy link
Contributor

Does what it says but maybe a bit premature, as versions are not independent movies (yet).

In the context of the info dialog, we always display information for a certain version. No need to ask which one to play.

Not in my tests. The information displayed is that of the "root" movie that versions were added to.
Versions can only have individual artwork at this point, but it is not used in the info dialog (tested by changing the poster from default on all versions of a movie).

@ksooo
Copy link
Member Author

ksooo commented Dec 26, 2023

Does what it says but maybe a bit premature, as versions are not independent movies (yet).

Step by step, in small chunks, okay? Otherwise we end up with monster PRs. The fix is needed as of now. Everything else can be fixed separately. Logically the info dialog displays for a certain version already. There are bugs (e.g. artwork) and due to the broken data model no real metadata per version. But this is where we are heading to.

@ksooo
Copy link
Member Author

ksooo commented Dec 26, 2023

Versions can only have individual artwork at this point, but it is not used in the info dialog (tested by changing the poster from default on all versions of a movie).

Please open an issue for the artwork problem.

@jurialmunkey
Copy link

I appreciate the quick fix. Behaviour is much improved.

I'm assuming a subsequent PR will change where select appears to make it consistent with how Action(Info) behaves? i.e.
Click movie > Opens default version info > Click play > Select version

Can see how this change sets up future possibilities for toggling whether info asks to select or shows default, so I'm not much fussed about the order itself, only the inconsistency between an info key press and select as info key press.

Very much low priority, but thought it worth mentioning. Can open a new issue for this if you'd like?

@ksooo
Copy link
Member Author

ksooo commented Dec 27, 2023

Write an issue, please. So we don’t forget.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants