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

Switch to lavalink's lavaplayer fork #1490

Merged
merged 4 commits into from
Mar 5, 2024

Conversation

MichailiK
Copy link
Collaborator

This pull request...

  • Fixes a bug
  • Introduces a new feature
  • Improves an existing feature
  • Boosts code quality or performance

Description

Switches lavaplayer dependency from our fork to lavalink's fork.

It uses the (as of writing) latest commit instead of the latest stable version, as I've encountered lavalink-devs/lavaplayer#69 during my testing, which prevented playing any YouTube tracks.

Purpose

Fixes several issues related to our outdated fork, particularly the "Error loading track from YouTube (403)"

Relevant Issue(s)

Fixes #885

Maybe fixes #1291

And honestly, closes #1375 because switching to ffmpeg requires monumental effort & im pretty sure there's other music bots that do precisely that anyways.

@MichailiK
Copy link
Collaborator Author

The lavalink devs have added the ability to login to a Google/YT account to play age-restricted videos, and will display the following warning when no credentials have been provided:

[WARN] [YoutubeAccessTokenTracker]: YouTube auth tokens can't be retrieved because email and password is not set in YoutubeAudioSourceManager, age restricted videos will throw exceptions

Since one of JMusicBot's "core principles" is to not require any credentials except for a Discord bot token, I suppose we want to suppress this warning specifically? Or perhaps do give an option to let people enter credentials, while expressly discouraging it?

@jagrosh
Copy link
Owner

jagrosh commented Mar 1, 2024

The lavalink devs have added the ability to login to a Google/YT account to play age-restricted videos, and will display the following warning when no credentials have been provided:

[WARN] [YoutubeAccessTokenTracker]: YouTube auth tokens can't be retrieved because email and password is not set in YoutubeAudioSourceManager, age restricted videos will throw exceptions

Since one of JMusicBot's "core principles" is to not require any credentials except for a Discord bot token, I suppose we want to suppress this warning specifically? Or perhaps do give an option to let people enter credentials, while expressly discouraging it?

I'm against including that functionality for 2 reasons. One is the intention to not require more than a bot token, but even more severe is that people should definitely not ever be including email and password in a config file. If we can suppress that warning, that would be preferable, because it's really bad practice to put passwords in plaintext and I don't want to confuse people into thinking that they should do that.

@jagrosh
Copy link
Owner

jagrosh commented Mar 1, 2024

We can maybe revisit allowing the functionality in some roundabout way (ex, env variables or something else not in a plaintext config) but in general I don't want to have any messages advertising this, since it's really not a good practice in the first place.

@MichailiK
Copy link
Collaborator Author

I have just created a logback TurboFilter to suppress that specific message

@prashker
Copy link

prashker commented Mar 2, 2024

Built MichailiK:fix/lavaplayer-fork-shenanigans from source and finally my MusicBot is back to life - just wanted to say thank you in advance!

@jagrosh jagrosh merged commit 49c3ec7 into jagrosh:master Mar 5, 2024
2 of 3 checks passed
CNote325

This comment was marked as spam.

@CNote325

This comment was marked as spam.

@CNote325

This comment was marked as spam.

DaniDipp added a commit to Team-Sneakymouse/MusicBot that referenced this pull request Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants