Implement Type-safe Kotlin DSL

[sya-ri]

trafficstars

#544

---

I didn't change the documentation because I didn't really understand it.

[DerEchtePilz]

I didn't change the documentation because I didn't really understand it.

The documentation is built using mdbook, I believe there are download links for the required executables in the wiki here on GitHub. The source for the documentation consists of .md files. Those can be found in docssrc/src. The documentation additionally uses compiled code so the code examples can be used in code without paying attention to the correct syntax. Its source resides in the commandapi-documentation-code. This module is the Bukkit module, there is not really documentation for Velocity yet.

[sya-ri]

Again, this should not be a replacement. This should rather be an alternative for those who want this.

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 argument(..., optional = false) or argument(..., optional = true); you should remove optional = false or use optionalArgument(...).

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 https://github.com/JorelAli/CommandAPI/pull/545/files#diff-67d7c60b582802c1efad0034b5b7bf5da58e284935c795f2959d5aa5bab2e654 https://github.com/JorelAli/CommandAPI/pull/545/files#diff-db52ae2a394c8abc971e1c4845fb833df06243d1096b6e8938dd042ed4eead99

I only changed the optional tests and added some tests.

[DerEchtePilz]

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?

Yeah, that would be ideal.

The only incompatibility caused by this replacement is when specifying argument(..., optional = false) or argument(..., optional = true); you should remove optional = false or use optionalArgument(...).

I mean, technically you can do whatever here, just don't introduce duplicate methods like the ones that have the Optional in their name. That is something we definitely don't want.

[DerEchtePilz]

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?

[willkroboth]

I think delegated properties are still syntactic sugar for CommandArguments#getUnchecked. You could still accidentally put the wrong class, and you wouldn't know until runtime when there's a ClassCastException or something.

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)
            }
        }
    }
}

[DerEchtePilz]

Yeah, true. I mean, it also depends on the variable name you put there so the type safety is still only sort of there. I mean yeah, in the end a call to CommandArguments#get(String) as T is done because the unsafe CommandArguments#getUnchecked wouldn't exactly work here (as we don't exactly work with fixed types there I think)