Conversation
# Conflicts: # HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/DataPackListPage.java # HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/DataPackListPageSkin.java # HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/WorldManagePage.java # HMCL/src/main/resources/assets/lang/I18N.properties # HMCLCore/src/main/java/org/jackhuang/hmcl/game/World.java
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/WorldBackupsPage.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Controllers.dialog(new InputDialogPane(i18n("world.duplicate.prompt"), world.getWorldName(), (newWorldName, handler) -> { | ||
| if (StringUtils.isBlank(newWorldName)) { | ||
| newWorldName = i18n("world.name.default"); | ||
| } | ||
| String finalNewWorldName = newWorldName; |
There was a problem hiding this comment.
copyWorld reassigns the lambda parameter newWorldName (line 71). Lambda parameters are effectively final in Java, so this won’t compile. Use a separate local variable (e.g., String name = ...) before computing finalNewWorldName.
HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/WorldManageUIUtils.java
Outdated
Show resolved
Hide resolved
| List<Path> files = Files.list(fs.getPath("/")).toList(); | ||
| if (files.size() != 1 || !Files.isDirectory(files.get(0))) { | ||
| throw new IOException("Not a valid world zip file"); |
There was a problem hiding this comment.
Files.list(fs.getPath("/")) returns a Stream that should be closed; here it’s consumed via toList() without try-with-resources. This can leak directory handles inside the zip filesystem. Wrap the Files.list(...) call in a try-with-resources and collect within it.
HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ImportableWorld.java
Outdated
Show resolved
Hide resolved
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request overhauls the world management system by introducing a WorldLock mechanism for session control and an ImportableWorld class for handling world imports. It adds functionality for restoring backups, renaming world folders, and refactors the UI to better handle read-only states and loading failures. Feedback highlights a logic error in the restoration task that could cause failures after renaming, a potential resource leak in the world import logic, and a regression in the file lock state detection for overlapping locks.
HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/WorldRestoreTask.java
Outdated
Show resolved
Hide resolved
HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ImportableWorld.java
Outdated
Show resolved
Hide resolved
| } catch (OverlappingFileLockException | NoSuchFileException overlappingFileLockException) { | ||
| return false; |
There was a problem hiding this comment.
OverlappingFileLockException 表示当前 JVM 已持有该文件的锁（虽然不是由当前 World 实例持有），因此在这种情况下应该返回 true（已锁定），而不是 false。这在旧版代码中是正确的，此处似乎是重构引入的回归。
| } catch (OverlappingFileLockException | NoSuchFileException overlappingFileLockException) { | |
| return false; | |
| } catch (OverlappingFileLockException overlappingFileLockException) { | |
| return true; | |
| } catch (NoSuchFileException noSuchFileException) { | |
| return false; |
There was a problem hiding this comment.
我觉得这个类的设计就很奇怪，尤其是 ImportableWorld 这个名字我认为非常不恰当。
我认为你可以把它改名叫 WorldInfo 之类的名字，或者修改 World 使其不可变，然后用新的可变类来管理对世界的修改。
There was a problem hiding this comment.
这个不是表达世界信息的类的，而是表达“即将被导入的世界的”，Import：导入，Importable：可导入的，为什么会认为是扩展的意思呢？
这个类的作用就是表示安装世界或在世界备份页面时的静态世界文件的，而World类现在仅表示在世界文件夹，可被世界管理页面管理的类的。
# Conflicts: # HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/WorldManageUIUtils.java
继续 #5215 对世界管理界面进行的优化第三期
用户可感知的更新
复用页面：
Controllers.getWorldManagePage().setWorld(World world, Profile profile, String instanceId)来获取和设置页面世界锁机制：
World类本身持有锁，而不是在其它类中。World类本身都不知道当前的锁是不是被自己锁定的，导致执行各种操作时需要协调WorldManagePage各种先释放锁再执行最后加锁，尽管某些操作根本不需要释放锁，现在World类本身知道自己是否持有锁，因此可以不再需要操作锁或者可以到内聚到World类中，简化了状态管理。ImportableWorld类：ImportableWorld类，用来表示在外部，即将被安装的世界。可以是压缩包或文件夹。World类仅表示在世界文件夹，以文件夹格式存储的世界。世界备份页面：
重命名/复制世界：
level.dat中的名称，也能修改文件夹名称。其它：
WorldManagePage页面，而是变为只读模式。