Skip to content

refactor: Redesign the Currency System API#1152

Merged
Controllerdestiny merged 4 commits intomasterfrom
ec
May 6, 2026
Merged

refactor: Redesign the Currency System API#1152
Controllerdestiny merged 4 commits intomasterfrom
ec

Conversation

@Controllerdestiny
Copy link
Copy Markdown
Contributor

@Controllerdestiny Controllerdestiny commented May 6, 2026

添加插件

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

更新插件/修复BUG

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

其他

  • ❤️熙恩我喜欢你

Summary by Sourcery

错误修复:

  • 通过将负值视为无效输入,防止 bank 命令接受负数。
Original summary in English

Summary by Sourcery

Bug Fixes:

  • Prevent bank command from accepting negative values by treating them as invalid input.

@Controllerdestiny Controllerdestiny requested a review from a team as a code owner May 6, 2026 00:04
@Controllerdestiny Controllerdestiny changed the title Ec fix: bank command negative number May 6, 2026
Copy link
Copy Markdown
Contributor

@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 个问题,并给出了一些整体性的反馈:

  • NumberValidator 中新增的负值检查很好,但条件 if(num < 0) 的空格与文件其余部分不一致(例如 if (num < 0)),建议和现有代码风格保持一致。
  • 请考虑这里是否应该允许 0,如果不允许,需要同时更新条件和错误信息(例如改为 num <= 0),以便校验逻辑和用户可见的信息都能清晰反映预期的业务规则。
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new negative-value check in `NumberValidator` is good, but the condition `if(num <  0)` has inconsistent spacing compared to the rest of the file (`if (num < 0)`), so it would be good to align this with the existing style.
- Consider whether zero should be allowed here and, if not, update the condition and error message (e.g., `num <= 0`) so the validation logic and user-facing message clearly match the intended business rule.

## Individual Comments

### Comment 1
<location path="src/Economics.Core/Command/BankCommand.cs" line_range="24-26" />
<code_context>
             args.Player.SendErrorMessage(GetString("请输入一个有效数值!"));
             return true;
         }
+        if(num <  0)
+        {
+            args.Player.SendErrorMessage(GetString("数值必须为正数!"));
+            return true;
+        }
</code_context>
<issue_to_address>
**question:** Clarify whether zero should be allowed given the "must be positive" error message.

`num < 0` treats `0` as valid, but the message "数值必须为正数!" typically means `num > 0`. If zero should be rejected, change the condition to `num <= 0`. If zero is allowed, update the message to something like "数值必须为非负数" to match the logic.
</issue_to_address>

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

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

  • The new negative-value check in NumberValidator is good, but the condition if(num < 0) has inconsistent spacing compared to the rest of the file (if (num < 0)), so it would be good to align this with the existing style.
  • Consider whether zero should be allowed here and, if not, update the condition and error message (e.g., num <= 0) so the validation logic and user-facing message clearly match the intended business rule.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new negative-value check in `NumberValidator` is good, but the condition `if(num <  0)` has inconsistent spacing compared to the rest of the file (`if (num < 0)`), so it would be good to align this with the existing style.
- Consider whether zero should be allowed here and, if not, update the condition and error message (e.g., `num <= 0`) so the validation logic and user-facing message clearly match the intended business rule.

## Individual Comments

### Comment 1
<location path="src/Economics.Core/Command/BankCommand.cs" line_range="24-26" />
<code_context>
             args.Player.SendErrorMessage(GetString("请输入一个有效数值!"));
             return true;
         }
+        if(num <  0)
+        {
+            args.Player.SendErrorMessage(GetString("数值必须为正数!"));
+            return true;
+        }
</code_context>
<issue_to_address>
**question:** Clarify whether zero should be allowed given the "must be positive" error message.

`num < 0` treats `0` as valid, but the message "数值必须为正数!" typically means `num > 0`. If zero should be rejected, change the condition to `num <= 0`. If zero is allowed, update the message to something like "数值必须为非负数" to match the logic.
</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/Economics.Core/Command/BankCommand.cs Outdated
@Controllerdestiny Controllerdestiny linked an issue May 6, 2026 that may be closed by this pull request
2 tasks
@pacenadhif778
Copy link
Copy Markdown
Contributor

you forgot prevent player send big amount to other player example player balance 400sen do /bank pay 99999 to target player i think no handled corectly please check curencymanager.cs
if config only have one curency (do not have redemption relathionship) /bank cash works like /bank add give free curency (main curenncy)

@Controllerdestiny

This comment was marked as outdated.

@pacenadhif778
Copy link
Copy Markdown
Contributor

maybe in bank cash check if there no redemption relathionship added this in dedudct
if (!RedemptionRelationships.Any()) return false;

@Controllerdestiny
Copy link
Copy Markdown
Contributor Author

maybe in bank cash check if there no redemption relathionship added this in dedudct if (!RedemptionRelationships.Any()) return false;

I believe the current plugin's currency exchange system has serious issues, and modifying it may require structural changes.

@Controllerdestiny Controllerdestiny requested a review from ACaiCat as a code owner May 6, 2026 05:20
@Controllerdestiny Controllerdestiny changed the title fix: bank command negative number refactor: Redesign the Currency System API May 6, 2026
@Controllerdestiny Controllerdestiny added this pull request to the merge queue May 6, 2026
Merged via the queue into master with commit c0228de May 6, 2026
3 checks passed
@Controllerdestiny
Copy link
Copy Markdown
Contributor Author

tmd

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.

[BUG] curency manager bug

3 participants