Matchmaker Overhaul #190

Spikey84 · 2023-10-19T23:40:17Z

Makes many improvements to the matchmaker tab of the client. Tested on linux, should work on windows too.

Made map vault search async
Removed the need for * to search for wildcards. Now u can search ‘wonder’ and see any map with that string in the title.
Added a map favoriting system. This can probably be further optimized in the future.You can leave the search bar empty and checkbox checked to get all favorited maps, but this is super slow since it is loading all maps then filtering out unfavorited ones.
Fixed mapgen info to be visible. I need to make the size number human readable next.
Added a button to remove all maps in a bracket. This should speed up making new pools without losing too much control.
Added a zoom in button for maps. This makes sure that you can see the map thumbnail better.

src/main/java/com/faforever/moderatorclient/ui/main_window/LadderMapPoolController.java

Brutus5000 · 2023-10-20T16:25:40Z

src/main/java/com/faforever/moderatorclient/ui/main_window/LadderMapPoolController.java

+            });
+            writer.close();
+        } catch (IOException e) {
+            e.printStackTrace();


logging as above

Brutus5000 · 2023-10-20T16:26:52Z

src/main/java/com/faforever/moderatorclient/ui/MapTableItemAdapter.java

+    public MapTableItemAdapter getThis() {
+        return this;
+    }


Makes no sense, please remove.

This is dumb, i wonder why i did this lol

When I remove this the favorite button breaks. I have no idea why, the getThis method is never called.

at com.sun.javafx.property.PropertyReference.getProperty(PropertyReference.java:191) ~[javafx-base-21-ea+24-linux.jar:na] at javafx.scene.control.cell.TreeItemPropertyValueFactory.getCellDataReflectively(TreeItemPropertyValueFactory.java:184) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.cell.TreeItemPropertyValueFactory.call(TreeItemPropertyValueFactory.java:157) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.cell.TreeItemPropertyValueFactory.call(TreeItemPropertyValueFactory.java:135) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.TreeTableColumn.getCellObservableValue(TreeTableColumn.java:578) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.TreeTableColumn.getCellObservableValue(TreeTableColumn.java:563) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.TreeTableCell.updateItem(TreeTableCell.java:676) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.TreeTableCell.indexChanged(TreeTableCell.java:482) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.IndexedCell$1.invalidated(IndexedCell.java:85) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.beans.property.IntegerPropertyBase.markInvalid(IntegerPropertyBase.java:113) ~[javafx-base-21-ea+24-linux.jar:na] at javafx.beans.property.IntegerPropertyBase.set(IntegerPropertyBase.java:148) ~[javafx-base-21-ea+24-linux.jar:na] at javafx.scene.control.IndexedCell.updateIndex(IndexedCell.java:130) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.skin.TableRowSkinBase.updateCells(TableRowSkinBase.java:525) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.skin.TreeTableRowSkin.updateCells(TreeTableRowSkin.java:280) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.skin.TableRowSkinBase.<init>(TableRowSkinBase.java:155) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.skin.TreeTableRowSkin.<init>(TreeTableRowSkin.java:90) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.TreeTableRow.createDefaultSkin(TreeTableRow.java:536) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.Control.doProcessCSS(Control.java:910) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.Control$1.doProcessCSS(Control.java:88) ~[javafx-controls-21-ea+24-linux.jar:na] at com.sun.javafx.scene.control.ControlHelper.processCSSImpl(ControlHelper.java:68) ~[javafx-controls-21-ea+24-linux.jar:na] at com.sun.javafx.scene.NodeHelper.processCSS(NodeHelper.java:147) ~[javafx-graphics-21-ea+24-linux.jar:na] at javafx.scene.Node.processCSS(Node.java:9555) ~[javafx-graphics-21-ea+24-linux.jar:na] at javafx.scene.Node.applyCss(Node.java:9642) ~[javafx-graphics-21-ea+24-linux.jar:na] at javafx.scene.control.skin.VirtualFlow.setCellIndex(VirtualFlow.java:1819) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.skin.VirtualFlow.getCell(VirtualFlow.java:1796) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.skin.VirtualFlow.getOrCreateCellSize(VirtualFlow.java:3068) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.skin.VirtualFlow.getOrCreateCellSize(VirtualFlow.java:3040) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.skin.VirtualFlow.recalculateAndImproveEstimatedSize(VirtualFlow.java:3144) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.skin.VirtualFlow$5.invalidated(VirtualFlow.java:865) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.beans.property.IntegerPropertyBase.markInvalid(IntegerPropertyBase.java:113) ~[javafx-base-21-ea+24-linux.jar:na] at javafx.beans.property.IntegerPropertyBase.set(IntegerPropertyBase.java:148) ~[javafx-base-21-ea+24-linux.jar:na] at javafx.scene.control.skin.VirtualFlow.setCellCount(VirtualFlow.java:911) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.skin.TreeTableViewSkin.updateItemCount(TreeTableViewSkin.java:354) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.skin.VirtualContainerBase.checkState(VirtualContainerBase.java:183) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.skin.VirtualContainerBase.layoutChildren(VirtualContainerBase.java:158) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.skin.TableViewSkinBase.layoutChildren(TableViewSkinBase.java:432) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.Control.layoutChildren(Control.java:612) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.control.TreeTableView.layoutChildren(TreeTableView.java:1681) ~[javafx-controls-21-ea+24-linux.jar:na] at javafx.scene.Parent.layout(Parent.java:1208) ~[javafx-graphics-21-ea+24-linux.jar:na] at javafx.scene.Parent.layout(Parent.java:1215) ~[javafx-graphics-21-ea+24-linux.jar:na] at javafx.scene.Parent.layout(Parent.java:1215) ~[javafx-graphics-21-ea+24-linux.jar:na] at javafx.scene.Parent.layout(Parent.java:1215) ~[javafx-graphics-21-ea+24-linux.jar:na] at javafx.scene.Parent.layout(Parent.java:1215) ~[javafx-graphics-21-ea+24-linux.jar:na] at javafx.scene.Parent.layout(Parent.java:1215) ~[javafx-graphics-21-ea+24-linux.jar:na] at javafx.scene.Parent.layout(Parent.java:1215) ~[javafx-graphics-21-ea+24-linux.jar:na] at javafx.scene.Parent.layout(Parent.java:1215) ~[javafx-graphics-21-ea+24-linux.jar:na] at javafx.scene.Parent.layout(Parent.java:1215) ~[javafx-graphics-21-ea+24-linux.jar:na] at javafx.scene.Scene.doLayoutPass(Scene.java:594) ~[javafx-graphics-21-ea+24-linux.jar:na] at javafx.scene.Scene$ScenePulseListener.pulse(Scene.java:2600) ~[javafx-graphics-21-ea+24-linux.jar:na] at com.sun.javafx.tk.Toolkit.lambda$runPulse$2(Toolkit.java:401) ~[javafx-graphics-21-ea+24-linux.jar:na] at java.base/java.security.AccessController.doPrivileged(AccessController.java:399) ~[na:na] at com.sun.javafx.tk.Toolkit.runPulse(Toolkit.java:400) ~[javafx-graphics-21-ea+24-linux.jar:na] at com.sun.javafx.tk.Toolkit.firePulse(Toolkit.java:430) ~[javafx-graphics-21-ea+24-linux.jar:na] at com.sun.javafx.tk.quantum.QuantumToolkit.pulse(QuantumToolkit.java:592) ~[javafx-graphics-21-ea+24-linux.jar:na] at com.sun.javafx.tk.quantum.QuantumToolkit.pulse(QuantumToolkit.java:572) ~[javafx-graphics-21-ea+24-linux.jar:na] at com.sun.javafx.tk.quantum.QuantumToolkit.pulseFromQueue(QuantumToolkit.java:565) ~[javafx-graphics-21-ea+24-linux.jar:na] at com.sun.javafx.tk.quantum.QuantumToolkit.lambda$runToolkit$11(QuantumToolkit.java:352) ~[javafx-graphics-21-ea+24-linux.jar:na] at com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95) ~[javafx-graphics-21-ea+24-linux.jar:na] at com.sun.glass.ui.gtk.GtkApplication._runLoop(Native Method) ~[javafx-graphics-21-ea+24-linux.jar:na] at com.sun.glass.ui.gtk.GtkApplication.lambda$runLoop$10(GtkApplication.java:263) ~[javafx-graphics-21-ea+24-linux.jar:na]```

Maybe @Sheikah45 can help here

It is because of line 1009 in the ViewHelper, you are creating a TreeItemPropertyValueFactory where you specify the property as this. So it is going to look for a getThis method to retrieve the property.

It would be better to change that cell factory to a lambda that provides the value like item -> item.getValue().valueProperty()

Generally for all of those I would use a lambda function rather than relying on the string name of the property to expect a getter. It is better for refactor and type safety

src/main/java/com/faforever/moderatorclient/FafModeratorClientApplication.java

src/main/java/com/faforever/moderatorclient/ui/main_window/LadderMapPoolController.java

Brutus5000 · 2023-10-20T16:29:22Z

src/main/java/com/faforever/moderatorclient/ui/main_window/LadderMapPoolController.java

+    public void addFavorite(MapTableItemAdapter mapTableItemAdapter) {
+        System.out.println(mapTableItemAdapter.getId());
+        System.out.println(mapTableItemAdapter.getMap().getId());
+        if (favoritesCache.contains(Integer.parseInt(mapTableItemAdapter.getId()))) return;


This whole integer parsing is EVERYWHERE. It seems much simpler to just use string everywhere.

Spikey84 · 2023-10-20T16:35:49Z

Will take care of all this tonight. Thanks for the review.

Sheikah45 · 2023-10-21T15:13:42Z

src/main/java/com/faforever/moderatorclient/ui/ViewHelper.java

+                        if (!empty && item != null && !item.isMapVersion()) {
+                            if (ladderMapPoolController.isMapFavorite(Integer.parseInt(item.getId()))) {
+                                Button button = new Button("Unfavorite");
+                                button.setOnMouseClicked(event -> removeFavorite.accept(item));


You have removeFavorite marked as nullable but no null check here

Sheikah45 · 2023-10-21T15:13:53Z

src/main/java/com/faforever/moderatorclient/ui/ViewHelper.java

+                                return;
+                            } else {
+                                Button button = new Button("Favorite");
+                                button.setOnMouseClicked(event -> addFavorite.accept(item));


nullability of addFavorite

What was changed here? I don't even know what @Sheikah45 refers to O.o

addFavorite is marked as nullable in the method parameters But there is no check to see if it is nonNull before calling accept on it. @Spikey84 I meant checking for null on addFavorite and removeFavorite not the item

Sheikah45 · 2023-10-21T15:15:39Z

src/main/java/com/faforever/moderatorclient/ui/main_window/LadderMapPoolController.java

 import org.apache.maven.artifact.versioning.ComparableVersion;
 import org.springframework.core.io.ClassPathResource;
 import org.springframework.stereotype.Component;

-import java.io.IOException;
+import java.io.*;


Personally I prefer not to use star imports to reduce name collisions

There are still some on import com.faforever.commons.api.dto.*;
This is a setting you can do in your IDE. Many IDEs like to summarize them to wildcard imports :(

Sheikah45 · 2023-10-21T15:16:57Z

src/main/java/com/faforever/moderatorclient/ui/main_window/LadderMapPoolController.java

@@ -182,10 +216,16 @@ private void loadMatchMakerQueue(MatchmakerQueue matchmakerQueue) {
            AddBracketController addBracketController = uiService.loadFxml("ui/main_window/addBracketLabelAndButton.fxml");
            addBracketController.setRatingLabelText(getBracketRatingString(bracketFX));
            addButtonsContainer.getChildren().add(addBracketController.getRoot());
+            Button button = new Button("--");
+            button.setOnAction((event -> {
+                bracketAssignments.removeAll(bracketAssignments.stream().toList());


bracketAssignments.clear() is easier

Sheikah45 · 2023-10-21T15:19:19Z

src/main/java/com/faforever/moderatorclient/ui/main_window/LadderMapPoolController.java

+            }
+        };
+
+        new Thread(task).start();


Rather than just creating threads and starting them it would be better to set up an executor service to execute them. Since this is spring boot one should even be pre configured that you just need to autowire into the class

Sheikah45 · 2023-10-21T15:20:42Z

src/main/java/com/faforever/moderatorclient/ui/main_window/LadderMapPoolController.java

+        File pwd = new File("").getAbsoluteFile();
+        File file = new File(pwd.getAbsoluteFile() + File.separator + "favoriteMaps.txt");
+
+        if (!file.exists()) {
+            file.createNewFile();
+        }


Meh even, I would probably suggest just using an ObjectMapper and saving the preferences as json file similar to how we do in the client.

It escapes the custom encoding and makes it easy to add to later.

src/main/java/com/faforever/moderatorclient/ui/main_window/LadderMapPoolController.java

Spikey84 · 2023-10-27T16:29:06Z

Im going to wait for another time to redo the cache system and figure out the threadpool thing.

Brutus5000 · 2023-10-27T17:47:41Z

src/main/java/com/faforever/moderatorclient/ui/main_window/BracketRatingController.java

@@ -2,6 +2,7 @@

 import com.faforever.moderatorclient.ui.Controller;
 import javafx.fxml.FXML;
+import javafx.scene.control.Button;


You gotta optimize the imports. Since there is no other change to this file it must be unused.

Spikey84 added 2 commits October 19, 2023 18:58

Add Matchmaker Fixes

68f5ef2

remove file

1a8552e

Spikey84 changed the title ~~Matchmaker overhaul~~ Matchmaker Overhaul Oct 19, 2023

Brutus5000 requested changes Oct 20, 2023

View reviewed changes

Sheikah45 requested changes Oct 21, 2023

View reviewed changes

Spikey84 added 3 commits October 27, 2023 11:47

Fixing stuff based on requested changes.

7dad7a2

Whoops

18c5785

Fix more things.

df337ba

Brutus5000 reviewed Oct 27, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Matchmaker Overhaul #190

Matchmaker Overhaul #190

Spikey84 commented Oct 19, 2023

Brutus5000 Oct 20, 2023

Spikey84 Oct 27, 2023

Brutus5000 Oct 20, 2023

Spikey84 Oct 20, 2023

Spikey84 Oct 27, 2023

Brutus5000 Oct 27, 2023

Sheikah45 Oct 28, 2023 •

edited

Loading

Sheikah45 Oct 28, 2023

Brutus5000 Oct 20, 2023

Spikey84 commented Oct 20, 2023

Sheikah45 Oct 21, 2023

Spikey84 Oct 27, 2023

Sheikah45 Oct 21, 2023

Spikey84 Oct 27, 2023

Brutus5000 Oct 27, 2023

Sheikah45 Oct 28, 2023

Sheikah45 Oct 21, 2023

Spikey84 Oct 27, 2023

Brutus5000 Oct 27, 2023

Sheikah45 Oct 21, 2023

Spikey84 Oct 27, 2023

Sheikah45 Oct 21, 2023

Sheikah45 Oct 21, 2023

Spikey84 commented Oct 27, 2023

Brutus5000 Oct 27, 2023

Matchmaker Overhaul #190

Are you sure you want to change the base?

Matchmaker Overhaul #190

Conversation

Spikey84 commented Oct 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sheikah45 Oct 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Spikey84 commented Oct 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Spikey84 commented Oct 27, 2023

Choose a reason for hiding this comment

Sheikah45 Oct 28, 2023 •

edited

Loading