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

Adds support for JDA 5 / Modify the Lavaplayer lib #1447

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MasqueOu
Copy link

@MasqueOu MasqueOu commented Oct 18, 2023

This pull request...

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

Description

This pull-request supports JDA 5.0 and modifies certain dependencies in order to fix the problems already listed.

Purpose

In the near future, this will enable us to use the full potential of Discord's latest features, including support for Threads, new usernames, Slash commands, etc.

Relevant Issue(s)

This PR closes issue #1434, #1437

@MichailiK MichailiK self-requested a review October 18, 2023 07:47
@MichailiK
Copy link
Collaborator

MichailiK commented Oct 18, 2023

Important to mention that this PR also switches to a jda-utilities fork and a lavaplayer fork.

jagrosh has stated that he's been working on adding slash commands to jda-utilities. As for lavaplayer, it seems likely to me that we will switch to a fork though.

Copy link
Collaborator

@MichailiK MichailiK left a comment

Choose a reason for hiding this comment

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

I'd wager that right now, it's too soon to update to JDA 5. Regardless, I imagine this PR will be nice to start off with once jda-utils gets updated, so we can quickly make the move to JDA 5 on MusicBot, to then start work on slash commands & i18n.

pom.xml Outdated Show resolved Hide resolved
pom.xml Show resolved Hide resolved
@MasqueOu MasqueOu changed the title Adds support for JDA 5 Adds support for JDA 5 / Modify the Lavaplayer lib Oct 18, 2023
@MasqueOu
Copy link
Author

Hello !
Thank you for your quick reply, I'll get back to you.

I don't explicitly mention the change to another Librairy Lavaplayer. I apologise for this.
This new library corrects problems relating to the loading of certain videos resulting in a 403 error in the bot console. I've added it to support JDA 5 and also to correct this problem.

As far as JDA Utilities is concerned, I am indeed using a fork, but as specified in the pom.xml, this is temporary until Jagrosh accepts the pull request on the JDA-Utilities Github or recodes JDA-Utilities to make it work under JDA5.

I've cancelled my changes to the pom.xml, I think there should still be a comment about JDA-Utilities, if it's disturbing I'll delete it. The compiler produces an obvious error because JDA5 is not supported by JDA-Utilities.

Apart from that, I sincerely thank you for the time you spend maintaining this project. If there is anything I can do to help, I am willing to do so.

Sorry for my English which can be a bit disastrous in some aspects, I mainly speak French. (Thanks DeepL)

@Celarye
Copy link

Celarye commented Jan 14, 2024

Hey, I experienced the bug which is mentioned to be fixed with this PR so I tried to build it using the GitHub action, but it failed. I'm not sure on how to fix this, it seems to fail to download some dependencies.

EDIT: I just noticed that these are the same errors as in the check bellow which fails...

@MasqueOu
Copy link
Author

Hey, I experienced the bug which is mentioned to be fixed with this PR so I tried to build it using the GitHub action, but it failed. I'm not sure on how to fix this, it seems to fail to download some dependencies.

EDIT: I just noticed that these are the same errors as in the check bellow which fails...

As requested by MichailiK, I've modified the .pom by putting back the old imports. You can only build the first commit, not the last.

@Celarye
Copy link

Celarye commented Jan 14, 2024

Alright, thank you for the quick response! I'll build with the previous commit in that case.

@Celarye
Copy link

Celarye commented Jan 23, 2024

I've compiled it using the second to last commit from MasqueOu's fork. This is how I achieved it:

  1. Fork MasqueOu's fork
  2. Go to the Actions tab and enable the actions
  3. Clone your fork (git clone <URL>)
  4. Reset to the second to last commit (git reset --hard 42fb14c635e0f738f9e0bd8e42bb9dc8990cf238)
  5. Force push to remote (git push --force origin <branchName>)
  6. Start the GitHub Action to build it

I must note though, in my case, this did not fix the issue it says it should.

@MasqueOu
Copy link
Author

MasqueOu commented Jan 24, 2024

I've compiled it using the second to last commit from MasqueOu's fork. This is how I achieved it:

  1. Fork MasqueOu's fork
  2. Go to the Actions tab and enable the actions
  3. Clone your fork (git clone <URL>)
  4. Reset to the second to last commit (git reset --hard 42fb14c635e0f738f9e0bd8e42bb9dc8990cf238)
  5. Force push to remote (git push --force origin <branchName>)
  6. Start the GitHub Action to build it

I must note though, in my case, this did not fix the issue it says it should.

Hello !
Can you try modifying the "Lavaplayer" library in the pom.xml to version 2.1.0?

EDIT: I've tested this library at considerable length, as well as the one that is included as standard in the pull request. I never got a 403 error again. The new version may correct the problem, but I can't be sure. Don't hesitate to give your feedback.

@IIWDTI
Copy link

IIWDTI commented Jan 24, 2024

After messing around with the code for along time I finally managed to make it compile.

I made changes to the pom.xml file, that fixed the 403 error. (might not be a final fix, but it solved my problems at least)

Source code and compiled binary available here: https://github.com/IIWDTI/MusicBot

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