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.

Sign up for GitHub

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

feat: add player MarkVideoAsWatched action #750

Merged
merged 1 commit into from
Dec 11, 2024
Merged

feat: add player MarkVideoAsWatched action #750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 2 additions & 13 deletions src/models/meta_details.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,20 +123,9 @@ impl<E: Env + 'static> UpdateWithCtx<E> for MetaDetails {
is_watched,
))) => match (&self.library_item, &self.watched) {
(Some(library_item), Some(watched)) => {
let mut watched = watched.to_owned();
watched.set_video(&video.id, *is_watched);
let mut library_item = library_item.to_owned();
library_item.state.watched = Some(watched.into());
if *is_watched {
library_item.state.last_watched =
match (&library_item.state.last_watched, &video.released) {
(Some(last_watched), Some(released)) if last_watched < released => {
Some(released.to_owned())
}
(None, released) => released.to_owned(),
(last_watched, _) => last_watched.to_owned(),
};
}
library_item.mark_video_as_watched::<E>(watched, video, *is_watched);

Effects::msg(Msg::Internal(Internal::UpdateLibraryItem(library_item)))
.unchanged()
}
Expand Down
53 changes: 27 additions & 26 deletions src/models/player.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,31 @@ impl<E: Env + 'static> UpdateWithCtx<E> for Player {
}))
.unchanged()
}
Msg::Action(Action::Player(ActionPlayer::MarkVideoAsWatched(video, is_watched))) => {
match (&self.library_item, &self.watched) {
(Some(library_item), Some(watched)) => {
let mut library_item = library_item.to_owned();
library_item.mark_video_as_watched::<E>(watched, video, *is_watched);

Effects::msg(Msg::Internal(Internal::UpdateLibraryItem(library_item)))
.unchanged()
}
_ => Effects::none().unchanged(),
}
}
Msg::Internal(Internal::LibraryChanged(_)) => {
let library_item_effects = library_item_update::<E>(
elpiel marked this conversation as resolved.
Show resolved Hide resolved
&mut self.library_item,
&self.selected,
&self.meta_item,
&ctx.library,
);

let watched_effects =
watched_update(&mut self.watched, &self.meta_item, &self.library_item);

library_item_effects.join(watched_effects)
}
Msg::Internal(Internal::StreamsChanged(_)) => {
stream_state_update(&mut self.stream_state, &self.selected, &ctx.streams)
}
Expand Down Expand Up @@ -1052,10 +1077,7 @@ fn library_item_update<E: Env + 'static>(
meta_request: Some(meta_request),
..
}) => {
let library_item = library_item
.as_ref()
.filter(|library_item| library_item.id == meta_request.path.id)
elpiel marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

@elpiel elpiel Dec 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

  • Before removing this we need to know why it was here in the first place and if this case still holds.

Please elaborate

Copy link
Member

Choose a reason for hiding this comment

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

Taking a second look after discussing the flow with Tim, I think this should be fine.
THe LibraryItem form the LIbrary will be the most up-to-date library item as we send any update to be updated from the Ctx

Copy link
Contributor

Choose a reason for hiding this comment

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

for the first 90 seconds of the Player lifecycle a library_item exists only in the Player, but not in ctx.library. (if you watch a movie for the first time ever)
All changes made to that temp library_item during those 90 seconds are gonna be lost. (in case youve marked video as watched)

Copy link
Contributor

@nklhtv nklhtv Dec 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

also self.push_library_item_time should be reset on Unload i think.
Perhaps you get library item from the ctx.library every time bc you dont reset push_library_item_time, which causes the library_item to be persisted even on the first TimeChanged action.
This is a case which i think we wanted to avoid, watching less than 90 seconds of stream should not end up in your library(only as temp library_item)

Copy link
Member

Choose a reason for hiding this comment

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

And "only as temp" meaning "existing only in the player until those 90 seconds end"?

At which point it will place the library in your Library Bucket.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, the library_item exits in the player for the first 90 seconds (or until you dispatch some action that triggers UpdateLibraryItem, like markvideoaswatch etc)
but with this change it recreates library_item from meta_item every time, until its not actually persisted in the library bucket

.or_else(|| library.items.get(&meta_request.path.id));
let library_item = library.items.get(&meta_request.path.id);
let meta_item = meta_item.as_ref().and_then(|meta_item| match meta_item {
ResourceLoadable {
content: Some(Loadable::Ready(meta_item)),
Expand All @@ -1077,28 +1099,7 @@ fn library_item_update<E: Env + 'static>(
}
_ => None,
};
if *library_item != next_library_item {
elpiel marked this conversation as resolved.
Show resolved Hide resolved
let update_library_item_effects = match &library_item {
Some(library_item) => Effects::msg(Msg::Internal(Internal::UpdateLibraryItem(
library_item.to_owned(),
)))
.unchanged(),
_ => Effects::none().unchanged(),
};
let update_next_library_item_effects = match &next_library_item {
Some(next_library_item) => Effects::msg(Msg::Internal(Internal::UpdateLibraryItem(
next_library_item.to_owned(),
)))
.unchanged(),
_ => Effects::none().unchanged(),
};
*library_item = next_library_item;
Effects::none()
.join(update_library_item_effects)
.join(update_next_library_item_effects)
} else {
Effects::none().unchanged()
}
eq_update(library_item, next_library_item)
}

fn watched_update(
Expand Down
6 changes: 6 additions & 0 deletions src/runtime/msg/action.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,12 @@ pub enum ActionPlayer {
/// - We've watched a movie to the last second
/// - We've watched a movie series to the last second
Ended,
/// Marks the given [`Video`] of the [`LibraryItem`] as watched.
///
/// Applicable only when you have a multi-video (e.g. movie series) item.
///
/// [`LibraryItem`]: crate::types::library::LibraryItem
MarkVideoAsWatched(Video, bool),
}

#[derive(Clone, Deserialize, Debug)]
Expand Down
22 changes: 22 additions & 0 deletions src/types/library/library_item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,28 @@ impl LibraryItem {
self.state.times_watched = 0;
}
}

pub fn mark_video_as_watched<E: Env>(
&mut self,
watched: &WatchedBitField,
video: &Video,
is_watched: bool,
) {
let mut watched = watched.to_owned();
watched.set_video(&video.id, is_watched);

self.state.watched = Some(watched.into());

if is_watched {
self.state.last_watched = match (&self.state.last_watched, &video.released) {
(Some(last_watched), Some(released)) if last_watched < released => {
Some(released.to_owned())
}
(None, released) => released.to_owned(),
(last_watched, _) => last_watched.to_owned(),
};
}
}
}

impl<E: Env + 'static> From<(&MetaItemPreview, PhantomData<E>)> for LibraryItem {
Expand Down
14 changes: 4 additions & 10 deletions stremio-core-web/src/model/serialize_player.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,16 +146,10 @@ pub fn serialize_player<E: stremio_core::runtime::Env + 'static>(
video,
upcoming: meta_item.preview.behavior_hints.has_scheduled_videos
&& video.released > Some(E::now()),
watched: ctx
.library
.items
.get(&meta_item.preview.id)
.map(|library_item| {
library_item
.state
.watched_bitfield(&meta_item.videos)
.get_video(&video.id)
})
watched: player
.watched
.as_ref()
.map(|watched| watched.get_video(&video.id))
.unwrap_or_default(),
// only the currently playing video can have the progress
// as we keep that information in the LibraryItem
Expand Down
Loading