-
Notifications
You must be signed in to change notification settings - Fork 594
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 discord clearActivity, menu, listen along option #380
Conversation
The callback sends multiple events, in particular two pause when going to the next song, so the timeout wasn't properly cleared. Add menu buttons for the two options
Currently we also don't handle (see #132 (reply in thread))
Supporting those would however require quite a bit of rewriting, is this in-scope? Should I do it? |
It seems to me that it would require constant listening to other process / (which is too much IMO) (I can't really find documentation about events at https://discord.js.org/#/docs/rpc/master/class/RPCClient) |
The RPC library emits a For the reconnecting, I would simply add a button to the menu, I agree that checking if discord is running periodically is too much For the events, you kinda have to look through https://github.com/discordjs/RPC/blob/master/src/client.js, but most of the events are just carried on from discord itself |
A button for reconnecting sounds neat 😎 |
Clear rpc on disconnect Add menu button to reconnect
Currently, if the plugin fails to connect, there is no message to the user (neither on start nor on reconnect). Is this behaviour desirable or do you think an alert would be better? |
Maybe show a message only when the reconnect button is clicked and it fails I don't think its necessary on startup, since you could easily start youtube-music with discord off - and it shouldn't be an error Great work btw :) |
@th-ch could you please review and merge? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did not fully test it, but diff looks good, thanks for the contribution! ✅
@cpiber Could you please explain the thinking process behind this export? youtube-music/plugins/discord/back.js Line 151 in f2e04f9
why can't you just export something like module.exports.isConnected = () => info.rpc !== null |
I had it differently before (used more from |
Could you open a small pr to clean this up? if you're busy I can do it np |
Will do ^^ |
Discord plugin: Clean Up Export (follow-up #380)
The callback sends multiple events, in particular two pause when going to the next song, so the timeout wasn't properly cleared. I have documented this.
Added menu buttons for the two options. Currently for the timeout time I simply open the config, to allow the user to properly input a number here a library is needed.
Now also clears activity directly if timeout enabled and set to 0.