Using PluginId as folder name

mamoe / mirai-console

mirai 的高效率 QQ 机器人控制台

GNU Affero General Public License v3.0

1.19k stars 412 forks source link

Closed Nambers closed 2 years ago

Nambers commented 2 years ago

close #276

Nambers commented 2 years ago

@Him188 这样子吗，需不需要直接写在onEnable()里而不是现在的lazy里目前就cancel后还要等onEnable()执行完才close console

如果在lazy里:

如果在onEnable()里:

Him188 commented 2 years ago

应该要写在 PluginManager 在获取到所有插件 Plugin 实例后进行

Nambers commented 2 years ago

应该要写在 PluginManager 在获取到所有插件 Plugin 实例后进行

在 BultInJvmPluginLoaderImpl 的 load 里面写会不会更好因为 PluginManager 里面都是 Plugin 类型，设计上看起来是不管内部，而 dataFolder 是在 JvmPlugin 里有

Him188 commented 2 years ago

@Nambers Indeed.

Nambers commented 2 years ago

Tested?

消息过于密集，适当添加换行 (使用两次 logger)

(仅建议) 可以考虑输出的路径为相对于 console 运行目录的路径 (caused by 2.)

如果目标文件夹下没有任何文件(listFiles().size == 0)，使用rename会返回false不抛出任何异常这样的话是要删除目标文件夹然后移动，还是发出log让用户手动移动?

Him188 commented 2 years ago

建议添加单元测试, 见 CommandTest 查看如何在 test 启动 console

Nambers commented 2 years ago

建议添加单元测试, 见 CommandTest 查看如何在 test 启动 console

差点忘了name == id的情况了, 为什么这个test每个单个执行就可以pass, 但是我整个类一起执行就会报错 kotlinx.coroutines.JobCancellationException: Job was cancelled; 问题在MiraiConsole.cancell

Him188 commented 2 years ago

因为 test 框架可能有 bug

Nambers commented 2 years ago

因为 test 框架可能有 bug

对协程的这方面不是很熟悉，目前没找到什么方法解决这个问题，在本地这个test是可以pass的你们看看怎么写? 看Coroutine context是supervisor job 应该没什么问题可能

Nambers commented 2 years ago

尝试修了下，没找到解决方法 :( 大概要注意的地方有

测试里plugin.load里的cancel方法会报错，可能要提出一个cancel方法在MiraiConsole，然后在testMiraiConsole里重载
MiraiConsoleImpl.instance.start里的isInitial, 可能需要传入一个testOnly的参数来在test里代替这个, 因为test里每个其实相当于重新开个MiraiConsole, 该属性应该是false
@afterEach的cancel会报错, 这个应该是就用目前这个cancelOnFinish控制

但是这些问题对于正常使用是没有问题的，因为正常设计下cancel了MiraiConsole就退出了(

Him188 commented 2 years ago

试试 rebase 到 master 然后换用新的 AbstractConsoleTest.kt?

Him188 commented 2 years ago

看了一下, 问题是现在 Console 的很多服务都是静态的, 没法重启, 得等 https://github.com/Him188/kotlin-dynamic-delegation