feat:load js script plugin - #1198
Conversation
There was a problem hiding this comment.
嘿——我发现了 3 个问题
面向 AI 代理的提示
请处理本次代码审查中的评论:
## 单独评论
### 评论 1
<location path="src/Economics.Plugin/Scripts/plugin-demo.js" line_range="82" />
<code_context>
+}
+
+function onNetGetData(args) {
+ log('[demo] 收包: 类型=' + args.MsgID + ' 玩家=' + TShockAPI.TShock.Players[args.Msg.whoAmI].Name + ' 长度=' + args.Length);
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** 随附示例中的 `onNetGetData` 回调读取了 `args.Msg.whoAmI`,但文档所述的 NetGetData 事件 API 将玩家索引暴露为 `args.Index`;因此,该回调会收到无效的玩家索引,并在处理网络数据包时失败。
**触发条件:** 加载随附的 `plugin-demo.js` 且触发 NetGetData 事件时。
**建议修复:** 使用 `playerName(args.Index)`,或以其他方式读取事件文档中规定的 `Index` 属性,而不是读取 `args.Msg.whoAmI`。
```suggestion
log('[demo] 收包: 类型=' + args.MsgID + ' 玩家=' + playerName(args.Index) + ' 长度=' + args.Length);
```
</issue_to_address>
### 评论 2
<location path="src/Economics.Plugin/README.md" line_range="13" />
<code_context>
+
+| 阶段 | 函数 | 说明 |
+| --- | --- | --- |
+| 加载 | `init()` | 插件启动 / `TShock /reload` / `/escript reload` 时,逐个执行,用于注册命令与钩子 |
+| 卸载 | `unload()` | 重载或插件停用时执行一次,建议在这里反注册钩子(命令由宿主自动移除) |
+
+## 脚本目录与加载约定
</code_context>
<issue_to_address>
**issue:** README 声称宿主会自动移除脚本命令,但 `ScriptHost` 从未跟踪或移除通过 `Commands.Add` 添加的命令;只有每个脚本的 `unload()` 显式调用 `Commands.Remove` 时才会移除命令。没有成功执行清理函数的脚本会在重载后留下过时命令。
**触发条件:** 脚本未实现 `unload()`,或其 `unload()` 函数在重载期间执行失败时。
**建议修复:** 要么在宿主端跟踪并移除脚本加载期间注册的命令,要么修改 README,说明脚本必须始终自行移除其命令。
```suggestion
| 卸载 | `unload()` | 重载或插件停用时执行一次,建议在这里反注册钩子(命令必须由脚本自行移除) |
```
</issue_to_address>
### 评论 3
<location path="src/Economics.Plugin/Scripting/ScriptHost.cs" line_range="109-110" />
<code_context>
+ {
+ foreach (var location in this._locations)
+ {
+ this._manager.GetOrCreate(location).TryInvoke(ExitFunction);
+ }
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** `UnloadAll` 会抑制脚本 `unload()` 抛出的所有异常,随后继续进行重载,导致该脚本注册的命令和事件钩子继续留在运行中的服务器上。之后,旧运行时会被重新使用或替换,但宿主不会对这些注册执行任何清理。
**触发条件:** 脚本在注册命令或钩子后,其 `unload()` 抛出异常时。
**建议修复:** 在宿主中跟踪注册信息,并在 `finally` 清理路径中移除这些注册;或者在清理失败时停止重载,而不是静默地继续执行。
</issue_to_address>请帮助我变得更有用!请对每条评论点击 👍 或 👎,我会利用这些反馈来改进审查结果。
Original comment in English
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/Economics.Plugin/Scripts/plugin-demo.js" line_range="82" />
<code_context>
+}
+
+function onNetGetData(args) {
+ log('[demo] 收包: 类型=' + args.MsgID + ' 玩家=' + TShockAPI.TShock.Players[args.Msg.whoAmI].Name + ' 长度=' + args.Length);
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** The bundled demo's `onNetGetData` callback reads `args.Msg.whoAmI`, but the documented NetGetData event API exposes the player index as `args.Index`; the callback therefore receives an invalid player index and fails when handling network packets.
**Triggers:** When the bundled `plugin-demo.js` is loaded and a NetGetData event fires.
**Suggested fix:** Use `playerName(args.Index)` or otherwise read the event's documented `Index` property instead of `args.Msg.whoAmI`.
```suggestion
log('[demo] 收包: 类型=' + args.MsgID + ' 玩家=' + playerName(args.Index) + ' 长度=' + args.Length);
```
</issue_to_address>
### Comment 2
<location path="src/Economics.Plugin/README.md" line_range="13" />
<code_context>
+
+| 阶段 | 函数 | 说明 |
+| --- | --- | --- |
+| 加载 | `init()` | 插件启动 / `TShock /reload` / `/escript reload` 时,逐个执行,用于注册命令与钩子 |
+| 卸载 | `unload()` | 重载或插件停用时执行一次,建议在这里反注册钩子(命令由宿主自动移除) |
+
+## 脚本目录与加载约定
</code_context>
<issue_to_address>
**issue:** The README claims that the host automatically removes script commands, but `ScriptHost` never tracks or removes commands added through `Commands.Add`; removal only happens if each script's `unload()` explicitly calls `Commands.Remove`. Scripts without a successful cleanup function leave stale commands after reload.
**Triggers:** When a script omits `unload()` or its `unload()` function fails during reload.
**Suggested fix:** Either implement host-side tracking and removal of commands registered during script loading, or correct the README to state that scripts must always remove their own commands.
```suggestion
| 卸载 | `unload()` | 重载或插件停用时执行一次,建议在这里反注册钩子(命令必须由脚本自行移除) |
```
</issue_to_address>
### Comment 3
<location path="src/Economics.Plugin/Scripting/ScriptHost.cs" line_range="109-110" />
<code_context>
+ {
+ foreach (var location in this._locations)
+ {
+ this._manager.GetOrCreate(location).TryInvoke(ExitFunction);
+ }
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** `UnloadAll` suppresses every exception from a script's `unload()` and then reload proceeds, leaving commands and event hooks registered by that script in the live server. The old runtime is subsequently reused or replaced without any host-side cleanup of those registrations.
**Triggers:** When a script's `unload()` throws after registering commands or hooks.
**Suggested fix:** Track registrations in the host and remove them in a `finally` cleanup path, or stop reload when cleanup fails instead of silently continuing.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
|
|
||
| function onNetGetData(args) { | ||
| log('[demo] 收包: 类型=' + args.MsgID + ' 玩家=' + TShockAPI.TShock.Players[args.Msg.whoAmI].Name + ' 长度=' + args.Length); |
There was a problem hiding this comment.
issue (bug_risk): 随附示例中的 onNetGetData 回调读取了 args.Msg.whoAmI,但文档所述的 NetGetData 事件 API 将玩家索引暴露为 args.Index;因此,该回调会收到无效的玩家索引,并在处理网络数据包时失败。
触发条件: 加载随附的 plugin-demo.js 且触发 NetGetData 事件时。
建议修复: 使用 playerName(args.Index),或以其他方式读取事件文档中规定的 Index 属性,而不是读取 args.Msg.whoAmI。
| log('[demo] 收包: 类型=' + args.MsgID + ' 玩家=' + TShockAPI.TShock.Players[args.Msg.whoAmI].Name + ' 长度=' + args.Length); | |
| log('[demo] 收包: 类型=' + args.MsgID + ' 玩家=' + playerName(args.Index) + ' 长度=' + args.Length); |
Original comment in English
issue (bug_risk): The bundled demo's onNetGetData callback reads args.Msg.whoAmI, but the documented NetGetData event API exposes the player index as args.Index; the callback therefore receives an invalid player index and fails when handling network packets.
Triggers: When the bundled plugin-demo.js is loaded and a NetGetData event fires.
Suggested fix: Use playerName(args.Index) or otherwise read the event's documented Index property instead of args.Msg.whoAmI.
| log('[demo] 收包: 类型=' + args.MsgID + ' 玩家=' + TShockAPI.TShock.Players[args.Msg.whoAmI].Name + ' 长度=' + args.Length); | |
| log('[demo] 收包: 类型=' + args.MsgID + ' 玩家=' + playerName(args.Index) + ' 长度=' + args.Length); |
| | 阶段 | 函数 | 说明 | | ||
| | --- | --- | --- | | ||
| | 加载 | `init()` | 插件启动 / `TShock /reload` / `/escript reload` 时,逐个执行,用于注册命令与钩子 | | ||
| | 卸载 | `unload()` | 重载或插件停用时执行一次,建议在这里反注册钩子(命令由宿主自动移除) | |
There was a problem hiding this comment.
issue: README 声称宿主会自动移除脚本命令,但 ScriptHost 从未跟踪或移除通过 Commands.Add 添加的命令;只有每个脚本的 unload() 显式调用 Commands.Remove 时才会移除命令。没有成功执行清理函数的脚本会在重载后留下过时命令。
触发条件: 脚本未实现 unload(),或其 unload() 函数在重载期间执行失败时。
建议修复: 要么在宿主端跟踪并移除脚本加载期间注册的命令,要么修改 README,说明脚本必须始终自行移除其命令。
| | 卸载 | `unload()` | 重载或插件停用时执行一次,建议在这里反注册钩子(命令由宿主自动移除) | | |
| | 卸载 | `unload()` | 重载或插件停用时执行一次,建议在这里反注册钩子(命令必须由脚本自行移除) | |
Original comment in English
issue: The README claims that the host automatically removes script commands, but ScriptHost never tracks or removes commands added through Commands.Add; removal only happens if each script's unload() explicitly calls Commands.Remove. Scripts without a successful cleanup function leave stale commands after reload.
Triggers: When a script omits unload() or its unload() function fails during reload.
Suggested fix: Either implement host-side tracking and removal of commands registered during script loading, or correct the README to state that scripts must always remove their own commands.
| | 卸载 | `unload()` | 重载或插件停用时执行一次,建议在这里反注册钩子(命令由宿主自动移除) | | |
| | 卸载 | `unload()` | 重载或插件停用时执行一次,建议在这里反注册钩子(命令必须由脚本自行移除) | |
| this._manager.GetOrCreate(location).TryInvoke(ExitFunction); | ||
| } |
There was a problem hiding this comment.
issue (bug_risk): UnloadAll 会抑制脚本 unload() 抛出的所有异常,随后继续进行重载,导致该脚本注册的命令和事件钩子继续留在运行中的服务器上。之后,旧运行时会被重新使用或替换,但宿主不会对这些注册执行任何清理。
触发条件: 脚本在注册命令或钩子后,其 unload() 抛出异常时。
建议修复: 在宿主中跟踪注册信息,并在 finally 清理路径中移除这些注册;或者在清理失败时停止重载,而不是静默地继续执行。
Original comment in English
issue (bug_risk): UnloadAll suppresses every exception from a script's unload() and then reload proceeds, leaving commands and event hooks registered by that script in the live server. The old runtime is subsequently reused or replaced without any host-side cleanup of those registrations.
Triggers: When a script's unload() throws after registering commands or hooks.
Suggested fix: Track registrations in the host and remove them in a finally cleanup path, or stop reload when cleanup fails instead of silently continuing.
添加插件
更新插件/修复BUG
其他
Sourcery 摘要
添加一个用于加载和管理基于 JavaScript 的服务器插件的 TShock 插件。
新功能:
增强功能:
构建:
文档:
Original summary in English
Summary by Sourcery
Add a TShock plugin for loading and managing JavaScript-based server plugins.
New Features:
Enhancements:
Build:
Documentation: