feat: 支持CompositeCommand间组合 #2125
Conversation
|
看了下英文下描述 |
There was a problem hiding this comment.
如果使用 ChildCommand 的话, 她给我的第一印象是
object TopLevel : CompositeCommand("toplevel") {
val myCommand = object : CompositeCommand() {
@SubCommand
suspend fun CommandSender.handle() {}
}
}命令会按照 /toplevel myCommand handle 注册, 我个人感觉这个名字会有很大的误解性.
也许可以考虑 @CombinedCommand 或者 @MixinedCommand? @Him188
|
@Karlatemp 我也是这样想的, 今天忙, 看了没来得及写 review 我认为造一个其他概念而不要用 |
|
另外现在这样， |
|
不过单看“XX助手”场景，有的功能点可能由SimpleCommand提供，所以需要的是能混合组合SimpleCommand和CompositeCommand 的工具。从这个角度来看， |
|
我考虑了 hierarchy 的,
|
看了下代码， 另外我还是不太理解你说的兼容性和复用性指什么，能更具体的说明吗？ |
|
@hundun000 是的 兼容性指的是有没有可能让开发者用 |
我的理解是要让
是指变成这样吗，那不就又没区别了？
|
|
|
懂了 |
hundun000
force-pushed
the
hundun/command2
branch
from
20a8b8a
to
df170b9
Compare
|
There was a problem hiding this comment.
总体来说是很不错的实现, 有一些考虑:
SubCommandGroup最好要提供一个可选的名称前缀的功能, 就像CompositeCommand那样. 我建议在使用注解时- 在实现上一点后,
CompositeCommand实际上也是一个SubCommandGroup. 把SubCommandGroup的provideOverloads改为overloads,CompositeCommand和AbstractSubCommandGroup就可以都继承SubCommandGroup. 这解决了各种类型兼容问题, 但这将会允许CompositeCommand和SubCommandGroup互相嵌套. 嵌套方案在之后讨论 - 互相嵌套时
CommandReflector会不会报错? 需要增加测试 - 需要增加对
generateUsage的测试
有关我上面说的设计方案, 由于 console 的指令是定义式的, 我想象了如下使用 group 的方式, 欢迎评论:
// 插件一个模块的部分功能
class ModuleAPart1 : AbstractSubCommandGroup() {
@SubCommand
fun function1();
}
// 插件一个模块的另一部分功能
class ModuleAPart2 : AbstractSubCommandGroup() {
@SubCommand
fun function2();
@SubCommand
fun function3();
}
// 插件的一个模块, 统合上面两个功能
class ModuleACommandGroup: AbstractSubCommandGroup() {
@SubCommand // 与在函数上标注这个注解类似, 它会带 `part1` 这个名称前缀来注册指令. 需要执行 /base part1 function1, 也可以使用 SubCommand 的参数来覆盖名称
val part1: ModuleAPart1
@FlattenSubCommands // 新增, 不带前缀注册指令, 执行 /base function2
val part1: ModuleAPart1
}
// 插件的总指令, 统合全部模块
class MyUnifiedCommand : CompositeCommand() {
@SubCommand
val moduleA: ModuleACommandGroup
@SubCommand // 与现有注册方式混合, 看起来也不错
fun about();
}注解方案
新设计的 CompositeCommand 与 SubCommandGroup 在定义指令上是一样的, 我希望让它们使用同样的注解. 我觉得可以把那些 @SubCommand 复制到新抽象的 SubCommandGroup 中并 public. 将旧的标注 @Deprecated(level = HIDDEN). 这样 Kotlin 用户只需要修复 import (因为 Kotlin 引用非直接父类的内部类需要带外部类类名), Java 用户无需更新.
嵌套方案
嵌套是否需要增加前缀, 在且只在外层定义中指定.
- 使用
@SubCommand时, 增加前缀. 前缀的名称的行为与这个注解标注在指令上时一致——有参数使用参数, 无参数使用指令反射名称 - 使用新增的
@FlattenSubCommands(这是我建议的名称, 可考虑更好名称) 时, 不增加前缀. 这样的行为类似标准库集合的flatMap.
有关混合嵌套问题, 我考虑:
- 允许
AbstractSubCommandGroup嵌入CompositeCommand——原 PR 计划的功能 - 允许
CompositeCommand嵌入AbstractSubCommandGroup, 即使这可能导致 confusion, 但这毕竟是用户自己要这么干的. 以后可考虑通过 IDE 插件提供预览来解决这个问题 (指令增加 group 之后变得复杂, 也确实可能需要预览了)
有关用户自己实现接口的考虑:
接口公开, 且容易实现. 我认为可以允许用户实现, 只要符合接口的设计规范.
mirai-console/backend/mirai-console/src/command/SubCommandGroup.kt
Outdated
Show resolved
Hide resolved
mirai-console/backend/mirai-console/src/internal/command/CommandReflector.kt
Outdated
Show resolved
Hide resolved
| public constructor( | ||
| ownerCommand: Command, | ||
| owner: Any, | ||
| correspondingFunction: KFunction<*>, | ||
| message: String?, | ||
| ) : super("Illegal command declaration: ${correspondingFunction.name} declared in ${ownerCommand::class.qualifiedName}") { | ||
| ) : super("Illegal command declaration: ${correspondingFunction.name} declared in ${owner::class.qualifiedName}") { | ||
| this.message = message | ||
| } |
There was a problem hiding this comment.
增加一个适用于 CommandGroup 的构造器, 而不是修改参数为 Any
…p.kt Co-authored-by: Him188 <Him188@mamoe.net>
…ndReflector.kt Co-authored-by: Him188 <Him188@mamoe.net>
# Conflicts: # mirai-console/backend/mirai-console/test/command/InstanceTestCommand.kt
即使已经(level = HIDDEN)且 计划开发期间真实移除
类似问题的issue已被记录为kotlin bug。 |
|
真实移除是 ABI 变更，是不可能进行的，只能考虑其他方案了... |
|
@hundun000 @Karlatemp 有什么想法吗，关于新注解 |
|
x Kotlin 用户只需要修复 import 这样？因为我看IDEA自动补全也是选择补全为这样（指以前CompositeCommand.SubCommand还未废弃的时候），是因为这样也更符合编码规范吗？ |
|
这个就是 Kotlin 设计问题，不允许用 simple name 引用超过一级继承关系的内部类型。Java 是可以的 |
效果：
@ChildCommad注解的CompositeCommand子类成员中的所有子指令，也会成为他的子指令。（见InstanceTestCommand.kt变动）改动说明：
@ChildCommad。也可以考虑继续使用@SubCommad？以及child的命名可能会引起歧义想到继承关系