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

Updates to the Command API to support implementation #2066

Merged
merged 3 commits into from
Jul 2, 2020

Conversation

dualspiral
Copy link
Contributor

The big thing here is the addition of a "CommandRegistrar". This is meant to be a lightweight interface that tries to give external command frameworks (Aikar's ACF, Katrix's Scammander, Faith's Butler, amongst others) a simple way to add their commands to Sponge. The idea of registrars was originally an implementation one - if you look at that code, you'll see that the vanilla brig system is separated from the Sponge one - it takes little work to add support for arbitary registrars, so why not add them?

The Javadocs should provide a fair amount of explanation. There will be more for client completions, I'm sure - though enabling that is the thing that is missing right now. That'll come later - it'll likely mimic Brig's suggestion provider with a tree like format to build the command elements up with.

This is simply for review of code style and Javadocs.

@dualspiral dualspiral added system: command api: 8 (u) version: 1.16 (unsupported since Oct 17th 2023) labels Dec 4, 2019
@dualspiral dualspiral added this to the Revision 8.0 milestone Dec 4, 2019
@dualspiral dualspiral requested a review from Zidane December 4, 2019 22:19
@dualspiral dualspiral self-assigned this Dec 4, 2019
@ItsDoot
Copy link
Member

ItsDoot commented Dec 4, 2019

So effectively, CommandRegistrar is similar to a Dispatcher + CatalogType?

@dualspiral
Copy link
Contributor Author

I guess, though it doesn't really need to be a dispatcher so much. A registrar could conceivebly be a command executor - I don't put a restriction on it.

Note that there are a couple of classes that don't have JDs. That's a mistake on my part, some API elements have made it through that were nowhere near ready. I won't be merging tonight as it is, so pick on anything and I'll review over the next couple of days.

@dualspiral dualspiral force-pushed the api8/command-registrars branch from 7fc207e to 437bbf5 Compare December 15, 2019 17:08
@dualspiral dualspiral force-pushed the api8/command-registrars branch from f36a5b3 to 456239f Compare December 30, 2019 13:54
@dualspiral dualspiral force-pushed the api8/command-registrars branch from d7582a3 to 35ca48a Compare March 1, 2020 14:55
@dualspiral dualspiral force-pushed the api8/command-registrars branch 2 times, most recently from e0cb097 to 0553016 Compare April 3, 2020 18:23
@dualspiral dualspiral force-pushed the api8/command-registrars branch from ce7d33a to 9d99d5b Compare May 23, 2020 10:26
@dualspiral dualspiral force-pushed the api8/command-registrars branch from bdf88ab to 69b7ac3 Compare June 5, 2020 17:05
@dualspiral dualspiral force-pushed the api8/command-registrars branch from 69b7ac3 to 78c7d8d Compare June 13, 2020 12:26
@dualspiral dualspiral force-pushed the api8/command-registrars branch from 78c7d8d to 9cab778 Compare June 24, 2020 12:39
* Added concept of CommandRegistrars to try to support external frameworks
* Updates to some JavaDocs
* Removed ArgumentReader parameter from completions, we just need to know what options there are.
* Add ClientCompletionKeys to try to enable non-Sponge client completions
* Update the CommandCause to directly implement Subject
* Introduce the notion of a terminal parameter
@dualspiral dualspiral force-pushed the api8/command-registrars branch from 2f912c9 to 0131ab9 Compare July 2, 2020 17:31
@Zidane Zidane merged commit a4d6aa2 into api-8 Jul 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: 8 (u) version: 1.16 (unsupported since Oct 17th 2023) system: command
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants