From 493c9ae7231cd7e7c658e29d171cfb6a06f251e5 Mon Sep 17 00:00:00 2001 From: Doomsdayrs <38189170+Doomsdayrs@users.noreply.github.com> Date: Tue, 11 Jul 2023 00:14:02 -0400 Subject: [PATCH] Fix Mod Loader Version not being changed during creation of instance (#781) * fix: Connect setLoaderVersion to loaderVersionsDropDown Unconnected method slipped through testing. Fixes #780 * fix: Prevent race runaway in VanillaPacksTab by filtering action events The issue is that java swing is not designed for any real concurrency, thus we need to filter the action events to only be ones produced by the user. * Update CHANGELOG.md --- CHANGELOG.md | 1 + .../atlauncher/gui/tabs/VanillaPacksTab.java | 41 +++++++++++----- .../base/IVanillaPacksViewModel.java | 4 +- .../viewmodel/impl/VanillaPacksViewModel.java | 47 +++++++++++++------ 4 files changed, 63 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ecef1ddd..7a47f9de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ This changelog only contains the changes that are unreleased. For changes for in - Add missing server_run analytics event - Run analytics only on version specified in config - Importing packs from CurseForge with distribution disabled not prompting to download +- Fix loader version always being the recommended one in VanillaPacksTab [#781] ### Misc - Upgrade to Gradle 8.2 diff --git a/src/main/java/com/atlauncher/gui/tabs/VanillaPacksTab.java b/src/main/java/com/atlauncher/gui/tabs/VanillaPacksTab.java index 549004c5..faed88e6 100644 --- a/src/main/java/com/atlauncher/gui/tabs/VanillaPacksTab.java +++ b/src/main/java/com/atlauncher/gui/tabs/VanillaPacksTab.java @@ -26,7 +26,6 @@ import java.awt.FlowLayout; import java.awt.GridBagConstraints; import java.awt.GridBagLayout; import java.util.List; -import java.util.Optional; import javax.annotation.Nullable; import javax.swing.Box; @@ -63,8 +62,6 @@ import com.atlauncher.utils.ComboItem; import com.atlauncher.viewmodel.base.IVanillaPacksViewModel; import com.atlauncher.viewmodel.impl.VanillaPacksViewModel; -import io.reactivex.rxjava3.disposables.Disposable; - public class VanillaPacksTab extends JPanel implements Tab, RelocalizationListener { private final JTextField nameField = new JTextField(32); private final JTextArea descriptionField = new JTextArea(2, 40); @@ -79,15 +76,20 @@ public class VanillaPacksTab extends JPanel implements Tab, RelocalizationListen private final JRadioButton loaderTypeForgeRadioButton = new JRadioButton("Forge"); private final JRadioButton loaderTypeLegacyFabricRadioButton = new JRadioButton("Legacy Fabric"); private final JRadioButton loaderTypeQuiltRadioButton = new JRadioButton("Quilt"); - private final JComboBox>> loaderVersionsDropDown = new JComboBox<>(); + private final JComboBox> loaderVersionsDropDown = new JComboBox<>(); private final JButton createServerButton = new JButton(getCreateServerText()); private final JButton createInstanceButton = new JButton(getCreateInstanceText()); private final IVanillaPacksViewModel viewModel = new VanillaPacksViewModel(); + /** + * Last time the loaderVersion has been changed. + *

+ * Used to prevent an infinite changes from occuring. + */ + private long loaderVersionLastChange = System.currentTimeMillis(); @Nullable private JTable minecraftVersionTable = null; @Nullable private DefaultTableModel minecraftVersionTableModel = null; - private Disposable selectionDisposable; public VanillaPacksTab() { super(new BorderLayout()); @@ -250,10 +252,6 @@ public class VanillaPacksTab extends JPanel implements Tab, RelocalizationListen viewModel.loaderVersionsDropDownEnabled().subscribe(loaderVersionsDropDown::setEnabled); viewModel.loaderVersions().subscribe((loaderVersionsOptional) -> { - if (selectionDisposable != null) { - selectionDisposable.dispose(); - selectionDisposable = null; - } loaderVersionsDropDown.removeAllItems(); if (!loaderVersionsOptional.isPresent()) { setEmpty(); @@ -279,12 +277,29 @@ public class VanillaPacksTab extends JPanel implements Tab, RelocalizationListen loaderVersionLength = min(400, loaderVersionLength); loaderVersionsDropDown.setPreferredSize(new Dimension(loaderVersionLength, 23)); - selectionDisposable = viewModel.selectedLoaderVersion().subscribe((it) -> { - it.ifPresent(loaderVersion -> loaderVersionsDropDown - .setSelectedIndex(loaderVersions.indexOf(loaderVersion))); - }); } }); + viewModel.selectedLoaderVersionIndex().subscribe((index) -> { + if (loaderVersionsDropDown.getItemAt(index) != null) { + loaderVersionLastChange = System.currentTimeMillis(); + loaderVersionsDropDown.setSelectedIndex(index); + } + }); + loaderVersionsDropDown.addActionListener((e) -> { + // A user cannot change the loader version in under 100 ms. It is physically impossible. + if (e.getWhen() > (loaderVersionLastChange + 100)) { + ComboItem comboItem = + (ComboItem) loaderVersionsDropDown.getSelectedItem(); + + if (comboItem != null) { + LoaderVersion version = comboItem.getValue(); + if (version != null) { + viewModel.setLoaderVersion(version); + } + } + } + }); + viewModel.loaderLoading().subscribe((it) -> { loaderVersionsDropDown.removeAllItems(); if (it) { diff --git a/src/main/java/com/atlauncher/viewmodel/base/IVanillaPacksViewModel.java b/src/main/java/com/atlauncher/viewmodel/base/IVanillaPacksViewModel.java index 4281bbc3..d978d5fd 100644 --- a/src/main/java/com/atlauncher/viewmodel/base/IVanillaPacksViewModel.java +++ b/src/main/java/com/atlauncher/viewmodel/base/IVanillaPacksViewModel.java @@ -328,7 +328,7 @@ public interface IVanillaPacksViewModel { /** * Set the selected loader version */ - void setLoaderVersion(@NotNull String loaderVersion); + void setLoaderVersion(@NotNull LoaderVersion loaderVersion); /** * Is the loader versions drop down enabled @@ -346,7 +346,7 @@ public interface IVanillaPacksViewModel { * The selected mod loader version */ @NotNull - Observable> selectedLoaderVersion(); + Observable selectedLoaderVersionIndex(); /** * Is the loader loading. diff --git a/src/main/java/com/atlauncher/viewmodel/impl/VanillaPacksViewModel.java b/src/main/java/com/atlauncher/viewmodel/impl/VanillaPacksViewModel.java index 6cd8f83e..39c0c744 100644 --- a/src/main/java/com/atlauncher/viewmodel/impl/VanillaPacksViewModel.java +++ b/src/main/java/com/atlauncher/viewmodel/impl/VanillaPacksViewModel.java @@ -136,6 +136,26 @@ public class VanillaPacksViewModel implements SettingsListener, IVanillaPacksVie true); // was lazy public final Boolean showQuiltOption = ConfigManager.getConfigItem("loaders.quilt.enabled", false); + public final Observable selectedLoaderVersionIndex = combineLatest( + loaderVersions, + selectedLoaderVersion, + (versionsOptional, selectedOptional) -> { + int index = -1; + + if (versionsOptional.isPresent()) { + List versions = versionsOptional.get(); + for (int i = 0; i < versions.size(); i++) { + if (Objects.equals(versions.get(i), selectedOptional.orElse(null))) { + index = i; + break; + } + } + } + + if (index == -1) + return 0; + return index; + }).subscribeOn(Schedulers.computation()); /** * Filters applied to the version table */ @@ -428,11 +448,6 @@ public class VanillaPacksViewModel implements SettingsListener, IVanillaPacksVie return description.observeOn(SwingSchedulers.edt()); } - @Override - public Observable> selectedLoaderVersion() { - return selectedLoaderVersion.observeOn(SwingSchedulers.edt()); - } - @Override public Observable loaderLoading() { return loaderLoading.observeOn(SwingSchedulers.edt()); @@ -759,17 +774,19 @@ public class VanillaPacksViewModel implements SettingsListener, IVanillaPacksVie selectedLoaderType.onNext(Optional.ofNullable(loader)); } - public void setLoaderVersion(String loaderVersion) { - Optional> versions = loaderVersions.getValue(); - - if (versions.isPresent()) { - for (LoaderVersion version : versions.get()) { - if (version.version.equals(loaderVersion)) { - selectedLoaderVersion.onNext(Optional.of(version)); - break; - } - } + public void setLoaderVersion(@Nonnull LoaderVersion loaderVersion) { + Optional currentOptional = selectedLoaderVersion.getValue(); + // Do not push two of the same? + if (currentOptional.isPresent()) { + if (currentOptional.get() == loaderVersion) + return; } + selectedLoaderVersion.onNext(Optional.of(loaderVersion)); + } + + @Override + public Observable selectedLoaderVersionIndex() { + return selectedLoaderVersionIndex.observeOn(SwingSchedulers.edt()); } public void createServer() {