search

vionya / discord-rich-presence

A cross-platform Discord Rich Presence library written in Rust
MIT License
89 stars 16 forks source link

I've implemented some features that might be helpful #15

Closed Toldoven closed 1 year ago

Toldoven commented 1 year ago

I did a bit of tinkering on this crate for my project. The implementation is janky, but I've added some features that make it a lot more convenient.

Maybe you can take these ideas and implement them in a better way.

https://github.com/KPidS/tonbun/tree/main/src-tauri/crates/discord-rich-presence

1) clear_activity method on DiscordIpc trait. Basically I implemented solution from #13 into the crate.

https://github.com/KPidS/tonbun/blob/main/src-tauri/crates/discord-rich-presence/src/discord_ipc.rs#L212

2) #14 connected field on the DiscordIpcClient now actually works. Not sure if it's the right way to implement it though.

3) set_activity_safe and clear_activity_safe.

https://github.com/KPidS/tonbun/blob/main/src-tauri/crates/discord-rich-presence/src/discord_ipc.rs#L212

Returns Ok If connectedis set to false, meaning the client isn't supposed to be connected.

If it's true, then tries to set the activity. If set_activity returns Err it will try to reconnect the client, and returns Err only if it failed to reconnect or failed to set_activity after reconnecting.

4) DiscordIpcClientMutex struct. Wraps around DiscordIpcClient and has enable and disable functions.

https://github.com/KPidS/tonbun/blob/main/src-tauri/crates/discord-rich-presence/src/discord_ipc_mutex.rs

enable will start an interval that is constantly going to try to connect to the client, so you don't have to worry about manually connecting it ever.

This is definitely WIP at the moment, since I need to implement a proper way to stop the interval thread when you disable it.

Right now if you spam enable disable you can launch multiple threads at the same time.

Rayrsn commented 1 year ago

Thank you for these, please make a PR so @sardonicism-04 can merge it into the main branch

Toldoven commented 1 year ago

Thank you for these, please make a PR so @sardonicism-04 can merge it into the main branch

Basically, I wanted to ask if these features are wanted, and if they are ready to be made into the PR.

Features 1 and 2 are likely are, so yeah, I'll try to polish them, add documentation, and make them into the PR.

vionya commented 1 year ago

1 and 2 look great, will happily merge that PR :)

3 looks like a bit of an unnecessary abstraction IMO

4 sounds interesting. I'll have to think about if something like that is in scope of the project, but would probably not merge it at the moment

Toldoven commented 1 year ago

1 and 2 look great, will happily merge that PR :)

3 looks like a bit of an unnecessary abstraction IMO

4 sounds interesting. I'll have to think about if something like that is in scope of the project, but would probably not merge it at the moment

Yeah, I agree. 3 is a bit too specific for my desires and 4 is definitely not ready to be merged.

I'll make a PR with 1 and 2 in couple of days. Thanks for the reply.