generated from UnrealMultiple/CaiBotMod
-
Notifications
You must be signed in to change notification settings - Fork 0
fix(Login): 修复重复登录请求导致断联 #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
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
There was a problem hiding this 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 ✨
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>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
LoiginQueue字段以保证每个玩家只能发起一次登录请求IsLoginIn字段忘记被设置的问题Sourcery 总结
通过引入
LoginQueue和SSCLogin标志来强制执行单一登录请求和正确的登录状态跟踪;修复了白名单流程中IsLoggedIn标志的赋值问题;更新了数据包劫持逻辑以遵循这些标志;并通过移除未使用的成员和虚方法来精简TSPlayer。新功能:
TSPlayer添加LoginQueue和SSCLogin标志,以强制每个玩家只发送一次登录请求错误修复:
IsLoggedIn标志赋值缺失的问题LoginQueue和IsLoggedIn,防止重复的登录请求导致意外断开连接改进:
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:
Bug Fixes:
Enhancements: