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

@@ -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

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<ComboItem<Optional<LoaderVersion>>> loaderVersionsDropDown = new JComboBox<>();
+    private final JComboBox<ComboItem<LoaderVersion>> 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.
+     * <p>
+     * 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<LoaderVersion> comboItem =
+                    (ComboItem<LoaderVersion>) loaderVersionsDropDown.getSelectedItem();
+
+                if (comboItem != null) {
+                    LoaderVersion version = comboItem.getValue();
+                    if (version != null) {
+                        viewModel.setLoaderVersion(version);
+                    }
+                }
+            }
+        });
+
         viewModel.loaderLoading().subscribe((it) -> {
             loaderVersionsDropDown.removeAllItems();
             if (it) {

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<Optional<LoaderVersion>> selectedLoaderVersion();
+    Observable<Integer> selectedLoaderVersionIndex();
 
     /**
      * Is the loader loading.

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<Integer> selectedLoaderVersionIndex = combineLatest(
+        loaderVersions,
+        selectedLoaderVersion,
+        (versionsOptional, selectedOptional) -> {
+            int index = -1;
+
+            if (versionsOptional.isPresent()) {
+                List<LoaderVersion> 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<Optional<LoaderVersion>> selectedLoaderVersion() {
-        return selectedLoaderVersion.observeOn(SwingSchedulers.edt());
-    }
-
     @Override
     public Observable<Boolean> 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<List<LoaderVersion>> 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<LoaderVersion> 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<Integer> selectedLoaderVersionIndex() {
+        return selectedLoaderVersionIndex.observeOn(SwingSchedulers.edt());
     }
 
     public void createServer() {