Skip to content

Commit

Permalink
Leaderboard improvements (#3181)
Browse files Browse the repository at this point in the history
* Sort leaderboards by player size

* Make season selectable

* Make UI functional

* Improve visuals

* Fix test

* Fix NPE in rendering thread
  • Loading branch information
BlackYps authored May 31, 2024
1 parent 27de3f6 commit 8d36cbb
Show file tree
Hide file tree
Showing 10 changed files with 74 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@
import com.faforever.client.fx.FxApplicationThreadExecutor;
import com.faforever.client.fx.JavaFxUtil;
import com.faforever.client.fx.NodeController;
import com.faforever.client.fx.ToStringOnlyConverter;
import com.faforever.client.i18n.I18n;
import com.faforever.client.notification.NotificationService;
import com.faforever.client.player.PlayerService;
import com.faforever.client.util.TimeService;
import javafx.beans.property.ObjectProperty;
import javafx.beans.property.SimpleObjectProperty;
import javafx.collections.FXCollections;
import javafx.scene.control.ComboBox;
import javafx.scene.control.Label;
import javafx.scene.layout.Pane;
import javafx.scene.layout.StackPane;
Expand Down Expand Up @@ -42,26 +45,29 @@ public class LeaderboardController extends NodeController<StackPane> {
public StackPane leaderboardRoot;
public Pane connectionProgressPane;
public Pane contentPane;
public Label seasonLabel;
public Label seasonDateLabel;
public ComboBox<LeagueSeason> seasonPicker;
public LeaderboardRankingsController leaderboardRankingsController;
public LeaderboardPlayerDetailsController leaderboardPlayerDetailsController;
public LeaderboardDistributionController leaderboardDistributionController;

private final ObjectProperty<LeagueSeason> leagueSeason = new SimpleObjectProperty<>();
private final ObjectProperty<List<LeagueSeason>> leagueSeasons = new SimpleObjectProperty<>(List.of());

@Override
protected void onInitialize() {
JavaFxUtil.bindManagedToVisible(contentPane, connectionProgressPane);
connectionProgressPane.visibleProperty().bind(contentPane.visibleProperty().not());

seasonLabel.textProperty()
.bind(leagueSeason.map(seasonBean -> i18n.getOrDefault(seasonBean.nameKey(),
"leaderboard.season.%s".formatted(
seasonBean.nameKey()),
seasonBean.seasonNumber()))
.map(String::toUpperCase)
.when(showing));
seasonPicker.setConverter(new ToStringOnlyConverter<>(
seasonBean -> i18n.get("leaderboard.season", seasonBean.seasonNumber())));

leagueSeason.bind(seasonPicker.getSelectionModel().selectedItemProperty());

leagueSeasons.map(FXCollections::observableList).when(showing).subscribe(seasons -> {
seasonPicker.getItems().setAll(seasons);
seasonPicker.getSelectionModel().selectFirst();
});

seasonDateLabel.textProperty().bind(leagueSeason.map(seasonBean -> {
String startDate = timeService.asDate(seasonBean.startDate(), FormatStyle.MEDIUM);
Expand Down Expand Up @@ -135,8 +141,8 @@ protected void onInitialize() {
});
}

public void setLeagueSeason(LeagueSeason leagueSeason) {
this.leagueSeason.set(leagueSeason);
public void setLeagueSeasons(List<LeagueSeason> leagueSeasons) {
this.leagueSeasons.set(leagueSeasons);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ private void updateSubdivisions(List<Subdivision> subdivisions) {

updateHighlightedSubdivision();

ratingDistributionChart.setAnimated(false);
ratingDistributionChart.setData(FXCollections.observableArrayList(series));
updateChartData(leagueEntries.getValue());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,26 +102,19 @@ public Flux<LeagueSeason> getActiveSeasons() {
}

@Cacheable(value = CacheNames.LEAGUE, sync = true)
public Mono<LeagueSeason> getLatestSeason(League league) {
public Flux<LeagueSeason> getSeasons(League league) {
ElideNavigatorOnCollection<com.faforever.commons.api.dto.LeagueSeason> navigator = ElideNavigator.of(
com.faforever.commons.api.dto.LeagueSeason.class)
.collection()
.setFilter(
qBuilder().intNum(
"league.id")
.eq(league.id())
.and()
.instant(
"startDate")
.before(
OffsetDateTime.now()
.toInstant(),
false))
.eq(league.id()))
.addSortingRule(
"startDate",
false);
return fafApiAccessor.getMany(navigator)
.next().map(leaderboardMapper::map);
return fafApiAccessor.getMany(navigator).map(leaderboardMapper::map)
.cache();
}

@Cacheable(value = CacheNames.LEAGUE_ENTRIES, sync = true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@
import org.springframework.stereotype.Component;
import reactor.core.publisher.Mono;

import java.util.Comparator;
import java.util.HashMap;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.stream.Collectors;


@Slf4j
Expand Down Expand Up @@ -61,17 +63,17 @@ protected void onInitialize() {
});

leaderboardService.getLeagues()
.map(league -> {
.collectSortedList(Comparator.comparing(League::technicalName))
.map(leagues -> leagues.stream().map(league -> {
String buttonText = i18n.getOrDefault(league.technicalName(), String.format("leaderboard.%s",
league.technicalName()));
league.technicalName()));
ToggleButton toggleButton = new ToggleButton(buttonText);
toggleButton.setToggleGroup(navigation);
toggleButton.getStyleClass().add("main-navigation-button");
toggleLeagueMap.put(toggleButton, league);
return toggleButton;
})
}).collect(Collectors.toList()))
.switchIfEmpty(Mono.error(new IllegalStateException("No leagues loaded")))
.collectList()
.doOnNext(leagueButtons -> {
League lastLeagueTab = navigationHandler.getLastLeagueTab();
Toggle startingToggle = toggleLeagueMap.entrySet()
Expand All @@ -93,9 +95,10 @@ protected void onInitialize() {

private void setLeague(League league) {
navigationHandler.setLastLeagueTab(league);
leaderboardService.getLatestSeason(league)
leaderboardService.getSeasons(league)
.collectList()
.publishOn(fxApplicationThreadExecutor.asScheduler())
.subscribe(leaderboardController::setLeagueSeason, throwable -> {
.subscribe(leaderboardController::setLeagueSeasons, throwable -> {
log.error("Error while loading seasons", throwable);
notificationService.addImmediateErrorNotification(throwable,
"leaderboard.failedToLoadLeaderboards");
Expand Down
11 changes: 1 addition & 10 deletions src/main/resources/i18n/messages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -1156,10 +1156,7 @@ leaderboard.1v1_league = 1v1 League
leaderboard.2v2_league = 2v2 League
leaderboard.4v4_full_share_league = 4v4 League
leaderboard.4v4_share_until_death_league = 4v4 No Share League
leaderboard.season.1v1_league = 1v1 Season {0}
leaderboard.season.2v2_league = 2v2 Season {0}
leaderboard.season.4v4_fs_league = 4v4 Season {0}
leaderboard.season.4v4_sud_league = 4v4 Season {0}
leaderboard.season = Season {0}
leaderboard.seasonDate = {0} – {1}
leaderboard.searchPrompt = Search player
leaderboard.score = Score
Expand All @@ -1173,10 +1170,6 @@ leagues.divisionName.diamond = Diamond
leagues.divisionName.master = Master
leagues.divisionName.grandmaster = Grandmaster
teammatchmaking.inPlacement = Unlisted
leaderboard.season.1v1_season = 1v1 Season {0}
leaderboard.season.2v2_season = 2v2 Season {0}
leaderboard.season.4v4_fs_season = 4v4 Season {0}
leaderboard.season.4v4_sud_season = 4v4 Season {0}
chat.category.self = Me
settings.data.mirrorURLs = Download Mirrors
settings.data.mirrorURLs.description = Add any mirrors here that you wish to use for downloading game patches instead of downloading from the main FAF server. This can help prevent extremely long download times.
Expand Down Expand Up @@ -1293,8 +1286,6 @@ ignoreWarning = Ignore warning
replay.replayRunning = Replay could not be started because another replay is already running.
teammatchmaking.queue.tmm3v3 = 3v3
leaderboard.3v3_league = 3v3 League
leaderboard.season.3v3_league = 3v3 Season {0}
leaderboard.season.3v3_season = 3v3 Season {0}
teammatchmaking.couldNotStart = Could not join queues. See error details below.
teammatchmaking.couldNotJoinQueue = Could not join queue {0}. See error details below.
teammatchmaking.match.searching = Searching...
Expand Down
5 changes: 3 additions & 2 deletions src/main/resources/theme/leaderboard/leaderboard.fxml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>

<?import javafx.geometry.Insets?>
<?import javafx.scene.control.ComboBox?>
<?import javafx.scene.control.Label?>
<?import javafx.scene.control.ProgressIndicator?>
<?import javafx.scene.layout.HBox?>
Expand All @@ -14,10 +15,10 @@
<children>
<VBox prefWidth="600">
<children>
<Label fx:id="seasonLabel" styleClass="season"/>
<ComboBox fx:id="seasonPicker" styleClass="season-selector" promptText="Seasons"/>
<Label fx:id="seasonDateLabel" styleClass="seasonDate">
<padding>
<Insets bottom="20.0"/>
<Insets bottom="20.0" left="10.0"/>
</padding>
</Label>
<fx:include fx:id="leaderboardRankings" source="leaderboard_rankings.fxml"/>
Expand Down
41 changes: 33 additions & 8 deletions src/main/resources/theme/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -824,16 +824,12 @@
-fx-font-family: 'Source Sans Pro';
}

.season {
-fx-text-fill: #e0e0e0;
-fx-font-family: 'Source Sans Pro Semibold';
.season-selector {
-fx-padding: 0;
-fx-border-width: 0;
-fx-font-size: 30px;
}

.seasonDate {
-fx-text-fill: #777;
-fx-background-color: #1e1e1ebb;
-fx-font-family: 'Source Sans Pro Semibold';
-fx-font-size: 20px;
}

.division-selector {
Expand All @@ -843,6 +839,35 @@
-fx-font-family: 'Source Sans Pro Semibold';
}

.division-selector:showing,
.season-selector:showing {
-fx-border-width: 0;
}

.division-selector .list-view,
.season-selector .list-view {
-fx-border-width: 0;
}

.season-selector .list-view .list-cell{
-fx-text-fill: #e0e0e0;
}

.season-selector .list-cell{
-fx-text-fill: #e0e0e0;
-fx-padding: 0 10;
}

.season-selector .combo-box-popup > .list-view > .virtual-flow > .clipped-container > .sheet > .list-cell {
-fx-padding: 0 10;
}

.seasonDate {
-fx-text-fill: #777;
-fx-font-family: 'Source Sans Pro Semibold';
-fx-font-size: 20px;
}

.subDivisionTabPane {
-fx-font-family: 'Source Sans Pro Semibold';
-fx-font-size: 14px;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,9 @@ public void setUp() throws Exception {

@Test
public void testSetSeason() {
runOnFxThreadAndWait(() -> instance.setLeagueSeason(season));
runOnFxThreadAndWait(() -> instance.setLeagueSeasons(List.of(season)));

assertEquals("SEASONNAME 1", instance.seasonLabel.getText());
assertEquals(season, instance.seasonPicker.getSelectionModel().getSelectedItem());
verifyNoInteractions(notificationService);

assertEquals(season, leagueSeasonProperty.get());
Expand All @@ -146,7 +146,7 @@ public void testInitializeWithSeasonError() {
when(leaderboardService.getActiveEntries(season)).thenReturn(Flux.error(new FakeTestException()));
when(leaderboardService.getLeagueEntryForPlayer(player, season)).thenReturn(Mono.error(new FakeTestException()));

runOnFxThreadAndWait(() -> instance.setLeagueSeason(season));
runOnFxThreadAndWait(() -> instance.setLeagueSeasons(List.of(season)));

verify(notificationService).addImmediateErrorNotification(any(), eq("leaderboard.failedToLoadEntry"));
verify(notificationService).addImmediateErrorNotification(any(), eq("leaderboard.failedToLoadEntries"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,11 @@ public void testGetActiveSeasons() {
}

@Test
public void testGetLatestSeason() {
public void testGetSeasons() {
when(fafApiAccessor.getMany(any())).thenReturn(Flux.empty());
League league = Instancio.create(League.class);

StepVerifier.create(instance.getLatestSeason(league)).verifyComplete();
StepVerifier.create(instance.getSeasons(league)).verifyComplete();

verify(fafApiAccessor).getMany(argThat(ElideMatchers.hasSort("startDate", false)));
verify(fafApiAccessor).getMany(argThat(ElideMatchers.filterPresent()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import org.mockito.InjectMocks;
import org.mockito.Mock;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
Expand Down Expand Up @@ -54,7 +53,7 @@ public class LeaderboardsControllerTest extends PlatformTest {
@BeforeEach
public void setUp() throws Exception {
when(leaderboardService.getLeagues()).thenReturn(Flux.just(Instancio.create(League.class)));
when(leaderboardService.getLatestSeason(any())).thenReturn(Mono.just(Instancio.create(LeagueSeason.class)));
when(leaderboardService.getSeasons(any())).thenReturn(Flux.just(Instancio.create(LeagueSeason.class)));
when(i18n.getOrDefault(anyString(), anyString())).thenReturn("league");

loadFxml("theme/leaderboard/leaderboards.fxml", clazz -> {
Expand Down Expand Up @@ -101,7 +100,7 @@ public void testInitializeWithLeagueError() {

@Test
public void testInitializeWithSeasonError() {
when(leaderboardService.getLatestSeason(any())).thenReturn(Mono.error(new FakeTestException()));
when(leaderboardService.getSeasons(any())).thenReturn(Flux.error(new FakeTestException()));

reinitialize(instance);

Expand Down

0 comments on commit 8d36cbb

Please sign in to comment.