Skip to content

修复一个空指令bug#7

Open
AX-17 wants to merge 2 commits intoUnrealMultiple:masterfrom
AX-17:master
Open

修复一个空指令bug#7
AX-17 wants to merge 2 commits intoUnrealMultiple:masterfrom
AX-17:master

Conversation

@AX-17
Copy link
Copy Markdown
Contributor

@AX-17 AX-17 commented May 6, 2026

由 Sourcery 提供的总结

更新 Vortex 适配器插件的元数据,并改进对可配置的空占位符聊天指令的处理,包括正确的注册、清理和重载行为。

新功能:

  • 支持可配置的空占位符聊天指令,带有明确的帮助文本,并对指令名称进行规范化处理。

错误修复:

  • 确保在插件卸载和配置重载时,空占位符聊天指令能够被正确移除并重新注册,避免指令陈旧或重复。

增强改进:

  • 提升插件版本号并更新作者元数据。
  • 将空指令的注册重构为专门的辅助方法,用于规范化并去重指令名称。
Original summary in English

Summary by Sourcery

Update the Vortex adapter plugin metadata and improve handling of configurable empty placeholder chat commands, including proper registration, cleanup, and reload behavior.

New Features:

  • Support configurable empty placeholder chat commands with explicit help text and normalization of command names.

Bug Fixes:

  • Ensure empty placeholder chat commands are correctly removed and re-registered on plugin disposal and config reload to avoid stale or duplicate commands.

Enhancements:

  • Increment the plugin version and update the author metadata.
  • Refactor empty command registration into dedicated helper methods that normalize and deduplicate command names.

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - 我发现了 1 个问题,并给出了一些整体反馈:

  • RegisterEmptyCommands 中,你先对空命令名称进行了规范化处理,然后又调用 RemoveRegisteredEmptyCommands,而它内部会再次对名称进行规范化;建议要么对 RemoveRegisteredEmptyCommands 进行重载以接受已规范化的名称,要么重构逻辑,使规范化只发生一次,以避免额外的分配和枚举。
  • NormalizeEmptyCommandNames 中,return []; 语法依赖较新的 C# 特性;如果项目没有明确定位到该 C# 版本,使用 Array.Empty<string>() 来创建数组会更安全也更清晰。
给 AI Agent 的提示
请针对本次代码评审中的评论进行修改:

## 总体意见
-`RegisterEmptyCommands` 中,你先对空命令名称进行了规范化处理,然后又调用 `RemoveRegisteredEmptyCommands`,而它内部会再次对名称进行规范化;建议要么对 `RemoveRegisteredEmptyCommands` 进行重载以接受已规范化的名称,要么重构逻辑,使规范化只发生一次,以避免额外的分配和枚举。
-`NormalizeEmptyCommandNames` 中,`return [];` 语法依赖较新的 C# 特性;如果项目没有明确定位到该 C# 版本,使用 `Array.Empty<string>()` 来创建数组会更安全也更清晰。

## 逐条评论

### 评论 1
<location path="Vortex.Adapter/Plugin.cs" line_range="132" />
<code_context>
+        }
+    }
+
+    private static void RemoveRegisteredEmptyCommands(IEnumerable<string>? emptyCommands)
+    {
+        var normalizedNames = NormalizeEmptyCommandNames(emptyCommands);
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 收紧删除谓词,以避免误删不相关的命令。

当前这个谓词同样可能匹配到真实命令,只要它们刚好只有一个名称、空权限,并且名称出现在 `nameSet` 中。如果这些删除操作只应该作用于该插件的占位命令,建议主要依赖 HelpText 标记(以及/或者来源程序集)来判定,并避免使用基于权限的回退逻辑,或者将那部分逻辑明确标记为临时/迁移用途。

建议实现:

```csharp
    private static void RemoveRegisteredEmptyCommands(IEnumerable<string>? emptyCommands)
    {
        var normalizedNames = NormalizeEmptyCommandNames(emptyCommands);
        var nameSet = new HashSet<string>(normalizedNames, StringComparer.OrdinalIgnoreCase);

        // Only remove commands that:
        //   - Were created as this plugin's empty/placeholder commands (identified by HelpText marker), and
        //   - Have a name in the current empty-command name set.
        //
        // This avoids accidentally removing real commands that happen to have:
        //   - A single name,
        //   - An empty permission set, and
        //   - A name that overlaps with our empty-command list.
        Commands.ChatCommands.RemoveAll(c =>
            c.HelpText == EmptyCommandHelpText &&
            nameSet.Contains(c.Name));

        // NOTE: If you still need a temporary/migration path that also cleans up older
        // placeholder commands that did not use the HelpText marker, implement that
        // explicitly and time-bound here, e.g.:
        //
        // var migrationCutoff = new DateTime(2025, 01, 01, 0, 0, 0, DateTimeKind.Utc);
        // Commands.ChatCommands.RemoveAll(c =>
        //     c.HelpText is null &&
        //     c.Permissions.Count == 0 &&
        //     nameSet.Contains(c.Name) &&
        //     c.OriginAssembly == typeof(Plugin).Assembly &&
        //     c.RegisteredAt < migrationCutoff);
        //
        // and remove that block once deployments are past the migration window.
    }

```

我目前只能看到 `Plugin.cs` 的一小部分,请你仔细核对并调整 `SEARCH` 代码块,使其与现有的 `RemoveRegisteredEmptyCommands` 实现相匹配。你需要确保的关键行为变化是:

1. 对于正常的删除操作,`RemoveAll` 的谓词应当 *始终* 同时满足 `c.HelpText == EmptyCommandHelpText``nameSet.Contains(c.Name)`2. 任何额外条件(例如基于权限的)都应明确标注为临时/迁移用途的代码块,最好还要受 `OriginAssembly`(如果你的 `Command` 类型上提供该属性)和/或时间窗口的约束,并在不再需要时删除。
3. 如果你的 `Command` 类使用的程序集 / 插件来源属性名称不是 `OriginAssembly`,而是其他名字,请在你采用迁移示例时相应地更新这个引用。
</issue_to_address>

Sourcery 对开源项目免费使用——如果你觉得这些评审有帮助,欢迎分享 ✨
帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续评审。
Original comment in English

Hey - I've found 1 issue, and left some high level feedback:

  • In RegisterEmptyCommands you normalize the empty command names and then call RemoveRegisteredEmptyCommands, which normalizes them again; consider either overloading RemoveRegisteredEmptyCommands to accept already-normalized names or refactoring so normalization happens only once to avoid extra allocations and enumeration.
  • In NormalizeEmptyCommandNames, the return []; syntax relies on newer C# features; if the project isn’t explicitly targeting that version, using Array.Empty<string>() would be safer and clearer for array creation.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `RegisterEmptyCommands` you normalize the empty command names and then call `RemoveRegisteredEmptyCommands`, which normalizes them again; consider either overloading `RemoveRegisteredEmptyCommands` to accept already-normalized names or refactoring so normalization happens only once to avoid extra allocations and enumeration.
- In `NormalizeEmptyCommandNames`, the `return [];` syntax relies on newer C# features; if the project isn’t explicitly targeting that version, using `Array.Empty<string>()` would be safer and clearer for array creation.

## Individual Comments

### Comment 1
<location path="Vortex.Adapter/Plugin.cs" line_range="132" />
<code_context>
+        }
+    }
+
+    private static void RemoveRegisteredEmptyCommands(IEnumerable<string>? emptyCommands)
+    {
+        var normalizedNames = NormalizeEmptyCommandNames(emptyCommands);
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Tighten the removal predicate to avoid accidentally removing unrelated commands.

This predicate can also match real commands that happen to have a single name, an empty permission, and a matching name in `nameSet`. If these removals are meant to be limited to this plugin’s placeholder commands, consider keying primarily off the help-text marker (and/or origin assembly) and avoiding the permissions-based fallback, or making that path explicitly temporary/migration-only.

Suggested implementation:

```csharp
    private static void RemoveRegisteredEmptyCommands(IEnumerable<string>? emptyCommands)
    {
        var normalizedNames = NormalizeEmptyCommandNames(emptyCommands);
        var nameSet = new HashSet<string>(normalizedNames, StringComparer.OrdinalIgnoreCase);

        // Only remove commands that:
        //   - Were created as this plugin's empty/placeholder commands (identified by HelpText marker), and
        //   - Have a name in the current empty-command name set.
        //
        // This avoids accidentally removing real commands that happen to have:
        //   - A single name,
        //   - An empty permission set, and
        //   - A name that overlaps with our empty-command list.
        Commands.ChatCommands.RemoveAll(c =>
            c.HelpText == EmptyCommandHelpText &&
            nameSet.Contains(c.Name));

        // NOTE: If you still need a temporary/migration path that also cleans up older
        // placeholder commands that did not use the HelpText marker, implement that
        // explicitly and time-bound here, e.g.:
        //
        // var migrationCutoff = new DateTime(2025, 01, 01, 0, 0, 0, DateTimeKind.Utc);
        // Commands.ChatCommands.RemoveAll(c =>
        //     c.HelpText is null &&
        //     c.Permissions.Count == 0 &&
        //     nameSet.Contains(c.Name) &&
        //     c.OriginAssembly == typeof(Plugin).Assembly &&
        //     c.RegisteredAt < migrationCutoff);
        //
        // and remove that block once deployments are past the migration window.
    }

```

I only see a small portion of `Plugin.cs`, so please double-check and adjust the `SEARCH` block to match your existing implementation of `RemoveRegisteredEmptyCommands`. The key behavior changes you want are:

1. The `RemoveAll` predicate should *always* require `c.HelpText == EmptyCommandHelpText` and `nameSet.Contains(c.Name)` for normal removals.
2. Any additional criteria (e.g., based on permissions) should be clearly marked as a temporary/migration-only block, ideally also constrained by `OriginAssembly` (if available on your `Command` type) and/or a time window, and removed once no longer needed.
3. If your `Command` class exposes an assembly / plugin origin property with a different name than `OriginAssembly`, update that reference accordingly in the migration example if you choose to implement it.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread Vortex.Adapter/Plugin.cs Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant