Closed Distractic closed 9 months ago
When we use the annotations to set argument, the default value from kotlin declaration is not supported.
Default arguments in Kotlin just end up generating overloaded methods. So, in order to support these default arguments we'd have to support overloaded methods. While possible, I am not entirely convinced that this is something we want.
A strategy for implementing this system would be to find all methods with identical @CommandMethod
annotations and then pick the most verbose method and infer argument types from that one. We'd also have to have to add support for multiple methods in the command executor.
I don't know if this can help you for the implementation, but with the kotlin reflection, you can call a function using the default variable (if you put not enough parameters) through the method KFunction.callBy. This is an exemple :
class A {
fun myMethod(
value1: Int,
value2: String = "test"
) {
println("$value1 : $value2")
}
}
fun main() {
val instance = A()
val kfun = instance::class.functions.find { it.name == "myMethod" }!!
val parameters = kfun.parameters
kfun.callBy(mapOf(parameters[0] to instance, parameters[1] to 1))
}
The console will print 1 : test
.
So if you use the KFunction
instance, you can directly call the function with default parameter. And for the suspending method, you can use the method KFunction.callSuspendBy.
To add on to this:
As per the docs, you can check if it's optional by checking KParameter#isOptional
. If it is optional, then it can be omitted when invoking KFunction#callBy
.
It seems there is no way to get what the default value is via the kotlin reflection api, however you can still use the default without knowledge of what it is.
Perhaps a new execution handle, similar to SuspendingExecutionHandler
could be introduced for this.
It could be added to a module with a name like kotlin-core
, perhaps?
Of course, the SuspendingExecutionHandler
would also need to be reworked to incorporate these changes as well, as you'd want feature parity for suspending and non suspending functions.
It may be possible to just have it extend this theoretical KotlinExecutionHandler
. (Any better ideas for names?)
Apparently overloads are only generated if you use @JvmOverloads
. I was under the impression that this was the default behaviour 🥴
If someone wants to go ahead and implement this, then it could probably become a part of cloud. It would have to go in some new module cloud-kotlin/cloud-kotlin-default-annotations
or something (similar to the couroutine support module)
Have you any clue to give us in order to start ? Which type of class must have a new implementation for that for example (I'm not fluent source code of cloud)
If someone wants to go ahead and implement this, then it could probably become a part of cloud. It would have to go in some new module cloud-kotlin/cloud-kotlin-default-annotations or something (similar to the couroutine support module)
That's why I proposed something like cloud-kotlin/cloud-kotlin-core
(in hindsight, I wasn't that clear about it)
And, @Distractic, look at SuspendingExecutionHandler
and its references.
Current Behavior
When we use the annotations to set argument, the default value from kotlin declaration is not supported.
With the current system, the command bellow cannot be executed when the player doesn't define the values for args :
If the user doesn't add an argument, a
null
value will be used to set the arguments.Effectively, in the
@Argument
, there is aString
to define the default value, but in the most cases, it's not enough.Expected Behavior
The command defined above can be used if the user writes :
/test
/test 1
/test 1 2
/test 1 2 3