From b06a5644a829be17f814cf8f88d2892173cd8a32 Mon Sep 17 00:00:00 2001 From: HowXu Date: Thu, 25 Jun 2026 12:03:09 +0800 Subject: [PATCH 1/5] fix: mod file name wrong after toggles on ModList Refresh the mod list row subtitle after the active checkbox toggles so the displayed filename reflects the current backing file path. This keeps the update local to the affected cell instead of reloading the whole mod list. Add regression tests covering enabled and disabled filenames, unknown loader display, and failed rename behavior. This refresh is a fallback of system-call filename change and will not perform if filename change failed, more see the test file. close #6200 --- .../hmcl/ui/versions/ModListPageSkin.java | 38 +++++-- .../hmcl/ui/versions/ModListPageTest.java | 100 ++++++++++++++++++ 2 files changed, 128 insertions(+), 10 deletions(-) create mode 100644 HMCL/src/test/java/org/jackhuang/hmcl/ui/versions/ModListPageTest.java diff --git a/HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java b/HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java index 1a4ecf3aa6a..12f16d7cf5b 100644 --- a/HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java +++ b/HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java @@ -20,6 +20,7 @@ import com.jfoenix.controls.*; import javafx.animation.PauseTransition; import javafx.application.Platform; +import javafx.beans.InvalidationListener; import javafx.beans.binding.Bindings; import javafx.beans.property.BooleanProperty; import javafx.beans.property.ObjectProperty; @@ -90,6 +91,16 @@ final class ModListPageSkin extends SkinBase { private final JFXListView listView; private final JFXTextField searchField; + /// Creates the file subtitle shown in the mod list. + static String createModSubtitle(LocalModFile modInfo) { + StringJoiner joiner = new StringJoiner(" | "); + if (modInfo.getModLoaderType() != ModLoaderType.UNKNOWN && StringUtils.isNotBlank(modInfo.getId())) + joiner.add(modInfo.getId()); + + joiner.add(FileUtils.getName(modInfo.getFile())); + return joiner.toString(); + } + @FXThread private boolean isSearching = false; @@ -568,6 +579,7 @@ final class ModInfoListCell extends MDListCell { JFXButton infoButton = FXUtils.newToggleButton4(SVG.INFO); JFXButton revealButton = FXUtils.newToggleButton4(SVG.FOLDER); BooleanProperty booleanProperty; + InvalidationListener activeListener; Tooltip warningTooltip; @@ -601,6 +613,15 @@ protected void updateControl(ModInfoObject dataItem, boolean empty) { warningTooltip = null; } + if (booleanProperty != null) { + if (activeListener != null) { + booleanProperty.removeListener(activeListener); + activeListener = null; + } + checkBox.selectedProperty().unbindBidirectional(booleanProperty); + booleanProperty = null; + } + if (empty) return; List warning = new ArrayList<>(); @@ -636,13 +657,7 @@ protected void updateControl(ModInfoObject dataItem, boolean empty) { } content.setTitle(displayName); - StringJoiner joiner = new StringJoiner(" | "); - if (modLoaderType != ModLoaderType.UNKNOWN && StringUtils.isNotBlank(modInfo.getId())) - joiner.add(modInfo.getId()); - - joiner.add(FileUtils.getName(modInfo.getFile())); - - content.setSubtitle(joiner.toString()); + content.setSubtitle(createModSubtitle(modInfo)); if (modLoaderType == ModLoaderType.UNKNOWN) { content.addTagWarning(i18n("mods.unknown")); @@ -664,10 +679,13 @@ protected void updateControl(ModInfoObject dataItem, boolean empty) { content.addTag(modVersion); } - if (booleanProperty != null) { - checkBox.selectedProperty().unbindBidirectional(booleanProperty); - } checkBox.selectedProperty().bindBidirectional(booleanProperty = dataItem.active); + // Re-read the current path after active toggling + // failed renames leave it unchanged + // Note: a more deep change is on HMCLCore, adding ReadOnlyProperty will forcing waiting for file name change result + // but it's more out of scope, these lines also does good + activeListener = observable -> content.setSubtitle(createModSubtitle(modInfo)); + dataItem.active.addListener(activeListener); restoreButton.setVisible(!modInfo.getMod().getOldFiles().isEmpty()); restoreButton.setOnAction(e -> { menu.get().getContent().setAll(modInfo.getMod().getOldFiles().stream() diff --git a/HMCL/src/test/java/org/jackhuang/hmcl/ui/versions/ModListPageTest.java b/HMCL/src/test/java/org/jackhuang/hmcl/ui/versions/ModListPageTest.java new file mode 100644 index 00000000000..7a8a7421246 --- /dev/null +++ b/HMCL/src/test/java/org/jackhuang/hmcl/ui/versions/ModListPageTest.java @@ -0,0 +1,100 @@ +/* + * Hello Minecraft! Launcher + * Copyright (C) 2026 huangyuhui and contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.jackhuang.hmcl.ui.versions; + +import org.jackhuang.hmcl.addon.LocalAddonFile; +import org.jackhuang.hmcl.addon.mod.LocalModFile; +import org.jackhuang.hmcl.addon.mod.ModLoaderType; +import org.jackhuang.hmcl.addon.mod.ModManager; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Tests for instance mods list file name change + * @author dev@howxu.cn + */ +final class ModListPageTest { + + @TempDir + Path tempDir; + + @Test + void modSubtitleUsesUpdatedFilePathAfterDisabling() throws IOException { + LocalModFile modFile = createModFile("example.jar", "testmod", ModLoaderType.FABRIC); + + modFile.setActive(false); + + assertEquals("testmod | example.jar.disabled", ModListPageSkin.createModSubtitle(modFile)); + } + + @Test + void modSubtitleUsesUpdatedFilePathAfterEnabling() throws IOException { + LocalModFile modFile = createModFile("example.jar.disabled", "testmod", ModLoaderType.FABRIC); + + modFile.setActive(true); + + assertEquals("testmod | example.jar", ModListPageSkin.createModSubtitle(modFile)); + } + + @Test + void modSubtitleShowsCurrentEnabledFilePath() throws IOException { + LocalModFile modFile = createModFile("example.jar", "testmod", ModLoaderType.FABRIC); + + assertEquals("testmod | example.jar", ModListPageSkin.createModSubtitle(modFile)); + } + + @Test + void modSubtitleOmitsUnknownLoaderId() throws IOException { + LocalModFile modFile = createModFile("unknown.jar", "unknown", ModLoaderType.UNKNOWN); + + assertEquals("unknown.jar", ModListPageSkin.createModSubtitle(modFile)); + } + + @Test + void modSubtitleDoesNotChangeWhenFileRenameFails() throws IOException { + LocalModFile modFile = createModFile("example.jar", "testmod", ModLoaderType.FABRIC); + Path disabledTarget = tempDir.resolve("example.jar.disabled"); + Files.createDirectory(disabledTarget); + Files.writeString(disabledTarget.resolve("child"), "occupied"); + + modFile.setActive(false); + + assertEquals("testmod | example.jar", ModListPageSkin.createModSubtitle(modFile)); + } + + // Note: this auxiliary function is generated by ChatGPT-5.5 + private LocalModFile createModFile(String fileName, String modId, ModLoaderType modLoaderType) throws IOException { + Path file = tempDir.resolve(fileName); + Files.writeString(file, "mod"); + + ModManager modManager = new ModManager(null, "test"); + return new LocalModFile( + modManager, + modManager.getLocalMod(modId, modLoaderType), + file, + "Test Mod", + new LocalAddonFile.Description("Test mod") + ); + } +} From 720397688e4d338fa3594a821cad52cdf374f04f Mon Sep 17 00:00:00 2001 From: HowXu Date: Thu, 25 Jun 2026 12:15:36 +0800 Subject: [PATCH 2/5] start ci actions From 4bd5e5122012250d3e09a2e2bb42d07b842a70c9 Mon Sep 17 00:00:00 2001 From: HowXu Date: Thu, 25 Jun 2026 12:39:52 +0800 Subject: [PATCH 3/5] review: fix weak listener cleanup in mod list cell Use WeakInvalidationListener for the mod active-state listener so reused list cells do not keep strong references through LocalModFile properties. Remove the weak wrapper when rebinding cells and keep the delegate listener referenced by the cell to prevent premature garbage collection. Preserve the existing subtitle refresh behavior after enabling or disabling mods. --- .../hmcl/ui/versions/ModListPageSkin.java | 17 +++++++++-------- .../hmcl/ui/versions/ModListPageTest.java | 3 ++- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java b/HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java index 12f16d7cf5b..a7797eaa3d6 100644 --- a/HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java +++ b/HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java @@ -21,6 +21,7 @@ import javafx.animation.PauseTransition; import javafx.application.Platform; import javafx.beans.InvalidationListener; +import javafx.beans.WeakInvalidationListener; import javafx.beans.binding.Bindings; import javafx.beans.property.BooleanProperty; import javafx.beans.property.ObjectProperty; @@ -580,6 +581,7 @@ final class ModInfoListCell extends MDListCell { JFXButton revealButton = FXUtils.newToggleButton4(SVG.FOLDER); BooleanProperty booleanProperty; InvalidationListener activeListener; + WeakInvalidationListener weakActiveListener; Tooltip warningTooltip; @@ -614,10 +616,11 @@ protected void updateControl(ModInfoObject dataItem, boolean empty) { } if (booleanProperty != null) { - if (activeListener != null) { - booleanProperty.removeListener(activeListener); - activeListener = null; + if (weakActiveListener != null) { + booleanProperty.removeListener(weakActiveListener); + weakActiveListener = null; } + activeListener = null; checkBox.selectedProperty().unbindBidirectional(booleanProperty); booleanProperty = null; } @@ -680,12 +683,10 @@ protected void updateControl(ModInfoObject dataItem, boolean empty) { } checkBox.selectedProperty().bindBidirectional(booleanProperty = dataItem.active); - // Re-read the current path after active toggling - // failed renames leave it unchanged - // Note: a more deep change is on HMCLCore, adding ReadOnlyProperty will forcing waiting for file name change result - // but it's more out of scope, these lines also does good + // Re-read the current path after active toggling; failed renames leave it unchanged. activeListener = observable -> content.setSubtitle(createModSubtitle(modInfo)); - dataItem.active.addListener(activeListener); + weakActiveListener = new WeakInvalidationListener(activeListener); + dataItem.active.addListener(weakActiveListener); restoreButton.setVisible(!modInfo.getMod().getOldFiles().isEmpty()); restoreButton.setOnAction(e -> { menu.get().getContent().setAll(modInfo.getMod().getOldFiles().stream() diff --git a/HMCL/src/test/java/org/jackhuang/hmcl/ui/versions/ModListPageTest.java b/HMCL/src/test/java/org/jackhuang/hmcl/ui/versions/ModListPageTest.java index 7a8a7421246..f930352b850 100644 --- a/HMCL/src/test/java/org/jackhuang/hmcl/ui/versions/ModListPageTest.java +++ b/HMCL/src/test/java/org/jackhuang/hmcl/ui/versions/ModListPageTest.java @@ -31,7 +31,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals; /** - * Tests for instance mods list file name change + * Tests for instance mods list file name change. + * * @author dev@howxu.cn */ final class ModListPageTest { From 1b71c5474c0128f6f6c1ccdaaa79c44262a945ab Mon Sep 17 00:00:00 2001 From: HowXu Date: Thu, 25 Jun 2026 12:46:50 +0800 Subject: [PATCH 4/5] chore: comments refact --- .../java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java | 4 +++- .../java/org/jackhuang/hmcl/ui/versions/ModListPageTest.java | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java b/HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java index a7797eaa3d6..82dd6711278 100644 --- a/HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java +++ b/HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java @@ -92,7 +92,9 @@ final class ModListPageSkin extends SkinBase { private final JFXListView listView; private final JFXTextField searchField; - /// Creates the file subtitle shown in the mod list. + /** + * Creates the file subtitle shown in the mod list. + */ static String createModSubtitle(LocalModFile modInfo) { StringJoiner joiner = new StringJoiner(" | "); if (modInfo.getModLoaderType() != ModLoaderType.UNKNOWN && StringUtils.isNotBlank(modInfo.getId())) diff --git a/HMCL/src/test/java/org/jackhuang/hmcl/ui/versions/ModListPageTest.java b/HMCL/src/test/java/org/jackhuang/hmcl/ui/versions/ModListPageTest.java index f930352b850..e6411dfba0e 100644 --- a/HMCL/src/test/java/org/jackhuang/hmcl/ui/versions/ModListPageTest.java +++ b/HMCL/src/test/java/org/jackhuang/hmcl/ui/versions/ModListPageTest.java @@ -84,7 +84,6 @@ void modSubtitleDoesNotChangeWhenFileRenameFails() throws IOException { assertEquals("testmod | example.jar", ModListPageSkin.createModSubtitle(modFile)); } - // Note: this auxiliary function is generated by ChatGPT-5.5 private LocalModFile createModFile(String fileName, String modId, ModLoaderType modLoaderType) throws IOException { Path file = tempDir.resolve(fileName); Files.writeString(file, "mod"); From 5d245baf304c147847011c5a59738fd1e8b9bb91 Mon Sep 17 00:00:00 2001 From: HowXu Date: Thu, 25 Jun 2026 12:57:44 +0800 Subject: [PATCH 5/5] refactor: reuse active-state listener per cell Initialize the active-state listener and WeakInvalidationListener once per mod list cell instead of recreating them on every updateControl call. Reuse the weak wrapper when rebinding cells and remove it from the previous active property during cleanup. This keeps the subtitle refresh behavior while reducing per-cell rebinding allocations and preserving weak listener cleanup. --- .../hmcl/ui/versions/ModListPageSkin.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java b/HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java index 82dd6711278..7ede7f4c198 100644 --- a/HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java +++ b/HMCL/src/main/java/org/jackhuang/hmcl/ui/versions/ModListPageSkin.java @@ -582,8 +582,13 @@ final class ModInfoListCell extends MDListCell { JFXButton infoButton = FXUtils.newToggleButton4(SVG.INFO); JFXButton revealButton = FXUtils.newToggleButton4(SVG.FOLDER); BooleanProperty booleanProperty; - InvalidationListener activeListener; - WeakInvalidationListener weakActiveListener; + final InvalidationListener activeListener = observable -> { + ModInfoObject item = getItem(); + if (item != null) { + content.setSubtitle(createModSubtitle(item.getModInfo())); + } + }; + final WeakInvalidationListener weakActiveListener = new WeakInvalidationListener(activeListener); Tooltip warningTooltip; @@ -618,11 +623,7 @@ protected void updateControl(ModInfoObject dataItem, boolean empty) { } if (booleanProperty != null) { - if (weakActiveListener != null) { - booleanProperty.removeListener(weakActiveListener); - weakActiveListener = null; - } - activeListener = null; + booleanProperty.removeListener(weakActiveListener); checkBox.selectedProperty().unbindBidirectional(booleanProperty); booleanProperty = null; } @@ -686,8 +687,6 @@ protected void updateControl(ModInfoObject dataItem, boolean empty) { checkBox.selectedProperty().bindBidirectional(booleanProperty = dataItem.active); // Re-read the current path after active toggling; failed renames leave it unchanged. - activeListener = observable -> content.setSubtitle(createModSubtitle(modInfo)); - weakActiveListener = new WeakInvalidationListener(activeListener); dataItem.active.addListener(weakActiveListener); restoreButton.setVisible(!modInfo.getMod().getOldFiles().isEmpty()); restoreButton.setOnAction(e -> {