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

Dev progress #2

Merged
merged 107 commits into from
Apr 17, 2024
Merged

Dev progress #2

merged 107 commits into from
Apr 17, 2024

Conversation

core1024
Copy link
Member

This PR is nowhere finished. It is just for tracking the progress.

@core1024 core1024 marked this pull request as draft July 13, 2021 14:37
Copy link
Member

@Ivshti Ivshti left a comment

Choose a reason for hiding this comment

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

Looks mostly good - would be great Niki to see it for a more detailed look. Seems like there are too many .unwrap()s that should be .expect()

I didn't know about partials - definitely a nice feature of NWG

}
Ok(())
}
fn process_event<'a>(
Copy link
Member

Choose a reason for hiding this comment

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

such a handler is a good idea for sure

src/stremio_app/stremio_wevbiew/stremio_wevbiew.rs Outdated Show resolved Hide resolved
pub struct WebView {
controller: Rc<OnceCell<Controller>>,
}
impl PartialUi for WebView {
Copy link
Member

Choose a reason for hiding this comment

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

That's interesting, didn't know NWG has PartialUi.

Copy link
Member

@elpiel elpiel left a comment

Choose a reason for hiding this comment

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

I went over this PR and gave feedback on things that popped up while looking at it.

I hope they are useful.

setup/CodeDependencies.iss Show resolved Hide resolved

#[derive(Serialize, Deserialize, Debug, Clone)]
pub struct RPCRequest {
pub id: u64,
Copy link
Member

Choose a reason for hiding this comment

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

Please check if we're building the code for 32bits if we have to change this to u32 or the compiler is able to do some magic and compile u64 on 32bit builds.

Comment on lines 4 to 7
use kernel32::{
CloseHandle, ConnectNamedPipe, CreateFileW, CreateNamedPipeW, DisconnectNamedPipe,
FlushFileBuffers, ReadFile, WaitNamedPipeW, WriteFile,
};
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity: Why do we use this kernel32 from windows?
Isn't it possible to use the std::fs module for e.g. Reading a file?

src/stremio_app/named_pipe.rs Outdated Show resolved Hide resolved
Comment on lines +222 to +231
trait MpvExt {
fn wake_up(&self);
}

impl MpvExt for Mpv {
// @TODO create a PR to the `libmpv` crate and then remove `libmpv-sys` from Cargo.toml?
fn wake_up(&self) {
unsafe { libmpv_sys::mpv_wakeup(self.ctx.as_ptr()) }
}
}
Copy link
Member

Choose a reason for hiding this comment

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

yes, I guess you can either export this unsafe function or add a function to Mpv that wraps it.

@core1024 core1024 marked this pull request as ready for review April 17, 2024 12:25
@core1024 core1024 merged commit 4022cbb into main Apr 17, 2024
2 checks passed
@core1024 core1024 deleted the dev-progress branch October 15, 2024 11:22
Ivshti pushed a commit that referenced this pull request Jan 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants