fix: mod file name wrong after toggles on ModList (#6200)

[HowXu]

Summary

Fixes filename unrefresh when toggle changed in the instance mod management page.

Close #6200

Mods are enabled or disabled by renaming the backing file:

• example.jar -> example.jar.disabled
• example.jar.disabled -> example.jar

Before this change, the checkbox state changed and the file was renamed, but the visible mod list row keep showing the old filename until the list was refreshed or reloaded.

Changes

• Extracted the mod row subtitle formatting into ModListPageSkin.createModSubtitle.
• Added a listener for the row's active state so the current row subtitle is recalculated after toggling the checkbox.
• Kept the update scoped to the affected visible cell instead of reloading all mods.
• Added regression tests in ModListPageTest.

Behavior

Video here:

2026-06-25.12-23-22.mp4

Tests

Added coverage for:

• Disabling a mod updates the displayed filename.
• Enabling a mod updates the displayed filename.
• Already-enabled mods show the current enabled filename.
• Failed rename leaves the displayed filename unchanged.

[HowXu]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the mod subtitle creation into a static helper method and introduces an InvalidationListener to dynamically update the subtitle when a mod's active state changes. It also adds unit tests to verify subtitle behavior under various scenarios. The review feedback highlights a potential memory leak in JavaFX and suggests wrapping the listener in a WeakInvalidationListener, along with properly cleaning it up when cells are reused.

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.

[HowXu]

/gemini review

[gemini-code-assist]

Code Review

This pull request updates ModListPageSkin to dynamically update the mod subtitle when a mod's active state changes, ensuring the UI correctly reflects file renames (or failures to rename). It also adds comprehensive unit tests in ModListPageTest to verify this behavior. The review feedback recommends replacing Java 23 triple-slash comments (///) with standard Javadoc comments to preserve compatibility with older JDKs, and removing an unnecessary comment referencing ChatGPT in the test file.

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.

[HowXu]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the mod list UI to dynamically update a mod's subtitle when its active state is toggled, and adds a comprehensive test suite to verify this behavior. The review feedback suggests a performance optimization to initialize the invalidation listeners once during cell construction rather than recreating them on every call to updateControl, which reduces GC pressure during list scrolling.

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.

[HowXu]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the mod list page UI to dynamically update the mod subtitle when a mod is enabled or disabled. It introduces a helper method createModSubtitle and utilizes a WeakInvalidationListener to listen to changes in the mod's active state, ensuring the UI reflects the correct file path (e.g., appending or removing .disabled). Additionally, unit tests have been added in ModListPageTest to verify the subtitle formatting and behavior under various scenarios, including failed file renames. There are no review comments, so no further feedback is provided.

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.

[HowXu]

@Glavo sorry for bother you, this pr is not complex and solve a small bug, If it's not too trouble, could you review it with some spare time? thanks a lot !

[Glavo]

你这个 activeListener 根本没有地方持有强引用啊，不怕被回收掉吗？

[HowXu]

这一段在之前被gemini指出内存泄露，我看了一下说的也在理，当时确实没发现，然后按照gemini的要求改了一下

Declaring activeListener and weakActiveListener as final and initializing them inline during instance construction avoids allocating a new lambda and a new WeakInvalidationListener instance every time updateControl is called. This reduces GC pressure and improves performance during list scrolling.

        final InvalidationListener activeListener = observable -> {
            ModInfoObject item = getItem();
            if (item != null) {
                content.setSubtitle(createModSubtitle(item.getModInfo()));
            }
        };
        final WeakInvalidationListener weakActiveListener = new WeakInvalidationListener(activeListener);

activeListener 是 ModInfoListCell 的 final 字段，cell 自身持有它的强引用，WeakInvalidationListener 防止 dataItem.active 反过来强引用整个 cell 导致泄漏（不包 weak ，addListener(strongListener) 会让 listener 链上 cell 的强引用，而 listener 通过 lambda 又反向捕获 cell）。

包了 weak 之后，dataItem.active -> weakActiveListener(weak) -> activeListener(strong via cell)，cell 才能被 GC。

这个写法我后来看了一下，是gemini按照已有的模式给出的，OptionsListSkin 的 Cell 里 updateStyleListener 大概也是也是这个写法 final listener套一层weak

HMCL/HMCL/src/main/java/org/jackhuang/hmcl/ui/construct/OptionsListSkin.java

Line 69 in 0eb1d3c

private final InvalidationListener updateStyleListener = o -> updateStyle();

    private final class Cell extends ListCell<OptionsList.Element> {
        private static final PseudoClass PSEUDO_CLASS_FIRST = PseudoClass.getPseudoClass("first");
        private static final PseudoClass PSEUDO_CLASS_LAST = PseudoClass.getPseudoClass("last");

        @SuppressWarnings("FieldCanBeLocal")
        private final InvalidationListener updateStyleListener = o -> updateStyle();

        private StackPane wrapper;

        public Cell() {
            FXUtils.limitCellWidth(listView, this);

            WeakInvalidationListener weakListener = new WeakInvalidationListener(updateStyleListener);
            listView.itemsProperty().addListener((o, oldValue, newValue) -> {
                if (oldValue != null)
                    oldValue.removeListener(weakListener);
                if (newValue != null)
                    newValue.addListener(weakListener);

                weakListener.invalidated(o);
            });
            itemProperty().addListener(weakListener);
            contentPaddings.addListener(weakListener);
        }

我认为 weakActiveListener 这个对象本身被 cell 当 final 字段持有，就不会被回收；它内部的弱引用指向的 activeListener 也被 cell 当 final 持有，也不用担心回收。两个对象生命周期和ListView或者Cell本身是一样长的，切走ListView肯定能GC了，只要Cell的生命周期是正常的，这个东西就没什么问题。

[Glavo]

哦，是我的错，我错把它看成方法局部变量了。