fix: statu msg Can't display - #1196
Merged
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
嘿——我发现了 5 个问题
AI 代理提示词
请处理本次代码审查中的评论：
## 单独评论
### 评论 1
<location path="src/Economics.Core/ConfigFiles/Setting.cs" line_range="26-27" />
<code_context>
[JsonProperty("显示信息下移")]
public int StatusTextShiftDown = 0;
+ [JsonProperty("显示信息内容")]
+ public List<string> StatusTextContent = [];
+
[JsonProperty("渐变颜色")]
</code_context>
<issue_to_address>
**问题（更广泛的影响）：** 现有的 Economics.json 文件在反序列化时不会调用 `SetDefault()`，因此当配置中不存在该属性时，新添加的 `StatusTextContent` 会保持其字段初始化时的空列表。随后，`CountertopUpdate` 只会发送一条空白状态消息，导致升级后现有用户无法显示状态文本。
**触发条件：** 从 Economics.json 不包含“显示信息内容”的版本升级时。
**建议修复：** 在加载/迁移期间，将默认状态内容合并到现有配置中，或者在 `StatusTextContent` 为空时使用回退值。
```suggestion
[JsonProperty("显示信息内容")]
public List<string> StatusTextContent =
[
"玩家名称：{player}",
"世界名称：{world}",
"玩家生命：{life}/{maxlife}",
"玩家魔法：{mana}/{maxmana}",
"当前延迟: {ping}",
"在线玩家: {online}/{maxonline}",
"货币信息: {currencies}",
"当前职业：{level}",
"升级职业：{levelRank}",
"当前技能：{skill}"
];
```
</issue_to_address>
### 评论 2
<location path="src/Economics.Core/Utils/Helper.cs" line_range="86" />
<code_context>
-using Economics.Core.ConfigFiles;
using Newtonsoft.Json;
+using Newtonsoft.Json.Serialization;
namespace Economics.Core.ConfigFiles;
</code_context>
<issue_to_address>
**问题（更广泛的影响）：** `CountertopUpdate` 不再对每个已配置的行调用 `GetGradientText`，因此 `Setting.Instance.GradientColor` 会被忽略，所有状态文本都会在未应用已配置渐变颜色的情况下发送。这使之前实现提供的行为发生了回归。
**触发条件：** 用户依赖现有的“渐变颜色”配置时。
**建议修复：** 在将每一行追加到状态消息之前，先通过 `GetGradientText` 处理解析后的内容。
```suggestion
Setting.Instance.StatusTextContent.ForEach(m => sb.AppendLine(GetGradientText(PlaceholderManager.Resolve(m, player)) + left));
```
</issue_to_address>
### 评论 3
<location path="src/Economics.Core/Services/PingService.cs" line_range="50" />
<code_context>
+ _records[player] = record;
+ }
+
+ record.ItemIndex = slot;
+ record.Stopwatch.Restart();
+
+ NetMessage.TrySendData(22, player.Index, -1, null, slot);
+ NetMessage.TrySendData(39, player.Index, -1, null, slot);
+ }
+
</code_context>
<issue_to_address>
**问题（潜在错误风险）：** 每次 ping 都只使用物品槽位作为负载，同时发送数据包 22 和数据包 39。数据包 22 是一个物品掉落/同步数据包，还需要其他物品字段；使用默认参数发送它会导致客户端处理无效的物品更新，而不是仅执行 ping 握手。
**触发条件：** 活跃玩家收到计划中的 ping 时。
**建议修复：** 只发送 ping 协议所需的数据包，或者根据 Terraria 协议填充数据包 22 所需的全部字段。
```suggestion
```
</issue_to_address>
### 评论 4
<location path="src/Economics.Core/Services/PingService.cs" line_range="69-72" />
<code_context>
+ return data;
+ }
+
+ public static void RemovePlayer(TSPlayer player)
+ {
+ _records.Remove(player);
+ }
+}
</code_context>
<issue_to_address>
**问题（潜在错误风险）：** `PingService._records` 会保留每个玩家的 `PingData`，但新的离开处理程序只从 `ServerPlayers` 中移除玩家，从未调用 `PingService.RemovePlayer`。因此，已断开连接的 `TSPlayer` 实例仍被静态字典强引用，导致玩家反复重连时出现陈旧引用的内存泄漏。
**触发条件：** 玩家从服务器断开连接时。
**建议修复：** 在 `OnLeave` 中调用 `PingService.RemovePlayer(player)`，并在那里清理所有未完成的物品占用。
</issue_to_address>
### 评论 5
<location path="src/Economics.RPG/RPG.cs" line_range="26-30" />
<code_context>
PlayerLevelManager = new();
PlayerHooks.PlayerPermission += this.PlayerHooks_PlayerPermission;
PlayerHooks.PlayerChat += this.PlayerHooks_PlayerChat;
- PlayerHandler.OnPlayerCountertop += this.OnCounterTop;
+ PlaceholderManager.Register("level", p => PlayerLevelManager.GetLevel(p.Name)?.Name ?? "?");
+ PlaceholderManager.Register("levelRank", p => string.Join(",", PlayerLevelManager.GetLevel(p.Name)?.RankLevels.Select(x => $"{x.Name}") ?? []));
}
</code_context>
<issue_to_address>
**问题（潜在错误风险）：** RPG 和 Skill 在进程级的 `PlaceholderManager` 中注册了委托，却从未在 `Dispose` 中注销。任一插件卸载后，状态更新仍会调用其已失效的委托；该委托会访问已卸载插件的管理器/本地化状态，并使过时的占位符继续处于活动状态。
**触发条件：** Economics.Core 仍处于加载状态，而 Economics.RPG 或 Economics.Skill 被卸载或重新加载时。
**建议修复：** 添加占位符注销支持，并在释放资源期间移除每个插件的注册。
</issue_to_address>请帮助我变得更有用！请对每条评论点击 👍 或 👎，我会利用反馈来改进审查结果。
Original comment in English
Hey - I've found 5 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/Economics.Core/ConfigFiles/Setting.cs" line_range="26-27" />
<code_context>
[JsonProperty("显示信息下移")]
public int StatusTextShiftDown = 0;
+ [JsonProperty("显示信息内容")]
+ public List<string> StatusTextContent = [];
+
[JsonProperty("渐变颜色")]
</code_context>
<issue_to_address>
**issue (broader_impact):** Existing Economics.json files are deserialized without calling `SetDefault()`, so the newly added `StatusTextContent` remains its field-initialized empty list when the property is absent. `CountertopUpdate` then sends only a blank status message after upgrading, which prevents the status text from displaying for existing users.
**Triggers:** When upgrading from a version whose Economics.json does not contain 显示信息内容.
**Suggested fix:** Merge the default status content into existing configurations during load/migration, or use a fallback when `StatusTextContent` is empty.
```suggestion
[JsonProperty("显示信息内容")]
public List<string> StatusTextContent =
[
"玩家名称：{player}",
"世界名称：{world}",
"玩家生命：{life}/{maxlife}",
"玩家魔法：{mana}/{maxmana}",
"当前延迟: {ping}",
"在线玩家: {online}/{maxonline}",
"货币信息: {currencies}",
"当前职业：{level}",
"升级职业：{levelRank}",
"当前技能：{skill}"
];
```
</issue_to_address>
### Comment 2
<location path="src/Economics.Core/Utils/Helper.cs" line_range="86" />
<code_context>
-using Economics.Core.ConfigFiles;
using Newtonsoft.Json;
+using Newtonsoft.Json.Serialization;
namespace Economics.Core.ConfigFiles;
</code_context>
<issue_to_address>
**issue (broader_impact):** `CountertopUpdate` no longer calls `GetGradientText` for each configured line, so `Setting.Instance.GradientColor` is ignored and all status text is sent without the configured gradient coloring. This regresses the behavior provided by the previous implementation.
**Triggers:** When users rely on the existing 渐变颜色 configuration.
**Suggested fix:** Pass each resolved line through `GetGradientText` before appending it to the status message.
```suggestion
Setting.Instance.StatusTextContent.ForEach(m => sb.AppendLine(GetGradientText(PlaceholderManager.Resolve(m, player)) + left));
```
</issue_to_address>
### Comment 3
<location path="src/Economics.Core/Services/PingService.cs" line_range="50" />
<code_context>
+ _records[player] = record;
+ }
+
+ record.ItemIndex = slot;
+ record.Stopwatch.Restart();
+
+ NetMessage.TrySendData(22, player.Index, -1, null, slot);
+ NetMessage.TrySendData(39, player.Index, -1, null, slot);
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Each ping sends both packet 22 and packet 39 using only the item slot as the payload. Packet 22 is an item-drop/synchronization packet with additional item fields, so sending it with default arguments causes clients to process an invalid item update instead of merely performing the ping handshake.
**Triggers:** When an active player receives the scheduled ping.
**Suggested fix:** Send only the packet required by the ping protocol, or populate every field required by packet 22 according to the Terraria protocol.
```suggestion
```
</issue_to_address>
### Comment 4
<location path="src/Economics.Core/Services/PingService.cs" line_range="69-72" />
<code_context>
+ return data;
+ }
+
+ public static void RemovePlayer(TSPlayer player)
+ {
+ _records.Remove(player);
+ }
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** `PingService._records` retains every player's `PingData`, but the new leave handler only removes the player from `ServerPlayers` and never calls `PingService.RemovePlayer`. Disconnected `TSPlayer` instances therefore remain strongly referenced by the static dictionary, causing a stale-reference memory leak across reconnects.
**Triggers:** When players disconnect from the server.
**Suggested fix:** Call `PingService.RemovePlayer(player)` from `OnLeave`, and clear any outstanding item reservation there.
</issue_to_address>
### Comment 5
<location path="src/Economics.RPG/RPG.cs" line_range="26-30" />
<code_context>
PlayerLevelManager = new();
PlayerHooks.PlayerPermission += this.PlayerHooks_PlayerPermission;
PlayerHooks.PlayerChat += this.PlayerHooks_PlayerChat;
- PlayerHandler.OnPlayerCountertop += this.OnCounterTop;
+ PlaceholderManager.Register("level", p => PlayerLevelManager.GetLevel(p.Name)?.Name ?? "?");
+ PlaceholderManager.Register("levelRank", p => string.Join(",", PlayerLevelManager.GetLevel(p.Name)?.RankLevels.Select(x => $"{x.Name}") ?? []));
}
</code_context>
<issue_to_address>
**issue (bug_risk):** RPG and Skill register delegates in the process-wide `PlaceholderManager` but never unregister them in `Dispose`. After either plugin is unloaded, a status update still invokes its stale delegate, which accesses the unloaded plugin's manager/localization state and leaves obsolete placeholders active.
**Triggers:** When Economics.Core remains loaded while Economics.RPG or Economics.Skill is unloaded or reloaded.
**Suggested fix:** Add placeholder unregister support and remove each plugin's registrations during disposal.
</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
+26
to
+27
| [JsonProperty("显示信息内容")] | ||
| public List<string> StatusTextContent = []; |
Contributor
There was a problem hiding this comment.
问题（更广泛的影响）： 现有的 Economics.json 文件在反序列化时不会调用 SetDefault()，因此当配置中不存在该属性时，新添加的 StatusTextContent 会保持其字段初始化时的空列表。随后，CountertopUpdate 只会发送一条空白状态消息，导致升级后现有用户无法显示状态文本。
触发条件： 从 Economics.json 不包含“显示信息内容”的版本升级时。
建议修复： 在加载/迁移期间，将默认状态内容合并到现有配置中，或者在 StatusTextContent 为空时使用回退值。
Suggested change
| [JsonProperty("显示信息内容")] | |
| public List<string> StatusTextContent = []; | |
| [JsonProperty("显示信息内容")] | |
| public List<string> StatusTextContent = | |
| [ | |
| "玩家名称：{player}", | |
| "世界名称：{world}", | |
| "玩家生命：{life}/{maxlife}", | |
| "玩家魔法：{mana}/{maxmana}", | |
| "当前延迟: {ping}", | |
| "在线玩家: {online}/{maxonline}", | |
| "货币信息: {currencies}", | |
| "当前职业：{level}", | |
| "升级职业：{levelRank}", | |
| "当前技能：{skill}" | |
| ]; |
Original comment in English
issue (broader_impact): Existing Economics.json files are deserialized without calling SetDefault(), so the newly added StatusTextContent remains its field-initialized empty list when the property is absent. CountertopUpdate then sends only a blank status message after upgrading, which prevents the status text from displaying for existing users.
Triggers: When upgrading from a version whose Economics.json does not contain 显示信息内容.
Suggested fix: Merge the default status content into existing configurations during load/migration, or use a fallback when StatusTextContent is empty.
Suggested change
| [JsonProperty("显示信息内容")] | |
| public List<string> StatusTextContent = []; | |
| [JsonProperty("显示信息内容")] | |
| public List<string> StatusTextContent = | |
| [ | |
| "玩家名称：{player}", | |
| "世界名称：{world}", | |
| "玩家生命：{life}/{maxlife}", | |
| "玩家魔法：{mana}/{maxmana}", | |
| "当前延迟: {ping}", | |
| "在线玩家: {online}/{maxonline}", | |
| "货币信息: {currencies}", | |
| "当前职业：{level}", | |
| "升级职业：{levelRank}", | |
| "当前技能：{skill}" | |
| ]; |
| sb.AppendLine(down); | ||
| args.Messages.OrderBy(x => x.Order).ForEach(x => sb.AppendLine(GetGradientText(x.Message) + Left)); | ||
| args.Player?.SendData(PacketTypes.Status, sb.ToString(), 0, 1); | ||
| Setting.Instance.StatusTextContent.ForEach(m => sb.AppendLine(PlaceholderManager.Resolve(m, player) + left)); |
Contributor
There was a problem hiding this comment.
问题（更广泛的影响）： CountertopUpdate 不再对每个已配置的行调用 GetGradientText，因此 Setting.Instance.GradientColor 会被忽略，所有状态文本都会在未应用已配置渐变颜色的情况下发送。这使之前实现提供的行为发生了回归。
触发条件： 用户依赖现有的“渐变颜色”配置时。
建议修复： 在将每一行追加到状态消息之前，先通过 GetGradientText 处理解析后的内容。
Suggested change
| Setting.Instance.StatusTextContent.ForEach(m => sb.AppendLine(PlaceholderManager.Resolve(m, player) + left)); | |
| Setting.Instance.StatusTextContent.ForEach(m => sb.AppendLine(GetGradientText(PlaceholderManager.Resolve(m, player)) + left)); |
Original comment in English
issue (broader_impact): CountertopUpdate no longer calls GetGradientText for each configured line, so Setting.Instance.GradientColor is ignored and all status text is sent without the configured gradient coloring. This regresses the behavior provided by the previous implementation.
Triggers: When users rely on the existing 渐变颜色 configuration.
Suggested fix: Pass each resolved line through GetGradientText before appending it to the status message.
Suggested change
| Setting.Instance.StatusTextContent.ForEach(m => sb.AppendLine(PlaceholderManager.Resolve(m, player) + left)); | |
| Setting.Instance.StatusTextContent.ForEach(m => sb.AppendLine(GetGradientText(PlaceholderManager.Resolve(m, player)) + left)); |
| record.ItemIndex = slot; | ||
| record.Stopwatch.Restart(); | ||
|
|
||
| NetMessage.TrySendData(22, player.Index, -1, null, slot); |
Contributor
There was a problem hiding this comment.
问题（潜在错误风险）： 每次 ping 都只使用物品槽位作为负载，同时发送数据包 22 和数据包 39。数据包 22 是一个物品掉落/同步数据包，还需要其他物品字段；使用默认参数发送它会导致客户端处理无效的物品更新，而不是仅执行 ping 握手。
触发条件： 活跃玩家收到计划中的 ping 时。
建议修复： 只发送 ping 协议所需的数据包，或者根据 Terraria 协议填充数据包 22 所需的全部字段。
Suggested change
| NetMessage.TrySendData(22, player.Index, -1, null, slot); |
Original comment in English
issue (bug_risk): Each ping sends both packet 22 and packet 39 using only the item slot as the payload. Packet 22 is an item-drop/synchronization packet with additional item fields, so sending it with default arguments causes clients to process an invalid item update instead of merely performing the ping handshake.
Triggers: When an active player receives the scheduled ping.
Suggested fix: Send only the packet required by the ping protocol, or populate every field required by packet 22 according to the Terraria protocol.
Suggested change
| NetMessage.TrySendData(22, player.Index, -1, null, slot); |
Comment on lines
26
to
+30
| PlayerLevelManager = new(); | ||
| PlayerHooks.PlayerPermission += this.PlayerHooks_PlayerPermission; | ||
| PlayerHooks.PlayerChat += this.PlayerHooks_PlayerChat; | ||
| PlayerHandler.OnPlayerCountertop += this.OnCounterTop; | ||
| PlaceholderManager.Register("level", p => PlayerLevelManager.GetLevel(p.Name)?.Name ?? "?"); | ||
| PlaceholderManager.Register("levelRank", p => string.Join(",", PlayerLevelManager.GetLevel(p.Name)?.RankLevels.Select(x => $"{x.Name}") ?? [])); |
Contributor
There was a problem hiding this comment.
问题（潜在错误风险）： RPG 和 Skill 在进程级的 PlaceholderManager 中注册了委托，却从未在 Dispose 中注销。任一插件卸载后，状态更新仍会调用其已失效的委托；该委托会访问已卸载插件的管理器/本地化状态，并使过时的占位符继续处于活动状态。
触发条件： Economics.Core 仍处于加载状态，而 Economics.RPG 或 Economics.Skill 被卸载或重新加载时。
建议修复： 添加占位符注销支持，并在释放资源期间移除每个插件的注册。
Original comment in English
issue (bug_risk): RPG and Skill register delegates in the process-wide PlaceholderManager but never unregister them in Dispose. After either plugin is unloaded, a status update still invokes its stale delegate, which accesses the unloaded plugin's manager/localization state and leaves obsolete placeholders active.
Triggers: When Economics.Core remains loaded while Economics.RPG or Economics.Skill is unloaded or reloaded.
Suggested fix: Add placeholder unregister support and remove each plugin's registrations during disposal.
Controllerdestiny
enabled auto-merge
September 1, 2026 05:11
ACaiCat
approved these changes
Sep 2, 2026
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.
添加插件
更新插件/修复BUG
其他
Sourcery 摘要
通过占位符使玩家状态消息可配置，并恢复整个插件套件中可靠的 Ping 信息。
新功能：
错误修复：
增强功能：
文档：
维护：
Original summary in English
Sourcery 总结
使玩家状态显示可配置，并在整个插件套件中恢复可靠的 Ping 信息。
新功能：
错误修复：
增强功能：
文档：
杂项：
Original summary in English
Sourcery 总结
使玩家状态显示可配置，并恢复整个插件套件中可靠的 Ping 信息。
新功能：
错误修复：
增强功能：
文档：
维护：
Original summary in English
Sourcery 摘要
使玩家状态显示可配置，并在整个插件套件中恢复可靠的 Ping 信息。
新功能：
错误修复：
增强功能：
文档：
维护：
Original summary in English
Sourcery 摘要
通过占位符使玩家状态显示可配置，并恢复整个插件套件中可靠的 Ping 信息。
新功能：
Bug 修复：
增强功能：
文档：
杂项：
Original summary in English
Sourcery 摘要
使玩家状态显示可配置、提升 Ping 的可靠性，并为技能和怪物 AI 引入可复用的脚本支持。
新功能：
错误修复：
增强功能：
构建：
文档：
杂项：
Original summary in English
Summary by Sourcery
Make player status displays configurable, improve ping reliability, and introduce reusable scripting support for skills and monster AI.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Chores: