-
Notifications
You must be signed in to change notification settings - Fork 24
LazyAPI更新 #1040
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
base: master
Are you sure you want to change the base?
LazyAPI更新 #1040
Conversation
…stCommand添加到addCommands和addRestCommands,移除对应的remove代码
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
你好 - 我已经审查了你的更改 - 这里有一些反馈:
- HookBase 的构造函数没有将提供的 loadType 赋值给 LoadType 属性,因此自动注册永远不会发生——请设置 LoadType = loadType。
- ActionHook 中基于 IL 的钩子注销依赖于脆弱的字节码模式;请考虑添加防御性检查或回退路径以优雅地处理意外的 MSIL 布局。
- 处置后,addCommands/addRestCommands/addHooks 集合未被清除,这可能导致插件重新初始化时出现重复注册——请考虑重置这些列表。
AI 代理提示
请解决此代码审查中的注释:
## 总体评论
- HookBase 的构造函数没有将提供的 loadType 赋值给 LoadType 属性,因此自动注册永远不会发生——请设置 LoadType = loadType。
- ActionHook 中基于 IL 的钩子注销依赖于脆弱的字节码模式;请考虑添加防御性检查或回退路径以优雅地处理意外的 MSIL 布局。
- 处置后,addCommands/addRestCommands/addHooks 集合未被清除,这可能导致插件重新初始化时出现重复注册——请考虑重置这些列表。
## 单独评论
### 评论 1
<location> `src/LazyAPI/Hooks/HookBase.cs:7` </location>
<code_context>
+{
+ public bool Registered { get; protected set; }
+ public HookLoadType LoadType { get; }
+ protected HookBase(HookLoadType loadType)
+ {
+ this.Registered = false;
</code_context>
<issue_to_address>
**问题 (bug_risk):** 构造函数未将 LoadType 参数赋值给属性。
在构造函数中赋值 'this.LoadType = loadType;' 以确保属性反映提供的参数。
</issue_to_address>帮助我更有用!请对每个评论点击 👍 或 👎,我将使用反馈来改进您的评论。
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- HookBase's constructor isn't assigning the provided loadType to the LoadType property, so auto-registration will never occur—please set LoadType = loadType.
- The IL-based hook unregistration in ActionHook relies on fragile bytecode patterns; consider adding defensive checks or fallback paths to handle unexpected MSIL layouts gracefully.
- After disposing, the addCommands/addRestCommands/addHooks collections aren’t cleared, which could cause duplicate registrations if a plugin is reinitialized—consider resetting these lists.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- HookBase's constructor isn't assigning the provided loadType to the LoadType property, so auto-registration will never occur—please set LoadType = loadType.
- The IL-based hook unregistration in ActionHook relies on fragile bytecode patterns; consider adding defensive checks or fallback paths to handle unexpected MSIL layouts gracefully.
- After disposing, the addCommands/addRestCommands/addHooks collections aren’t cleared, which could cause duplicate registrations if a plugin is reinitialized—consider resetting these lists.
## Individual Comments
### Comment 1
<location> `src/LazyAPI/Hooks/HookBase.cs:7` </location>
<code_context>
+{
+ public bool Registered { get; protected set; }
+ public HookLoadType LoadType { get; }
+ protected HookBase(HookLoadType loadType)
+ {
+ this.Registered = false;
</code_context>
<issue_to_address>
**issue (bug_risk):** Constructor does not assign LoadType parameter to property.
Assign 'this.LoadType = loadType;' in the constructor to ensure the property reflects the provided parameter.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| { | ||
| public bool Registered { get; protected set; } | ||
| public HookLoadType LoadType { get; } | ||
| protected HookBase(HookLoadType loadType) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
问题 (bug_risk): 构造函数未将 LoadType 参数赋值给属性。
在构造函数中赋值 'this.LoadType = loadType;' 以确保属性反映提供的参数。
Original comment in English
issue (bug_risk): Constructor does not assign LoadType parameter to property.
Assign 'this.LoadType = loadType;' in the constructor to ensure the property reflects the provided parameter.
|
别摸鱼了,看看吧 😭😭😭 |
| ServerApi.Hooks.ProjectileAIUpdate.Register(this, this.ProjectAiUpdate); | ||
| TShockAPI.Commands.ChatCommands.Add(new Command("autofish", Commands.Afs, "af", "autofish")); | ||
| this.addCommands.Add(new Command("autofish", Commands.Afs, "af", "autofish")); | ||
| base.Initialize(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
每个地方都base.Initialize()太恶心了吧,有没有什么更好的办法
ACaiCat
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我觉得最大的问题就是和LazyAPI的attribute有点重复,我觉得这两个应该得二选一
添加类型: ActionHook,HandlerListHook,ServerApiHook
LazyPlugin: 添加了addCommands,addRestCommands, addHooks和内部使用的addReloadEvents以提供自动卸载功能。移动构造的内容到Initialize()以解决某些bug,修改子类的Initialize()以匹配更改,并把ChatCommands.Add()和RestApi.Register()全部移动到addCommands和addRestCommands
修改了两个子类的Initialize()以展示Hook如何使用
Sourcery 总结
通过引入统一的钩子抽象层和延迟注册集合,在 LazyPlugin 中自动化并集中化插件命令、REST 端点和钩子注册,并将现有插件迁移到新系统。
新功能:
改进:
Original summary in English
Summary by Sourcery
Automate and centralize plugin command, REST endpoint, and hook registration in LazyPlugin by introducing a unified hook abstraction layer and deferred registration collections, migrating existing plugins to the new system.
New Features:
Enhancements: