fix: Download manager button breaks when one of multiple tasks completes - #3573
Open
sensen0025 wants to merge 1 commit into
Open
fix: Download manager button breaks when one of multiple tasks completes#3573sensen0025 wants to merge 1 commit into
sensen0025 wants to merge 1 commit into
Conversation
… of multiple tasks completes
pcl-ce-automation
Bot
added
🛠️ 等待审查
Contributor
审查者指南（小型 PR 中折叠显示）审查者指南更新任务栏进度刷新逻辑，使每个非加载中的任务在完成或退出后立即被移除，从而避免速度面板 UI 在监视器更新期间反复创建和销毁下载管理器卡片。 进度刷新期间任务栏清理的时序图sequenceDiagram
participant Watcher
participant ModLoader
participant PageSpeedLeft
participant Taskbar
loop Every 300ms watcher tick
Watcher->>ModLoader: LoaderTaskbarProgressRefresh()
ModLoader->>Taskbar: Inspect task State
alt Task.State != Loading
ModLoader->>PageSpeedLeft: TaskRefresh(Task)
ModLoader->>Taskbar: Remove(Task)
end
end
文件级变更
针对关联 Issue 的评估
可能关联的 Issue
提示和命令与 Sourcery 交互
自定义使用体验访问你的仪表板以：
获取帮助Original review guide in EnglishReviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates taskbar progress refresh to remove each non-loading task as soon as it completes or exits, preventing the speed-panel UI from repeatedly recreating and disposing the download-manager card during watcher updates. Sequence diagram for taskbar cleanup during progress refreshsequenceDiagram
participant Watcher
participant ModLoader
participant PageSpeedLeft
participant Taskbar
loop Every 300ms watcher tick
Watcher->>ModLoader: LoaderTaskbarProgressRefresh()
ModLoader->>Taskbar: Inspect task State
alt Task.State != Loading
ModLoader->>PageSpeedLeft: TaskRefresh(Task)
ModLoader->>Taskbar: Remove(Task)
end
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
您好——我发现了 2 个问题
AI Agent 提示词
请处理本次代码审查中的评论：
## 单独评论
### 评论 1
<location path="Plain Craft Launcher 2/Modules/Base/ModLoader.cs" line_range="44" />
<code_context>
var newProgress = LoaderTaskbarProgressGet();
// 若单个任务已中止，或全部任务已完成，则刷新并移除
foreach (var Task in loaderTaskbar)
- if (loaderTaskbar.All(l => l.State != ModBase.LoadState.Loading) ||
</code_context>
<issue_to_address>
**nitpick：** 该注释表示只有在某个任务中止或所有任务都完成时才会移除任务，但新条件会移除任何不是 `Loading` 状态的单个任务，包括其他任务仍处于活动状态时的 `Waiting`、`Failed` 和 `Finished` 任务。该注释现在错误地描述了方法的行为。
**建议修复：** 更新注释，说明每个非 `Loading` 状态的任务都会被刷新并移除。
</issue_to_address>
### 评论 2
<location path="Plain Craft Launcher 2/Modules/Base/ModLoader.cs" line_range="43-49" />
<code_context>
var newProgress = LoaderTaskbarProgressGet();
// 若单个任务已中止，或全部任务已完成，则刷新并移除
foreach (var Task in loaderTaskbar)
- if (loaderTaskbar.All(l => l.State != ModBase.LoadState.Loading) ||
- Task.State == ModBase.LoadState.Waiting || Task.State == ModBase.LoadState.Aborted)
+ if (Task.State != ModBase.LoadState.Loading)
{
ModMain.frmSpeedLeft?.TaskRefresh(Task);
</code_context>
<issue_to_address>
**issue (bug_risk)：** `newProgress` 在已完成任务被移除之前根据 `loaderTaskbar` 计算，因此进度更新会包含那些已经不再被跟踪的终止状态任务。当一个已完成任务的进度为 1，而剩余任务的进度较低时，任务栏进度会在本次刷新时被高估，并且平滑状态会根据错误的任务集合进行更新。
**触发条件：** 当一个任务完成时，其他任务栏任务仍在加载且它们的进度不同时。
**建议修复：** 在计算 `LoaderTaskbarProgressGet()` 之前移除非加载状态的任务，或者在移除循环之后重新计算 `newProgress`。
```suggestion
// 若单个任务已中止，或全部任务已完成，则刷新并移除
foreach (var Task in loaderTaskbar)
if (Task.State != ModBase.LoadState.Loading)
{
ModMain.frmSpeedLeft?.TaskRefresh(Task);
loaderTaskbar.Remove(Task);
ModBase.Log($"[Taskbar] {Task.name} 已移出任务列表");
}
var newProgress = LoaderTaskbarProgressGet();
```
</issue_to_address>帮助我变得更有用！请在每条评论上点击 👍 或 👎，我会利用反馈来改进审查结果。
Original comment in English
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="Plain Craft Launcher 2/Modules/Base/ModLoader.cs" line_range="44" />
<code_context>
var newProgress = LoaderTaskbarProgressGet();
// 若单个任务已中止，或全部任务已完成，则刷新并移除
foreach (var Task in loaderTaskbar)
- if (loaderTaskbar.All(l => l.State != ModBase.LoadState.Loading) ||
</code_context>
<issue_to_address>
**nitpick:** The comment says tasks are removed only when one task is aborted or all tasks are complete, but the new condition removes any individual task that is not `Loading`, including `Waiting`, `Failed`, and `Finished` tasks while other tasks remain active. The comment now gives an incorrect description of the method's behavior.
**Suggested fix:** Update the comment to state that every non-`Loading` task is refreshed and removed.
</issue_to_address>
### Comment 2
<location path="Plain Craft Launcher 2/Modules/Base/ModLoader.cs" line_range="43-49" />
<code_context>
var newProgress = LoaderTaskbarProgressGet();
// 若单个任务已中止，或全部任务已完成，则刷新并移除
foreach (var Task in loaderTaskbar)
- if (loaderTaskbar.All(l => l.State != ModBase.LoadState.Loading) ||
- Task.State == ModBase.LoadState.Waiting || Task.State == ModBase.LoadState.Aborted)
+ if (Task.State != ModBase.LoadState.Loading)
{
ModMain.frmSpeedLeft?.TaskRefresh(Task);
</code_context>
<issue_to_address>
**issue (bug_risk):** `newProgress` is calculated from `loaderTaskbar` before completed tasks are removed, so the progress update includes terminal tasks that are no longer tracked. When a completed task has progress 1 and a remaining task has lower progress, the taskbar progress is overstated for the refresh tick and the smoothing state is updated from the wrong task set.
**Triggers:** When one task completes while other taskbar tasks are still loading and their progress differs.
**Suggested fix:** Remove non-loading tasks before calculating `LoaderTaskbarProgressGet()`, or recalculate `newProgress` after the removal loop.
```suggestion
// 若单个任务已中止，或全部任务已完成，则刷新并移除
foreach (var Task in loaderTaskbar)
if (Task.State != ModBase.LoadState.Loading)
{
ModMain.frmSpeedLeft?.TaskRefresh(Task);
loaderTaskbar.Remove(Task);
ModBase.Log($"[Taskbar] {Task.name} 已移出任务列表");
}
var newProgress = LoaderTaskbarProgressGet();
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -43,8 +43,7 @@ public static void LoaderTaskbarProgressRefresh() | |||
| var newProgress = LoaderTaskbarProgressGet(); | |||
| // 若单个任务已中止，或全部任务已完成，则刷新并移除 | |||
Contributor
There was a problem hiding this comment.
nitpick： 该注释表示只有在某个任务中止或所有任务都完成时才会移除任务，但新条件会移除任何不是 Loading 状态的单个任务，包括其他任务仍处于活动状态时的 Waiting、Failed 和 Finished 任务。该注释现在错误地描述了方法的行为。
建议修复： 更新注释，说明每个非 Loading 状态的任务都会被刷新并移除。
Original comment in English
nitpick: The comment says tasks are removed only when one task is aborted or all tasks are complete, but the new condition removes any individual task that is not Loading, including Waiting, Failed, and Finished tasks while other tasks remain active. The comment now gives an incorrect description of the method's behavior.
Suggested fix: Update the comment to state that every non-Loading task is refreshed and removed.
Comment on lines
43
to
49
| var newProgress = LoaderTaskbarProgressGet(); | ||
| // 若单个任务已中止，或全部任务已完成，则刷新并移除 | ||
| foreach (var Task in loaderTaskbar) | ||
| if (loaderTaskbar.All(l => l.State != ModBase.LoadState.Loading) || | ||
| Task.State == ModBase.LoadState.Waiting || Task.State == ModBase.LoadState.Aborted) | ||
| if (Task.State != ModBase.LoadState.Loading) | ||
| { | ||
| ModMain.frmSpeedLeft?.TaskRefresh(Task); | ||
| loaderTaskbar.Remove(Task); |
Contributor
There was a problem hiding this comment.
issue (bug_risk)： newProgress 在已完成任务被移除之前根据 loaderTaskbar 计算，因此进度更新会包含那些已经不再被跟踪的终止状态任务。当一个已完成任务的进度为 1，而剩余任务的进度较低时，任务栏进度会在本次刷新时被高估，并且平滑状态会根据错误的任务集合进行更新。
触发条件： 当一个任务完成时，其他任务栏任务仍在加载且它们的进度不同时。
建议修复： 在计算 LoaderTaskbarProgressGet() 之前移除非加载状态的任务，或者在移除循环之后重新计算 newProgress。
Suggested change
| var newProgress = LoaderTaskbarProgressGet(); | |
| // 若单个任务已中止，或全部任务已完成，则刷新并移除 | |
| foreach (var Task in loaderTaskbar) | |
| if (loaderTaskbar.All(l => l.State != ModBase.LoadState.Loading) || | |
| Task.State == ModBase.LoadState.Waiting || Task.State == ModBase.LoadState.Aborted) | |
| if (Task.State != ModBase.LoadState.Loading) | |
| { | |
| ModMain.frmSpeedLeft?.TaskRefresh(Task); | |
| loaderTaskbar.Remove(Task); | |
| // 若单个任务已中止，或全部任务已完成，则刷新并移除 | |
| foreach (var Task in loaderTaskbar) | |
| if (Task.State != ModBase.LoadState.Loading) | |
| { | |
| ModMain.frmSpeedLeft?.TaskRefresh(Task); | |
| loaderTaskbar.Remove(Task); | |
| ModBase.Log($"[Taskbar] {Task.name} 已移出任务列表"); | |
| } | |
| var newProgress = LoaderTaskbarProgressGet(); |
Original comment in English
issue (bug_risk): newProgress is calculated from loaderTaskbar before completed tasks are removed, so the progress update includes terminal tasks that are no longer tracked. When a completed task has progress 1 and a remaining task has lower progress, the taskbar progress is overstated for the refresh tick and the smoothing state is updated from the wrong task set.
Triggers: When one task completes while other taskbar tasks are still loading and their progress differs.
Suggested fix: Remove non-loading tasks before calculating LoaderTaskbarProgressGet(), or recalculate newProgress after the removal loop.
Suggested change
| var newProgress = LoaderTaskbarProgressGet(); | |
| // 若单个任务已中止，或全部任务已完成，则刷新并移除 | |
| foreach (var Task in loaderTaskbar) | |
| if (loaderTaskbar.All(l => l.State != ModBase.LoadState.Loading) || | |
| Task.State == ModBase.LoadState.Waiting || Task.State == ModBase.LoadState.Aborted) | |
| if (Task.State != ModBase.LoadState.Loading) | |
| { | |
| ModMain.frmSpeedLeft?.TaskRefresh(Task); | |
| loaderTaskbar.Remove(Task); | |
| // 若单个任务已中止，或全部任务已完成，则刷新并移除 | |
| foreach (var Task in loaderTaskbar) | |
| if (Task.State != ModBase.LoadState.Loading) | |
| { | |
| ModMain.frmSpeedLeft?.TaskRefresh(Task); | |
| loaderTaskbar.Remove(Task); | |
| ModBase.Log($"[Taskbar] {Task.name} 已移出任务列表"); | |
| } | |
| var newProgress = LoaderTaskbarProgressGet(); |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c169238ca
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| foreach (var Task in loaderTaskbar) | ||
| if (loaderTaskbar.All(l => l.State != ModBase.LoadState.Loading) || | ||
| Task.State == ModBase.LoadState.Waiting || Task.State == ModBase.LoadState.Aborted) | ||
| if (Task.State != ModBase.LoadState.Loading) |
There was a problem hiding this comment.
When one task fails while another remains loading and the task-manager page has not yet been opened, this condition removes the failed task immediately and frmSpeedLeft?.TaskRefresh(Task) is a no-op because that page is created lazily. Opening the manager through the still-visible download button then shows only the remaining task, so the failed task's error details cannot be inspected or copied. Previously, failed tasks remained in loaderTaskbar until no task was loading; restrict the new early-removal behavior to successfully finished tasks while retaining the existing handling for failures.
Useful? React with 👍 / 👎.
LuLu-ling
requested changes
Aug 28, 2026
LuLu-ling
left a comment
LuLu-ling
left a comment
Member
There was a problem hiding this comment.
请严格遵守 技术规范 重新修整你的 PR：
- PR 中的所有 Commit 必须要附带有效的、已验证的签名。参考 提交信息规范-提交签名要求 章节与 GPG 签名配置指南。
- 若原文件使用不同换行符或编码，不应为了统一格式而产生大量无关 diff，以免与其他分支中的修改产生冲突。
LuLu-ling
changed the title
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.
Fixes #3567.
Summary
When multiple download/update tasks are running and one finishes, the original condition \loaderTaskbar.All(l => l.State != ModBase.LoadState.Loading)\ prevented finished tasks from being removed from \loaderTaskbar.
This caused the taskbar manager UI (\PageSpeedLeft) to repeatedly re-create and dispose the UI card on every 300ms Watcher tick, resulting in UI glitches and a broken download manager button.
This fix removes tasks from \loaderTaskbar\ as soon as they are no longer \Loading.
Sourcery 摘要
错误修复：
Original summary in English
Summary by Sourcery
Bug Fixes: