fix: mod file name wrong after toggles on ModList (#6200)#6216
Conversation
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 HMCL-dev#6200
|
/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 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.
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.
|
/gemini review |
There was a problem hiding this comment.
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.
|
/gemini review |
There was a problem hiding this comment.
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.
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.
|
/gemini review |
There was a problem hiding this comment.
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.
|
@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 ! |
1 participant
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.disabledexample.jar.disabled->example.jarBefore 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
ModListPageSkin.createModSubtitle.ModListPageTest.Behavior
Video here:
2026-06-25.12-23-22.mp4
Tests
Added coverage for: