Type-safe Kotlin DSL #544
Comments
|
Hmm, interesting. |
|
@DerEchtePilz Thank you for your interest. We will need to make a breaking change to "optional". Currently, specifying For example: commandTree("sendmessageto") {
playerArgument("player", /* Actually: optional = false */) { getPlayer ->
greedyStringArgument("msg", optional = true) { getMessage ->
anyExecutor { _, args ->
val player = getPlayer(args) // Expected: Player (not-null)
val message = getMessage(args) // Expected: String? (nullable)
player.sendMessage(message)
}
}
}
}However, depending on the value of the argument and changing the type is difficult and requires changing the method to call. commandTree("sendmessageto") {
playerArgument("player") { getPlayer ->
greedyStringOptionalArgument("msg") { getMessage ->
anyExecutor { _, args ->
val player = getPlayer(args) // Returns Player (not-null)
val message = getMessage(args) // Returns String? (nullable)
player.sendMessage(message)
}
}
}
}I would like to discuss this 🙏 |
|
@sya-ri On a different note, iirc @willkroboth currently is working on updating the way commands are registered and has also made changes to optional argument handling. During that, we also kinda decided that optional arguments don't really make sense in a
This will also never change. Optional arguments just have the risk of not being present which is why we return |
Yes, that's exactly right 😢 I would like to take this opportunity to remove the deprecated methods. I would to leave deprecated methods that are not related to arguments. What do you think? |
|
I hate that MultiLiteralArgument mess... A bit of history about that But yeah, those deprecations are already in the process of being removed, we just can't do it until 10.0.0 |
I see. Changes to this issue will also be merged in 10.0.0. Right? |
|
That solely depends on the nature of your PR and what it changes. |
|
OK. I will create a PR as soon as possible 😉 |
|
I wonder if this could be added for Java as well. If I add this method to public <T, N extends AbstractArgument<T, ?, Argument, CommandSender>> Impl then(final N tree, BiConsumer<Function<CommandArguments, T>, N> consumer) {
this.arguments.add(tree);
consumer.accept(args -> args.getUnchecked(tree.getNodeName()), tree);
return instance();
}Then this command syntax becomes possible: new CommandTree("sendMessageTo")
.then(new PlayerArgument("player"), (getPlayer, playerArgument) -> playerArgument
.then(new GreedyStringArgument("msg"), (getMessage, messageArgument) -> messageArgument
.executes((sender, args) -> {
Player player = getPlayer.apply(args);
String message = getMessage.apply(args);
player.sendMessage(message);
})
)
)
.register();Java lambdas aren't as clean as Kotlin's, but it still hides the unsafe casting (calling With public <T> Impl withArgument(AbstractArgument<T, ?, Argument, CommandSender> argument, BiConsumer<Function<CommandArguments, T>, Impl> consumer) {
this.arguments.add((Argument) argument);
consumer.accept(args -> args.getUnchecked(argument.getNodeName()), instance());
return instance();
}
public <T> Impl withOptionalArgument(AbstractArgument<T, ?, Argument, CommandSender> argument, BiConsumer<OptionalArgumentProvider<T>, Impl> consumer) {
argument.setOptional(true);
this.arguments.add((Argument) argument);
consumer.accept(new OptionalArgumentProvider<>(argument), instance());
return instance();
}I can make a command like this: new CommandAPICommand("sayhi")
.withArgument(new PlayerArgument("target"), (getTarget, command1) -> command1
.withOptionalArgument(new GreedyStringArgument("message"), (getMessage, command2) -> command2 // This is a little annoying, where the lambda parameters must have different names --- Although command1 and command2 do refer to the same object (`new CommandAPICommand("sayhi")`), so maybe command2 can be removed
.executes((sender, args) -> {
// target is not only type-safe, but also guaranteed to be not null
Player target = getTarget.apply(args);
target.sendMessage("Hello!");
Optional<String> message = getMessage.getOptional(args);
if(message.isPresent()) {
target.sendMessage(message.get());
} else {
target.sendMessage("How are you doing?");
}
})))
.register();For the optional argument here, I created an public class OptionalArgumentProvider<T> {
private final AbstractArgument<T, ?, ?, ?> argument;
public OptionalArgumentProvider(AbstractArgument<T, ?, ?, ?> argument) {
this.argument = argument;
}
@Nullable
public T get(CommandArguments args) {
return args.getUnchecked(this.argument.getNodeName());
}
@NotNull
public T getOrDefault(CommandArguments args, T defaultValue) {
return args.getOrDefaultUnchecked(this.argument.getNodeName(), defaultValue);
}
@NotNull
public Optional<T> getOptional(CommandArguments args) {
return args.getOptionalUnchecked(this.argument.getNodeName());
}
}Just a way of giving more options than a basic |
Description
The current Kotlin DSL requires casts, so it is not type-safe. We can't get the most out of Kotlin.
Expected code
I have confirmed that this code can actually be rewritten. You can try it out using this library: https://github.com/sya-ri/commandapi-kotlin-improved
In my library, the Kotlin DSL for CommandAPICommand is not supported, but we should be able to rewrite it similarly.
Extra details
If necessary, I would like to submit a PR for the changes made by my library 😉
The text was updated successfully, but these errors were encountered: