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

Add arbitrary JDA event support #237

Closed
Zabuzard opened this issue Oct 30, 2021 · 8 comments · Fixed by #345
Closed

Add arbitrary JDA event support #237

Zabuzard opened this issue Oct 30, 2021 · 8 comments · Fixed by #345
Assignees
Labels
enhancement New feature or request priority: normal

Comments

@Zabuzard
Copy link
Member

The command system, as of now, only supports SlashCommands. However, we slowly get more and more commands with a need to listen for any sort of events.

While we do favor offering specific solutions (SlashCommands, MessageListener, ...), we should also offer a catch-them-all solution where commands can just listen to all incoming JDA events. It should be well integrated with existing solutions, i.e. it should be possible to create a slash command that also receives all other events (for example if using the interfaces).

There is no need to put a lot of effort into the arbitrary event support, since it is only meant as backup solution in case we do not offer more concrete and specific support. Commands that need this are for example the mod audit system (#225 )

@Zabuzard Zabuzard added enhancement New feature or request priority: normal labels Oct 30, 2021
@Zabuzard Zabuzard added this to the Improvement phase 1 milestone Oct 30, 2021
@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the stale label Dec 17, 2021
@Zabuzard
Copy link
Member Author

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

This is actually still relevant, we need it at some point.

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the stale label Jan 17, 2022
@Zabuzard Zabuzard removed the stale label Jan 17, 2022
@Tais993
Copy link
Member

Tais993 commented Jan 17, 2022

How would you see it exactly implemented then?

You mean addition of a class that extends both the SlashCommand thing and ListenerAdapter, and adding a class which contains a list of all EventListeners? This way it stays optional, and works with SlashCommand's

@Zabuzard
Copy link
Member Author

You mean addition of a class that extends both the SlashCommand thing and ListenerAdapter, and adding a class which contains a list of all EventListeners? This way it stays optional, and works with SlashCommand's

No, I would seperate it from SlashCommand. 2 separate interfaces and 2 separate adapters with 2 separate flows in the command system.

And if someone really needs, they can still create a class that implements both interfaces (and for example forwards to both adapters) but that will be custom then.

@Zabuzard
Copy link
Member Author

Basically, the command system should offer a way to register a EventListener and manage it accordingly (could be as simple as just jda.addEventListener(listener) in the command system). In parallel to the existing slash command setup.

@Tais993
Copy link
Member

Tais993 commented Jan 17, 2022

Issue with this is that you'll get bad UX, you can't both use ListenerAdapter and the SlashCommand adapter

@Zabuzard
Copy link
Member Author

Zabuzard commented Jan 17, 2022

Issue with this is that you'll get bad UX, you can't both use ListenerAdapter and the SlashCommand adapter

I do not think we will have a lot of people who need that both. But still, the crucial part is not that the adapter allow combination but that the interfaces do - and they do.

Our design is based on the interfaces; we are not forced to use the adapters.

Putting event-capabilities into slash commands would mean you overload the class/interface with more and more stuff that regular users, such as /ping or similar just never asked for, which is equally not good.

@Zabuzard Zabuzard self-assigned this Jan 17, 2022
@Zabuzard Zabuzard linked a pull request Jan 17, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority: normal
Projects
None yet
2 participants