Skip to content

迁移 ModpackHelper#findMinecraftDirectoryInManuallyCreatedModpack 到 kala-compress [Qwen3.7-Plus]#6465

Open
CiiLu wants to merge 2 commits into
HMCL-dev:mainfrom
CiiLu:k
Open

迁移 ModpackHelper#findMinecraftDirectoryInManuallyCreatedModpack 到 kala-compress [Qwen3.7-Plus]#6465
CiiLu wants to merge 2 commits into
HMCL-dev:mainfrom
CiiLu:k

Conversation

@CiiLu

@CiiLu CiiLu commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions github-actions Bot added the 40+ label Jul 21, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the modpack installation logic to use ArchiveFileTree instead of FileSystem when locating the Minecraft directory within manually created modpacks. The reviewer identified critical issues regarding character encoding and path separators: ignoring the charset parameter when opening zip files can lead to ZipException or incorrect directory parsing in non-UTF-8 environments (such as GBK on Windows). Additionally, converting the returned directory string to a Path object introduces platform-specific path separators (backslashes on Windows) which can break zip extraction utilities that expect forward slashes. The reviewer suggested updating the method signatures to accept and respect the charset parameter and keeping the subdirectory as a String.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread HMCL/src/main/java/org/jackhuang/hmcl/game/ManuallyCreatedModpackInstallTask.java Outdated
Comment thread HMCL/src/main/java/org/jackhuang/hmcl/game/ModpackHelper.java
Comment thread HMCL/src/main/java/org/jackhuang/hmcl/game/ModpackHelper.java
@Glavo

Glavo commented Jul 21, 2026

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5c0dbb96e6

ℹ️ 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".

public static String findMinecraftDirectoryInManuallyCreatedModpack(String modpackName, Path zipPath)
throws IOException, UnsupportedModpackException {

try (ArchiveFileTree<?, ?> tree = ArchiveFileTree.open(zipPath)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve the selected ZIP filename charset

For manually created ZIPs whose wrapper directory has non-UTF-8 entry names (for example, a GBK-named 整合包/.minecraft/versions/...), this opens the archive with ArchiveFileTree.open(zipPath), which constructs ZipArchiveReader without the charset selected by findSuitableEncoding. The detected prefix is therefore mojibake (or detection fails), while Unzipper subsequently decodes entries with the correct charset; no entries then match that prefix and the installer can create an empty game directory. Thread the charset through this helper and use it when creating the reader.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants