-
-
Notifications
You must be signed in to change notification settings - Fork 405
Commands rework - Brigadier backend #8111
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/feature
Are you sure you want to change the base?
Conversation
…hin a section node
…le entry container
I likely won't review this but I'm more concerned about how this will work, since I'm not a huge fan of the implications this makes. How exactly are you intending to handle multiple arguments and their suggestions? If anything I would prefer them handled in a way similar to I also don't see anything like sub arguments/commands of sub commands |
Thanks for the feedback, let me clear up some things. :)
Correct, if you structure the command as command /foo <first: word> <second: word> <third: word>:
suggestions:
# something the suggestions will apply only for the last node (argument command /foo:
suggestions:
# suggestions for first
subcommand <first: word>:
suggestions:
# suggestions for second
subcommand <second: word>:
suggestions:
# suggestions for third
subcommand <third: word>: But it is good to note that the command nodes themselves have own suggestion logic, for command such as command /foo:
subcommand bar
subcommand world both
Yes, MessageComponent -> SpigotComponent -> PaperComponent -> Suggestion. Suggestions can only be plain text with a component tooltip, I felt like using existing MessageComponent fits well here. Possible improvement would be to convert MessageComponent directly to Suggestion, but I think introducing Suggestion as a separate type is too specific.
Subcommands can already have other subcommands if that is the question, SubCommandEntryData can be nested. |
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.
looks like a great start!
I'm sure it's in progress but just a reminder to add a bunch more javadocs to stuff, too.
src/main/java/ch/njol/skript/command/brigadier/BrigadierCommandEvent.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/command/brigadier/DefaultArgumentTypes.java
Outdated
Show resolved
Hide resolved
src/main/java/ch/njol/skript/command/brigadier/DefaultArgumentTypes.java
Outdated
Show resolved
Hide resolved
moved paper platform impl from ch/njo/skript to org/skriptlang/skript, improved syntax patterns for existing command argument types
fixed the syntax registration, small changes to argument types
…osed of multiple command nodes
This is a continuation of #7032 from the ground up. x)
Problem
The current Skript command system has several drawbacks for both Skript users and maintainers. I think the biggest are:
hello <string> <number>
, where the inputhello foo 1 2
is considered valid. I also see this implementation as a maintenance burden.Solution
The solution is to switch to Brigadier as the command backend.
The new implementation is now fully separate from the existing command system, mainly because of the many changes between the two. It is very unlikely this will change. My aim is to integrate most of the features from the old command system, deprecate it, and fully remove it later. This will and should take a lot of time.
Not using Brigadier as a backend would take a lot of extra effort for what I believe is no extra gain, why reinvent the wheel if we don't have to..
Planned Syntax:
Existing Work:
integer from %integer% to %integer%
or simple ones likeword
. Right now, you cannot simply use different class infos as arguments, but I plan on supporting this later. I prefer this approach over a validation section because:The current work is divided into three different packages:
org.skriptlang.skript.lang.command
- general classes related to syntax, parsing, and the new command API.org.skriptlang.skript.brigadier
- classes that expand on the Brigadier library.ch.njol.skript.command.brigadier
- platform-specific (Paper) implementation of the new command API.Work to be Done:
(req|request)
node would accept bothreq
andrequest
literalsset suggestions of "name" to {_s::*}
)What will almost surely be lost compared to the old command system:
Testing Completed
Only local testing has been performed for now. I need to spend more time with this PR before I can start writing proper tests.
This PR is meant for Paper servers running on 1.21 and above, that's why the existing tests on lower versions might fail.
Completes: none
Related: #7032