Skip to content

Commit

Permalink
Don't recreate games container every switch (#2962)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Sheikah45
Sheikah45 committed Apr 23, 2023
1 parent 8a72c8f commit 2cb56a7
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 83 deletions.
24 changes: 24 additions & 0 deletions src/main/java/com/faforever/client/fx/ToStringOnlyConverter.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.faforever.client.fx;

import javafx.util.StringConverter;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;

import java.util.function.Function;

@RequiredArgsConstructor
public class ToStringOnlyConverter<T> extends StringConverter<T> {

@NonNull
private final Function<T, String> toStringFunction;

@Override
public String toString(T object) {
return toStringFunction.apply(object);
}

@Override
public T fromString(String string) {
throw new UnsupportedOperationException();
}
}
67 changes: 23 additions & 44 deletions src/main/java/com/faforever/client/game/CustomGamesController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.faforever.client.filter.CustomGamesFilterController;
import com.faforever.client.fx.AbstractViewController;
import com.faforever.client.fx.JavaFxUtil;
import com.faforever.client.fx.ToStringOnlyConverter;
import com.faforever.client.game.GamesTilesContainerController.TilesSortingOrder;
import com.faforever.client.i18n.I18n;
import com.faforever.client.main.event.HostGameEvent;
Expand All @@ -20,6 +21,7 @@
import com.google.common.eventbus.Subscribe;
import javafx.beans.binding.Bindings;
import javafx.beans.binding.IntegerBinding;
import javafx.beans.value.ObservableValue;
import javafx.collections.transformation.FilteredList;
import javafx.geometry.Bounds;
import javafx.scene.Node;
Expand All @@ -34,7 +36,6 @@
import javafx.scene.layout.StackPane;
import javafx.stage.Popup;
import javafx.stage.PopupWindow.AnchorLocation;
import javafx.util.StringConverter;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.jetbrains.annotations.Nullable;
Expand All @@ -60,6 +61,8 @@ public class CustomGamesController extends AbstractViewController<Node> {
private final Preferences preferences;

public GameDetailController gameDetailController;
public GamesTableController gamesTableController;
public GamesTilesContainerController gamesTilesContainerController;

public ToggleButton tableButton;
public ToggleButton tilesButton;
Expand All @@ -76,48 +79,31 @@ public class CustomGamesController extends AbstractViewController<Node> {
private FilteredList<GameBean> filteredGames;
private CustomGamesFilterController customGamesFilterController;
private Popup gameFilterPopup;
private GamesTableController gamesTableController;
private GamesTilesContainerController gamesTilesContainerController;

private final Predicate<GameBean> openGamesPredicate = game -> game.getStatus() == GameStatus.OPEN && game.getGameType() == GameType.CUSTOM;

public void initialize() {
JavaFxUtil.bindManagedToVisible(chooseSortingTypeChoiceBox);
JavaFxUtil.bindManagedToVisible(chooseSortingTypeChoiceBox, gameDetailPane);
ObservableValue<Boolean> showing = JavaFxUtil.showingProperty(getRoot());

initializeFilterController();

JavaFxUtil.bind(createGameButton.disableProperty(), gameService.gameRunningProperty());
getRoot().sceneProperty().addListener((observable, oldValue, newValue) -> {
if (newValue == null) {
createGameButton.disableProperty().unbind();
}
});
createGameButton.disableProperty().bind(gameService.gameRunningProperty().when(showing));

chooseSortingTypeChoiceBox.getItems().addAll(TilesSortingOrder.values());
chooseSortingTypeChoiceBox.setConverter(new StringConverter<>() {
@Override
public String toString(TilesSortingOrder tilesSortingOrder) {
return tilesSortingOrder == null ? "null" : i18n.get(tilesSortingOrder.getDisplayNameKey());
}

@Override
public TilesSortingOrder fromString(String string) {
throw new UnsupportedOperationException("Not supported");
}
});
chooseSortingTypeChoiceBox.setConverter(new ToStringOnlyConverter<>(tilesSortingOrder -> tilesSortingOrder == null ? "null" : i18n.get(tilesSortingOrder.getDisplayNameKey())));

filteredGames = new FilteredList<>(gameService.getGames());
filteredGames.predicateProperty().bind(customGamesFilterController.predicateProperty());

IntegerBinding filteredGameCount = Bindings.size(filteredGames);
IntegerBinding gameCount = Bindings.size(gameService.getGames().filtered(openGamesPredicate));
JavaFxUtil.bind(filteredGamesCountLabel.visibleProperty(), filteredGameCount.isNotEqualTo(gameCount));
filteredGamesCountLabel.visibleProperty().bind(filteredGameCount.isNotEqualTo(gameCount).when(showing));

filteredGamesCountLabel.textProperty()
.bind(Bindings.createStringBinding(() -> {
int numGames = gameCount.intValue();
return i18n.get("filteredOutItemsCount", numGames - filteredGameCount.intValue(), numGames);
}, gameCount, filteredGameCount));
filteredGamesCountLabel.textProperty().bind(Bindings.createStringBinding(() -> {
int numGames = gameCount.intValue();
return i18n.get("filteredOutItemsCount", numGames - filteredGameCount.intValue(), numGames);
}, gameCount, filteredGameCount).when(showing));

if (tilesButton.getId().equals(preferences.getGamesViewMode())) {
viewToggleGroup.selectToggle(tilesButton);
Expand All @@ -126,23 +112,30 @@ public TilesSortingOrder fromString(String string) {
viewToggleGroup.selectToggle(tableButton);
tableButton.getOnAction().handle(null);
}

preferences.gamesViewModeProperty()
.bind(viewToggleGroup.selectedToggleProperty()
.map(toggle -> (ToggleButton) toggle)
.map(ToggleButton::getId)
.when(showing));

viewToggleGroup.selectedToggleProperty().addListener((observable, oldValue, newValue) -> {
if (newValue == null) {
if (oldValue != null) {
viewToggleGroup.selectToggle(oldValue);
} else {
viewToggleGroup.selectToggle(viewToggleGroup.getToggles().get(0));
}
return;
}
preferences.setGamesViewMode(((ToggleButton) newValue).getId());
});

gameDetailPane.visibleProperty().bind(toggleGameDetailPaneButton.selectedProperty());
gameDetailPane.managedProperty().bind(gameDetailPane.visibleProperty());

toggleGameDetailPaneButton.selectedProperty().bindBidirectional(preferences.showGameDetailsSidePaneProperty());

gamesTilesContainerController.createTiledFlowPane(filteredGames, chooseSortingTypeChoiceBox);
gamesTableController.initializeGameTable(filteredGames);

eventBus.register(this);
}

Expand Down Expand Up @@ -193,11 +186,7 @@ public Node getRoot() {
}

public void onTableButtonClicked() {
disposeGamesContainer();

gamesTableController = uiService.loadFxml("theme/play/games_table.fxml");
gameDetailController.gameProperty().bind(gamesTableController.selectedGameProperty());
gamesTableController.initializeGameTable(filteredGames);
populateContainer(gamesTableController.getRoot());
}

Expand All @@ -211,12 +200,8 @@ private void populateContainer(Node root) {
}

public void onTilesButtonClicked() {
disposeGamesContainer();

gamesTilesContainerController = uiService.loadFxml("theme/play/games_tiles_container.fxml");
gameDetailController.gameProperty().bind(gamesTilesContainerController.selectedGameProperty());
populateContainer(gamesTilesContainerController.getRoot());
gamesTilesContainerController.createTiledFlowPane(filteredGames, chooseSortingTypeChoiceBox);
}

@Subscribe
Expand All @@ -234,12 +219,6 @@ public void onMapGeneratedEvent(MapGeneratedEvent event) {
});
}

private void disposeGamesContainer() {
if (gamesTilesContainerController != null) {
gamesTilesContainerController.removeListeners();
}
}

public void onFilterButtonClicked() {
if (gameFilterPopup == null) {
gameFilterPopup = PopupUtil.createPopup(AnchorLocation.CONTENT_TOP_LEFT, customGamesFilterController.getRoot());
Expand Down
13 changes: 2 additions & 11 deletions src/main/java/com/faforever/client/game/GameDetailController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,10 @@
import javafx.beans.binding.Bindings;
import javafx.beans.binding.BooleanExpression;
import javafx.beans.property.BooleanProperty;
import javafx.beans.property.MapProperty;
import javafx.beans.property.ObjectProperty;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.beans.property.SimpleMapProperty;
import javafx.beans.property.SimpleObjectProperty;
import javafx.beans.value.ObservableValue;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import javafx.collections.transformation.SortedList;
import javafx.scene.Node;
import javafx.scene.control.Button;
import javafx.scene.control.Label;
Expand Down Expand Up @@ -117,12 +112,8 @@ public void initialize() {

playTimeTimeline.setCycleCount(Timeline.INDEFINITE);

root.parentProperty().addListener(observable -> {
if (!(root.getParent() instanceof Pane)) {
return;
}
root.maxWidthProperty().bind(((Pane) root.getParent()).widthProperty());
});
root.maxWidthProperty()
.bind(root.parentProperty().flatMap(parent -> parent instanceof Pane pane ? pane.widthProperty() : null));

root.visibleProperty().bind(game.isNotNull());
gameTitleLabel.textProperty()
Expand Down
7 changes: 1 addition & 6 deletions src/main/java/com/faforever/client/game/GamesTilesContainerController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public GamesTilesContainerController(UiService uiService, PlayerService playerSe
this.playerService = playerService;
this.preferences = preferences;

averageRatingComparator = Comparator.comparingDouble(o -> this.playerService.getAverageRatingForGame((GameBean) o.getUserData()));
averageRatingComparator = Comparator.comparingDouble(tile -> this.playerService.getAverageRatingForGame((GameBean) tile.getUserData()));
}

private void sortNodes() {
Expand Down Expand Up @@ -187,11 +187,6 @@ public Node getRoot() {
return tiledScrollPane;
}

public void removeListeners() {
JavaFxUtil.removeListener(games, gameListChangeListener);
JavaFxUtil.removeListener(sortingTypeChoiceBox.getSelectionModel().selectedItemProperty(), sortingListener);
}

public enum TilesSortingOrder {
PLAYER_DES("tiles.comparator.playersDescending"),
PLAYER_ASC("tiles.comparator.playersAscending"),
Expand Down
14 changes: 10 additions & 4 deletions src/main/resources/theme/play/custom_games.fxml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,16 @@
<Insets bottom="10.0" left="10.0" top="10.0" />
</padding>
</HBox>
<AnchorPane fx:id="gameViewContainer" maxHeight="1.7976931348623157E308" maxWidth="1.7976931348623157E308" VBox.vgrow="ALWAYS">
<padding>
<Insets left="10.0" right="10.0" />
</padding></AnchorPane>
<AnchorPane fx:id="gameViewContainer" maxHeight="1.7976931348623157E308"
maxWidth="1.7976931348623157E308" VBox.vgrow="ALWAYS">
<padding>
<Insets left="10.0" right="10.0"/>
</padding>
<children>
<fx:include fx:id="gamesTilesContainer" source="games_tiles_container.fxml"/>
<fx:include fx:id="gamesTable" source="games_table.fxml"/>
</children>
</AnchorPane>
</children>
</VBox>
<ScrollPane fx:id="gameDetailPane" fitToWidth="true" minWidth="200.0" prefWidth="300.0" styleClass="game-detail-pane">
Expand Down
6 changes: 6 additions & 0 deletions src/test/java/com/faforever/client/game/CustomGamesControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ public void setUp() throws Exception {
when(gameDetailController.getRoot()).thenReturn(new Pane());

loadFxml("theme/play/custom_games.fxml", clazz -> {
if (clazz == GamesTableController.class) {
return gamesTableController;
}
if (clazz == GamesTilesContainerController.class) {
return gamesTilesContainerController;
}
if (clazz == GameDetailController.class) {
return gameDetailController;
}
Expand Down
29 changes: 11 additions & 18 deletions src/test/java/com/faforever/client/test/ElideMatchers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import com.faforever.commons.api.elide.ElideNavigatorOnId;
import com.github.rutledgepaulv.qbuilders.conditions.Condition;
import com.github.rutledgepaulv.qbuilders.visitors.RSQLVisitor;
import lombok.Value;
import org.hamcrest.Matcher;
import org.hamcrest.Matchers;
import org.mockito.ArgumentMatcher;
Expand All @@ -27,7 +26,8 @@ public final class ElideMatchers {
public static final String INCLUDE = "include=";
public static final String SORT = "sort=";

public static <T extends ElideEntity> ArgumentMatcher<ElideNavigator<T>> hasDtoClass(Class<? extends ElideEntity> clazz) {
public static <T extends ElideEntity> ArgumentMatcher<ElideNavigator<T>> hasDtoClass(
Class<? extends ElideEntity> clazz) {
return builder -> builder != null && builder.getDtoClass().equals(clazz);
}

Expand All @@ -44,7 +44,9 @@ public static ElideParamListMatcher<ElideNavigatorOnCollection<?>> hasSort(Strin
}

public static ElideParamListMatcher<?> hasIncludes(List<String> includes) {
return new ElideParamListMatcher<>(containsInAnyOrder(includes.stream().map(Matchers::equalTo).collect(Collectors.toList())), INCLUDE);
return new ElideParamListMatcher<>(containsInAnyOrder(includes.stream()
.map(Matchers::equalTo)
.collect(Collectors.toList())), INCLUDE);
}

public static ElideParamMatcher hasPageSize(int pageSize) {
Expand All @@ -67,42 +69,33 @@ public static ElideRelationshipMatcher hasRelationship(String relationship) {
return new ElideRelationshipMatcher(matchesPattern(String.format("^/data/\\w+/\\d+/%s.*", relationship)));
}

@Value
private static class ElideParamMatcher implements ArgumentMatcher<ElideNavigatorOnCollection<?>> {
Matcher<String> matcher;

private record ElideParamMatcher(Matcher<String> matcher) implements ArgumentMatcher<ElideNavigatorOnCollection<?>> {
@Override
public boolean matches(ElideNavigatorOnCollection<?> argument) {
return argument != null && matcher.matches(argument.build());
}
}

@Value
private static class ElideIdMatcher implements ArgumentMatcher<ElideNavigatorOnId<?>> {
Matcher<String> matcher;

private record ElideIdMatcher(Matcher<String> matcher) implements ArgumentMatcher<ElideNavigatorOnId<?>> {
@Override
public boolean matches(ElideNavigatorOnId<?> argument) {
return argument != null && matcher.matches(argument.build());
}
}

@Value
private static class ElideRelationshipMatcher implements ArgumentMatcher<ElideNavigatorOnCollection<?>> {
Matcher<String> matcher;

private record ElideRelationshipMatcher(
Matcher<String> matcher) implements ArgumentMatcher<ElideNavigatorOnCollection<?>> {
@Override
public boolean matches(ElideNavigatorOnCollection<?> argument) {
return argument != null && matcher.matches(argument.build());
}
}

@Value
private static class ElideParamListMatcher<T extends ElideEndpointBuilder<?>> implements ArgumentMatcher<T> {
private record ElideParamListMatcher<T extends ElideEndpointBuilder<?>>(Matcher<Iterable<? extends String>> matcher,
String paramPrefix) implements ArgumentMatcher<T> {
private static final String PARAM_DELIMITER = "&";
private static final String PROPERTY_DELIMITER = ",";
Matcher<Iterable<? extends String>> matcher;
String paramPrefix;

@Override
public boolean matches(T argument) {
Expand Down

0 comments on commit 2cb56a7

Please sign in to comment.