重构 WorldListPage 以使用 ListCell 展示元素 by Glavo · Pull Request #5224 · HMCL-dev/HMCL

Glavo · 2026-01-13T14:03:08Z

No description provided.

Copilot

Pull request overview

This PR refactors WorldListPage to use JavaFX's standard ListCell pattern for displaying world items, eliminating the need for custom WorldListItem and WorldListItemSkin classes.

Changes:

Replaced custom WorldListItem Control class with direct use of World domain objects in the list
Migrated UI rendering logic from WorldListItemSkin into a new WorldListCell inner class that properly implements the ListCell pattern
Moved world operation methods (export, delete, copy, reveal, launch, etc.) from WorldListItem to WorldListPage as they now operate directly on World objects

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
WorldListPage.java	Changed generic type from `WorldListItem` to `World`, added new `WorldListCell` inner class implementing `ListCell<World>`, moved world operation methods from `WorldListItem` to this class, and added `createListCell()` override in `WorldListPageSkin`
WorldListItem.java	Deleted - functionality moved to `WorldListPage` and `WorldListCell`
WorldListItemSkin.java	Deleted - UI rendering logic moved to `WorldListCell.updateItem()` method

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-13T14:30:07Z

+    }
+
+    public void delete(World world) {
+        WorldManageUIUtils.delete(world, () -> this.getItems().remove(world));


The delete method removes the world from the displayed items list but does not update the worlds field. This creates an inconsistency: if the user toggles the "show all" filter or triggers a refresh, the deleted world may reappear briefly until the refresh completes. Consider also removing the world from the worlds list or calling refresh() after deletion instead of just removing from the items.

Suggested change

WorldManageUIUtils.delete(world, () -> this.getItems().remove(world));

WorldManageUIUtils.delete(world, this::refresh);

Copilot · 2026-01-13T14:30:07Z

                        Task.runAsync(() -> world.install(savesDir, name))
                                .whenComplete(Schedulers.javafx(), () -> {
-                                    itemsProperty().add(new WorldListItem(this, new World(savesDir.resolve(name)), backupsDir, profile, id));
+                                    itemsProperty().add(new World(savesDir.resolve(name)));


When a world is successfully installed, it's added directly to the items list but not to the worlds field. This creates inconsistency similar to the delete method: toggling filters or other operations that rely on the worlds field won't include the newly added world until a full refresh occurs. Consider calling refresh() after successful installation or updating both the worlds field and the items list.

Suggested change

itemsProperty().add(new World(savesDir.resolve(name)));

itemsProperty().add(new World(savesDir.resolve(name)));

// Ensure backing 'worlds' list and filters are updated as well

refresh();

Copilot

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.

Copilot · 2026-01-14T12:28:31Z

                                    resolve.run();
+                                    refresh();


The refresh() is called after resolve.run() instead of before it, which could lead to incorrect UI state. The resolve.run() callback likely dismisses the prompt dialog, and calling refresh() after it means the list might update after the dialog has already closed. This ordering could cause the new world to not appear in the list until a manual refresh. Consider moving refresh() call before resolve.run().

Suggested change

resolve.run();

refresh();

refresh();

resolve.run();

Copilot · 2026-01-14T12:28:32Z

-                    }
-                }).start();
+        Task.supplyAsync(Schedulers.io(), () -> {
+            // Ensure the game version number is parsed


The comment states 'Ensure the game version number is parsed' but the return value is ignored. This comment should clarify why we're calling this method for its side effects (likely to cache the version) rather than using the result.

Suggested change

// Ensure the game version number is parsed

// Preload and cache the game version number for this profile.

// The result is intentionally ignored; parsing may trigger repository-side caching.

Copilot · 2026-01-14T12:28:32Z

+                return stream.toList();
+            }
+        }).whenComplete(Schedulers.javafx(), (result, exception) -> {
+            if (refreshCount != currentRefresh) {


The refreshCount comparison uses != instead of <, which could theoretically fail if refreshCount wraps around after Integer.MAX_VALUE refreshes. While unlikely in practice, using < would be more robust: if (currentRefresh < refreshCount).

Suggested change

if (refreshCount != currentRefresh) {

if (currentRefresh < refreshCount) {

Copilot · 2026-01-14T12:28:32Z

+            } else {
+                imageView.setImage(world.getIcon() == null ? FXUtils.newBuiltinImage("/assets/img/unknown_server.png") : world.getIcon());
+                leftTooltip.setText(world.getFile().toString());
+                content.setTitle(world.getWorldName() != null ? parseColorEscapes(world.getWorldName()) : "");


Setting an empty string when world.getWorldName() is null is inconsistent with the logic in the 'if (empty || world == null)' branch at line 311 which also sets title to "". Consider using a more descriptive placeholder or consistent null handling pattern.

Suggested change

content.setTitle(world.getWorldName() != null ? parseColorEscapes(world.getWorldName()) : "");

String worldName = world.getWorldName();

if (worldName == null || worldName.isEmpty()) {

Path worldFile = world.getFile();

worldName = worldFile != null && worldFile.getFileName() != null ? worldFile.getFileName().toString() : "";

}

content.setTitle(worldName.isEmpty() ? "" : parseColorEscapes(worldName));

Glavo added 2 commits January 13, 2026 21:57

update

2079c6d

update

5d7ff6b

Glavo requested a review from Copilot January 13, 2026 14:03

Copilot started reviewing on behalf of Glavo January 13, 2026 14:03 View session

update

a1623b4

Copilot AI reviewed Jan 13, 2026

View reviewed changes

Comment thread HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/WorldListPage.java

Glavo added 3 commits January 13, 2026 22:08

update

3dabe55

update

1e8cb6a

update

76d45f4

Glavo requested a review from Copilot January 13, 2026 14:25

Copilot started reviewing on behalf of Glavo January 13, 2026 14:25 View session

Copilot AI reviewed Jan 13, 2026

View reviewed changes

Glavo added 2 commits January 14, 2026 20:19

update

cf72bc8

Merge remote-tracking branch 'upstream/main' into WorldListPage

88c443f

Glavo requested a review from Copilot January 14, 2026 12:24

Copilot started reviewing on behalf of Glavo January 14, 2026 12:24 View session

Copilot AI reviewed Jan 14, 2026

View reviewed changes

Glavo merged commit 7d76fea into HMCL-dev:main Jan 14, 2026
8 checks passed

Glavo deleted the WorldListPage branch January 14, 2026 12:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

重构 WorldListPage 以使用 ListCell 展示元素#5224

重构 WorldListPage 以使用 ListCell 展示元素#5224
Glavo merged 8 commits into
HMCL-dev:mainfrom
Glavo:WorldListPage

Glavo commented Jan 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 13, 2026

Uh oh!

Copilot AI Jan 13, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 14, 2026

Uh oh!

Copilot AI Jan 14, 2026

Uh oh!

Copilot AI Jan 14, 2026

Uh oh!

Copilot AI Jan 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	WorldManageUIUtils.delete(world, () -> this.getItems().remove(world));
	WorldManageUIUtils.delete(world, this::refresh);

	// Ensure the game version number is parsed
	// Preload and cache the game version number for this profile.
	// The result is intentionally ignored; parsing may trigger repository-side caching.

	if (refreshCount != currentRefresh) {
	if (currentRefresh < refreshCount) {

-                content.setTitle(world.getWorldName() != null ? parseColorEscapes(world.getWorldName()) : "");
+                String worldName = world.getWorldName();
+                if (worldName == null || worldName.isEmpty()) {
+                    Path worldFile = world.getFile();
+                    worldName = worldFile != null && worldFile.getFileName() != null ? worldFile.getFileName().toString() : "";
+                }
+                content.setTitle(worldName.isEmpty() ? "" : parseColorEscapes(worldName));

Conversation

Glavo commented Jan 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants