#Brigadier Command API

1 messages · Page 3 of 1

granite hound
#

read the PR?

#

and the pinned message

proud timber
#

Wait is the PR about to be merged?

neat radish
#

hopefully soon, yes!

lime zephyr
#

2000nd message

unreal tangle
#

WOMP WOMP

lime zephyr
#

poop

unreal tangle
#

change it to 2001

#

do it

lime zephyr
#

i was waiting for that thumb to get deleted

unreal tangle
lime zephyr
#

YAY

proud timber
#

Does this need to be resolved though?

neat radish
#

Exception stuff is properly coming later

#

(existing types, that is)

proud timber
#

But it's broken now?

neat radish
#

could u try to show code here actually? That may have gotten lost

proud timber
#

Lost?

neat radish
#

404 on ur pastebin

proud timber
#

Huh

#

Oh it has probably expired then

neat radish
#

mhm

proud timber
# 
class StructureArgumentType extends CustomArgumentType.Converted<Structure, NamespacedKey> {

    private static final DynamicCommandExceptionType INVALID_STRUCTURE_ID_EXCEPTION = new DynamicCommandExceptionType(id -> MessageComponentSerializer.message().serialize(Component.translatable("structure_block.invalid_structure_name", id.toString())));

    private static final Set<NamespacedKey> STRUCTURES = StreamSupport.stream(Registry.STRUCTURE.spliterator(), false)
        .map(Keyed::getKey)
        .collect(Collectors.toUnmodifiableSet());

    private StructureArgumentType() {
        super(ArgumentTypes.namespacedKey());
    }

    public static StructureArgumentType structure() {
        return new StructureArgumentType();
    }

    @Override
    public @NotNull Structure convert(@NotNull NamespacedKey key) throws CommandSyntaxException {
        Structure structure = Registry.STRUCTURE.get(key);
        if (structure == null) {
            throw INVALID_STRUCTURE_ID_EXCEPTION.create(key);
        }
        return structure;
    }

    @Override
    public @NotNull CompletableFuture<Suggestions> listSuggestions(@NotNull CommandContext context, @NotNull SuggestionsBuilder builder) {
        return SuggestionProviderHelper.suggestNamespacedKeys(STRUCTURES, builder);
    }
}
neat radish
#

@tacit umbra Was this potentially one of the issues u were talking about

#

@proud timber So is the issue is that the "%s" isn't being resolved?

proud timber
#

Yes

golden crypt
#

Any thoughts on the message up there and this comment ? Is registering commands during server runtime going to be possible and/or cause that CME message?

glass smelt
#

Theres basically 0 way to guard it

#

it's also generally not really worth messing around to do so

proud timber
#

Oh and I would really like to request being able to lowercase the command "namespace"

#

Uppercased "namespaces" for commands are so ugly

neat radish
#

Yes fixed that

proud timber
#

Oh really nice

#

Does it automatically lowercase it or is it a config option?

river flame
tacit umbra
#

@neat radish there was some issue with Registry stuff right? Some reason to do the RegistryAccess stuff before this

neat radish
#

Oh right eys

tacit umbra
#

that is why I split that bit out from the registry mod API. But I forget what that reason was

neat radish
#

It was accessing those stuff during bootstrap

#

for arguments

tacit umbra
#

oh right, yeah. But doesn't that usually happen in the parse/convert methods? Like that stuff doesn't run during bootstrap ever does it?

neat radish
#

That was the reason you described but yeah i never really thouht it was fully needed

tacit umbra
#

I thought I had a more specific reasion. I think someone actually had an issue with it

neat radish
#

Also, it seems there are some issues with the help command, will need to be looked into.

tacit umbra
#

I guess its weird to be able to use Registry.WHATEVER within a method in CustomArgumentType, but not in like a static block or something

#

that is a little bit weird from an API perspective

golden crypt
neat radish
#

This is an issue because console uses the command map logic

#

(My pr changes that)

golden crypt
#

Oh yeah, it looks like Brigadier-based-command-API.patch removes a few of the lines Add-Unknown-Command-Event.patch added :P. A little silly how one patch adds lines and another undos the changes.

But yeah, it looks like custom Brigadier commands will correctly pass through the if statement to show the syntax error, whereas Bukkit commands nodes will show the Unknown command message.

I also wonder if https://github.com/PaperMC/Paper/pull/9251 will be fixed as well. Maybe I'll test these issues myself to double check.

neat radish
distant hinge
#

Is there any schedule for merging this awesome api?

unreal tangle
#

Presumably towards the end of 1.20.6's experimental phase

slender agate
#

is that at eta???

pure vesselBOT
#
__There Is No ETA__

Updates to Paper do not have any sort of estimate for when they release, ever. Any and all updates will arrive when they are ready, and the only thing to do is wait for them patiently along with everyone else.

ETA means 'Estimated Time of Arrival'!
gusty iris
river flame
#

There Is No ETA in Ba Sing Se

stray remnant
#

joo dee

neat radish
#

rebased

pure vesselBOT
#
__There Is No ETA__

Updates to Paper do not have any sort of estimate for when they release, ever. Any and all updates will arrive when they are ready, and the only thing to do is wait for them patiently along with everyone else.

ETA means 'Estimated Time of Arrival'!
neat radish
gusty iris
#

merged 🎉🎉

unreal tangle
#

🥳

river flame
#

yayyyyy 🎊

golden crypt
#

Is there documentation for how to create commnads with this anywhere?

marsh shard
#

not yet

neat radish
#

Or see Commands docuemntation

golden crypt
neat radish
#

Yes

#

For now, we are releasing it to just test the bukkit legacy logic.

limpid fulcrum
#

It is available now.
Based on Owen's comment above - making sure old command API continues to work unaffected is the top most priority.
I'm sure that more testing is welcome and appreciated.

proud timber
#

But 🎉 🎉 it's merged

static owl
#

Given that ArgumentTypes#player() and ArgumentTypes#players() both return a PlayerSelectorArgumentResolver and only differ in the internal implementation, why doesn't ArgumentTypes#player() return a Player instance?

glass smelt
#

The internal impl returns a list

buoyant leaf
#

What the point of it returning a list? Seems odd that every time you only player() argument that you would need to use getFirst or something

glass smelt
#

because player() and players() use the same argument class

static owl
# glass smelt The internal impl returns a list

Well yeah, and I understand that this is supposed to mirror Brigadier as close as possible while also supporting vanilla arguments, but given API can make things easier, why isn't this changed to return a single player instance?

tacit umbra
#

Because it would just mean more types to add in the api

#

There would have to be 2 more interfaces added. Just using .getFirst() is fine

#

Brig doesn’t really support argument types with generic parameters

#

So every argument type has to be a type without a generic parameter

distant hinge
#

Im a bit confused, whats the proper way of registering a brigadier command now?

unreal tangle
#

see the javadocs

distant hinge
#

Thank you!

#

But I can register multiple command in one eventhandler right? Im asking cause in the test plugin there is one eventhandler for each command

ebon hill
#

in the test plugin it's registering multiple commands in one handler as well

#

but yeah

distant hinge
#

Ah missed that

tacit umbra
distant hinge
#

Thanks a lot, moved my message to paper-dev as I thought this thread is dead.

distant hinge
tacit umbra
#

not sure what that is

covert wren
distant hinge
#

Yes this is what I meant. I wasn't sure if this is command api specific or kinda part of brigardier or minecraft

glass smelt
#

that's commandapi specific

proud timber
#

It seems pointless anyway

#

Their example would be shorter if you hadn't used them

modern pagoda
covert wren
#

Quick question, is there a fancier way to get a CommandSourceStack instance in listSuggestions than to just cast the result from .getSource()?

image.png
tacit umbra
#

Nah. Cause its supposed to be sender agnostic. I think it's pretty safe to just always cast

covert wren
#

Alright, thanks 👍

static owl
#

Based on Owen's comment (https://github.com/PaperMC/Paper/pull/8235#issuecomment-1369131980) on the Brigadier PR, I would have assumed that using MinecraftServer#getCommands() to register commands using internals would have kept the commands when reloading using minecraft:reload, however they seem to not exist anymore after executing that command as paper seems to reset the resources field in MinecraftServer with a fresh one. The commands registered using API methods seem to be put back via the lifecycle API and Bukkit commands also seem to be copied over.
I know using internals itself isn't exactly supported here and that there are better alternatives like this API and I could probably use the ServerResourcesReloadedEvent alongside Player#updateCommands() to put the commands back but again, based on that comment I would've thought that commands registered when using internals would still exist after reloading.
Is there something that can be done about this or is that simply the intended behaviour now and I misunderstood Owen's comment?

neat radish
#

For registering commands you’ll need to use life cycle if you wanna use brig

static owl
#

Alright, thanks for clarifying!

lusty sierra
#

Honestly the brigadier command api is awesome! At first I thought it looked extremely messy and confusing but honestly it's just different is all. Makes you use that shift + scroll lol.

I don't know if I'm doing it "right" but it's working pretty well!

image.png
#

It's exciting to think just how plugins are getting closer to mods. Thank you to all of the paper team for your hard work! 😄

tacit umbra
#

You gotta turn off that 8 space continuation indent…

lusty sierra
tacit umbra
#

IntelliJ editor -> code style

#

Then maybe Java, idk. But it’s called the continuation indent

#

As opposed to the “normal” indent

lusty sierra
#

YO! That is so much better. Thank you so much!

Settings -> Code Style -> Java -> Tabs and Indents. Changing continuation indent to 4 makes a huge difference

neat radish
#

Glad you’re enjoying it! 🙂

frosty sand
#

btw something that I've noticed: brigadier declarations "registered" with CommandRegisteredEvent (yeah yeah I know, it is deprecated, I'm migrating to the new API rn) do not have autocomplete working sad_cat5 (actually now that I've noticed all declarations don't work, not just autocomplete, the commands are still executed correctly tho)

#

but at least migrating to the new API is going well: I just needed to replace the Mojang packages with Paper packages, replacing the registering stuff with the lifecycle API and it worked (and now autocomplete works again) smol_gessy

lusty sierra
#

Is this a proper way to go about creating custom argument types?
https://pastebin.com/tDQjSyQ1

In this case, EntityInflictor is just an object that holds an entity which is obtained using ArgumentTypes.entity().

You basically create an "argument" class that implements CustomArgumentType.
Then you create a "resolver" class that implements SelectorArgumentResolver.

Pastebin
//ENTITY INFLICTOR ARGUMENTimport com.mojang.brigadier.StringReader...

Pastebin.com is the number one paste tool since 2002. Pastebin is a website where you can store text online for a set period of time.

tacit umbra
#

Implementing custom argument type is the way to create custom arguments

#

I’m not sure what you’re asking, you seem to have what you need?

lusty sierra
#

It works as expected. I suppose I just wanted to make sure I was doing it "right".

tacit umbra
#

Since you are just directly using the native type to parse it

lusty sierra
#

Oh, I see. Thank you!

golden crypt
#

I was wondering, is there an "official" way to tell if a LiteralCommandNode in the NMS dispatcher (From MinecraftServer#getCommands()) came from a Bukkit command? Previously, if node.getCommand() instanceof org.bukkit.craftbukkit.command.BukkitCommandWrapper was true, then node was a command wrapped from the Bukkit CommandMap into Brigadier's CommandDispatcher. I'm currently looking for a way to replicate this check after the internal changes from this PR.

stuck trail
#

What's the usecase?

neat radish
#

@golden crypt Check BukkitCommandNode

golden crypt
# stuck trail What's the usecase?

Compatibility with old API meant for Spigot. The CommandAPI has a feature for unregistering commands. With the two separated Brigadier dispatchers on Spigot, unregistering commands added by other plugins using Bukkit API required extra work. Detecting which commands were added by Bukkit also meant that the CommandAPI could offer the ability to only unregister commands added by the Bukkit API.

So, since currently the CommandAPI supports both Spigot and Paper in one jar, it is possible to run CommandAPIBukkit.unregister("luckperms", false, false) for example. This is supposed to unregister commands with the name "luckperms" that aren't added by Bukkit. Currently on Spigot I can tell which commands are from Bukkit, so the command from the LuckPerms plugin still exists. On Paper I have not yet implemented this check, so it acts differently and just removes all "luckperms" commands no matter where they came from.

Command unregistration - CommandAPI Documentation v9.4.1
#

So not really a good reason, just I could do it before.

golden crypt
neat radish
#

Yep

golden crypt
glass smelt
#

No, because we cannot publish such a thing

#

if you want to access internals, you'd need to use the gradle userdev plugin

golden crypt
#

Unfortunately, the CommandAPI project is currently maven based, so I suppose direct access isn't currently possible.

#

Thanks for your help! Saved me a lot of time looking through this myself.

glass smelt
#

There was a community plugin for maven, unsupported by us, however

#

and they're just shipping jars against mojangs EULA

peak charm
#

Also a GPL license violation, the reason the DMCA takedown happened

tacit umbra
#

@neat radish so I think CommandSourceStack needs to point to using the executor more where available

#

as well as using the location instead of the senders location

#

how best to do that? To make it the default to automatically support the /execute command as much as possible

neat radish
#

needs to point to using the executor more where available
What does this mean exactly here?

proud timber
#

You have the command sender and the command executor

#

The sender actually typed the command, but the command executor is the entity as who it is executed

#

And Machine Maker is suggesting, I think, to use the executor rather than the sender for certain things

#

Idk what that entails precisely but I'd match vanilla as much as possible

peak charm
#

I think I've mentioned that a few times, you have to go out of your way to support the executor right now. The obvious/default way of doing things ignores them and since the API for getting the executor is nullable it's annoying to deal with

lusty sierra
# 
@Override
    public @NotNull <S> CompletableFuture<Suggestions> listSuggestions(@NotNull CommandContext<S> ctx, @NotNull SuggestionsBuilder builder) {
                Player p = ctx.getArgument("target", PlayerSelectorArgumentResolver.class)
                    .resolve((CommandSourceStack) ctx.getSource()).getFirst();
}

Is this sort of behaviour unsupported- Building a list of suggestions based on other command arguments?
I keep getting "No such argument 'target' exists on this command" when that command does in fact have Commands.argument("target", ArgumentTypes.player()). The player argument is also before the custom argument that is trying to get the "target" argument.

tacit umbra
#

brig might not parse arguments as its computing suggestions

#

that'd be a brig limitation (a reason to use a more robust command framework)

golden crypt
#

This shouldn't be a Brigadier limitation. The previous arguments in a command should be accessible when completing suggestions. A framework like CommandAPI is able to allow suggestions to depend on previous arguments because it is possible to pull those values from Brigadier.

golden crypt
golden crypt
#

Oh, hm... I'm tyring it myself and it is currently not working. So maybe something with Paper?

#

Hm, the command added by Paper to the Brigadier dispatcher does indeed look curious. So yea, it seems to me something about how Paper is getting its commands to work is making it impossible to get previous arguments inside the suggestions provider.

For reference, I'm using a command like this: https://pastes.dev/RxLcEK1kzG

pastes | RxLcEK1kzG

A paste containing 33 lines of java.

#

Ooooohhhh, wait, nevermind.

tacit umbra
#

nevermind as in it is a brig limitation?

golden crypt
#

This is a silly quirk in how Brigadier works. I made a bug report about this in their repo because I thought it was strange behavior. (https://github.com/Mojang/brigadier/pull/142#:~:text=I also thought it was weird that this example test for suggestions failed%3A). When a command goes through a redirect, it splits into a child context. When executing the command, Brig gives you the child context, but when suggesting the command, Brig gives you the original context.

The way Paper registered my command, the /test I was typing in gets redirected to the /bukkittestplugin:test node. That's where the redirect comes from, invoking this strange behavior.

So, in order to access the arguments, you have to get the child of the given context, which actually contains the arguments. In the command I sent, to get the Player target, I can do this:

Player player = context
    .getLastChild() // Get the child context that actually holds the args
    .getArgument("target", PlayerSelectorArgumentResolver.class)
    .resolve(context.getSource())
    .get(0);

So, while it works weirdly, you should be able to get the arguments while completing suggestions if you use CommandContext#getLastChild. I was a bit thrown off since I didn't expect Paper to insert a redirect into my command, but it seems reasonable. I'd also err on the side that this is intended Brigadier behavior given the lack of a response on Brigadier#142.

Suggestions.png
lusty sierra
proud timber
#

I think Brig is more of a "source available" thing than open source

#

They don't really care about the framework unless they need to for Minecraft

golden crypt
#

Yea, tho I suppose if they’re not changing anything then everything is “working as intended”

lusty sierra
#

Hey so I have an argument that ultimately returns an adventure Key (from what I've seen, Paper is moving towards adventure keys so I've replaced most of all of my custom stuff to used Keyed and Key from adventure).

However, I want to make it so the user can input the argument without needing to explicitly put the separator :. If no separator is inputted, then the namespace will default to the plugin's namespace. Is that possible with Brigadier? There doesn't seem to be any string argument types that allow any and all characters.

# 
public class CruxKeyArgument implements CustomArgumentType<Key, String> {
    @Override
    public @NotNull Key parse(@NotNull StringReader reader) throws CommandSyntaxException {
        return Crux.key(getNativeType().parse(reader));
    }

    @Override
    public @NotNull ArgumentType<String> getNativeType() {
        return StringArgumentType.string();
    }
}

I tried doing this but this makes it so you can't put the separator, : since, for example, if I input one:two, it only returns "one".

primal coral
#

maybe try using the ArgumentTypes.key() as the native type, and parse it yourself in parse

ebon hill
#

key() and namespacedKey() will use the minecraft namespace by default so you'd have to parse it yourself if you want your namespace as default

proud timber
lusty sierra
#

This is the new code. It is getting the string properly and everything now, it just thinks there's an error while typing.

public class CruxKeyArgument implements CustomArgumentType<Key, Key> {
    @Override
    public @NotNull Key parse(@NotNull StringReader reader) throws CommandSyntaxException {
        String string = readString(reader);
        Bukkit.broadcastMessage(string);
        return Crux.key(string);
    }

    public String readString(@NotNull StringReader reader) {
        final int start = reader.getCursor();
        while (reader.canRead() && reader.peek() != ' ') {
            reader.skip();
        }
        return reader.getString().substring(start, reader.getCursor());
    }

    @Override
    public @NotNull ArgumentType<Key> getNativeType() {
        return ArgumentTypes.key();
    }
}
primal coral
glass smelt
#

public @NotNull ArgumentType<NamespacedKey> getNativeType() {

#

MaterialArgumentType example from the paper repo

lusty sierra
#

Yeah this seems funky..

lusty sierra
proud timber
lusty sierra
#

I allow all other namespaces

proud timber
#

If it's always your thing just use StringArgumentType.word

proud timber
lusty sierra
#

That's the tricky part it seems

proud timber
#

It shouldn't be hard at all

lusty sierra
#

That was my first thought until actually trying to do it lol

proud timber
lusty sierra
#

It's weird because ArgumentTypes.key() works by itself. But when creating a custom argument that uses it, it's like you can't use : anymore (at least in the client's eyes)

proud timber
proud timber
#

Maybe an implementation mistake somewhere

#

I also use that in my plugin

#

And that works, and I assume the same for the tests

#

So it's likely that CustomArgumentType.Converted works but just CustomArgumentType doesn't

#

Just CustomArgumentType should probably also have been tested...

#

@lusty sierra probably best to open an issue

covert wren
# lusty sierra It's weird because `ArgumentTypes.key()` works by itself. But when creating a cu...

I went ahead and tested this code with a custom key argument and it seemed to work fine on 1.20.6:

  @Override
  public void onEnable() {
    getServer().getPluginManager().registerEvents(this, this);
    getLifecycleManager().registerEventHandler(LifecycleEvents.COMMANDS, commands -> {
      commands.registrar().register(
        Commands.literal("test")
          .then(Commands.argument("someKey", new CustomArg())
            .then(Commands.literal("anotherLiteral")
              .executes(commandContext -> {
                commandContext.getSource().getExecutor().sendPlainMessage("The key in question: " + commandContext.getArgument("someKey", Key.class).asString());
                return Command.SINGLE_SUCCESS;
              })
            )
          ).build());
    });
  }

  private static class CustomArg implements CustomArgumentType<Key, Key> {

    @Override
    public @NotNull Key parse(@NotNull StringReader reader) throws CommandSyntaxException {
      int start = reader.getCursor();

      while (reader.canRead() && reader.peek() != ' ') {
        reader.skip();
      }

      String key = reader.getString().substring(start, reader.getCursor());
      if (key.indexOf(':') == -1) {
        return Key.key("custom", key);
      } else {
        return Key.key(key);
      }
    }

    @Override
    public @NotNull ArgumentType<Key> getNativeType() {
      return ArgumentTypes.key();
    }
  }

The client handled the key argument like expected

proud timber
#

Interesting

viral fossil
#

or a permission string getter instead of canUse to keep it simple, have the full capabilities only in the brig builder

golden crypt
#

Would it be possible to do both, and have the permissions method delegate to a requirement? So somewhere the implementation would look like:

// Command is acessible by default
Predicate<CommandSender> requirement = sender -> true;

withRequirement(Predicate<CommandSender> requirement) {
    this.requirement = this.requirement.and(requirement);
}

withPermission(String permission) {
    this.withRequirement(player -> player.hasPermission(permission));
}
viral fossil
#

BasicCommand is an interface, so it has to be a method where a return value is used

golden crypt
#

Ok, so something like?

interface BasicCommand {
    default String getPermission() {
        // Null means no permission set
        // Or maybe Optional would be better
        return null;
    }

    default boolean canUse(CommandSender sender) {
        return true;
    }
}

// When creating CommandNode
BasicCommand command;

Predicate<CommandSource> requirement;
String permission = command.getPermission();
Predicate<CommandSender> canUse = command::canUse;
if (permission == null) {
    requirement = source -> {
        CommandSender sender = source.bukkitCommandSender();
        return canUse.test(sender);
    }
} else {
    requirement = source -> {
        CommandSender sender = source.bukkitCommandSender();
        return sender.hasPermission(permission) && canUse.test(sender);
    }
}

LiteralArgumentBuidler
    .literal(commandName)
    .requires(requirement);
viral fossil
#

hmm having both might be confusing, but could work too

#

so in any case, you'd prefer one or both methods there over the pr?

golden crypt
#

Just an alternative idea. Imo if the BasicCommand is basically a Spigot command then just a permission seems reasonable in most cases, but I use full Brigadier commands anyway.

modern pagoda
#

is it possible to allow any character in single string arguments (word not greedy)
by default only alphanumeric values are accepted which is kinda stupid

glass smelt
#

no

modern pagoda
#

excuse me but thats stupid

#

why the hell is this even a limitation

glass smelt
#

ask mojang

viral fossil
#

Mojang hates non english speakers

glass smelt
#

a quoted string can accept non-ascii but needs to be, well, quoted

modern pagoda
#

yeah in my case quoted is stupid and totally unintuitive because " is literally an allowed input character

unreal tangle
#

\" 😏

modern pagoda
#

the suggestions would look like shit

#

i could use key arguments to support colons but !"/ are expected inputs so what else could i use

#

and putting the argument at the end is not really an option (so it can be greedy)

#

okay well... doesnt look that bad

image.png
golden crypt
#

I wonder if there is a Minecraft bug report for this to possibly get it classified as a gameplay issue and not a Brigadier API issue...

neat radish
#

This is because of brig

lusty sierra
# covert wren I went ahead and tested this code with a custom key argument and it seemed to wo...

So I found out that using the Key argument directly works but once you try getting an object from the key, that's when it produces the issue. For example, let's say you have something like this:

public abstract class CruxKeyedArgument<T extends Keyed> implements CustomArgumentType<T, String> {
    public @NotNull T parse(@NotNull StringReader reader) throws CommandSyntaxException {
//CruxCmdArguments.CRUX_KEY is basically the same CustomArg class that you sent.
        return this.parse(CruxCmdArguments.CRUX_KEY.parse(reader));
    }

    public abstract @NotNull T parse(@NotNull Key var1);

    public @NotNull ArgumentType<String> getNativeType() {
        return StringArgumentType.string();
    }
}

And you extend it:

public class SomethingKeyedArgument extend CruxKeyedArgument<Something> {
    @Override
    public @NotNull Something parse(@NotNull Key key) {
        //get it in some way
    }

If you try using the SomethingKeyedArgument directly, like so:
Commands.argument("something", SomethingKeyedArgument),
it produces the issue.

#

Seems like a weird issue, I don't know. At least it can be fixed by approaching it differently:

Commands.argument("something", CustomKeyArg)
.executes(ctx ->{
  Key key = ctx.getArgument("something", Key.class);
  Something something = //do the parsing here instead of within the actual argument itself.
})```
lusty sierra
#

It appears my brain decided to take a long vacation. The reason the error was present was because CruxCmdArguments.CRUX_KEY was still using CustomArgumentType<T, String> and StringArgumentType.string() when it should have been using CustomArgumentType<T, Key> and ArgumentTypes.key().

proud timber
#

So does it work for you now or no?

lusty sierra
#

Yeah it's working now

lusty sierra
#

Is there no way to use a CustomArgumentType to parse the argument based on the command context? Like, for example, getting data from the player using the command.

tacit umbra
#

Nope, brig doesn’t support that. That’s why lots of the built in argument types are “resolvers” where you have to pass the source to get the value

golden crypt
#

Technically, it is possible to achieve this by extending the ArgumentCommandNode class, though https://github.com/Mojang/brigadier/pull/144 makes that a little weird to pull off. Otherwise, yeah, having your ArgumentType return a Function<Player, T> or equivalent and passing it the previous context in your suggests or executes should always work.

proud timber
#

Contributing to Brigadier is a lost cause

lusty sierra
#

Alright. Thank you!

#

Brigadier is a pretty nice command framework in general though. So thanks to the Paper team for opening it up for us (:

modern pagoda
#

i am missing methods to define my own namespace fallback for command arguments

image.png
covert wren
covert wren
modern pagoda
#

is there some way of checking whether a user has access to any of the subcommands without reperforming all of the checks?
there are to many conditions and commands but i dont want to add a permission to the root command or smth

image.png
#

dont think so lol
requires only returns the command source stack so i have to do all checks right ?

ebon hill
#

the subcommand nodes can have their own requirement predicate, not sure what you mean otherwise

modern pagoda
#

i know they do
but the root command gets suggested even when no subcommand can be used

peak charm
#

Well, yeah, because you don't have a permission on the root command blocking access

#

So I guess I'm saying you need a requirement predicate on the root that ORs all the requirements for the subcommands, like you feared

modern pagoda
#

great 🙂

image.png
#

not too bad for this one tho

image.png
glass smelt
#

what you'd do is make all of those perms have world.command as a child perm

modern pagoda
#

smart call

golden crypt
# modern pagoda great 🙂

Could maybe also put all your subcommands in a list and loop over each of them to check permissions and add them as children to your root.

modern pagoda
#

naw im doing it the cat way but thanks tho

#

thats so smart

image.png
glass smelt
#

would be the other way around

#

child perms are given when they have the root perm

modern pagoda
#

holy thats even smarter

glass smelt
#

so, having access to world.command.list would grant access to world.command

modern pagoda
#

i named the permpack for all commands
worlds.commands is that a too vague distinction in your opinion?

proud timber
modern pagoda
#

It's useless

#

If you can't use the command why even showing it

proud timber
#

I wouldn't have cared about it 🤷‍♂️

stray remnant
#

no reason to clog your tab complete if you dont need to

proud timber
#

"clog" as if one command makes a difference when you have a million other plugns with each a million other commands

#

Anyway I don't think it's worth a discussion

modern pagoda
#

All my plugins only expose their commands to users with the required permissions or conditions
A new user without any permissions will see about 4-8 commands based on the server they are connected to

#

I even made a plugin just to add a permission to the LuckPerms root command if a user can't use luckperms

#

I absolutely hate it when plugins dont properly use permissions for their commands
For example essentials exposes (or at least did at some point) all of its commands even if the player has no permission to use them
A new user will have no clue what they can and can't do if that is the case

fickle gulch
#

Question, does this api respect aliases in commands.yml? or are there other ways to achieve command overwriting?

#

[00:51:26 WARN]: Could not register alias warp because it contains commands that do not exist: warps:warp $1-
[00:51:26 WARN]: Could not register alias setwarp because it contains commands that do not exist: warps:setwarp $1-

trying to overwrite some of essentials commands, but logs claim the commands are non existing

#

just disabling the commands did the trick:

command-block-overrides: []
ignore-vanilla-permissions: false
aliases:
  warp:
  - []
  setwarp:
  - []
modern pagoda
#

Pretty sure essentials is using Bukkit commands

tacit umbra
#

@vernal fractal before I dive into this... is there a potential issue with CommandAPI (since the brig changes) where running /minecraft:reload "erases" plugins registered via your lib? One of my plugins triggers such a reload, and plugins that used CommandAPI suddenly don't have their commands anymore. I'll dive into it if you aren't sure.

golden crypt
modern pagoda
#

isnt it possible to get a parsed argument from the context within a suggestion provider?

image.png image.png
#

iirc velocity can do it

tacit umbra
#

try on the child context of the context

#

or the last child, I forget exactly

modern pagoda
#

i can only access the context provided by the getSuggestions method

image.png
tacit umbra
#

context.getChild()?

#

context.getLastChild()?

modern pagoda
#

oh i see

#

nice works thanks for the tip

lapis relic
#

is there a way to modify this unknown command message when a player attempts to use a command that they don't have permission to use?

I know there is the UnkownCommandEvent I could listen to, but is there a way to do it when registering the command via the new command api?

image.png
glass smelt
#

No

granite hound
#

if you do change it, a lot of server owners will just use something like pl hide or whatever to effectively revert it to the default message anyways

lapis relic
#

wouldnt really call that a reason to not have it. not all servers want that, nor does that really have customization over each different commands deny message.

glass smelt
#

Everything is handled by the dispatcher

#

Adding rejection messages requires us to basically excessively hack brig just to support that

lapis relic
#

yeah, not saying it should/shouldnt be added... having to hack brig is a valid reason to not add it. The reason of "oh some servers install this 3rd party plugin to change the output anyways" isnt a valid reason

tacit umbra
#

I feel like in pretty much every situation, you want commands they don't have permission for to not be even known to the client and so that message makes perfect sense

#

if you have some conditionally-allowed command, clients should just always have permission and then do a separate check on whatever that condition is within the execute method

lapis relic
tacit umbra
#

I would just do the perm check in the execute in that case.

#

not the canUse

lapis relic
#

Yeah, thats a good work around, a lot better than using the unknowncommandevent. Not perfect, but i'll make do

#

`
On a different note. What are the thoughts on adding more info the BasicCommand interface, such as description, label and aliases.
That way to register a command its as simple as:
commands.register(new FunCommand())

glass smelt
#

brig doesn't have some of those concepts

#

and trying to pretend that they exist just presribes us to bukkit-isms

lapis relic
#

i thought the whole point of the BasicCommand was to provide a similar way to bukkits implementation. (as described on the docs).

And I dont think this wouldnt be some hack or anything, something as simple as a couple of methods that need to be overridden

unreal tangle
#

BONK feature creeped

glass smelt
#

The goal is to provide a minimal basic interface to register basic commands without having to learn brig as a means to quasi deprecate the legacy bukkit command map

lapis relic
#

dont see how this is a feature creep. u could argue the entire basic command is a feature creep

unreal tangle
#

it is

#

that has been argued

#

its the middle ground of "we know brig is complex, here if you wanna hack something together real quick" vs "pls use a proper framework for brig or learn brig yourself"

glass smelt
#

vanilla has 0 concept of labels or description afaict

lapis relic
#

well yeah, but we're not vanilla, this is paper

unreal tangle
#

again, this is a minimal abstraction over brig

glass smelt
#

That information does not exist on the brig nodes

lapis relic
#

well labels/desriptions exist with or without this

#

ur still having to implement them

glass smelt
#

no?

lapis relic
#

if i just supply the literral command node, what is shown up on bukkits /help then?

#

is it just not there?

#

i figured it was just there with a blank description, but i haven't tested it

glass smelt
#

probably a blank description as vanilla does have descriptions for commands

lapis relic
#

yeah, but paper (technically bukkit) is adding it.

glass smelt
#

within the legacy bukkit system

#

but that is all going to be blank strings

#

There is literally interest in our part hacking brig to add such things

#

if it was up to me I'd remove bukkits help command right now

lapis relic
#

thats a whole other discussion

glass smelt
lapis relic
#

well, if "descriptions" and "labels" dont exist, aliases do, (redirects) so seems valid to me that those should be apart of the basic command

mellow jolt
#

do custom brig commands show up as mojang provided?

lapis relic
#

i can test it right now

glass smelt
#

nope

lapis relic
#

beat me to it, yeah its just blank

glass smelt
#

the string is hardcoded for the mojang commands in the legacy bukkit wrapper class

lapis relic
#

maybe should add a "A plugin provided command." if no description given

#

better than it being blank

mellow jolt
#

huh PluginVanillaCommandWrapper extends VanillaCommandWrapper

ebon hill
#

clearly this means we need to expand on the o.b.help API instead kappa

glass smelt
#

I think I've seen like 2 plugins interface with that API, ever

ebon hill
#

well maybe if it was expanded it would be used more often

glass smelt
#

ah, so, the register method lets you put in stuff for the legacy command wrapper

#

default @Unmodifiable @NotNull Set<String> register(final @NotNull String label, final @Nullable String description, final @NotNull BasicCommand basicCommand) {

mellow jolt
lapis relic
#

finally someone making some sense

tacit umbra
#

@neat radish how do you think the resource_or_tag and resource_or_tag_key argument types should work? They directly use the Either type

#

we have all the needed types to add those to the ArgumentTypes API

#

one is an Either<Holder.Reference<T>, HolderSet.Named<T>> and the other is Either<ResourceKey<T>, TagKey<T>>

tacit umbra
#

actually, don't wanna do the first one cause it would require an ugly T extends Keyed in a spot one shouldn't be

modern pagoda
#

I cant find the example admin command anymore
did you delete it?

sacred juniper
#

^

tacit umbra
#

it still exists in the test-plugin folder of the repo

tacit umbra
#

@neat radish how tricky would it be to implement a way to convert an nms ParseResult<CommandSourceStack> to the api one? There's a couple places where exposing the parsed command might be useful

neat radish
#

Cause lowkey that should already be exposed? Like in event.registrar().getDispatcher().parse

tacit umbra
#

well I mean an already parsed one from vanilla

#

I'm looking through open feature requests, and noticed one to format whisper messages, and realized that is possible via AsyncCommandChatDecorateEvent, but there is no context about what command was run

#

if we could convert the existing nms ParseResults into the API CommandSourceStack type, we could add that to the event and then the command should be known

neat radish
tacit umbra
#

those node wrapping/unwrapping get cached tho right?

neat radish
#

ye

#

We will just need to move that logic outside of the mirror node

proud timber
#

🤔 CustomArgumentType.Converted.html#convert should probably not only allow native argument types, but also other Converted types

#

That's how Quilt does it too

glass smelt
#

Not sure what you mean there

#

Quilt is also a low level thing so mods can add custom types natively in there

#

At least in terms of what that would actually do given that the underlying type needs to be supported by brig, nesting them for conversions makes very little sense imho

proud timber
proud timber
lapis relic
#

is it possible to unregister a command after its been registered via the lifecyclemanager?

tacit umbra
#

do a /minecraft:reload or Server#reloadData and don’t register the command again when the event fires

lapis relic
tacit umbra
#

well you are using the event right? the event re-runs during those reloads, so itll just run the event again

#

so if nothing changed, the command will be re-registered

lapis relic
#

i think i follow

#

it's all making sense now, I guess some feedback would be maybe for reloading data via api, would be to specify what should be reloaded. for example, if we wanted to just reload commands, we wouldnt have to reload everything else, (tbh idk apart from datapacks, and i guess now commands what all gets reloaded from it)

tacit umbra
#

That… isn’t simple. Lots of things depend on other things. Like if you reload only tags, you also have to reload commands I think. And trying to map out those “dependencies” isn’t a good idea. Just don’t be reloading too often

warm cargo
#

since 1.21.2 nms Entity is no longer CommandSource and CommandSource.NULL is used for creating CommandSourceStack. How would this api handle that?

amber shell
#

Is it possible that there is an thread sync problem using by the new command api?

stuck trail
#

elaborate please

amber shell
# stuck trail elaborate please

In my clan system it is sometimes the case that when you transfer a clan, for example, the ranks are not displayed correctly and are not recognized correctly by parts of the system. For example, the /clan command in a GUI indicates that you still have the Owner rank. However, it is displayed correctly via the /claninfo command. Subsystems are also affected by this. However, the whole thing is thread-safe via the concurrents. The individual clan information is also managed centrally via a single manager that can be accessed by all systems. I therefore cannot explain what the problem could be.

glass smelt
#

"in a GUI"

#

brigadier just deals with parsing + dispatching

#

anything display related is basically 100% on you

#

the only caveat would be that you can't change the command tree at runtime, i.e. removing/adding things based on perms, without resending the entire tree

amber shell
#

Okay thank you

wind venture
#

Potentially off topic, can we get some kind of API to check if a command is safe to execute async? Allow commands to declare themselves as being able to run asynchornously which will default to false.

The use case for me is I make a couple plugins that allow you to execute commands under certain conditions, one of them is a simple conditional and timed command executor plugin and the other is an anticheat that allows you to execute commands on flags.

Performance can get really awful for running tons of commands on the main thread whose implementation is often already thread-safe or could be made thread safe with very little modification.

neat radish
wind venture
#

Yeah I do that in my code the problem is my plugins execute arbitrary commands including from other plugins

#

There's no way I can know that calling /aopetihe defined in the config for a server by one of my users can be invoekd async or not

neat radish
#

Then you should ask those plugin developers to expose API that allows you to interface with it async.

wind venture
peak charm
#

Executing a command from another thread is unsafe regardless of what the command does

wind venture
#

that all use diffefenrt apis?

peak charm
#

Someone on the main thread could be registering a command at the same time

wind venture
neat radish
#

If you are executing another command async that you did not make, it does not mean that its all the sudden thread safe.

wind venture
#

I know

#

I am not doing that

#

I am only doing that for my own commands

peak charm
#

I'm saying it doesn't matter if you were careful in your command implementation, the command machinery itself isn't thread safe

wind venture
#

I was hoping that could change

peak charm
#

Unless you bypass brigadier that doesn't really help

wind venture
#

I do actually

#

I went as low as I could go

#

following the stack trace

#

but anyways

#

that's a side note

#

I don't think we're on the same page as to what I"m asking.

I have a plugin is to have config.yml where you can go

whenSomeEventHappens:
- /somearbitraryCommand
- /anotherArbitraryCommand
#

I would like to be able to go

String command;
if (Bukkit.getServer().getCommandCanBeExceutedAsync(command)) {
    Bukkit.executeCommandAsync(commandSender, command);
} else {
   Bukkit.dispatchCommand(commandSender, command);
}```
#

If I were to implement a PR adding support to execute commands like this, would you accept it?

#

For Bukkit Commands its pretty simple, add an annotation or something on top of OnCommand() to mark it as executable async

For Brigadier Commands something similar is already done in Cloud with command executors, though I'll probably take a different approach.

Point is I'm fully willing to implement this all myself and make it work. My only question is if you would be open to merging it (@peak charm @neat radish?) once I'm done because there's no way this will be adopted if it doesn't make it way into Paper.

neat radish
# wind venture I do actually

Relying on this is an implementation detail, Server#dispatchCommand not using brig will be removed in the future as generally its an old implemenmtation.

neat radish
#

So sadly this will not be adopted.

#

Making brig support async commands is too invasive and not really a goal in Paper.

#

Especially when plugin devs already mostly delegate expensive commands async anyways

stuck trail
#

Commands also aren't API?

#

If you talk to another plugin? Expose am API?

glass smelt
#

Brigadier doesn't store such context and it's generally not something that we're inclined to hack into brigadier to provide support for unsupported behavior

#

if you wanted async dispatch support, you would need to get such support and annotation added to brigadier, and I really doubt that they're interested in that either

viral fossil
#

Even then I would expect like 0.1% of plugin commands to actually be thread safe, unless all they do is send a message

proud timber
#

Also how would async commands work with datapacks and command blocks, those expect a return value directly

full geyser
#

Also, you should know that making something "async" doesn't autmatically fix all problems and make the stuff more performant. It is more worthwhile for people to fix their performance leaks than to try to offload the work

#

If your command takes 20 ms to run, putting it async will still lead to terrible performance — it just wouldn't affect the main thread as much

#

Furthermore, unless you are genuinely running into performance issues, there is no need to over-optimize your code. Modern day computers are really fast, you know?

#

Async is great for http requests or file io. But that should be handled by the command creator already anyways

lucid steppe
#

Add a material argument

unreal tangle
#

Item and BlockType will do that

lucid steppe
unreal tangle
#

(they already do btw)

lucid steppe
unreal tangle
#

in what scenario do you need both o.O

#

with how different items and blocks are

lucid steppe
#

wait an item registry contains like stone right?

unreal tangle
#

Yes

lucid steppe
#

oh ok then nvm

full geyser
glass smelt
#

They're two seperate distinct things, Material is a blatently bad system which consistenly leads to errors because it blindly confuses those concepts together which no longer exist

lucid steppe
lucid steppe
peak charm
lucid steppe
glass smelt
#

Items and Blocks are already distinct types in vanilla, thankfully many items are blocks and so they match up 1:1, but then there are the fun cases where there is no alignment there; and generally, trying to pretend that they exist in one happy bundle like they did 10 years ago is a constant source of headaches with the API and plugin devs not understanding that carrots and carrot are not the same thing

icy moon
#

Not sure if this is related but thanks to whoever made CommandNode requirement field public ❤️

image.png
proud timber
#

.requires is a thing yk (and getRequirement)

wicked garnet
#

brigader crash tho hoo snow

marsh shard
#

wat

full geyser
wicked garnet
#

nonono im not saying i have issues

#

im joking around

#

there used to be a crash for brigader

#

but its fully patched now

full geyser
#

Tbh @neat radish, we might just be able to close this post now since this is getting un-experimentaled soon anyways and I believe there is no more constructive feedback to be given anymore

mortal zealot
glacial mist
tacit umbra
#

Yep

light vessel
#

what's the diffrence between brigrader command API vs regular command registration from bukkit?

glass smelt
#

the brigadier API is basically a fairly low level means of accessing mojangs command system

#

the bukkit command system dates back from when craftbukkit replaced the vanilla command system, now it's just in some weird state of basically trying to mirror the vanilla command system

#

it's a lot more primative but somewhat easier from a beginner perspective if you don't want a lesson on trees

viral fossil
#

It gives you proper client-side completion of certain arguments, like number ranges, resource locations, quoted strings and whatnot, also telling the client beforehand what goes where

full geyser
#

Using Brigadier gives your command users a much better experience using your command

limpid fulcrum
#

There are downsides also though. Your commands must more strictly follow mojang's command style to get these enhancements

full geyser
#

The very popular example of StringArgumentType.word() being incredibly restrictive in terms of which characters you can enter

light vessel
#

is using brigradier harder? if so how much?

full geyser
#

Well, it's just more technical

#

More "fancy sounding classes"

#

Bukkit commands is just a String array

light vessel
#

does it work w/ folia

full geyser
#

Yes

light vessel
#

is there any good examples of brigraider command

full geyser
full geyser
#

(Seen with code)

light vessel
#

bruh so complicated

full geyser
#

(It's really not that complicated)

#

As I said, it's just a whole bunch of complicated sounding classes

#

Once you understands its essence, it is incredibly easy

light vessel
#

branching is cpmplicated

full geyser
#

Not much more complicated than nested switch statements shrug

#

(Which is basically what Bukkit commands are)

light vessel
#

should I stick w/ bukkit or go to brigradier

full geyser
#

I heavily suggest that you learn Brigadier. If you do not feel comfortable with plain Brigadier, feel free to use a command framework

#

Like CommandAPI

static owl
#

Brigadier. Much nicer user and developer experience.

light vessel
#

but so hard, uhhh

full geyser
#

Bukkit commands are a legacy system

full geyser
light vessel
#

is there any youtube tutorials

full geyser
#

No, and that's good, because youtube tutorials gets outdated way too quickly

#

Reading is a developers best skill :3

light vessel
#

but it looks so complicated

#

can i get some direct help please?

full geyser
light vessel
#

uh, I'm trying to send a reosurcepack in the config phase of the API, but I don't know how to do that

#

if you can't answer this queston, here's another thing I need help with

#

the brigraider command API. I want to change my command system in my plugin to use that API

light vessel
full geyser
light vessel
#

oh

full geyser
light vessel
#

I want to move to using brigraider api

#

i'm reading the docs

full geyser
#

For the permission, you should use a requirement:

You can display the help message by putting a .executes method onto your root node:

Commands.literal("gem")
    // ....
    .executes((ctx) -> sendHelpMessage(ctx.getSource().getSender())
    .build();

Since your first argument is a player, you can add a player argument type and resolve that inside your executes:

Commands.literal("gem")
    .then(Commands.argument("target", ArgumentTypes.player())
        // at some point later
        .executes(ctx -> {
            Player target = ctx.getArgument("target", PlayerSelectorArgumentResolver.class).resolve(ctx.getSource()).getFirst();
            // ...
        });
    );

Your second argument is a number, so I suggest you do the same with a number argument type:

For the final argument, which seems to be the gem type, I would suggest you use multiple literals:

Alternatively you could also just use a word string argument type and add suggestions:

light vessel
#

wait, this actualy might be better than bukkit

full geyser
#

That's all the spoonfeeding I will be giving for today. I am tired and gotta sleep. I wrote the docs very verbosely, so I hope you will take your time reading them foliaheart

light vessel
#

because you can set subcommands to require permissions

light vessel
full geyser
#

Yeah ikr

light vessel
#

thank you so much

full geyser
#

Yw

light vessel
#

bye

full geyser
#

goodnight