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

Added Discord rich presence and added extra properties to songInfo provider #124

Merged
merged 2 commits into from
Jan 13, 2021
Merged

Added Discord rich presence and added extra properties to songInfo provider #124

merged 2 commits into from
Jan 13, 2021

Conversation

semvis123
Copy link
Contributor

  • Added Discord rich presence

image

  • Added extra properties to songInfo provider:
{
    songDuration: Integer,
    ElapsedSeconds: Integer
}

This was referenced Jan 12, 2021
Copy link
Owner

@th-ch th-ch left a comment

Choose a reason for hiding this comment

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

Looks promising! Just a question/remark and should be good to go!
Note: #125 will refactor the providers, the second PR to be merged will have to be updated to take into account changes.

transport: 'ipc'
});

const clientId = '790655993809338398';
Copy link
Owner

Choose a reason for hiding this comment

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

Quick question about the ID, did you register an application in Discord under your own account?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

uhm yes, but it can be changed to yours if you want :)

@@ -0,0 +1,41 @@
const DiscordRPC = require('discord-rpc');
Copy link
Owner

Choose a reason for hiding this comment

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

nit: would have just called the plugin discord (rpc being an implementation detail, it might not be necessary to have it in the plugin name)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Like this?

Copy link
Owner

Choose a reason for hiding this comment

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

Ah no, sorry for the confusion, I was talking about the filename plugins/discord-rpc/back.js -> plugins/discord/back.js! The code looks good 👍

Copy link
Owner

@th-ch th-ch left a comment

Choose a reason for hiding this comment

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

Looks good, merging, thanks for the contribution!

@th-ch th-ch merged commit f0200e7 into th-ch:master Jan 13, 2021
@ghost
Copy link

ghost commented Jan 23, 2021

The individual discord account needs to have its respective youtube account connected in order to show valid information.

Could this be written in some docs (README) on these changes with the other available plugins?

@semvis123
Copy link
Contributor Author

@AlxBrn Hmm that shouldn't be necessary, I don't have my YouTube account connected to my Discord and it works fine for me.

Have you restarted both discord and YouTube music before you checked?

Also could you clarify what you mean with "in order to show valid information"?

@NNowakowski
Copy link
Contributor

@semvis123 Would it be possible to rename the client to YouTube Music instead of Youtube Music? :p

@semvis123
Copy link
Contributor Author

@NNowakowski It should be updated right now, thanks for your suggestion!

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.

3 participants