refactor(mod-base): 重构 ModBase.cs#3294
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
# Conflicts: # Plain Craft Launcher 2/Modules/Minecraft/ModLocalComp.cs # Plain Craft Launcher 2/Modules/Network/Downloader/FileDownloader.cs
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
pcl-ce-automation
Bot
added
🛠️ 等待审查
![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
PCL-CE/PCL.Core/IO/Directories.cs
Line 230 in 2d7a963
Keep copy progress callbacks incremental
For modpacks with an override folder containing multiple files, CopyOverrideDirectory still adds the callback value to loader.Progress, matching the old CopyDirectory contract of reporting 1 / fileCount increments. This new implementation reports the cumulative fraction (copiedFiles / totalFiles), so the caller adds 1/n + 2/n + ... + 1 and the progress jumps far past the intended range during override copying.
Lines 123 to 127 in 2d7a963
Parse WPF hex colors as ARGB
When a caller passes an 8-digit WPF color string that the previous ColorConverter path accepted, such as #80FF0000, this now treats it as #RRGGBBAA instead of WPF's #AARRGGBB. Semi-transparent colors are therefore decoded with the alpha byte in the blue channel and often become fully transparent or the wrong color when converted through NColor.
ℹ️ 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".
| // 解压文件，ProgressIncrementHandler 通过 Lambda 更新进度 | ||
| ModBase.ExtractFile(fileAddress, installTemp, encode, | ||
| delta => loader.Progress += delta * progressIncrement); | ||
| Files.ExtractFileAsync(fileAddress, installTemp, delta => loader.Progress += delta * progressIncrement).GetAwaiter().GetResult(); |
There was a problem hiding this comment.
When importing a ZIP modpack that contains GB18030/Chinese entry names, this replacement drops the encode value that the old ModBase.ExtractFile(..., encode, ...) passed into ZipFile.Open; Files.ExtractFileAsync opens ZIPs without an encoding override, so non-Unicode entry names are decoded with the library default and extract to garbled paths or fail. This affects local compressed-pack installs with localized filenames.
Useful? React with 👍 / 👎.
| (JsonObject)JsonCompat.ParseNode( | ||
| Files.ReadAllTextOrEmptyAsync(LauncherFileSystem.ResolvePath(ModFolder.mcFolderSelected + "launcher_profiles.json")).GetAwaiter().GetResult()); |
There was a problem hiding this comment.
This reads launcher_profiles.json with Files.ReadAllTextOrEmptyAsync's default UTF-8 path, but the same code writes the file as GB18030 immediately below and the old ModBase.ReadFile auto-detected GB18030. If an existing launcher profile contains non-UTF8 bytes, such as Chinese profile names, launch-time profile updates can corrupt the JSON text or fall into the delete-and-recreate path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 400e7fe3df
ℹ️ 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".
| var queryIndex = path.IndexOfAny(['?', '#']); | ||
| if (queryIndex >= 0) path = path[..queryIndex]; |
There was a problem hiding this comment.
When users install or export a local file whose Windows filename contains a legal # (for example mods\foo#1.jar), this branch treats it like a URL fragment and truncates the name before the filename is returned. Callers use this helper to build destination filenames for local mods/resource packs, so these files can be copied as foo instead of foo#1.jar and lose their extension; only strip # after the input has been identified as a URL.
Useful? React with 👍 / 👎.
# Conflicts: # Plain Craft Launcher 2/Application.xaml.cs # Plain Craft Launcher 2/Modules/Minecraft/ModComp.cs # Plain Craft Launcher 2/Modules/Network/Loaders/LoaderDownload.cs # Plain Craft Launcher 2/Modules/Network/Models/DownloadFile.cs # Plain Craft Launcher 2/Pages/PageLaunch/PageLoginProfile.xaml.cs # Plain Craft Launcher 2/Pages/PageSetup/PageSetupUpdate.xaml.cs
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 788cd35247
ℹ️ 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".
| // 解压文件，ProgressIncrementHandler 通过 Lambda 更新进度 | ||
| ModBase.ExtractFile(fileAddress, installTemp, encode, | ||
| delta => loader.Progress += delta * progressIncrement); | ||
| Files.ExtractFileAsync(fileAddress, installTemp, delta => loader.Progress += delta * progressIncrement).GetAwaiter().GetResult(); |
There was a problem hiding this comment.
When fileAddress is a Modrinth .mrpack (recognized earlier and passed into InstallPackModrinth), this now calls Files.ExtractFileAsync, whose extension dispatch only treats .zip/.jar as zip archives and throws NotSupportedException for .mrpack. The previous ModBase.ExtractFile treated every non-gz archive as a zip, so local Modrinth modpacks can no longer be installed unless .mrpack is explicitly routed through the zip extraction path or normalized before this call.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ded03161e7
ℹ️ 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".
| Files.ExtractFileAsync( | ||
| fileAddress, | ||
| installTemp, | ||
| delta => loader.Progress += delta * progressIncrement, | ||
| encode) |
There was a problem hiding this comment.
When the selected local pack has an uppercase extension such as PACK.ZIP, foo.JAR, or pack.MRPACK, this new path now delegates to Files.ExtractFileAsync, whose extension dispatch uses case-sensitive EndsWithF checks and falls through to NotSupportedException. The previous ModBase.ExtractFile path accepted these on Windows by treating non-.gz archives as zip after a case-insensitive .gz check, so valid local modpacks can no longer be installed unless the extension checks are normalized or made case-insensitive.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Line 547 in 7b46b90
Reopen tar streams before extracting entries
When a .tar or .tar.gz archive reaches this path, the first loop has already advanced the TarInputStream to EOF while collecting entries; Reset() does not rewind the archive stream, so the subsequent CopyEntryContentsAsync calls run with no current entry data and tar extraction produces empty/failing output. This affects the newly advertised tar/tar.gz extraction path, including .tar.gz files accepted by crash-log import through the .gz suffix; process entries in a single pass or reopen/seek the underlying stream before copying contents.
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bbb06ba481
ℹ️ 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".
| @@ -101,7 +101,7 @@ private void RefreshReal() | |||
| { | |||
| // 本地文件 | |||
| LogWrapper.Info("[Page] 主页自定义数据来源：本地文件"); | |||
| content = ModBase.ReadFile(Path.Combine(ModBase.exePath, "PCL", "Custom.xaml")); | |||
| content = Files.ReadAllTextOrEmptyAsync(LauncherFileSystem.ResolvePath(Path.Combine(LauncherPaths.ExecutableDirectoryWithSlash, "PCL", "Custom.xaml"))).GetAwaiter().GetResult(); | |||
There was a problem hiding this comment.
When users enable a local custom homepage and PCL\Custom.xaml was saved with GB18030/ANSI bytes (common for Chinese text in manually edited XAML), this now reads it with ReadAllTextOrEmptyAsync's default UTF-8 path instead of the old ModBase.ReadFile byte decoder with GB18030 fallback. In that scenario the homepage content is corrupted or read as empty and the custom homepage fails to load; pass the legacy encoding/detection path here as was done for other GB18030 files.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bbb06ba481
ℹ️ 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".
| { | ||
| CollectionAssert.AreEqual(Expected, "a12b34".RegexSearch(@"\\d+")); | ||
| Assert.AreEqual("12", "a12b34".RegexSeek(@"\\d+")); | ||
| Assert.IsTrue("a12".RegexCheck(@"\\d+")); |
There was a problem hiding this comment.
These new regex tests double-escape \d inside a verbatim string, so the pattern searches for a literal \d instead of digits. With inputs like "a12b34", RegexSearch, RegexSeek, and RegexCheck therefore return no matches and the added test fails before validating the extension behavior; use @"\d+" here and in the replacement test below.
Useful? React with 👍 / 👎.
1 participant
close: #3288