Conversation
Reviewer's Guide在删除隔离实例时新增可配置的个人文件备份流程,包括新的备份模式设置、UI 提示,以及用于截图和示意图的递归复制工具;当备份失败时会中止删除操作。 在删除隔离实例期间进行个人文件备份的时序图sequenceDiagram
actor User
participant PageSelectRight
participant ModPersonalFiles
participant ModMain
participant FileSystem
User->>PageSelectRight: DeleteVersion(mcInstance)
PageSelectRight->>ModPersonalFiles: GetDeleteHint()
PageSelectRight->>ModMain: MyMsgBox(confirmFullMsg)
ModMain-->>PageSelectRight: confirmResult = 1
PageSelectRight->>ModPersonalFiles: TryHandleBeforeDelete(mcInstance)
alt PersonalFilesBackupMode.Disabled
ModPersonalFiles-->>PageSelectRight: return true
else PersonalFilesBackupMode.AskEveryTime
ModPersonalFiles->>ModPersonalFiles: _HasScreenshots(mcInstance.PathIndie)
alt [no screenshots]
ModPersonalFiles-->>PageSelectRight: return true
else [screenshots exist]
ModPersonalFiles->>ModMain: MyMsgBox(Ask.Message, Ask.Title)
ModMain-->>ModPersonalFiles: promptResult
alt promptResult == 2 (DeleteWithoutBackup)
ModPersonalFiles-->>PageSelectRight: return true
else promptResult != 1 (Cancel)
ModPersonalFiles-->>PageSelectRight: return false
else promptResult == 1 (BackupAndDelete)
ModPersonalFiles->>ModPersonalFiles: _Backup(mcInstance)
ModPersonalFiles->>FileSystem: Copy screenshots & schematics
ModPersonalFiles->>HintService: Hint(Backup.Success)
ModPersonalFiles-->>PageSelectRight: return true
end
end
else PersonalFilesBackupMode.Always
ModPersonalFiles->>ModPersonalFiles: _Backup(mcInstance)
ModPersonalFiles->>FileSystem: Copy screenshots & schematics
ModPersonalFiles->>HintService: Hint(Backup.Success)
ModPersonalFiles-->>PageSelectRight: return true
end
PageSelectRight->>ModBase: IniClearCache(options.txt)
PageSelectRight->>ConfigService: InvalidateCache(mcInstance.PathInstance)
PageSelectRight->>FileSystem: Delete instance files
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 以:
Getting HelpOriginal review guide in EnglishReviewer's GuideAdds a configurable personal-files backup workflow when deleting isolated instances, including a new backup mode setting, UI hints, and a recursive copy utility for screenshots and schematics that aborts deletion on backup failure. Sequence diagram for personal files backup during isolated instance deletionsequenceDiagram
actor User
participant PageSelectRight
participant ModPersonalFiles
participant ModMain
participant FileSystem
User->>PageSelectRight: DeleteVersion(mcInstance)
PageSelectRight->>ModPersonalFiles: GetDeleteHint()
PageSelectRight->>ModMain: MyMsgBox(confirmFullMsg)
ModMain-->>PageSelectRight: confirmResult = 1
PageSelectRight->>ModPersonalFiles: TryHandleBeforeDelete(mcInstance)
alt PersonalFilesBackupMode.Disabled
ModPersonalFiles-->>PageSelectRight: return true
else PersonalFilesBackupMode.AskEveryTime
ModPersonalFiles->>ModPersonalFiles: _HasScreenshots(mcInstance.PathIndie)
alt [no screenshots]
ModPersonalFiles-->>PageSelectRight: return true
else [screenshots exist]
ModPersonalFiles->>ModMain: MyMsgBox(Ask.Message, Ask.Title)
ModMain-->>ModPersonalFiles: promptResult
alt promptResult == 2 (DeleteWithoutBackup)
ModPersonalFiles-->>PageSelectRight: return true
else promptResult != 1 (Cancel)
ModPersonalFiles-->>PageSelectRight: return false
else promptResult == 1 (BackupAndDelete)
ModPersonalFiles->>ModPersonalFiles: _Backup(mcInstance)
ModPersonalFiles->>FileSystem: Copy screenshots & schematics
ModPersonalFiles->>HintService: Hint(Backup.Success)
ModPersonalFiles-->>PageSelectRight: return true
end
end
else PersonalFilesBackupMode.Always
ModPersonalFiles->>ModPersonalFiles: _Backup(mcInstance)
ModPersonalFiles->>FileSystem: Copy screenshots & schematics
ModPersonalFiles->>HintService: Hint(Backup.Success)
ModPersonalFiles-->>PageSelectRight: return true
end
PageSelectRight->>ModBase: IniClearCache(options.txt)
PageSelectRight->>ConfigService: InvalidateCache(mcInstance.PathInstance)
PageSelectRight->>FileSystem: Delete instance files
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
关于备份截图的想法,得和 @Pigeon0v0 探讨下,删除实例损失了游玩的回忆会很可惜,但仍不确定这个功能,要以何种方式实现,目前的想法是全局设置打开后,删除游戏时会先备份截图。投影原理图是顺带的。 |
|
可以加一个每次均询问的选项,如果截图文件夹里有东西就弹个窗问一下 |
There was a problem hiding this comment.
Hey - 我在这里给出了一些总体反馈:
- 请考虑在
ModPersonalFiles._ArchiveRoot中针对ModFolder.mcFolderSelected为空或未设置的情况进行保护,这样在未选择游戏根目录时触发的删除流程就不会抛出异常或生成无效的备份路径。 - 在
_CopyDirectory中,确保在调用ModBase.CopyFile之前先创建目标子目录(例如通过Directory.CreateDirectory(Path.GetDirectoryName(targetFile))),这样无论CopyFile的具体行为如何,嵌套的screenshots/schematics文件都能被可靠地复制。
供 AI 代理使用的提示
Please address the comments from this code review:
## Overall Comments
- Consider guarding `ModPersonalFiles._ArchiveRoot` against a null or unset `ModFolder.mcFolderSelected` so that delete flows invoked when no game root is selected don’t throw or produce an invalid backup path.
- In `_CopyDirectory`, ensure the target subdirectories are created (e.g. via `Directory.CreateDirectory(Path.GetDirectoryName(targetFile))`) before calling `ModBase.CopyFile` so nested `screenshots/schematics` files are copied reliably regardless of `CopyFile`'s behavior.帮我变得更有用!请对每条评论点击 👍 或 👎,我会根据这些反馈改进为你提供的代码审查。
Original comment in English
Hey - I've left some high level feedback:
- Consider guarding
ModPersonalFiles._ArchiveRootagainst a null or unsetModFolder.mcFolderSelectedso that delete flows invoked when no game root is selected don’t throw or produce an invalid backup path. - In
_CopyDirectory, ensure the target subdirectories are created (e.g. viaDirectory.CreateDirectory(Path.GetDirectoryName(targetFile))) before callingModBase.CopyFileso nestedscreenshots/schematicsfiles are copied reliably regardless ofCopyFile's behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider guarding `ModPersonalFiles._ArchiveRoot` against a null or unset `ModFolder.mcFolderSelected` so that delete flows invoked when no game root is selected don’t throw or produce an invalid backup path.
- In `_CopyDirectory`, ensure the target subdirectories are created (e.g. via `Directory.CreateDirectory(Path.GetDirectoryName(targetFile))`) before calling `ModBase.CopyFile` so nested `screenshots/schematics` files are copied reliably regardless of `CopyFile`'s behavior.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec82020b51
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| { | ||
| case 1: | ||
| { | ||
| if (isHintIndie && !ModPersonalFiles.TryHandleBeforeDelete(mcInstance)) return; |
There was a problem hiding this comment.
Back up isolated instances even when validation fails
When an isolated instance has a corrupt or unreadable version definition, its state is McInstanceState.Error, which forces isHintIndie to false and skips this new backup handler even if PathIndie is the instance directory. The delete operation still removes PathInstance, so screenshots and schematics are silently lost under both Ask and Always modes; determine isolation from the configured/path relationship or probe the instance directory independently of validation state.
Useful? React with 👍 / 👎.
This comment was marked as spam.
This comment was marked as spam.
|
Request |
whitecat346
left a comment
There was a problem hiding this comment.
看了下,整体思路我觉得没问题,配置项、枚举取值和下拉映射、默认值、12 个本地化键我都对着代码核过了,没漏 key,两个架构的 build 也都是绿的。另外那 5 个语言文件是骨架,不用跟着加,这个没问题。
有几处想请你改一下,第一个比较要紧:
-
ModPersonalFiles.cs:83 那个
if (!Directory.Exists(sourceFolder)) return 0;感觉会出事。Directory.Exists 在没权限访问目录的时候是返回 false 而不是抛异常的,所以「始终备份」下如果 screenshots 或 schematics 读不到(权限、被安全软件占着、目录本身坏了之类),这里会当成「没有文件」返回 0,函数正常返回 true,实例照样被删,一份都没备份。这跟你 PR 里写的「备份失败时取消删除」正好是反的。同一个文件里 IgnoreInaccessible = false 又会让读不到的子目录抛异常、把删除拦住,等于同一件事两种结果,建议统一一下:把「目录不存在」和「目录存在但读不了」分开处理,后者得算失败。还有就是「始终备份」这一档下备份失败只会弹个错,用户在框里没有「我知道,还是删」这个选项,只能先去设置里改成「关闭」或者「每次询问」才能把实例删掉,感觉不太顺,可以给个出口。
-
codex 之前提的那个我也去确认了下,确实存在:state 是 Error 的时候 isHintIndie 直接就被判成 false 了,备份整个跳过,但下面照样删 PathInstance,隔离实例的截图和原理图就这么没了。isHintIndie 原本只是控制提示文案的,现在拿它来决定要不要备份,我不太放心。隔离与否其实看 PathIndie 和 PathInstance 的关系就够了,没必要跟校验状态绑在一起。
-
「每次询问」只判断 screenshots(:49),只有原理图、没有截图的实例会直接静默删掉,跟这个功能想做事的初衷有点冲突。要不要改成 screenshots 或 schematics 任一有文件就问?
-
递归复制是在按钮点击里同步跑的(:85-92,调用点在 PageInstanceOverall.xaml.cs:769 和 PageSelectRight.xaml.cs:549),截图攒多了会把界面卡住,也没有任何进度。仓库里这类操作一般都会扔到 RunInNewThread 或者 LoaderTask,比如 PageSelectLeft.xaml.cs:618。
-
_CopyDirectory 和 ModBase.CopyDirectory(ModBase.cs:1499)基本是一回事,重复写了一份。你这个版本多出来的只有计数、时间戳和 ReparsePoint 跳过这三样,如果确实需要,麻烦在注释里写一句为什么不复用,不然过段时间没人记得原因。
-
顺手说下 diff 里的空白改动:zh-CN.xaml 和 en-US.xaml 末尾补了换行,还有 PageSelectRight.xaml.cs:541 那行续行的缩进多了一格(它本来是对齐上一行的 confirmMsg,现在错开一格)。跟这次功能没关系,按技术规范要么单独提一个 style 提交,要么在描述里说明下。
-
流程上还有个事:这次一共动了 9 个 .cs/.xaml,已经过了技术规范里「大型 AI PR」那条线(超过 5 个文件),规则要求拆小提交并保证每一处都人工理解过。这个我不好替你判断,建议在 PR 描述里补一句说明。
剩下一些小地方,改不改都行,一起说了:两个删除入口各拼了一遍提示文案(PageInstanceOverall.xaml.cs:754-756 和 PageSelectRight.xaml.cs:542-543),变量名还不统一,PageSelectRight.xaml.cs:528 那条注释本来就是说这俩得一起改,要不要干脆让 ModPersonalFiles 把「提示后缀 + 是否放行」一起返回;GetDeleteHint 这名字看不出它会按配置返回两种提示;两处 switch 里 Always 都是用 _ 兜底,以后枚举加值会静默掉进 Always 分支;:19 那句中文异常消息没走 Lang.Text,是这次新增里唯一没本地化的用户可见文案,而且「回忆文件」和配置里的 PersonalFiles/个人文件两套叫法混着用;ToolTip 里把 PCL\PersonalFiles 写死了,路径本身定义在 :10-11,改一处容易漏。
最后提一个可能是有意为之的:删整个游戏文件夹那个入口(PageSelectLeft.xaml.cs:623)会把 PCL/PersonalFiles 一起删掉。备份目录放在游戏文件夹里,这个入口就没保护了,想确认下是不是可以接受。
改完 1、2 我觉得就能合了,后面那些有空再顺手弄也行。
修改内容
screenshots中存在文件时弹窗,可选择备份并删除、直接删除或取消screenshots与schematics递归复制到所选游戏目录下的PCL/PersonalFiles/<实例名>验证情况
dotnet build "Plain Craft Launcher 2/Plain Craft Launcher 2.csproj" -c Debug -p:Platform=x64 --no-restore:通过,0 个错误CI配置dotnet publish --no-self-contained:通过;生成的 .NET 10 可执行文件启动冒烟通过git diff --check dev...feat/protect-instance-personal-files:通过AI 使用
Summary by Sourcery
在删除隔离的 Minecraft 实例时,新增可配置的个人文件保护机制,包括可选地将截图和结构文件备份到专用归档文件夹。
New Features:
Enhancements:
Original summary in English
Summary by Sourcery
Add configurable protection for personal files when deleting isolated Minecraft instances, including optional backup of screenshots and schematics to a dedicated archive folder.
New Features:
Enhancements: