Skip to content
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

Brigadier: providing argument contexts (future work) #31

Open
Cypher121 opened this issue Jul 10, 2022 · 0 comments
Open

Brigadier: providing argument contexts (future work) #31

Cypher121 opened this issue Jul 10, 2022 · 0 comments

Comments

@Cypher121
Copy link
Contributor

Cypher121 commented Jul 10, 2022

Issue

It would be useful to provide additional contexts for builder lambdas in the Brigadier DSL. Those could, for instance, hold additional extension functions that cannot currently be implemented due to requiring too many receivers, or, in case of optional arguments, safely accessing the argument's exact builder type only on the branch containing the argument:

//This shorthand is not possible as the only argument for unary minus is the ArgumentConstructor from `integer("test")`
+integer("test") {
...
}

optional(player("target")) { target ->
    //there is no valid class to hold this function
    ifPresent {
        requires { ... } // possible by instead checking target != null
        suggests { ... } // requires checking target != null and then performing an unsafe cast on `this`
    }
}

Solutions

This could be resolved in two ways:

  1. Providing functions with a custom implementation of ArgumentBuilder as their receiver. Requires manually delegating to existing implementations as ArgumentBuilder is not an interface and has a self upper bound.
  2. Providing context arguments in addition to the ArgumentBuilder. This requires context arguments to become a stable Kotlin feature.
  3. Providing the context as a second argument next to the argument accessor. Causes a lot of visual noise, can lead to unexpected behavior if accessed from a child builder.

Prior work

Possible implementation of 2 can be found at this fork branch. Note context arguments in CommandArgument.register.

Compatibility

Adding a context changes lambda types from Function1 to Function2, causing an ABI break, but maintaining source compatibility. It may be possible to keep both versions and go through a deprecation cycle with the non-contextual ones, though that depends on the compiler defaulting to the non-deprecated context version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant