update：为应对新版ts，改为延迟分批清扫 - #1176
Closed
AX-17 wants to merge 3 commits into
Closed
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我发现了两个问题，并给出了一些整体性的反馈：
AutoClearService.ValidateStartState中的主线程检测依赖于在Update()中初始化的mainThreadId，因此在主线程第一次执行Update()之前发生的任何TryStart/TryStartAutomatic调用都会错误地返回MainThreadRequired；建议尽早初始化mainThreadId（例如在插件的Initialize或第一次TryStart中），或者把-1视为未初始化，并将第一个调用者视为主线程。- Starver 集成（
HasStarverCleanupProtection和IsStarverCleanupRunning）在每次调用时都会对所有插件做基于反射的扫描；如果在大型插件集上这条路径变成热点，建议在发现协调器Type或插件容器之后对其进行缓存，以避免重复的反射和枚举。
给 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- `AutoClearService.ValidateStartState` 中的主线程检测依赖于在 `Update()` 中初始化的 `mainThreadId`，因此在主线程第一次执行 `Update()` 之前发生的任何 `TryStart`/`TryStartAutomatic` 调用都会错误地返回 `MainThreadRequired`；建议尽早初始化 `mainThreadId`（例如在插件的 `Initialize` 或第一次 `TryStart` 中），或者把 `-1` 视为未初始化，并将第一个调用者视为主线程。
- Starver 集成（`HasStarverCleanupProtection` 和 `IsStarverCleanupRunning`）在每次调用时都会对所有插件做基于反射的扫描；如果在大型插件集上这条路径变成热点，建议在发现协调器 `Type` 或插件容器之后对其进行缓存，以避免重复的反射和枚举。
## Individual Comments
### Comment 1
<location path="src/AutoClear/AutoClearPlugin.cs" line_range="107-116" />
<code_context>
+ && string.Equals(command.CommandDelegate.Method.Name, "Clear", StringComparison.Ordinal));
+ }
+
+ private void OnPrePlayerCommand(PrePlayerCommandEventArgs args)
+ {
+ if (HasStarverCleanupProtection()
+ || !ReferenceEquals(args.Command, tshockClearCommand)
+ || !AutoClearCommandRules.TryParseItemClearParameters(
+ args.Arguments.Parameters,
+ out int radiusTiles))
+ {
+ return;
+ }
+
+ args.Handled = true;
+ StartCleanup(args.Arguments.Player, radiusTiles, args.Arguments.Silent);
+ }
+
</code_context>
<issue_to_address>
**suggestion (performance):** 可以对每次命令执行时重复进行的基于反射的 Starver 检测进行缓存。
`OnPrePlayerCommand` 和 `OnSafeClearItems` 都会调用 `HasStarverCleanupProtection()`，每次都会遍历 `ServerApi.Plugins` 并使用反射。相反，可以依赖已有的 `starverProtectionActive` 字段，只在重载/插件初始化（或插件发生变化）时重新计算，这样在命令处理的热点路径上就能避免重复的反射。
建议实现如下：
```csharp
private void OnPrePlayerCommand(PrePlayerCommandEventArgs args)
{
if (starverProtectionActive
|| !ReferenceEquals(args.Command, tshockClearCommand)
|| !AutoClearCommandRules.TryParseItemClearParameters(
args.Arguments.Parameters,
out int radiusTiles))
{
return;
}
args.Handled = true;
StartCleanup(args.Arguments.Player, radiusTiles, args.Arguments.Silent);
}
```
1. 更新其它热点调用方（例如 `OnSafeClearItems`），改为使用缓存标志而不是反射：
- 在条件中将 `HasStarverCleanupProtection()` 替换为 `starverProtectionActive`。
2. 将 `HasStarverCleanupProtection()` 改造成*重新计算*例程，用来设置 `starverProtectionActive`（如果仍然需要返回值，可以在设置之后返回该值），而不是在每次命令时遍历 `ServerApi.Plugins`：
- 将现有的反射逻辑抽取到一个类似 `RecalculateStarverCleanupProtection()` 的方法中，由它来设置 `starverProtectionActive`。
- 在插件初始化（例如 `Initialize`/构造函数）、配置重载以及你可能拥有的插件加载/卸载事件钩子中调用该方法。
3. 如果 `starverProtectionActive` 可能在非主线程中被更新，请确保将其声明为 `private volatile bool`（或以其他方式安全发布），以便命令处理程序读取到一致的值。
</issue_to_address>
### Comment 2
<location path="src/AutoClear/AutoClearConfiguration.cs" line_range="67-76" />
<code_context>
+ internal static AutoClearConfiguration Load()
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 始终通过 `JObject` 和 `FromLegacyObject` 加载会丢失未知或未来的配置字段。
由于 `Load` 总是将 `AutoClear.json` 解析为 `JObject`，然后使用 `FromLegacyObject`，任何未被 `FromLegacyObject` 明确处理的字段（包括未来新增的配置属性）都会丢失。为避免静默数据丢失并保持配置的前向兼容性，建议优先使用 `JsonConvert.DeserializeObject<AutoClearConfiguration>`，只有在反序列化失败或检测到旧版格式时，才回退到 `JObject` + `FromLegacyObject`。
建议实现如下：
```csharp
internal static AutoClearConfiguration Load()
{
try
{
AutoClearConfiguration configuration;
if (File.Exists(ConfigPath))
{
var json = File.ReadAllText(ConfigPath, Encoding.UTF8);
try
{
// Prefer direct deserialization to preserve unknown/future fields
configuration = JsonConvert.DeserializeObject<AutoClearConfiguration>(json)
?? new AutoClearConfiguration();
}
catch (JsonException)
{
// Fallback: legacy format, parse as JObject and convert
JObject root = JObject.Parse(json);
configuration = FromLegacyObject(root);
}
}
else if (!TryImportLegacy(out configuration))
```
如果 `JsonConvert` 或 `JsonException` 尚未在作用域内，请确保在 `AutoClearConfiguration.cs` 文件顶部添加：
1. `using Newtonsoft.Json;`
`JObject` 已经在使用，因此 `using Newtonsoft.Json.Linq;` 应该已经存在。
</issue_to_address>帮我变得更有用！请对每条评论点 👍 或 👎，我会根据反馈改进后续的评审。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The main-thread detection in
AutoClearService.ValidateStartStaterelies onmainThreadIdbeing initialized inUpdate(), so anyTryStart/TryStartAutomaticcalls that occur before the firstUpdate()on the main thread will incorrectly returnMainThreadRequired; consider initializingmainThreadIdeagerly (e.g., in pluginInitializeor firstTryStart) or falling back to treating-1as uninitialized and accepting the first caller as the main thread. - The Starver integration (
HasStarverCleanupProtectionandIsStarverCleanupRunning) performs reflection-based scans over all plugins on each call; if this becomes a hot path on large plugin sets, consider caching the coordinatorTypeor the plugin container once discovered to avoid repeated reflection and enumeration.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The main-thread detection in `AutoClearService.ValidateStartState` relies on `mainThreadId` being initialized in `Update()`, so any `TryStart`/`TryStartAutomatic` calls that occur before the first `Update()` on the main thread will incorrectly return `MainThreadRequired`; consider initializing `mainThreadId` eagerly (e.g., in plugin `Initialize` or first `TryStart`) or falling back to treating `-1` as uninitialized and accepting the first caller as the main thread.
- The Starver integration (`HasStarverCleanupProtection` and `IsStarverCleanupRunning`) performs reflection-based scans over all plugins on each call; if this becomes a hot path on large plugin sets, consider caching the coordinator `Type` or the plugin container once discovered to avoid repeated reflection and enumeration.
## Individual Comments
### Comment 1
<location path="src/AutoClear/AutoClearPlugin.cs" line_range="107-116" />
<code_context>
+ && string.Equals(command.CommandDelegate.Method.Name, "Clear", StringComparison.Ordinal));
+ }
+
+ private void OnPrePlayerCommand(PrePlayerCommandEventArgs args)
+ {
+ if (HasStarverCleanupProtection()
+ || !ReferenceEquals(args.Command, tshockClearCommand)
+ || !AutoClearCommandRules.TryParseItemClearParameters(
+ args.Arguments.Parameters,
+ out int radiusTiles))
+ {
+ return;
+ }
+
+ args.Handled = true;
+ StartCleanup(args.Arguments.Player, radiusTiles, args.Arguments.Silent);
+ }
+
</code_context>
<issue_to_address>
**suggestion (performance):** Repeated reflection-based Starver detection on every command could be cached.
`OnPrePlayerCommand` and `OnSafeClearItems` both call `HasStarverCleanupProtection()`, which walks `ServerApi.Plugins` and uses reflection each time. Instead, rely on the existing `starverProtectionActive` field and only recompute it on reload/plugin init (or when plugins change), so command handling avoids repeated reflection on this hot path.
Suggested implementation:
```csharp
private void OnPrePlayerCommand(PrePlayerCommandEventArgs args)
{
if (starverProtectionActive
|| !ReferenceEquals(args.Command, tshockClearCommand)
|| !AutoClearCommandRules.TryParseItemClearParameters(
args.Arguments.Parameters,
out int radiusTiles))
{
return;
}
args.Handled = true;
StartCleanup(args.Arguments.Player, radiusTiles, args.Arguments.Silent);
}
```
1. Update any other hot-path callers (e.g. `OnSafeClearItems`) to use the cached flag instead of reflection:
- Replace `HasStarverCleanupProtection()` with `starverProtectionActive` in their conditionals.
2. Change `HasStarverCleanupProtection()` to become the *recalculation* routine that sets `starverProtectionActive` (and returns it if you still need a return value), rather than walking `ServerApi.Plugins` on every command:
- Extract the existing reflection logic into a method like `RecalculateStarverCleanupProtection()` that sets `starverProtectionActive`.
- Call this method from plugin initialization (e.g. `Initialize`/constructor), configuration reload, and any hooks you may have for plugin load/unload events.
3. Ensure `starverProtectionActive` is a `private volatile bool` (or otherwise safely published) if it can be updated from non-main threads, so command handlers read a consistent value.
</issue_to_address>
### Comment 2
<location path="src/AutoClear/AutoClearConfiguration.cs" line_range="67-76" />
<code_context>
+ internal static AutoClearConfiguration Load()
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Loading always via `JObject` and `FromLegacyObject` can drop unknown or future config fields.
Because `Load` always parses `AutoClear.json` into a `JObject` and then uses `FromLegacyObject`, any fields not explicitly handled by `FromLegacyObject` (including future config properties) are lost. To avoid silent data loss and keep configs forward‑compatible, prefer `JsonConvert.DeserializeObject<AutoClearConfiguration>` first, and only fall back to `JObject` + `FromLegacyObject` when deserialization fails or you detect a legacy format.
Suggested implementation:
```csharp
internal static AutoClearConfiguration Load()
{
try
{
AutoClearConfiguration configuration;
if (File.Exists(ConfigPath))
{
var json = File.ReadAllText(ConfigPath, Encoding.UTF8);
try
{
// Prefer direct deserialization to preserve unknown/future fields
configuration = JsonConvert.DeserializeObject<AutoClearConfiguration>(json)
?? new AutoClearConfiguration();
}
catch (JsonException)
{
// Fallback: legacy format, parse as JObject and convert
JObject root = JObject.Parse(json);
configuration = FromLegacyObject(root);
}
}
else if (!TryImportLegacy(out configuration))
```
If `JsonConvert` or `JsonException` are not yet in scope, ensure the file has:
1. `using Newtonsoft.Json;` at the top of `AutoClearConfiguration.cs`.
`JObject` is already used, so `using Newtonsoft.Json.Linq;` should already be present.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+107
to
+116
| private void OnPrePlayerCommand(PrePlayerCommandEventArgs args) | ||
| { | ||
| if (HasStarverCleanupProtection() | ||
| || !ReferenceEquals(args.Command, tshockClearCommand) | ||
| || !AutoClearCommandRules.TryParseItemClearParameters( | ||
| args.Arguments.Parameters, | ||
| out int radiusTiles)) | ||
| { | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
suggestion (performance): 可以对每次命令执行时重复进行的基于反射的 Starver 检测进行缓存。
OnPrePlayerCommand 和 OnSafeClearItems 都会调用 HasStarverCleanupProtection()，每次都会遍历 ServerApi.Plugins 并使用反射。相反，可以依赖已有的 starverProtectionActive 字段，只在重载/插件初始化（或插件发生变化）时重新计算，这样在命令处理的热点路径上就能避免重复的反射。
建议实现如下：
private void OnPrePlayerCommand(PrePlayerCommandEventArgs args)
{
if (starverProtectionActive
|| !ReferenceEquals(args.Command, tshockClearCommand)
|| !AutoClearCommandRules.TryParseItemClearParameters(
args.Arguments.Parameters,
out int radiusTiles))
{
return;
}
args.Handled = true;
StartCleanup(args.Arguments.Player, radiusTiles, args.Arguments.Silent);
}- 更新其它热点调用方（例如
OnSafeClearItems），改为使用缓存标志而不是反射：- 在条件中将
HasStarverCleanupProtection()替换为starverProtectionActive。
- 在条件中将
- 将
HasStarverCleanupProtection()改造成重新计算例程，用来设置starverProtectionActive（如果仍然需要返回值，可以在设置之后返回该值），而不是在每次命令时遍历ServerApi.Plugins：- 将现有的反射逻辑抽取到一个类似
RecalculateStarverCleanupProtection()的方法中，由它来设置starverProtectionActive。 - 在插件初始化（例如
Initialize/构造函数）、配置重载以及你可能拥有的插件加载/卸载事件钩子中调用该方法。
- 将现有的反射逻辑抽取到一个类似
- 如果
starverProtectionActive可能在非主线程中被更新，请确保将其声明为private volatile bool（或以其他方式安全发布），以便命令处理程序读取到一致的值。
Original comment in English
suggestion (performance): Repeated reflection-based Starver detection on every command could be cached.
OnPrePlayerCommand and OnSafeClearItems both call HasStarverCleanupProtection(), which walks ServerApi.Plugins and uses reflection each time. Instead, rely on the existing starverProtectionActive field and only recompute it on reload/plugin init (or when plugins change), so command handling avoids repeated reflection on this hot path.
Suggested implementation:
private void OnPrePlayerCommand(PrePlayerCommandEventArgs args)
{
if (starverProtectionActive
|| !ReferenceEquals(args.Command, tshockClearCommand)
|| !AutoClearCommandRules.TryParseItemClearParameters(
args.Arguments.Parameters,
out int radiusTiles))
{
return;
}
args.Handled = true;
StartCleanup(args.Arguments.Player, radiusTiles, args.Arguments.Silent);
}- Update any other hot-path callers (e.g.
OnSafeClearItems) to use the cached flag instead of reflection:- Replace
HasStarverCleanupProtection()withstarverProtectionActivein their conditionals.
- Replace
- Change
HasStarverCleanupProtection()to become the recalculation routine that setsstarverProtectionActive(and returns it if you still need a return value), rather than walkingServerApi.Pluginson every command:- Extract the existing reflection logic into a method like
RecalculateStarverCleanupProtection()that setsstarverProtectionActive. - Call this method from plugin initialization (e.g.
Initialize/constructor), configuration reload, and any hooks you may have for plugin load/unload events.
- Extract the existing reflection logic into a method like
- Ensure
starverProtectionActiveis aprivate volatile bool(or otherwise safely published) if it can be updated from non-main threads, so command handlers read a consistent value.
Contributor
|
纯ai改的改完还没有检查，多个相关文件被删除，i18n相关全部被移除。 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
update：为应对新版ts，改为延迟分批清扫
Summary by Sourcery
引入一个全新的 AutoClear 插件，以渐进节奏、主线程安全的方式清理世界物品，并与 TShock 及可选的 Starver 保护进行集成。
新功能：
改进：
Original summary in English
Summary by Sourcery
Introduce a new AutoClear plugin that performs paced, main-thread-safe world item cleanup and integrates with TShock and optional Starver protection.
New Features:
Enhancements: