Skip to content

Conversation

Tais993
Copy link
Member

@Tais993 Tais993 commented Feb 15, 2022

This PR is ready to be reviewed, but will not be merged!

Once #363 is merged, the target will be changed to develop.
For now this is only there so the chances can be reviewed in advance so the load will be lightened

Changes in this PR

Added interfaces

  • Added BotCommand (getName, getVisibility, onButtonClick, onSelectionMenu, acceptComponentGenerator
  • MessageContextCommand (onMessageContext)
  • UserContextCommand (onUserContext)

Added adapters

  • Added BotCommandAdapter adds generateComponentId, and implements the methods listed above
  • ContextCommandAdapter

Renamed

  • Renamed SlashCommandVisibility to CommandVisibility

Edited

  • Stripped down SlashCommand (moved to BotCommand)
  • Stripped down SlashCommandAdapter (moved to BotCommandAdapter)
  • Made BotCommandProvider return BotCommand instead of SlashCommand

TODO

More info

Check the issue, #368

@Tais993 Tais993 added documentation Improvements or additions to documentation enhancement New feature or request blocked This issue is currently blocked by another issue (see comments) priority: major labels Feb 15, 2022
@Tais993 Tais993 self-assigned this Feb 15, 2022
@Tais993 Tais993 linked an issue Feb 15, 2022 that may be closed by this pull request
14 tasks
@Tais993 Tais993 changed the title Feature/jda5 command system Rework command-system for JDA5 + new Discord features Feb 15, 2022
@Tais993 Tais993 force-pushed the feature/JDA5-command-system branch from 7bf8c3c to 0886132 Compare February 15, 2022 18:18
@Tais993 Tais993 force-pushed the feature/JDA5-command-system branch from 0886132 to 18ecfbf Compare February 16, 2022 22:21
@Tais993 Tais993 marked this pull request as ready for review February 16, 2022 22:22
@Tais993
Copy link
Member Author

Tais993 commented Feb 16, 2022

This PR can be reviewed already, if we agree on my design I'll be able to already implement things into TJ-Bot so the size of this PR can decrease.

Otherwise we will receive #363 and this PR at the same time, which will take a lot of time.

@Tais993
Copy link
Member Author

Tais993 commented Feb 16, 2022

@Zabuzard compilation issues, compiles alright locally..

Weird caching issue or sum? Not sure, IntelliJ is complicated

@Tais993 Tais993 requested a review from Zabuzard March 2, 2022 12:58
@Tais993 Tais993 mentioned this pull request Mar 3, 2022
@Zabuzard
Copy link
Member

Zabuzard commented Mar 12, 2022

@Tais993 For your info, for #394 I have to do changes on the command system (BotCommand in your case) that abstract out the capability to create buttons and selection menus from commands. So that non-commands (routines, event listeners, ...) like ScamDetector can create them as well.

That means that I will have to move:

  • acceptComponentIdGenerator - to create buttons etc
  • getName() - unique name for the routing
  • onButtonClick - callback
  • onSelectionMenu - callback

out of the command system into a separate interface, which the commands then also implement.

For you, that means, that you will probably have to do a minor refactoring on BotCommand once its merged. Nothing critical though, I suppose.

I am thinking about calling it sth along the lines of InteractionCreator, something that can create an interaction with the user. (name to be decided lmao)

@Tais993 Tais993 force-pushed the feature/JDAV5 branch 2 times, most recently from f78a63e to 8de7a4b Compare March 17, 2022 15:01
Base automatically changed from feature/JDAV5 to develop March 29, 2022 08:56
@github-actions
Copy link

This pull request is stale because it has been open 30 days with no activity. Remove stale label, comment or add the valid label or this will be closed in 5 days.

@github-actions github-actions bot added stale inactivity-closed Issues that have been closed due to inactivity, but are otherwise valid and might be reopened later labels Apr 29, 2022
@github-actions github-actions bot closed this May 5, 2022
@Tais993 Tais993 removed inactivity-closed Issues that have been closed due to inactivity, but are otherwise valid and might be reopened later stale labels May 5, 2022
@Tais993 Tais993 reopened this May 5, 2022
@Tais993
Copy link
Member Author

Tais993 commented May 17, 2022

Changing this to a draft, there's a few points that need to be addressed first;

No promises on when this will happen, if someone needs any of the new features hit me up and I'll prioritize this so it gets merged

The issue contains what has to be done

@Tais993 Tais993 marked this pull request as draft May 17, 2022 11:31
@Tais993 Tais993 mentioned this pull request May 31, 2022
8 tasks
@Zabuzard
Copy link
Member

Zabuzard commented Jun 1, 2022

@Tais993 For this PR individually, what is left to do? Nothing? Just rebasing, fixing merge conflicts, fixing sonar, quick test whether it works and done? Or is there still any content-stuff you want to be done in this PR before its shipped?

If its just getting the PR, as it is, ready for merge, I can probably do that.

@Tais993
Copy link
Member Author

Tais993 commented Jun 1, 2022

See issue

If we implement those features later, that would be fine too. In that case it's rebasing, and maybe sonarcloud?
So kinda just preparing for merge

@Zabuzard
Copy link
Member

Zabuzard commented Jun 1, 2022

See issue

I did see it. But since you said in illus PR that the other PR can probably be merged later, I assumed that you want to ship this individually now.

I do not quite get why you want to hold back the general command rework until the extra features are there. Isn't it easier to just ship the rework now and the extra features afterwards?

Or am I confusing the two PRs now? 😆

@Tais993
Copy link
Member Author

Tais993 commented Jun 1, 2022

I have no idea what you mean.

The intention of this PR was to include everything the issue says, currently I haven't finished everything, therefor I'm saying "you can merge and I'll do it later" to not delay for any longer.

I never talked about an existing second PR, I just said a new PR can be made in the future for these missing features.

@illuminator3 illuminator3 mentioned this pull request Jun 28, 2022
@illuminator3
Copy link
Contributor

bump

@github-actions
Copy link

github-actions bot commented Aug 6, 2022

This pull request is stale because it has been open 30 days with no activity. Remove stale label, comment or add the valid label or this will be closed in 5 days.

@github-actions github-actions bot added stale inactivity-closed Issues that have been closed due to inactivity, but are otherwise valid and might be reopened later labels Aug 6, 2022
@github-actions github-actions bot closed this Aug 11, 2022
Tais993 added a commit that referenced this pull request Sep 3, 2022
@Tais993 Tais993 deleted the feature/JDA5-command-system branch September 13, 2022 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked This issue is currently blocked by another issue (see comments) documentation Improvements or additions to documentation enhancement New feature or request inactivity-closed Issues that have been closed due to inactivity, but are otherwise valid and might be reopened later priority: major stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rework command system to add context command and autocomplete support
5 participants