Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for allowing duplicate filenames when exporting game logs to a ZIP file. When duplicate entries are encountered, the system will automatically append a numeric suffix (e.g., .1, .2) to avoid conflicts, with a maximum of 10 attempts before throwing an exception.
Key changes:
- Modified
Zipperclass to optionally track entry names and handle duplicates by appending numeric suffixes - Updated
LogExporterto enable duplicate entry handling when creating log export archives
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
HMCLCore/src/main/java/org/jackhuang/hmcl/util/io/Zipper.java |
Added duplicate entry handling mechanism with allowDuplicateEntry flag and newEntry() method to generate unique names |
HMCL/src/main/java/org/jackhuang/hmcl/game/LogExporter.java |
Enabled duplicate entry support when creating the zipper for log exports |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for (long i = 0; ; i++) { | ||
| String newName = name + "." + i; | ||
| if (entryNames.add(newName)) { | ||
| return newName; | ||
| } | ||
| } |
There was a problem hiding this comment.
This infinite loop could potentially run forever if there's an issue with the Set implementation or if Long.MAX_VALUE duplicates are encountered. While extremely unlikely in practice, consider adding a reasonable upper bound (e.g., 1000 or 10000) and throwing an exception if that limit is reached, similar to what's done in Zipper.newEntry(). This would make the behavior more predictable and prevent potential infinite loops.
| for (long i = 0; ; i++) { | |
| String newName = name + "." + i; | |
| if (entryNames.add(newName)) { | |
| return newName; | |
| } | |
| } | |
| for (int i = 0; i < 10_000; i++) { | |
| String newName = name + "." + i; | |
| if (entryNames.add(newName)) { | |
| return newName; | |
| } | |
| } | |
| throw new IllegalStateException("Failed to generate unique entry name for \"" + name + "\" after 10000 attempts"); |
| for (long i = 0; ; i++) { | ||
| String newName = name + "." + i; | ||
| if (entryNames.add(newName)) { | ||
| return newName; | ||
| } |
There was a problem hiding this comment.
The duplicate naming strategy is inconsistent with Zipper.newEntry(), which uses a 1-based counter (e.g., "file.1", "file.2"). This implementation uses a 0-based counter (e.g., "file.0", "file.1"). For consistency and better user experience across the application, both implementations should use the same numbering scheme. Consider starting from 1 instead of 0 to match the Zipper implementation, or align both to start from 0.
There was a problem hiding this comment.
应当直接迁移导出日志的逻辑到 Zipper 以共享逻辑。
There was a problem hiding this comment.
这里的逻辑更稳定，能够区分读取文件时发生的异常和写入文件时发生的异常，并在读取文件异常时跳过该文件处理其他文件。
这种稳定性对于导出启动器日志有意义，因为我们需要启动器日志才能 DEBUG，所以我们希望避免在导出启动器日志时发生任何意外，但是对于 Zipper 来说并没有这种需求，所以我不认为它们应该共享代码。
2 participants
No description provided.