Streamline memory allocation options and improve preset management#6330
Streamline memory allocation options and improve preset management#6330Glavo wants to merge 2 commits into
Conversation
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the game settings UI to dynamically track and update inherited settings when parent presets change, replacing global listeners on SettingsManager with parent-specific property listeners. It also updates how parent presets are retrieved and introduces a dynamic description for the memory allocation sublist. Feedback on this PR highlights an issue where the collapsed memory sublist description may initially display incorrect information due to lazy loading of the sublist content. It is recommended to bind memorySelectedValue to the active setting's autoMemoryProperty using a weak listener to ensure the description is accurate from the start.
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.
| memorySelectedValue.addListener((observable, oldValue, newValue) -> | ||
| memorySublist.setDescription(getMemoryAllocationDisplayName(newValue))); | ||
| memorySublist.setDescription(getMemoryAllocationDisplayName(memorySelectedValue.get())); |
There was a problem hiding this comment.
Since ComponentSublist content is loaded lazily via a Supplier, the memoryItem and its listener that updates memorySelectedValue are not initialized until the sublist is expanded. Consequently, the collapsed sublist description will initially display the default value (settings.memory.auto_allocate) even if the active setting is configured for manual memory allocation.[361, 387, 430]\n\nTo ensure the description is always accurate from the start, we should also bind memorySelectedValue to the active setting's autoMemoryProperty using a weak listener.
memorySelectedValue.addListener((observable, oldValue, newValue) ->
memorySublist.setDescription(getMemoryAllocationDisplayName(newValue)));
InvalidationListener memoryDescriptionListener = observable -> {
S setting = currentSetting.get();
if (setting != null) {
memorySelectedValue.set(IndependentSettingBinder.getEffectiveValue(
setting,
GameSettings::autoMemoryProperty,
this::getEffectiveParentGameSettings));
}
};
InvalidationListener weakMemoryDescriptionListener = holder.weak(memoryDescriptionListener);
currentSetting.addListener((observable, oldValue, newValue) -> {
if (oldValue != null) {
oldValue.autoMemoryProperty().removeListener(weakMemoryDescriptionListener);
}
if (newValue != null) {
newValue.autoMemoryProperty().addListener(weakMemoryDescriptionListener);
}
memoryDescriptionListener.invalidated(newValue);
});
S initialSetting = currentSetting.get();
if (initialSetting != null) {
initialSetting.autoMemoryProperty().addListener(weakMemoryDescriptionListener);
}
memoryDescriptionListener.invalidated(initialSetting);
Fix #6307