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
Implement Type-safe Kotlin DSL #545
base: dev/dev
Are you sure you want to change the base?
Conversation
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.
Again, this should not be a replacement. This should rather be an alternative for those who want this.
Please also remove the additional methods containing the Optional
part, especially in the CommandTreeDSL
because as mentioned in your issue, optional arguments are going to be removed from CommandTree
s in the future. Regardless, these methods are not wanted in the CommandAPICommandDSL
either.
Other than that I honestly don't really understand how this works exactly so I definitely want to test this myself first, but that's obviously my problem :D
The documentation is built using mdbook, I believe there are download links for the required executables in the wiki here on GitHub. |
Does that mean I should leave the method's arguments and processing as they are rather than changing them, and define new methods in another file? The only incompatibility caused by this replacement is when specifying Retrieval methods using get and cast are still supported. I just made it possible to use the getter inside the lambda. https://github.com/JorelAli/CommandAPI/pull/545/files#diff-a396609c4f42dbc82e492e9976de00ceb4c97805935fc741902fbb6cb4eae000 I only changed the optional tests and added some tests. |
Yeah, that would be ideal.
I mean, technically you can do whatever here, just don't introduce duplicate methods like the ones that have the |
Hmm, I just realized, we do have Delegated properties support for the Kotlin DSL. That of course doesn't prevent this feature from being added but generally it is sort of the same functionality, right? |
I think delegated properties are still syntactic sugar for commandTree("sendmessageto") {
playerArgument("player") {
greedyStringArgument("msg") {
anyExecutor { _, args ->
val player: String by args // At runtime: Inconvertible types, Player is not a String
val message: String by args
player.sendMessage(message)
}
}
}
} The changes here allow there to be a compile time exception. IDEs can also statically analyze the code and catch the error, which makes it easier for the developer to notice and fix. commandTree("sendmessageto") {
playerArgument("player") { getPlayer ->
greedyStringArgument("msg") { getMessage ->
anyExecutor { _, args ->
String player = getPlayer(args) // At compile time: Inconvertible types, Player is not a String
val message = getMessage(args)
player.sendMessage(message)
}
}
}
} |
Yeah, true. I mean, it also depends on the variable name you put there so the type safety is still only sort of there. |
#544
I didn't change the documentation because I didn't really understand it.