Skip to content

feat: Add command placeholder#1186

Merged
Controllerdestiny merged 2 commits into
masterfrom
servertool
Jul 26, 2026
Merged

feat: Add command placeholder#1186
Controllerdestiny merged 2 commits into
masterfrom
servertool

Conversation

@Controllerdestiny

@Controllerdestiny Controllerdestiny commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

添加插件

  • 插件已加入解决方案 (Plugin.sln)
  • 插件项目已导入template.targets ()
  • 插件信息已添加至对应manifest.json
  • 插件的文件夹名字和插件的插件项目名字一样 (XXX/XXX.csproj)
  • 添加插件单独的README.md文件 (XXX/README.md)
  • 插件可以正常工作

更新插件/修复BUG

  • 插件已修改版本号
  • 更新插件README.md中的更新日志
  • 插件可以正常工作

其他

  • ❤️熙恩我喜欢你

Summary by Sourcery

在玩家指令中引入命令占位符处理机制,根据当前在线玩家和玩家群组展开特殊标记。

新特性:

  • 添加命令占位符处理器,在执行指令之前,将诸如 all 以及基于群组的占位符解析为目标玩家名称。

增强内容:

  • 在插件初始化期间注册新的命令占位符处理钩子,用于拦截并处理玩家指令。
Original summary in English

Summary by Sourcery

Introduce command placeholder handling to expand special tokens in player commands based on active players and groups.

New Features:

  • Add a command placeholder handler that resolves tokens like all and group-based placeholders into target player names before executing commands.

Enhancements:

  • Register new command placeholder handling hooks during plugin initialization to intercept and process player commands.

@Controllerdestiny
Controllerdestiny requested a review from a team as a code owner July 24, 2026 13:39
@Controllerdestiny Controllerdestiny linked an issue Jul 24, 2026 that may be closed by this pull request
2 tasks

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

  • CommandPlaceholderHandle 中的静态 _resolvers 列表只会在 Register() 中不断追加,从未被清空或加以保护,因此在重复初始化/重载插件时会注册重复的 resolver;建议让注册过程具备幂等性或改为非静态,并在需要时在插件销毁时进行清理。
  • Handle 中,当遇到未知占位符,或者某个占位符解析为空列表时,会立刻 return 并静默丢弃该命令;建议要么给玩家一些反馈(例如通过 args.Player.SendErrorMessage),要么在失败时直接执行原始命令,而不是静默失败。
  • 递归的 ExpandAndExecute 会为占位符展开的每一种组合生成一条命令,这个数量可能呈组合爆炸式增长(例如,大型分组中存在多个占位符时);建议为总展开数添加上限或安全保护,以避免过量命令执行和潜在的服务器卡顿。
面向 AI 代理的提示
Please address the comments from this code review:

## Overall Comments
- The static `_resolvers` list in `CommandPlaceholderHandle` is only ever appended to in `Register()` and never cleared or guarded, so repeated plugin initialization/reload will register duplicate resolvers; consider making registration idempotent or non-static, and ensure cleanup on plugin disposal if needed.
- In `Handle`, encountering an unknown placeholder or a placeholder that resolves to an empty list causes an immediate `return` and silently drops the command; consider either giving the player feedback (e.g., via `args.Player.SendErrorMessage`) or executing the original command unchanged instead of failing silently.
- The recursive `ExpandAndExecute` will generate a command for every combination of placeholder expansions, which can grow combinatorially (e.g., large groups with multiple placeholders); consider adding a cap on total expansions or a safety guard to prevent excessive command execution and potential server lag.

## Individual Comments

### Comment 1
<location path="src/ServerTools/CommandPlaceholderHandle.cs" line_range="11-17" />
<code_context>
+{
+    private static readonly List<PlaceholderResolver> _resolvers = [];
+
+    public static void Register()
+    {
+        // *all*
+        _resolvers.Add(new PlaceholderResolver(
+            placeholder => placeholder == "*all*",
+            _ => Plugin.ActivePlayers.Select(p => p.Name)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Guard against repeated Register calls to avoid growing the static resolver list.

Since `Register` appends to the static `_resolvers` list without checking for prior registration, calling `Plugin.Initialize` multiple times (e.g., on plugin reload) will add duplicate resolvers and cause redundant resolution. Make registration idempotent by either clearing `_resolvers` at the start of `Register` or tracking initialization (e.g., a `bool _initialized` or checking for existing resolvers before adding).

```suggestion
    public static void Register()
    {
        // Ensure repeated registrations (e.g., on plugin reload) don't accumulate duplicate resolvers
        _resolvers.Clear();

        // *all*
        _resolvers.Add(new PlaceholderResolver(
            placeholder => placeholder == "*all*",
            _ => Plugin.ActivePlayers.Select(p => p.Name)
        ));
```
</issue_to_address>

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

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

  • The static _resolvers list in CommandPlaceholderHandle is only ever appended to in Register() and never cleared or guarded, so repeated plugin initialization/reload will register duplicate resolvers; consider making registration idempotent or non-static, and ensure cleanup on plugin disposal if needed.
  • In Handle, encountering an unknown placeholder or a placeholder that resolves to an empty list causes an immediate return and silently drops the command; consider either giving the player feedback (e.g., via args.Player.SendErrorMessage) or executing the original command unchanged instead of failing silently.
  • The recursive ExpandAndExecute will generate a command for every combination of placeholder expansions, which can grow combinatorially (e.g., large groups with multiple placeholders); consider adding a cap on total expansions or a safety guard to prevent excessive command execution and potential server lag.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The static `_resolvers` list in `CommandPlaceholderHandle` is only ever appended to in `Register()` and never cleared or guarded, so repeated plugin initialization/reload will register duplicate resolvers; consider making registration idempotent or non-static, and ensure cleanup on plugin disposal if needed.
- In `Handle`, encountering an unknown placeholder or a placeholder that resolves to an empty list causes an immediate `return` and silently drops the command; consider either giving the player feedback (e.g., via `args.Player.SendErrorMessage`) or executing the original command unchanged instead of failing silently.
- The recursive `ExpandAndExecute` will generate a command for every combination of placeholder expansions, which can grow combinatorially (e.g., large groups with multiple placeholders); consider adding a cap on total expansions or a safety guard to prevent excessive command execution and potential server lag.

## Individual Comments

### Comment 1
<location path="src/ServerTools/CommandPlaceholderHandle.cs" line_range="11-17" />
<code_context>
+{
+    private static readonly List<PlaceholderResolver> _resolvers = [];
+
+    public static void Register()
+    {
+        // *all*
+        _resolvers.Add(new PlaceholderResolver(
+            placeholder => placeholder == "*all*",
+            _ => Plugin.ActivePlayers.Select(p => p.Name)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Guard against repeated Register calls to avoid growing the static resolver list.

Since `Register` appends to the static `_resolvers` list without checking for prior registration, calling `Plugin.Initialize` multiple times (e.g., on plugin reload) will add duplicate resolvers and cause redundant resolution. Make registration idempotent by either clearing `_resolvers` at the start of `Register` or tracking initialization (e.g., a `bool _initialized` or checking for existing resolvers before adding).

```suggestion
    public static void Register()
    {
        // Ensure repeated registrations (e.g., on plugin reload) don't accumulate duplicate resolvers
        _resolvers.Clear();

        // *all*
        _resolvers.Add(new PlaceholderResolver(
            placeholder => placeholder == "*all*",
            _ => Plugin.ActivePlayers.Select(p => p.Name)
        ));
```
</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 src/ServerTools/CommandPlaceholderHandle.cs
@Controllerdestiny
Controllerdestiny added this pull request to the merge queue Jul 26, 2026
Merged via the queue into master with commit 2cf63df Jul 26, 2026
3 checks passed
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.

[建议|Suggestion] 请求添加如*all*这类的变量功能

2 participants