添加自动钓鱼重制版插件 #1068
Merged
添加自动钓鱼重制版插件 #1068
Conversation
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
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我发现了 6 个问题，并给出了一些整体性的反馈：
- 在
Vector2Ext中，Magnitude/Normalized对零长度向量会出现除以零的问题，而且在紧密循环中使用时开销相对较大；建议对零长度向量进行保护，并在合适的地方使用SafeNormalize/LengthSquared语义，同时重命名那些叫做distanceSquared、但实际与真实距离比较的参数，以避免让人误以为这里使用的是“距离平方”。 - 在
ExitMod中的消耗模式过期逻辑会构造一个expiredMessage，并使用静态的ClearCount，但最终只把组合后的消息发给当前玩家；如果你的意图是为所有过期玩家广播一条聚合通知，建议在同一处累积列表并广播一次，而不是按玩家逐个发送；或者干脆简化为返回一个 bool，让调用方负责消息发送。 - 在
Vector2Ext.HasNanOrInf中，显式的IsInfinity和IsPositiveInfinity检查存在重叠（正无穷本身就是无穷的一种）；你可以把它简化为对每个分量使用IsNaN || IsInfinity，这样意图更清晰，也避免了冗余检查。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `Vector2Ext`, `Magnitude`/`Normalized` can divide by zero for zero-length vectors and are relatively heavy for use in tight loops; consider guarding against zero-length vectors and using `SafeNormalize`/`LengthSquared` semantics where appropriate, and rename `distanceSquared` parameters (which are compared against actual distances) to avoid the misleading implication that squared distances are being used.
- The consumption-mode expiry logic in `ExitMod` builds an `expiredMessage` and uses a static `ClearCount` but only ever sends the composed message back to the current player; if the intent is to broadcast a single aggregated notice for all expired players, you may want to accumulate the list in one place and broadcast once instead of per-player, or simplify by returning a bool and letting the caller handle messaging.
- In `Vector2Ext.HasNanOrInf`, the explicit `IsInfinity` and `IsPositiveInfinity` checks overlap (positive infinity is a subset of infinity); you can simplify this to `IsNaN || IsInfinity` for each component, which makes the intent clearer and avoids redundant checks.
## Individual Comments
### Comment 1
<location> `src/AutoFishR/Utils/Vector2Ext.cs:63-66` </location>
<code_context>
+ return Convert.ToSingle(Math.Sqrt(vel.X * vel.X + vel.Y * vel.Y));
+ }
+
+ public static Vector2 Normalized(this Vector2 vel)
+ {
+ var magnitude = vel.Magnitude();
+ return new Vector2(vel.X / magnitude, vel.Y / magnitude);
+ }
+
</code_context>
<issue_to_address>
**issue:** Handle zero-length vectors in Normalized() to avoid NaN/Infinity results.
For a `(0, 0)` vector, `magnitude` is 0 and this division will yield NaN/Infinity. Add a guard (e.g., return `Vector2.Zero` when the magnitude is very small) or align with `Vector2.Normalize`/`Vector2.SafeNormalize` behavior to prevent invalid values from propagating.
</issue_to_address>
### Comment 2
<location> `src/AutoFishR/Utils/Vector2Ext.cs:9-13` </location>
<code_context>
+
+public static class Vector2Ext
+{
+ public static bool HasNanOrInf(this Vector2 vec)
+ {
+ return float.IsNaN(vec.X) || float.IsInfinity(vec.X)
+ || float.IsNaN(vec.Y) || float.IsInfinity(vec.Y)
+ || float.IsPositiveInfinity(vec.X) || float.IsPositiveInfinity(vec.Y);
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Simplify and correct HasNanOrInf to avoid redundant / asymmetric checks.
`float.IsInfinity` already covers both positive and negative infinity, so the extra `IsPositiveInfinity` checks are redundant and asymmetrical. This can be simplified to:
```csharp
return float.IsNaN(vec.X) || float.IsNaN(vec.Y) ||
float.IsInfinity(vec.X) || float.IsInfinity(vec.Y);
```
which keeps the behavior while making the intent clearer.
</issue_to_address>
### Comment 3
<location> `src/AutoFishR/Utils/Vector2Ext.cs:70-75` </location>
<code_context>
+ }
+
+
+ public static List<NPC> FindRangeNPCs(this Vector2 pos, float distanceSquared)
+ {
+ var npcs = new List<NPC>();
+ foreach (var npc in Main.npc)
+ if (npc != null && npc.active && npc.CanBeChasedBy())
+ if (distanceSquared > Math.Abs(npc.Center.Distance(pos)))
+ npcs.Add(npc);
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Fix distanceSquared parameter usage; currently comparing a squared radius to a non-squared distance.
`distanceSquared` is compared to `npc.Center.Distance(pos)` (and wrapped in `Math.Abs`, which isn’t needed for a distance). This treats a squared radius as if it were a linear distance, so callers passing `r * r` will actually get a range of `r * r`.
Apply a consistent approach here and in `FindRangeProjectiles`, `FindRangePlayers`, and `FindRangeTSPlayers` by either:
- using a true distance parameter (e.g., `maxDistance`) and comparing with `Vector2.Distance`, or
- keeping it squared and comparing with `Vector2.DistanceSquared`.
This will avoid subtle range bugs and keep the API semantics clear.
</issue_to_address>
### Comment 4
<location> `src/AutoFishR/Utils/Vector2Ext.cs:163-169` </location>
<code_context>
+ }
+
+
+ public static List<Vector2> CreateCircle(this Vector2 startPoint, Vector2 centerPoint, int angleIncrement,
+ int numberOfPoints)
+ {
+ var points = new List<Vector2>();
+ for (var i = 0; i < numberOfPoints; i++)
+ {
+ float angle = angleIncrement * i;
+ var x = centerPoint.X + (float)Math.Cos(angle) * (startPoint.X - centerPoint.X);
+ var y = centerPoint.Y + (float)Math.Sin(angle) * (startPoint.Y - centerPoint.Y);
</code_context>
<issue_to_address>
**issue (bug_risk):** Clarify angle units in CreateCircle; current implementation mixes degrees/radians conventions used elsewhere.
Other helpers here (`RotationAngle`, `GetArcPoints`, `GetPointsOnCircle`) use degrees and explicitly convert to radians (`* Math.PI / 180`). `CreateCircle` instead passes `angle` (an `int` increment) directly to `Math.Cos`/`Math.Sin`, so `angleIncrement` is effectively in radians.
To avoid confusion and incorrect circles when callers pass degrees, either:
- treat `angleIncrement` as degrees and convert to radians inside, or
- keep it in radians but rename/document the parameter accordingly.
</issue_to_address>
### Comment 5
<location> `src/AutoFishR/AFMain/Commands/Commands.Admin.cs:94-53` </location>
<code_context>
+ : "common.disabledVerb")));
+ AutoFish.Config.Write();
+ return true;
+ case "set":
+ caller.SendInfoMessage(Lang.T("info.currentConsumeCount", AutoFish.Config.BaitConsumeCount));
+ return true;
+ case "time":
+ caller.SendInfoMessage(
</code_context>
<issue_to_address>
**suggestion:** Admin numeric subcommands silently ignore invalid input; consider returning an error message on parse failure.
For the `set`, `duo`, and `time` subcommands, `int.TryParse` failures are ignored and the command still returns `true`, so callers get no feedback when input is invalid. Consider sending a shared error message on parse failure, e.g.:
```csharp
if (!int.TryParse(args.Parameters[1], out var value)) {
caller.SendErrorMessage(Lang.T("error.invalidNumber"));
return true;
}
```
Then reuse `value` for the valid case so admins can see when they’ve mistyped a numeric argument.
Suggested implementation:
```csharp
var matchedItems = TShock.Utils.GetItemByIdOrName(args.Parameters[1]);
if (matchedItems.Count > 1)
```
I cannot see the numeric-handling branches for the `set`, `duo`, and `time` subcommands in the provided snippet. You should update each place where those subcommands parse a numeric argument from `args.Parameters[1]` as follows:
**Before (pattern that currently ignores parse failure)**:
```csharp
// Example for "set"
int.TryParse(args.Parameters[1], out var consumeCount);
// code that uses consumeCount and always returns true afterward
AutoFish.Config.BaitConsumeCount = consumeCount;
caller.SendSuccessMessage(Lang.T("success.setConsumeCount", AutoFish.Config.BaitConsumeCount));
return true;
```
**After (with explicit parse-failure handling)**:
```csharp
// Example for "set"
if (!int.TryParse(args.Parameters[1], out var consumeCount))
{
caller.SendErrorMessage(Lang.T("error.invalidNumber"));
return true;
}
AutoFish.Config.BaitConsumeCount = consumeCount;
caller.SendSuccessMessage(Lang.T("success.setConsumeCount", AutoFish.Config.BaitConsumeCount));
return true;
```
Apply the same pattern for the `duo` and `time` admin subcommands, reusing `consumeCount`/`duoValue`/`timeMinutes` (or whatever variables you currently use) in the success path, and returning `true` immediately after sending `error.invalidNumber` in the failure path. Make sure all three subcommands return `true` in both success and failure cases so the command is considered handled and callers receive feedback instead of silent failure.
</issue_to_address>
### Comment 6
<location> `src/AutoFishR/CHANGELOG.md:11` </location>
<code_context>
+对玩家命令和管理员命令进行拆分
+添加贵重鱼饵保护功能和对应的命令
+命令对控制台添加特判
+重拍配置顺序
+引入负权限系统
+
</code_context>
<issue_to_address>
**issue (typo):** Possible typo: “重拍配置顺序” likely should be “重排配置顺序”.
“重排”更能表达“重新排序配置”的含义，建议改用该词。
```suggestion
重排配置顺序
```
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey - I've found 6 issues, and left some high level feedback:
- In
Vector2Ext,Magnitude/Normalizedcan divide by zero for zero-length vectors and are relatively heavy for use in tight loops; consider guarding against zero-length vectors and usingSafeNormalize/LengthSquaredsemantics where appropriate, and renamedistanceSquaredparameters (which are compared against actual distances) to avoid the misleading implication that squared distances are being used. - The consumption-mode expiry logic in
ExitModbuilds anexpiredMessageand uses a staticClearCountbut only ever sends the composed message back to the current player; if the intent is to broadcast a single aggregated notice for all expired players, you may want to accumulate the list in one place and broadcast once instead of per-player, or simplify by returning a bool and letting the caller handle messaging. - In
Vector2Ext.HasNanOrInf, the explicitIsInfinityandIsPositiveInfinitychecks overlap (positive infinity is a subset of infinity); you can simplify this toIsNaN || IsInfinityfor each component, which makes the intent clearer and avoids redundant checks.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `Vector2Ext`, `Magnitude`/`Normalized` can divide by zero for zero-length vectors and are relatively heavy for use in tight loops; consider guarding against zero-length vectors and using `SafeNormalize`/`LengthSquared` semantics where appropriate, and rename `distanceSquared` parameters (which are compared against actual distances) to avoid the misleading implication that squared distances are being used.
- The consumption-mode expiry logic in `ExitMod` builds an `expiredMessage` and uses a static `ClearCount` but only ever sends the composed message back to the current player; if the intent is to broadcast a single aggregated notice for all expired players, you may want to accumulate the list in one place and broadcast once instead of per-player, or simplify by returning a bool and letting the caller handle messaging.
- In `Vector2Ext.HasNanOrInf`, the explicit `IsInfinity` and `IsPositiveInfinity` checks overlap (positive infinity is a subset of infinity); you can simplify this to `IsNaN || IsInfinity` for each component, which makes the intent clearer and avoids redundant checks.
## Individual Comments
### Comment 1
<location> `src/AutoFishR/Utils/Vector2Ext.cs:63-66` </location>
<code_context>
+ return Convert.ToSingle(Math.Sqrt(vel.X * vel.X + vel.Y * vel.Y));
+ }
+
+ public static Vector2 Normalized(this Vector2 vel)
+ {
+ var magnitude = vel.Magnitude();
+ return new Vector2(vel.X / magnitude, vel.Y / magnitude);
+ }
+
</code_context>
<issue_to_address>
**issue:** Handle zero-length vectors in Normalized() to avoid NaN/Infinity results.
For a `(0, 0)` vector, `magnitude` is 0 and this division will yield NaN/Infinity. Add a guard (e.g., return `Vector2.Zero` when the magnitude is very small) or align with `Vector2.Normalize`/`Vector2.SafeNormalize` behavior to prevent invalid values from propagating.
</issue_to_address>
### Comment 2
<location> `src/AutoFishR/Utils/Vector2Ext.cs:9-13` </location>
<code_context>
+
+public static class Vector2Ext
+{
+ public static bool HasNanOrInf(this Vector2 vec)
+ {
+ return float.IsNaN(vec.X) || float.IsInfinity(vec.X)
+ || float.IsNaN(vec.Y) || float.IsInfinity(vec.Y)
+ || float.IsPositiveInfinity(vec.X) || float.IsPositiveInfinity(vec.Y);
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Simplify and correct HasNanOrInf to avoid redundant / asymmetric checks.
`float.IsInfinity` already covers both positive and negative infinity, so the extra `IsPositiveInfinity` checks are redundant and asymmetrical. This can be simplified to:
```csharp
return float.IsNaN(vec.X) || float.IsNaN(vec.Y) ||
float.IsInfinity(vec.X) || float.IsInfinity(vec.Y);
```
which keeps the behavior while making the intent clearer.
</issue_to_address>
### Comment 3
<location> `src/AutoFishR/Utils/Vector2Ext.cs:70-75` </location>
<code_context>
+ }
+
+
+ public static List<NPC> FindRangeNPCs(this Vector2 pos, float distanceSquared)
+ {
+ var npcs = new List<NPC>();
+ foreach (var npc in Main.npc)
+ if (npc != null && npc.active && npc.CanBeChasedBy())
+ if (distanceSquared > Math.Abs(npc.Center.Distance(pos)))
+ npcs.Add(npc);
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Fix distanceSquared parameter usage; currently comparing a squared radius to a non-squared distance.
`distanceSquared` is compared to `npc.Center.Distance(pos)` (and wrapped in `Math.Abs`, which isn’t needed for a distance). This treats a squared radius as if it were a linear distance, so callers passing `r * r` will actually get a range of `r * r`.
Apply a consistent approach here and in `FindRangeProjectiles`, `FindRangePlayers`, and `FindRangeTSPlayers` by either:
- using a true distance parameter (e.g., `maxDistance`) and comparing with `Vector2.Distance`, or
- keeping it squared and comparing with `Vector2.DistanceSquared`.
This will avoid subtle range bugs and keep the API semantics clear.
</issue_to_address>
### Comment 4
<location> `src/AutoFishR/Utils/Vector2Ext.cs:163-169` </location>
<code_context>
+ }
+
+
+ public static List<Vector2> CreateCircle(this Vector2 startPoint, Vector2 centerPoint, int angleIncrement,
+ int numberOfPoints)
+ {
+ var points = new List<Vector2>();
+ for (var i = 0; i < numberOfPoints; i++)
+ {
+ float angle = angleIncrement * i;
+ var x = centerPoint.X + (float)Math.Cos(angle) * (startPoint.X - centerPoint.X);
+ var y = centerPoint.Y + (float)Math.Sin(angle) * (startPoint.Y - centerPoint.Y);
</code_context>
<issue_to_address>
**issue (bug_risk):** Clarify angle units in CreateCircle; current implementation mixes degrees/radians conventions used elsewhere.
Other helpers here (`RotationAngle`, `GetArcPoints`, `GetPointsOnCircle`) use degrees and explicitly convert to radians (`* Math.PI / 180`). `CreateCircle` instead passes `angle` (an `int` increment) directly to `Math.Cos`/`Math.Sin`, so `angleIncrement` is effectively in radians.
To avoid confusion and incorrect circles when callers pass degrees, either:
- treat `angleIncrement` as degrees and convert to radians inside, or
- keep it in radians but rename/document the parameter accordingly.
</issue_to_address>
### Comment 5
<location> `src/AutoFishR/AFMain/Commands/Commands.Admin.cs:94-53` </location>
<code_context>
+ : "common.disabledVerb")));
+ AutoFish.Config.Write();
+ return true;
+ case "set":
+ caller.SendInfoMessage(Lang.T("info.currentConsumeCount", AutoFish.Config.BaitConsumeCount));
+ return true;
+ case "time":
+ caller.SendInfoMessage(
</code_context>
<issue_to_address>
**suggestion:** Admin numeric subcommands silently ignore invalid input; consider returning an error message on parse failure.
For the `set`, `duo`, and `time` subcommands, `int.TryParse` failures are ignored and the command still returns `true`, so callers get no feedback when input is invalid. Consider sending a shared error message on parse failure, e.g.:
```csharp
if (!int.TryParse(args.Parameters[1], out var value)) {
caller.SendErrorMessage(Lang.T("error.invalidNumber"));
return true;
}
```
Then reuse `value` for the valid case so admins can see when they’ve mistyped a numeric argument.
Suggested implementation:
```csharp
var matchedItems = TShock.Utils.GetItemByIdOrName(args.Parameters[1]);
if (matchedItems.Count > 1)
```
I cannot see the numeric-handling branches for the `set`, `duo`, and `time` subcommands in the provided snippet. You should update each place where those subcommands parse a numeric argument from `args.Parameters[1]` as follows:
**Before (pattern that currently ignores parse failure)**:
```csharp
// Example for "set"
int.TryParse(args.Parameters[1], out var consumeCount);
// code that uses consumeCount and always returns true afterward
AutoFish.Config.BaitConsumeCount = consumeCount;
caller.SendSuccessMessage(Lang.T("success.setConsumeCount", AutoFish.Config.BaitConsumeCount));
return true;
```
**After (with explicit parse-failure handling)**:
```csharp
// Example for "set"
if (!int.TryParse(args.Parameters[1], out var consumeCount))
{
caller.SendErrorMessage(Lang.T("error.invalidNumber"));
return true;
}
AutoFish.Config.BaitConsumeCount = consumeCount;
caller.SendSuccessMessage(Lang.T("success.setConsumeCount", AutoFish.Config.BaitConsumeCount));
return true;
```
Apply the same pattern for the `duo` and `time` admin subcommands, reusing `consumeCount`/`duoValue`/`timeMinutes` (or whatever variables you currently use) in the success path, and returning `true` immediately after sending `error.invalidNumber` in the failure path. Make sure all three subcommands return `true` in both success and failure cases so the command is considered handled and callers receive feedback instead of silent failure.
</issue_to_address>
### Comment 6
<location> `src/AutoFishR/CHANGELOG.md:11` </location>
<code_context>
+对玩家命令和管理员命令进行拆分
+添加贵重鱼饵保护功能和对应的命令
+命令对控制台添加特判
+重拍配置顺序
+引入负权限系统
+
</code_context>
<issue_to_address>
**issue (typo):** Possible typo: “重拍配置顺序” likely should be “重排配置顺序”.
“重排”更能表达“重新排序配置”的含义，建议改用该词。
```suggestion
重排配置顺序
```
</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
+63
to
+66
| public static Vector2 Normalized(this Vector2 vel) | ||
| { | ||
| var magnitude = vel.Magnitude(); | ||
| return new Vector2(vel.X / magnitude, vel.Y / magnitude); |
Contributor
There was a problem hiding this comment.
issue: 在 Normalized() 中处理零长度向量以避免产生 NaN/Infinity 结果。
对于 (0, 0) 向量，magnitude 为 0，这里的除法会产生 NaN/Infinity。请增加保护逻辑（例如当模长非常小时返回 Vector2.Zero），或者对齐 Vector2.Normalize/Vector2.SafeNormalize 的行为，以避免无效值继续向下传播。
Original comment in English
issue: Handle zero-length vectors in Normalized() to avoid NaN/Infinity results.
For a (0, 0) vector, magnitude is 0 and this division will yield NaN/Infinity. Add a guard (e.g., return Vector2.Zero when the magnitude is very small) or align with Vector2.Normalize/Vector2.SafeNormalize behavior to prevent invalid values from propagating.
Comment on lines
+9
to
+13
| public static bool HasNanOrInf(this Vector2 vec) | ||
| { | ||
| return float.IsNaN(vec.X) || float.IsInfinity(vec.X) | ||
| || float.IsNaN(vec.Y) || float.IsInfinity(vec.Y) | ||
| || float.IsPositiveInfinity(vec.X) || float.IsPositiveInfinity(vec.Y); |
Contributor
There was a problem hiding this comment.
suggestion: 简化并修正 HasNanOrInf，避免冗余且不对称的检查。
float.IsInfinity 已经同时覆盖正无穷和负无穷，因此额外的 IsPositiveInfinity 检查既冗余又不对称。可以简化为：
return float.IsNaN(vec.X) || float.IsNaN(vec.Y) ||
float.IsInfinity(vec.X) || float.IsInfinity(vec.Y);这样既保持行为一致，又能让意图更清晰。
Original comment in English
suggestion: Simplify and correct HasNanOrInf to avoid redundant / asymmetric checks.
float.IsInfinity already covers both positive and negative infinity, so the extra IsPositiveInfinity checks are redundant and asymmetrical. This can be simplified to:
return float.IsNaN(vec.X) || float.IsNaN(vec.Y) ||
float.IsInfinity(vec.X) || float.IsInfinity(vec.Y);which keeps the behavior while making the intent clearer.
Comment on lines
+70
to
+75
| public static List<NPC> FindRangeNPCs(this Vector2 pos, float distanceSquared) | ||
| { | ||
| var npcs = new List<NPC>(); | ||
| foreach (var npc in Main.npc) | ||
| if (npc != null && npc.active && npc.CanBeChasedBy()) | ||
| if (distanceSquared > Math.Abs(npc.Center.Distance(pos))) |
Contributor
There was a problem hiding this comment.
issue (bug_risk): 修复 distanceSquared 参数的用法；目前是把平方半径与未平方的距离进行比较。
distanceSquared 被拿来和 npc.Center.Distance(pos) 比较（并包了一层 Math.Abs，但对距离来说这也没有必要）。这实际上是把“半径的平方”当成线性距离来用，因此调用方若传入 r * r，最终实际范围会是 r * r。
请在这里以及 FindRangeProjectiles、FindRangePlayers 和 FindRangeTSPlayers 中使用一致的方式，要么：
- 使用真实距离参数（例如
maxDistance），并与Vector2.Distance比较；要么 - 保持使用平方距离，并与
Vector2.DistanceSquared比较。
这样可以避免细微的范围错误，并让 API 语义更加清晰。
Original comment in English
issue (bug_risk): Fix distanceSquared parameter usage; currently comparing a squared radius to a non-squared distance.
distanceSquared is compared to npc.Center.Distance(pos) (and wrapped in Math.Abs, which isn’t needed for a distance). This treats a squared radius as if it were a linear distance, so callers passing r * r will actually get a range of r * r.
Apply a consistent approach here and in FindRangeProjectiles, FindRangePlayers, and FindRangeTSPlayers by either:
- using a true distance parameter (e.g.,
maxDistance) and comparing withVector2.Distance, or - keeping it squared and comparing with
Vector2.DistanceSquared.
This will avoid subtle range bugs and keep the API semantics clear.
Comment on lines
+163
to
+169
| public static List<Vector2> CreateCircle(this Vector2 startPoint, Vector2 centerPoint, int angleIncrement, | ||
| int numberOfPoints) | ||
| { | ||
| var points = new List<Vector2>(); | ||
| for (var i = 0; i < numberOfPoints; i++) | ||
| { | ||
| float angle = angleIncrement * i; |
Contributor
There was a problem hiding this comment.
issue (bug_risk): 明确 CreateCircle 中角度的单位；当前实现与其它地方使用的度/弧度约定不一致。
这里的其它辅助方法（RotationAngle、GetArcPoints、GetPointsOnCircle）都使用“度”，并显式转换为弧度（* Math.PI / 180）。而 CreateCircle 直接把 angle（一个 int 增量）传入 Math.Cos/Math.Sin，因此 angleIncrement 实际上是弧度。
为避免调用者按“度”传参时出现混淆和错误的圆形，要么：
- 把
angleIncrement视为“度”，并在内部转换为弧度；要么 - 保持使用弧度，但相应地重命名/文档化该参数。
Original comment in English
issue (bug_risk): Clarify angle units in CreateCircle; current implementation mixes degrees/radians conventions used elsewhere.
Other helpers here (RotationAngle, GetArcPoints, GetPointsOnCircle) use degrees and explicitly convert to radians (* Math.PI / 180). CreateCircle instead passes angle (an int increment) directly to Math.Cos/Math.Sin, so angleIncrement is effectively in radians.
To avoid confusion and incorrect circles when callers pass degrees, either:
- treat
angleIncrementas degrees and convert to radians inside, or - keep it in radians but rename/document the parameter accordingly.
| : "common.disabledVerb"); | ||
| caller.SendSuccessMessage(Lang.T("success.toggle.globalMulti", caller.Name, multiToggle)); | ||
| AutoFish.Config.Write(); | ||
| return true; |
Contributor
There was a problem hiding this comment.
suggestion: 管理员的数字子命令在输入无效时会静默忽略错误；建议在解析失败时返回错误消息。
对于 set、duo 和 time 子命令，int.TryParse 失败时会被忽略，命令仍然返回 true，因此调用者在输入无效时得不到任何反馈。建议在解析失败时发送统一的错误消息，例如：
if (!int.TryParse(args.Parameters[1], out var value)) {
caller.SendErrorMessage(Lang.T("error.invalidNumber"));
return true;
}然后在解析成功的情况下复用 value，这样管理员就能知道什么时候把数字参数敲错了。
建议的实现方式：
var matchedItems = TShock.Utils.GetItemByIdOrName(args.Parameters[1]);
if (matchedItems.Count > 1)在当前提供的代码片段中，我看不到 set、duo 和 time 子命令处理数字的分支。你需要在这些子命令从 args.Parameters[1] 解析数字参数的每一处进行如下更新：
更新前（当前会忽略解析失败的模式）：
// 以 "set" 为例
int.TryParse(args.Parameters[1], out var consumeCount);
// 使用 consumeCount 的代码，并在之后总是返回 true
AutoFish.Config.BaitConsumeCount = consumeCount;
caller.SendSuccessMessage(Lang.T("success.setConsumeCount", AutoFish.Config.BaitConsumeCount));
return true;更新后（显式处理解析失败）：
// 以 "set" 为例
if (!int.TryParse(args.Parameters[1], out var consumeCount))
{
caller.SendErrorMessage(Lang.T("error.invalidNumber"));
return true;
}
AutoFish.Config.BaitConsumeCount = consumeCount;
caller.SendSuccessMessage(Lang.T("success.setConsumeCount", AutoFish.Config.BaitConsumeCount));
return true;对 duo 和 time 管理员子命令也应用相同模式，在成功路径中复用 consumeCount/duoValue/timeMinutes（或你当前使用的变量名），并在发送 error.invalidNumber 后立即返回 true。要保证这三个子命令在成功和失败时都返回 true，这样命令会被视为已处理，调用者也能收到反馈而不是静默失败。
Original comment in English
suggestion: Admin numeric subcommands silently ignore invalid input; consider returning an error message on parse failure.
For the set, duo, and time subcommands, int.TryParse failures are ignored and the command still returns true, so callers get no feedback when input is invalid. Consider sending a shared error message on parse failure, e.g.:
if (!int.TryParse(args.Parameters[1], out var value)) {
caller.SendErrorMessage(Lang.T("error.invalidNumber"));
return true;
}Then reuse value for the valid case so admins can see when they’ve mistyped a numeric argument.
Suggested implementation:
var matchedItems = TShock.Utils.GetItemByIdOrName(args.Parameters[1]);
if (matchedItems.Count > 1)I cannot see the numeric-handling branches for the set, duo, and time subcommands in the provided snippet. You should update each place where those subcommands parse a numeric argument from args.Parameters[1] as follows:
Before (pattern that currently ignores parse failure):
// Example for "set"
int.TryParse(args.Parameters[1], out var consumeCount);
// code that uses consumeCount and always returns true afterward
AutoFish.Config.BaitConsumeCount = consumeCount;
caller.SendSuccessMessage(Lang.T("success.setConsumeCount", AutoFish.Config.BaitConsumeCount));
return true;After (with explicit parse-failure handling):
// Example for "set"
if (!int.TryParse(args.Parameters[1], out var consumeCount))
{
caller.SendErrorMessage(Lang.T("error.invalidNumber"));
return true;
}
AutoFish.Config.BaitConsumeCount = consumeCount;
caller.SendSuccessMessage(Lang.T("success.setConsumeCount", AutoFish.Config.BaitConsumeCount));
return true;Apply the same pattern for the duo and time admin subcommands, reusing consumeCount/duoValue/timeMinutes (or whatever variables you currently use) in the success path, and returning true immediately after sending error.invalidNumber in the failure path. Make sure all three subcommands return true in both success and failure cases so the command is considered handled and callers receive feedback instead of silent failure.
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
ACaiCat
approved these changes
Jan 20, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
添加插件
其他
来自 Sourcery 的总结
新增 AutoFishR TShock 插件，提供可配置的自动钓鱼、多钩、增益效果和“消耗模式”等功能，并支持本地化以及玩家/管理员指令处理。
新功能：
/af）和管理员指令（/afa）处理，用于切换功能、查看状态以及管理鱼饵和战利品列表。增强改进：
文档：
Original summary in English
Summary by Sourcery
Add a new AutoFishR TShock plugin providing configurable automatic fishing, multi-hook, buff, and consumption-mode features with localization and player/admin command handling.
New Features:
Enhancements:
Documentation:
Original summary in English
来自 Sourcery 的总结
新增 AutoFishR TShock 插件，提供可配置的自动钓鱼、多钩、增益效果和“消耗模式”等功能，并支持本地化以及玩家/管理员指令处理。
新功能：
/af）和管理员指令（/afa）处理，用于切换功能、查看状态以及管理鱼饵和战利品列表。增强改进：
文档：
Original summary in English
Summary by Sourcery
Add a new AutoFishR TShock plugin providing configurable automatic fishing, multi-hook, buff, and consumption-mode features with localization and player/admin command handling.
New Features:
Enhancements:
Documentation: