-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Matchmaker Overhaul #190
base: develop
Are you sure you want to change the base?
Matchmaker Overhaul #190
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,6 +40,7 @@ | |
import com.faforever.moderatorclient.ui.domain.VotingChoiceFX; | ||
import com.faforever.moderatorclient.ui.domain.VotingQuestionFX; | ||
import com.faforever.moderatorclient.ui.domain.VotingSubjectFX; | ||
import com.faforever.moderatorclient.ui.main_window.LadderMapPoolController; | ||
import javafx.beans.binding.Bindings; | ||
import javafx.beans.property.SimpleIntegerProperty; | ||
import javafx.beans.property.SimpleObjectProperty; | ||
|
@@ -61,6 +62,7 @@ | |
import javafx.scene.control.TextArea; | ||
import javafx.scene.control.Tooltip; | ||
import javafx.scene.control.TreeItem; | ||
import javafx.scene.control.TreeTableCell; | ||
import javafx.scene.control.TreeTableColumn; | ||
import javafx.scene.control.TreeTableView; | ||
import javafx.scene.control.cell.CheckBoxTableCell; | ||
|
@@ -990,7 +992,7 @@ protected void updateItem(PlayerFX item, boolean empty) { | |
}; | ||
} | ||
|
||
public static void buildMapTreeView(TreeTableView<MapTableItemAdapter> mapTreeView) { | ||
public static void buildMapTreeView(TreeTableView<MapTableItemAdapter> mapTreeView, @Nullable Consumer<MapTableItemAdapter> removeFavorite, @Nullable Consumer<MapTableItemAdapter> addFavorite, LadderMapPoolController ladderMapPoolController) { | ||
TreeTableColumn<MapTableItemAdapter, String> idColumn = new TreeTableColumn<>("ID"); | ||
idColumn.setCellValueFactory(new TreeItemPropertyValueFactory<>("id")); | ||
idColumn.setComparator(Comparator.comparingInt(Integer::parseInt)); | ||
|
@@ -1002,6 +1004,43 @@ public static void buildMapTreeView(TreeTableView<MapTableItemAdapter> mapTreeVi | |
nameColumn.setMinWidth(300); | ||
mapTreeView.getColumns().add(nameColumn); | ||
|
||
TreeTableColumn<MapTableItemAdapter, MapTableItemAdapter> favoriteColumn = new TreeTableColumn<>("Favorite"); | ||
favoriteColumn.setMinWidth(100); | ||
favoriteColumn.setCellValueFactory(new TreeItemPropertyValueFactory<>("this")); | ||
Callback<TreeTableColumn<MapTableItemAdapter, MapTableItemAdapter>, TreeTableCell<MapTableItemAdapter, MapTableItemAdapter>> cellFactory = new Callback<TreeTableColumn<MapTableItemAdapter, MapTableItemAdapter>, TreeTableCell<MapTableItemAdapter, MapTableItemAdapter>>() { | ||
@Override | ||
public TreeTableCell<MapTableItemAdapter, MapTableItemAdapter> call(TreeTableColumn<MapTableItemAdapter, MapTableItemAdapter> param) { | ||
return new TreeTableCell<MapTableItemAdapter, MapTableItemAdapter>() { | ||
@Override | ||
public void updateItem(MapTableItemAdapter item, boolean empty) { | ||
super.updateItem(item, empty); | ||
if (!empty && item != null && !item.isMapVersion() && removeFavorite != null && addFavorite != null) { | ||
if (ladderMapPoolController.isMapFavorite(Integer.parseInt(item.getId()))) { | ||
Button button = new Button("Unfavorite"); | ||
button.setOnMouseClicked(event -> removeFavorite.accept(item)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You have removeFavorite marked as nullable but no null check here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
button.setTextFill(Color.rgb(200, 10, 10)); | ||
|
||
setGraphic(button); | ||
return; | ||
} else { | ||
Button button = new Button("Favorite"); | ||
button.setOnMouseClicked(event -> addFavorite.accept(item)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nullability of addFavorite There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What was changed here? I don't even know what @Sheikah45 refers to O.o There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
button.setTextFill(Color.rgb(10, 200, 10)); | ||
|
||
setGraphic(button); | ||
return; | ||
} | ||
} | ||
setGraphic(null); | ||
} | ||
}; | ||
} | ||
}; | ||
|
||
favoriteColumn.setCellFactory(cellFactory); | ||
|
||
mapTreeView.getColumns().add(favoriteColumn); | ||
|
||
TreeTableColumn<MapTableItemAdapter, ComparableVersion> versionColumn = new TreeTableColumn<>("Version"); | ||
versionColumn.setCellValueFactory(new TreeItemPropertyValueFactory<>("version")); | ||
mapTreeView.getColumns().add(versionColumn); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
|
||
import com.faforever.moderatorclient.ui.Controller; | ||
import javafx.fxml.FXML; | ||
import javafx.scene.control.Button; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You gotta optimize the imports. Since there is no other change to this file it must be unused. |
||
import javafx.scene.control.Label; | ||
import javafx.scene.layout.HBox; | ||
import lombok.RequiredArgsConstructor; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,7 @@ | ||
package com.faforever.moderatorclient.ui.main_window; | ||
|
||
import com.faforever.commons.api.dto.MapPoolAssignment; | ||
import com.faforever.commons.api.dto.MapVersion; | ||
import com.faforever.commons.api.dto.MatchmakerQueue; | ||
import com.faforever.commons.api.dto.MatchmakerQueueMapPool; | ||
import com.faforever.commons.api.dto.NeroxisGeneratorParams; | ||
import com.faforever.commons.api.dto.*; | ||
import com.faforever.commons.api.dto.Map; | ||
import com.faforever.moderatorclient.api.domain.MapService; | ||
import com.faforever.moderatorclient.mapstruct.MapPoolAssignmentMapper; | ||
import com.faforever.moderatorclient.mapstruct.MatchmakerQueueMapPoolMapper; | ||
|
@@ -16,14 +13,17 @@ | |
import com.faforever.moderatorclient.ui.domain.MapPoolAssignmentFX; | ||
import com.faforever.moderatorclient.ui.domain.MapPoolFX; | ||
import com.faforever.moderatorclient.ui.domain.MatchmakerQueueMapPoolFX; | ||
import javafx.application.Platform; | ||
import javafx.beans.property.ObjectProperty; | ||
import javafx.beans.property.ReadOnlyObjectProperty; | ||
import javafx.beans.property.SimpleObjectProperty; | ||
import javafx.beans.value.ChangeListener; | ||
import javafx.collections.FXCollections; | ||
import javafx.collections.ObservableList; | ||
import javafx.concurrent.Task; | ||
import javafx.event.ActionEvent; | ||
import javafx.fxml.FXML; | ||
import javafx.scene.Scene; | ||
import javafx.scene.control.Button; | ||
import javafx.scene.control.CheckBox; | ||
import javafx.scene.control.ComboBox; | ||
|
@@ -40,19 +40,25 @@ | |
import javafx.scene.image.ImageView; | ||
import javafx.scene.layout.HBox; | ||
import javafx.scene.layout.VBox; | ||
import javafx.stage.Stage; | ||
import javafx.util.StringConverter; | ||
import lombok.RequiredArgsConstructor; | ||
import lombok.extern.slf4j.Slf4j; | ||
import org.apache.maven.artifact.versioning.ComparableVersion; | ||
import org.springframework.core.io.ClassPathResource; | ||
import org.springframework.stereotype.Component; | ||
|
||
import java.io.BufferedWriter; | ||
import java.io.File; | ||
import java.io.FileWriter; | ||
import java.io.IOException; | ||
import java.net.URL; | ||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.Collection; | ||
import java.util.List; | ||
import java.util.Objects; | ||
import java.util.Scanner; | ||
import java.util.function.BiPredicate; | ||
import java.util.stream.Collectors; | ||
|
||
|
@@ -73,7 +79,7 @@ public class LadderMapPoolController implements Controller<SplitPane> { | |
public HBox bracketListContainer; | ||
public HBox bracketHeaderContainer; | ||
public TreeTableView<MapTableItemAdapter> mapVaultView; | ||
public CheckBox filterByMapNameCheckBox; | ||
public CheckBox filterByFavorites; | ||
public TextField mapNamePatternTextField; | ||
public ImageView ladderPoolImageView; | ||
public Button refreshButton; | ||
|
@@ -85,6 +91,10 @@ public class LadderMapPoolController implements Controller<SplitPane> { | |
public Spinner<Double> neroxisSizeSpinner; | ||
public Spinner<Integer> neroxisSpawnsSpinner; | ||
public Label mapParamsLabel; | ||
public Button zoomInButton; | ||
|
||
private ObservableList<Integer> favoritesCache; | ||
private List<Map> cachedMaps; | ||
|
||
private final ObjectProperty<MapPoolAssignmentFX> selectedMap = new SimpleObjectProperty<>(); | ||
private final BiPredicate<MapPoolAssignmentFX, MapPoolAssignmentFX> matchingPoolAssignmentPredicate = (assignmentFX1, assignmentFX2) -> | ||
|
@@ -100,7 +110,8 @@ public SplitPane getRoot() { | |
|
||
@FXML | ||
public void initialize() { | ||
ViewHelper.buildMapTreeView(mapVaultView); | ||
cachedMaps = new ArrayList<>(); | ||
ViewHelper.buildMapTreeView(mapVaultView, this::removeFavorite, this::addFavorite, this); | ||
bindSelectedMapPropertyToImageView(); | ||
mapVaultView.getSelectionModel().selectedItemProperty().addListener((observable, oldValue, newValue) -> { | ||
if (newValue != null && newValue.getValue() != null && newValue.getValue().isMapVersion()) { | ||
|
@@ -131,8 +142,15 @@ public MatchmakerQueue fromString(String string) { | |
|
||
bracketsScrollPane.setHbarPolicy(ScrollPane.ScrollBarPolicy.NEVER); | ||
mapParamsLabel.managedProperty().bind(mapParamsLabel.visibleProperty()); | ||
try { | ||
favoritesCache = FXCollections.observableArrayList(getFavorites()); | ||
} catch (IOException e) { | ||
log.error("Failed to load favorite maps cache", e); | ||
} | ||
|
||
} | ||
|
||
|
||
public void queueComboAction(ActionEvent event) { | ||
clearUiAndLoadQueue(queueComboBox.getValue()); | ||
} | ||
|
@@ -153,6 +171,24 @@ private void clearUI() { | |
|
||
|
||
private void loadMatchMakerQueue(MatchmakerQueue matchmakerQueue) { | ||
zoomInButton.setOnAction((event) -> { | ||
|
||
if (selectedMap == null) return; | ||
ZoomInMapWindow zoomInMapWindow = uiService.loadFxml("ui/LadderMapZoomIn.fxml"); | ||
Stage newCategoryDialog = new Stage(); | ||
newCategoryDialog.setTitle("Zoom In"); | ||
Scene scene = new Scene(zoomInMapWindow.getRoot()); | ||
newCategoryDialog.setScene(scene); | ||
|
||
ImageView imageView = (ImageView) scene.getRoot().getChildrenUnmodifiable().get(0); | ||
if (selectedMap != null) imageView.setImage(new Image(selectedMap.getValue().getMapVersion().getThumbnailUrlLarge().toString())); | ||
imageView.setFitWidth(1000); | ||
imageView.setFitHeight(1000); | ||
|
||
newCategoryDialog.showAndWait(); | ||
|
||
}); | ||
|
||
List<MatchmakerQueueMapPool> brackets = mapService.getListOfBracketsInMatchmakerQueue(matchmakerQueue); | ||
List<MapPoolAssignment> mapPoolAssignments = mapService.getListOfMapsInBrackets(brackets); | ||
List<MatchmakerQueueMapPoolFX> bracketsFX = matchmakerQueueMapPoolMapper.mapToFx(brackets); | ||
|
@@ -182,10 +218,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.clear(); | ||
})); | ||
addBracketController.getRoot().addColumn(3, button); | ||
|
||
bindListViewSelectionToSelectedMapProperty(listViewController.mapListView); | ||
bindSelectedMapPropertyToAddRemoveButtons(bracketAssignments, addBracketController, bracketFX.getMapPool()); | ||
} | ||
|
||
uploadToDatabaseButton.setOnAction(event -> { | ||
List<MapPoolAssignment> oldMapPoolAssignments = mapService.getListOfMapsInBrackets(brackets); | ||
List<MapPoolAssignmentFX> oldMapPoolAssignmentsFX = mapPoolAssignmentMapper.mapToFX(oldMapPoolAssignments); | ||
|
@@ -229,7 +271,7 @@ private void bindSelectedMapPropertyToImageView() { | |
} catch (IOException e) { | ||
log.warn("Could not load generated map icon", e); | ||
} | ||
mapParamsLabel.setText(String.format("Version: %s\nSpawns: %d\nSize: %d\n", | ||
mapParamsLabel.setText(String.format(" \nVersion: %s\nSpawns: %d\nSize: %d\n", | ||
neroxisParams.getVersion(), | ||
neroxisParams.getSpawns(), | ||
neroxisParams.getSize())); | ||
|
@@ -306,17 +348,49 @@ public void refresh() { | |
selectedMap.setValue(currentMap); | ||
} | ||
|
||
public List<Map> filterByFavorites(List<Map> in) { | ||
List<Map> out = new ArrayList<>(); | ||
|
||
return in.stream() | ||
.filter(map -> favoritesCache.contains(Integer.parseInt(map.getId()))) | ||
.toList(); | ||
} | ||
|
||
public void onSearchMapVault() { | ||
mapVaultView.getRoot().getChildren().clear(); | ||
mapVaultView.getSortOrder().clear(); | ||
String mapNamePattern = null; | ||
|
||
if (filterByMapNameCheckBox.isSelected()) { | ||
mapNamePattern = mapNamePatternTextField.getText(); | ||
} | ||
|
||
ViewHelper.fillMapTreeView(mapVaultView, | ||
mapService.findMaps(mapNamePattern).stream()); | ||
Task<Void> task = new Task<Void>() { | ||
@Override | ||
protected Void call() throws Exception { | ||
String mapNamePattern = mapNamePatternTextField.getText(); | ||
|
||
if (mapNamePattern.length() > 0) { | ||
mapNamePattern = '*' + mapNamePattern + '*'; | ||
} | ||
|
||
List<Map> maps = mapService.findMaps(mapNamePattern); | ||
|
||
List<Map> filteredMaps; | ||
|
||
if (filterByFavorites.isSelected()) { | ||
maps = filterByFavorites(maps); | ||
} | ||
|
||
cachedMaps.clear(); | ||
cachedMaps.addAll(maps); | ||
|
||
List<Map> finalMaps = maps; | ||
Platform.runLater( () -> { | ||
mapVaultView.getRoot().getChildren().clear(); | ||
mapVaultView.getSortOrder().clear(); | ||
ViewHelper.fillMapTreeView(mapVaultView, finalMaps.stream()); | ||
}); | ||
return null; | ||
} | ||
}; | ||
|
||
new Thread(task).start(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
|
||
} | ||
|
||
public void onGeneratedMapButton() { | ||
|
@@ -329,4 +403,78 @@ public void onGeneratedMapButton() { | |
selectedMap.setValue(mapPoolAssignmentMapper.map(mapPoolAssignment)); | ||
} | ||
|
||
public List<Integer> getFavorites() throws IOException { | ||
List<Integer> list = new ArrayList<>(); | ||
|
||
File pwd = new File("").getAbsoluteFile(); | ||
File file = new File(pwd.getAbsoluteFile() + File.separator + "favoriteMaps.txt"); | ||
|
||
if (!file.exists()) { | ||
file.createNewFile(); | ||
} | ||
Comment on lines
+409
to
+414
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here is a reusable code block that also supports the new Java IO packages: public List<Integer> getFavorites() throws IOException {
try(var lines = Files.lines(getOrCreateFavoriteMapsPath())) {
return lines
.filter(line -> !line.startsWith("#"))
.filter(line -> !line.trim().isEmpty())
.map(Integer::parseInt)
.toList();
} catch (IOException|NumberFormatException e) {
log.error("Could not read favorite maps. Falling back to empty list.", e);
return List.of();
}
} Gimmick:allows comments just for fun. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
||
Scanner reader = new Scanner(file); | ||
while (reader.hasNextLine()) { | ||
String line = reader.nextLine(); | ||
if (line != null && line.length() > 0) { | ||
list.add(Integer.parseInt(line)); | ||
} | ||
} | ||
reader.close(); | ||
return list; | ||
} | ||
|
||
public void addFavorite(MapTableItemAdapter mapTableItemAdapter) { | ||
if (favoritesCache.contains(Integer.parseInt(mapTableItemAdapter.getId()))) return; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This whole integer parsing is EVERYWHERE. It seems much simpler to just use string everywhere. |
||
|
||
int id = Integer.parseInt(mapTableItemAdapter.getId()); | ||
|
||
favoritesCache.add(id); | ||
|
||
try { | ||
File pwd = new File("").getAbsoluteFile(); | ||
File file = new File(pwd.getAbsoluteFile() + File.separator + "favoriteMaps.txt"); | ||
if (!file.exists()) { | ||
file.createNewFile(); | ||
} | ||
|
||
BufferedWriter writer = new BufferedWriter(new FileWriter(file, true)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. try(var writer = Files.newBufferedWriter(getOrCreateFavoriteMapsPath()))
{...} |
||
writer.append(String.valueOf(id) + "\n"); | ||
writer.close(); | ||
} catch (IOException e) { | ||
log.error("Failed to write to favorite maps cache", e); | ||
} | ||
onSearchMapVault(); | ||
} | ||
|
||
public void removeFavorite(MapTableItemAdapter mapTableItemAdapter) { | ||
int id = Integer.parseInt(mapTableItemAdapter.getId()); | ||
|
||
favoritesCache.removeAll(Arrays.asList(id)); | ||
|
||
try { | ||
File pwd = new File("").getAbsoluteFile(); | ||
File file = new File(pwd.getAbsoluteFile() + File.separator + "favoriteMaps.txt"); | ||
if (!file.exists()) { | ||
file.createNewFile(); | ||
} | ||
List<String> list = new ArrayList<>(); | ||
|
||
BufferedWriter writer = new BufferedWriter(new FileWriter(file, false)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as above |
||
|
||
for (Integer line : favoritesCache) { | ||
writer.write(line + "\n"); | ||
} | ||
|
||
writer.close(); | ||
} catch (IOException e) { | ||
log.error("Failed to write to favorite maps cache", e); | ||
} | ||
onSearchMapVault(); | ||
} | ||
|
||
public boolean isMapFavorite(int id) { | ||
return favoritesCache.contains(id); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes no sense, please remove.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is dumb, i wonder why i did this lol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I remove this the favorite button breaks. I have no idea why, the getThis method is never called.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe @Sheikah45 can help here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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