-
Notifications
You must be signed in to change notification settings - Fork 1
Real Brigadier Commands #64
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
base: dev
Are you sure you want to change the base?
Conversation
| this(label, CommandSettings.DEFAULT_COMMAND_SETTINGS); | ||
| } | ||
|
|
||
| public void execute(@NotNull CommandContext<@NotNull CommandSourceStack> context) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would it feel to wrap CommandContext in this case? I feel like this API could be nicer if we served something "easier" Paper has a lot of these really annoying inbetween types, which if we wrap, we could resolve in a more "beautiful manner" Mainly what comes to my mind is any form of Registry entry or Location is absolutely annoying to resolve.
It's also nice because we know we'll always have a CommandSourceStack so we can chuck away that annoying generic too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It depends on the info we actually end up needing from the context, we could technically use the arguments the command knows about to pass a CommandSourceStack with some form of argument collection, just depends on how we handle the generics on that collection
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It depends on the info we actually end up needing from the context, we could technically use the arguments the command knows about to pass a CommandSourceStack with some form of argument collection, just depends on how we handle the generics on that collection
Perhaps I'll try building something out this weekend and we see how we like it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me, lmk if there's any ideas you have that I could attempt
# Conflicts: # settings.gradle.kts
This PR aims to allow for a commands to use brigadier to its advantage while using a similar layout to how normal Commands work. It also migrates the existing registerCommands to use the short-hand provided in java plugin and converts most references of Plugin to JavaPlugin. Also adds a test plugin using a similar setup from Paper, currently includes a test for commands but will be removed prior to merge.
Not finished yet, feedback wanted.
Main points, wanted are about design and should quick getters be added for arguments, eg: getString, getPlayer, getLocation etc etc for built in argument types from brigadier or paper