Skip to content

Conversation

@ACaiCat
Copy link
Member

@ACaiCat ACaiCat commented Nov 2, 2025

  • 添加LoiginQueue字段以保证每个玩家只能发起一次登录请求
  • 修复IsLoginIn字段忘记被设置的问题
  • 清理代码

Sourcery 总结

通过引入 LoginQueueSSCLogin 标志来强制执行单一登录请求和正确的登录状态跟踪;修复了白名单流程中 IsLoggedIn 标志的赋值问题;更新了数据包劫持逻辑以遵循这些标志;并通过移除未使用的成员和虚方法来精简 TSPlayer

新功能:

  • TSPlayer 添加 LoginQueueSSCLogin 标志,以强制每个玩家只发送一次登录请求

错误修复:

  • 修复白名单接受后 IsLoggedIn 标志赋值缺失的问题
  • 通过在数据包处理器中检查 LoginQueueIsLoggedIn,防止重复的登录请求导致意外断开连接

改进:

  • TSPlayer 中移除大量未使用的字段和 virtual 修饰符,以简化该类
  • 清理多个服务文件中的空格、格式和细微的字符串/枚举解析不一致问题
Original summary in English

Summary by Sourcery

Enforce single login requests and proper login state tracking by introducing LoginQueue and SSCLogin flags, fixing the IsLoggedIn assignment in the whitelist flow, updating packet hijacking logic to respect these flags, and streamlining TSPlayer by removing unused members and virtual methods.

New Features:

  • Add LoginQueue and SSCLogin flags to TSPlayer to enforce a single login request per player

Bug Fixes:

  • Fix missing IsLoggedIn flag assignment after whitelist acceptance
  • Prevent duplicate login requests from causing unexpected disconnects by checking LoginQueue and IsLoggedIn in packet handlers

Enhancements:

  • Remove numerous unused fields and virtual modifiers from TSPlayer to simplify the class
  • Clean up whitespace, formatting, and minor string/enum parsing inconsistencies across multiple service files

Copy link

@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.

你好 - 我已审阅你的更改 - 以下是一些反馈:

  • 确保在白名单成功和失败路径上都重置 LoginQueue 标志,以免玩家被永久阻塞。
  • 考虑将 SSCLogin 和 LoginQueue 重命名为更具描述性的名称(例如 HasPerformedSscLogin, IsLoginPending),以便它们在登录序列中的作用更清晰。
  • 仔细检查删除 virtual 修饰符和 TSRestPlayer 子类不会破坏任何期望挂接到 SendMessage 或相关方法的插件覆盖。
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- 确保在白名单成功和失败路径上都重置 LoginQueue 标志,以免玩家被永久阻塞。
- 考虑将 SSCLogin 和 LoginQueue 重命名为更具描述性的名称(例如 HasPerformedSscLogin, IsLoginPending),以便它们在登录序列中的作用更清晰。
- 仔细检查删除 virtual 修饰符和 TSRestPlayer 子类不会破坏任何期望挂接到 SendMessage 或相关方法的插件覆盖。

## Individual Comments

### Comment 1
<location> `CaiBotLiteMod/Moudles/TSPlayer.cs:258-83` </location>
<code_context>
     ///     Initializes a new instance of the <see cref="TSPlayer" /> class.\n     /// </summary>\n     /// <param name="playerName">The player's name.</param>\n-    protected TSPlayer(string playerName)\n+    private TSPlayer(string playerName)\n     {\n         this.Index = -1;\n</code_context>

<issue_to_address>
**issue (bug_risk):** 将 TSPlayer 构造函数更改为 private 可能会限制子类化。

此更改可能会破坏依赖于受保护访问构造函数的现有子类,例如 TSRestPlayer。
</issue_to_address>

### Comment 2
<location> `CaiBotLiteMod/Services/Packet.cs:38-42` </location>
<code_context>
                 break;\n             case MessageID.SyncPlayer:\n-\n-                if (!Config.Settings.WhiteList || !player!.SscLogin || player.IsLoggedIn)\n+                if (!Config.Settings.WhiteList || !player!.SSCLogin || player.LoginQueue || player.IsLoggedIn)\n                 {\n                     return false;\n</code_context>

<issue_to_address>
**suggestion:** 在白名单逻辑中添加了 LoginQueue 检查;请验证预期流程。

请确认更新后的带有 LoginQueue 的条件不会无意中影响玩家访问,尤其是在并发登录场景中。

```suggestion
            case MessageID.SyncPlayer:
                // 仅当玩家在登录队列中且尚未登录时才进行阻塞。
                // 这可以防止重复登录尝试,但允许合法的并发登录。
                if (!Config.Settings.WhiteList || !player!.SSCLogin || (player.LoginQueue && !player.IsLoggedIn) || player.IsLoggedIn)
                {
                    return false;
                }
```
</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.
Original comment in English

Hey there - I've reviewed your changes - here's some feedback:

  • Ensure the LoginQueue flag is reset on both whitelist success and failure paths so players aren’t left permanently blocked.
  • Consider renaming SSCLogin and LoginQueue to more descriptive names (e.g. HasPerformedSscLogin, IsLoginPending) so their roles in the login sequence are clearer.
  • Double‐check that removing virtual modifiers and the TSRestPlayer subclass doesn’t break any plugin overrides that expect to hook into SendMessage or related methods.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Ensure the LoginQueue flag is reset on both whitelist success and failure paths so players aren’t left permanently blocked.
- Consider renaming SSCLogin and LoginQueue to more descriptive names (e.g. HasPerformedSscLogin, IsLoginPending) so their roles in the login sequence are clearer.
- Double‐check that removing virtual modifiers and the TSRestPlayer subclass doesn’t break any plugin overrides that expect to hook into SendMessage or related methods.

## Individual Comments

### Comment 1
<location> `CaiBotLiteMod/Moudles/TSPlayer.cs:258-83` </location>
<code_context>
     ///     Initializes a new instance of the <see cref="TSPlayer" /> class.
     /// </summary>
     /// <param name="playerName">The player's name.</param>
-    protected TSPlayer(string playerName)
+    private TSPlayer(string playerName)
     {
         this.Index = -1;
</code_context>

<issue_to_address>
**issue (bug_risk):** Changing TSPlayer constructor to private may restrict subclassing.

This change may break existing subclasses like TSRestPlayer that rely on protected access to the constructor.
</issue_to_address>

### Comment 2
<location> `CaiBotLiteMod/Services/Packet.cs:38-42` </location>
<code_context>
                 break;
             case MessageID.SyncPlayer:
-
-                if (!Config.Settings.WhiteList || !player!.SscLogin || player.IsLoggedIn)
+                if (!Config.Settings.WhiteList || !player!.SSCLogin || player.LoginQueue || player.IsLoggedIn)
                 {
                     return false;
</code_context>

<issue_to_address>
**suggestion:** Added LoginQueue check to whitelist logic; verify intended flow.

Please confirm that the updated condition with LoginQueue does not unintentionally affect player access, particularly in concurrent login scenarios.

```suggestion
            case MessageID.SyncPlayer:
                // Only block if player is in the login queue and not yet logged in.
                // This prevents duplicate login attempts but allows legitimate concurrent logins.
                if (!Config.Settings.WhiteList || !player!.SSCLogin || (player.LoginQueue && !player.IsLoggedIn) || player.IsLoggedIn)
                {
                    return false;
                }
```
</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.

@ACaiCat ACaiCat merged commit bf67a4b into master Nov 2, 2025
2 checks passed
@ACaiCat ACaiCat deleted the fix/duplicate-login-request branch November 2, 2025 14:55
@ACaiCat ACaiCat self-assigned this Nov 2, 2025
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.

2 participants