From fc66eddfdd44822201aaabfbcb473404def21ff7 Mon Sep 17 00:00:00 2001 From: alex Date: Sun, 25 Sep 2022 12:09:57 +0200 Subject: [PATCH] Address review --- .../com/faforever/client/game/GameService.java | 17 +++++++++-------- .../client/remote/FafServerAccessor.java | 2 +- .../game/IsReadyForGameController.java | 15 +++++++++------ .../tournament/game/TournamentGameService.java | 12 ++++++------ .../ui/progress/ProgressCircleIndicator.java | 4 ++-- .../ui/progress/RingProgressIndicator.java | 14 +++++++------- .../ui/progress/RingProgressIndicatorSkin.java | 2 +- .../faforever/client/game/GameServiceTest.java | 18 +++++++++--------- .../client/remote/ServerAccessorTest.java | 2 +- .../game/TournamentGameServiceTest.java | 2 +- 10 files changed, 46 insertions(+), 42 deletions(-) diff --git a/src/main/java/com/faforever/client/game/GameService.java b/src/main/java/com/faforever/client/game/GameService.java index 5c3c3c839f..830e01d03f 100644 --- a/src/main/java/com/faforever/client/game/GameService.java +++ b/src/main/java/com/faforever/client/game/GameService.java @@ -41,6 +41,7 @@ import com.faforever.client.util.ConcurrentUtil; import com.faforever.client.util.MaskPatternLayout; import com.faforever.commons.lobby.GameInfo; +import com.faforever.commons.lobby.GameLaunchResponse; import com.faforever.commons.lobby.GameStatus; import com.faforever.commons.lobby.GameVisibility; import com.google.common.annotations.VisibleForTesting; @@ -514,30 +515,30 @@ public CompletableFuture startSearchMatchmaker() { return matchmakerFuture; } - matchmakerFuture = listenToServerInitiatedGame(FAF.getTechnicalName()); + matchmakerFuture = listenForServerInitiatedGame(FAF.getTechnicalName()); return matchmakerFuture; } - public CompletableFuture startListeningToTournamentGame(String featuredModTechnicalName) { + public CompletableFuture startListeningForTournamentGame(String featuredModTechnicalName) { if (isRunning()) { log.info("Game is running, ignoring tournament search request"); notificationService.addImmediateWarnNotification("game.gameRunning"); return completedFuture(null); } - return listenToServerInitiatedGame(featuredModTechnicalName); + return listenForServerInitiatedGame(featuredModTechnicalName); } - private CompletableFuture listenToServerInitiatedGame(String featuredModTechnicalName) { + private CompletableFuture listenForServerInitiatedGame(String featuredModTechnicalName) { if (!preferencesService.isGamePathValid()) { CompletableFuture gameDirectoryFuture = postGameDirectoryChooseEvent(); return gameDirectoryFuture.thenCompose(path -> startSearchMatchmaker()); } - log.info("Listening to server made game has been started"); + log.info("Started listening for game launch message"); - final var gameLaunchMessageFuture = fafServerAccessor.getGameLaunchMessage(); - final var matchFuture = modService.getFeaturedMod(featuredModTechnicalName) + final CompletableFuture gameLaunchMessageFuture = fafServerAccessor.getGameLaunchMessageFuture(); + final CompletableFuture matchFuture = modService.getFeaturedMod(featuredModTechnicalName) .thenAccept(featuredModBean -> updateGameIfNecessary(featuredModBean, Set.of())) .thenCompose(aVoid -> gameLaunchMessageFuture) .thenCompose(gameLaunchResponse -> downloadMapIfNecessary(gameLaunchResponse.getMapName()) @@ -563,7 +564,7 @@ private CompletableFuture listenToServerInitiatedGame(String featuredModTe process.destroy(); } } else { - log.warn("Server initiated game could not be started", throwable); + log.warn("Game could not be started", throwable); } } else { log.info("Matchmaker queue exited"); diff --git a/src/main/java/com/faforever/client/remote/FafServerAccessor.java b/src/main/java/com/faforever/client/remote/FafServerAccessor.java index aa9d8840c8..1a4120a875 100644 --- a/src/main/java/com/faforever/client/remote/FafServerAccessor.java +++ b/src/main/java/com/faforever/client/remote/FafServerAccessor.java @@ -212,7 +212,7 @@ public void requestMatchmakerInfo() { lobbyClient.requestMatchmakerInfo(); } - public CompletableFuture getGameLaunchMessage() { + public CompletableFuture getGameLaunchMessageFuture() { return lobbyClient.getEvents() .filter(event -> event instanceof GameLaunchResponse) .next() diff --git a/src/main/java/com/faforever/client/tournament/game/IsReadyForGameController.java b/src/main/java/com/faforever/client/tournament/game/IsReadyForGameController.java index 65ead2b361..b920c522c8 100644 --- a/src/main/java/com/faforever/client/tournament/game/IsReadyForGameController.java +++ b/src/main/java/com/faforever/client/tournament/game/IsReadyForGameController.java @@ -1,6 +1,7 @@ package com.faforever.client.tournament.game; import com.faforever.client.fx.Controller; +import com.faforever.client.fx.JavaFxUtil; import com.faforever.client.i18n.I18n; import com.faforever.client.ui.progress.RingProgressIndicator; import javafx.animation.KeyFrame; @@ -40,11 +41,12 @@ public class IsReadyForGameController implements Controller { private Runnable readyCallback; @Setter private Runnable dismissCallBack; + private boolean clickedReady = false; @Override public void initialize() { - progressIndicator.setProgressLableStringConverter(new StringConverter<>() { + progressIndicator.setProgressLabelStringConverter(new StringConverter<>() { @Override public String toString(Integer object) { return i18n.number(timeLeft); @@ -67,26 +69,26 @@ public void setTimeout(int responseTimeSeconds) { OffsetDateTime start = OffsetDateTime.now(); queuePopTimeUpdater = new Timeline(1, new KeyFrame(javafx.util.Duration.seconds(0), (ActionEvent event) -> { - updateQueue(responseTimeSeconds, start); + updateTimer(responseTimeSeconds, start); }), new KeyFrame(javafx.util.Duration.seconds(1))); queuePopTimeUpdater.setCycleCount(Timeline.INDEFINITE); queuePopTimeUpdater.play(); } - private void updateQueue(int responseTimeSeconds, OffsetDateTime start) { + private void updateTimer(int responseTimeSeconds, OffsetDateTime start) { OffsetDateTime now = OffsetDateTime.now(); Duration timeGone = Duration.between(start, now); final var percent = timeGone.toSeconds() / (double) responseTimeSeconds; this.timeLeft = (int) (responseTimeSeconds - timeGone.toSeconds()); progressIndicator.setProgress((int) (percent * 100)); if (timeLeft <= 0 && queuePopTimeUpdater != null) { - end(); + queuePopTimeUpdater.stop(); + JavaFxUtil.runLater(this::end); } } private void end() { - queuePopTimeUpdater.stop(); - if (isReadyButton.isDisable()) { + if (clickedReady) { isReadyButton.setText(i18n.get("isReady.launching")); } else { dismissCallBack.run(); @@ -96,6 +98,7 @@ private void end() { public void onReady() { readyCallback.run(); isReadyButton.setDisable(true); + clickedReady = true; isReadyButton.setText(i18n.get("isReady.waiting")); } } diff --git a/src/main/java/com/faforever/client/tournament/game/TournamentGameService.java b/src/main/java/com/faforever/client/tournament/game/TournamentGameService.java index de7b649f95..afb3ec932e 100644 --- a/src/main/java/com/faforever/client/tournament/game/TournamentGameService.java +++ b/src/main/java/com/faforever/client/tournament/game/TournamentGameService.java @@ -60,19 +60,18 @@ private void onReadRequest(IsReadyRequest isReadyRequest) { log.info("Tournament game is ready, asking user."); if (notification != null) { log.warn("Tournament ready request ignored because tournament is already in progress."); - respondToReadyRequest(isReadyRequest.getRequestId(), isReadyRequest); return; } uiService.bringMainStageToFront(); - final var controller = initializeIsReadyController(isReadyRequest); + final IsReadyForGameController controller = initializeIsReadyController(isReadyRequest); notification = new ImmediateNotification(i18n.get("isReady.title"), i18n.get("isReady.message", isReadyRequest.getGameName()), Severity.INFO, null, List.of(), controller.getRoot(), false); notificationService.addNotification(notification); - safetyCloseMechanism(); + ensureNotificationCloses(); } - private void safetyCloseMechanism() { + private void ensureNotificationCloses() { final var currentNotification = notification; timer.schedule(new TimerTask() { @Override @@ -98,18 +97,19 @@ private IsReadyForGameController initializeIsReadyController(IsReadyRequest isRe IsReadyForGameController controller = uiService.loadFxml("theme/tournaments/is_ready_for_game.fxml"); controller.setTimeout(isReadyRequest.getResponseTimeSeconds()); controller.setReadyCallback(() -> respondToReadyRequest(isReadyRequest.getRequestId(), isReadyRequest)); - controller.setDismissCallBack(() -> JavaFxUtil.runLater(this::dismissNotification)); + controller.setDismissCallBack(this::dismissNotification); return controller; } private void respondToReadyRequest(String requestId, IsReadyRequest isReadyRequest) { - matchFuture = gameService.startListeningToTournamentGame(isReadyRequest.getFeaturedMod()); + matchFuture = gameService.startListeningForTournamentGame(isReadyRequest.getFeaturedMod()); try { fafServerAccessor.sendIsReady(requestId); } catch (Exception e) { dismissNotification(); cancelMatch(); + log.error("Could not send the server that player is ready for the tournament game", e); notificationService.addImmediateErrorNotification(e, "isReady.readyUpFailed"); } } diff --git a/src/main/java/com/faforever/client/ui/progress/ProgressCircleIndicator.java b/src/main/java/com/faforever/client/ui/progress/ProgressCircleIndicator.java index 9ba9ec9b04..3b87534f6b 100644 --- a/src/main/java/com/faforever/client/ui/progress/ProgressCircleIndicator.java +++ b/src/main/java/com/faforever/client/ui/progress/ProgressCircleIndicator.java @@ -33,8 +33,8 @@ /** - * Base class for the progress indicator controls represented by circualr shapes - * + * Base class for the progress indicator controls represented by circular shapes + * * * @author Andrea Vacondio */ abstract class ProgressCircleIndicator extends Control { diff --git a/src/main/java/com/faforever/client/ui/progress/RingProgressIndicator.java b/src/main/java/com/faforever/client/ui/progress/RingProgressIndicator.java index 57d7d2c915..4a65364806 100644 --- a/src/main/java/com/faforever/client/ui/progress/RingProgressIndicator.java +++ b/src/main/java/com/faforever/client/ui/progress/RingProgressIndicator.java @@ -38,7 +38,7 @@ * @author Andrea Vacondio */ public class RingProgressIndicator extends ProgressCircleIndicator { - public ObjectProperty> progressLableStringConverter = new SimpleObjectProperty<>(new StringConverter<>() { + public ObjectProperty> progressLabelStringConverter = new SimpleObjectProperty<>(new StringConverter<>() { @Override public String toString(Integer object) { return String.format("%d%%", object); @@ -121,15 +121,15 @@ public StyleableProperty getStyleableProperty(RingProgressIndicator n) { return StyleableProperties.STYLEABLES; } - public StringConverter getProgressLableStringConverter() { - return progressLableStringConverter.get(); + public StringConverter getProgressLabelStringConverter() { + return progressLabelStringConverter.get(); } - public ObjectProperty> progressLableStringConverterProperty() { - return progressLableStringConverter; + public ObjectProperty> progressLabelStringConverterProperty() { + return progressLabelStringConverter; } - public void setProgressLableStringConverter(StringConverter progressLableStringConverter) { - this.progressLableStringConverter.set(progressLableStringConverter); + public void setProgressLabelStringConverter(StringConverter progressLabelStringConverter) { + this.progressLabelStringConverter.set(progressLabelStringConverter); } } \ No newline at end of file diff --git a/src/main/java/com/faforever/client/ui/progress/RingProgressIndicatorSkin.java b/src/main/java/com/faforever/client/ui/progress/RingProgressIndicatorSkin.java index 23922c8197..8a38f3ad73 100644 --- a/src/main/java/com/faforever/client/ui/progress/RingProgressIndicatorSkin.java +++ b/src/main/java/com/faforever/client/ui/progress/RingProgressIndicatorSkin.java @@ -90,7 +90,7 @@ public RingProgressIndicatorSkin(final RingProgressIndicator indicator) { private void setProgressLabel(int value) { if (value >= 0) { - percentLabel.setText(indicator.getProgressLableStringConverter().toString(value)); + percentLabel.setText(indicator.getProgressLabelStringConverter().toString(value)); } } diff --git a/src/test/java/com/faforever/client/game/GameServiceTest.java b/src/test/java/com/faforever/client/game/GameServiceTest.java index 32d342a3b8..0e38c960a3 100644 --- a/src/test/java/com/faforever/client/game/GameServiceTest.java +++ b/src/test/java/com/faforever/client/game/GameServiceTest.java @@ -205,7 +205,7 @@ private void mockMatchmakerChain() { when(modService.getFeaturedMod(FAF.getTechnicalName())) .thenReturn(completedFuture(FeaturedModBeanBuilder.create().defaultValues().get())); when(gameUpdater.update(any(), any(), any(), any())).thenReturn(completedFuture(null)); - when(fafServerAccessor.getGameLaunchMessage()).thenReturn(new CompletableFuture<>()); + when(fafServerAccessor.getGameLaunchMessageFuture()).thenReturn(new CompletableFuture<>()); } @Test @@ -536,7 +536,7 @@ public void testStartSearchLadder1v1() throws Exception { mockStartGameProcess(gameParameters); when(leaderboardService.getActiveLeagueEntryForPlayer(junitPlayer, LADDER_1v1_RATING_TYPE)).thenReturn(completedFuture(Optional.empty())); - when(fafServerAccessor.getGameLaunchMessage()).thenReturn(completedFuture(gameLaunchMessage)); + when(fafServerAccessor.getGameLaunchMessageFuture()).thenReturn(completedFuture(gameLaunchMessage)); when(gameUpdater.update(featuredMod, Set.of(),null, null)).thenReturn(completedFuture(null)); when(mapService.isInstalled(map)).thenReturn(false); when(mapService.download(map)).thenReturn(completedFuture(null)); @@ -545,7 +545,7 @@ public void testStartSearchLadder1v1() throws Exception { instance.startSearchMatchmaker().join(); - verify(fafServerAccessor).getGameLaunchMessage(); + verify(fafServerAccessor).getGameLaunchMessageFuture(); verify(mapService).download(map); verify(replayServer).start(eq(uid), any()); verify(forgedAllianceService).startGameOnline(gameParameters); @@ -573,7 +573,7 @@ public void testStartSearchLadder1v1WithLeagueEntry() throws Exception { mockStartGameProcess(gameParameters); LeagueEntryBean leagueEntry = LeagueEntryBeanBuilder.create().defaultValues().get(); when(leaderboardService.getActiveLeagueEntryForPlayer(junitPlayer, LADDER_1v1_RATING_TYPE)).thenReturn(completedFuture(Optional.of(leagueEntry))); - when(fafServerAccessor.getGameLaunchMessage()).thenReturn(completedFuture(gameLaunchMessage)); + when(fafServerAccessor.getGameLaunchMessageFuture()).thenReturn(completedFuture(gameLaunchMessage)); when(gameUpdater.update(featuredMod, Set.of(),null, null)).thenReturn(completedFuture(null)); when(mapService.isInstalled(map)).thenReturn(false); when(mapService.download(map)).thenReturn(completedFuture(null)); @@ -582,7 +582,7 @@ public void testStartSearchLadder1v1WithLeagueEntry() throws Exception { instance.startSearchMatchmaker().join(); - verify(fafServerAccessor).getGameLaunchMessage(); + verify(fafServerAccessor).getGameLaunchMessageFuture(); verify(mapService).download(map); verify(replayServer).start(eq(uid), any()); verify(forgedAllianceService).startGameOnline(gameParameters); @@ -608,7 +608,7 @@ public void testStartSearchLadderTwiceReturnsSameFutureWhenSearching() throws Ex mockStartGameProcess(gameParameters); when(leaderboardService.getActiveLeagueEntryForPlayer(junitPlayer, LADDER_1v1_RATING_TYPE)).thenReturn(completedFuture(Optional.empty())); - when(fafServerAccessor.getGameLaunchMessage()).thenReturn(completedFuture(gameLaunchMessage)); + when(fafServerAccessor.getGameLaunchMessageFuture()).thenReturn(completedFuture(gameLaunchMessage)); when(gameUpdater.update(featuredMod, Set.of(), null, null)).thenReturn(completedFuture(null)); when(mapService.isInstalled(map)).thenReturn(false); when(mapService.download(map)).thenReturn(completedFuture(null)); @@ -776,7 +776,7 @@ public void startSearchMatchmakerWithGameOptions() throws IOException { when(leaderboardService.getActiveLeagueEntryForPlayer(junitPlayer, "global")).thenReturn(completedFuture(Optional.empty())); when(gameUpdater.update(any(), any(), any(), any())).thenReturn(completedFuture(null)); when(mapService.download(gameLaunchMessage.getMapName())).thenReturn(completedFuture(null)); - when(fafServerAccessor.getGameLaunchMessage()).thenReturn(completedFuture(gameLaunchMessage)); + when(fafServerAccessor.getGameLaunchMessageFuture()).thenReturn(completedFuture(gameLaunchMessage)); instance.startSearchMatchmaker().join(); verify(forgedAllianceService).startGameOnline(gameParameters); } @@ -799,7 +799,7 @@ public void startSearchMatchmakerThenCancelledWithGame() throws IOException { when(leaderboardService.getActiveLeagueEntryForPlayer(junitPlayer, "global")).thenReturn(completedFuture(Optional.empty())); when(gameUpdater.update(any(), any(), any(), any())).thenReturn(completedFuture(null)); when(mapService.download(gameLaunchMessage.getMapName())).thenReturn(completedFuture(null)); - when(fafServerAccessor.getGameLaunchMessage()).thenReturn(completedFuture(gameLaunchMessage)); + when(fafServerAccessor.getGameLaunchMessageFuture()).thenReturn(completedFuture(gameLaunchMessage)); CompletableFuture future = instance.startSearchMatchmaker(); when(process.isAlive()).thenReturn(true); future.cancel(false); @@ -821,7 +821,7 @@ public void startSearchMatchmakerThenCancelledNoGame() throws IOException { mockStartGameProcess(gameMapper.map(gameLaunchMessage)); when(gameUpdater.update(any(), any(), any(), any())).thenReturn(completedFuture(null)); when(mapService.download(gameLaunchMessage.getMapName())).thenReturn(completedFuture(null)); - when(fafServerAccessor.getGameLaunchMessage()).thenReturn(completedFuture(gameLaunchMessage)); + when(fafServerAccessor.getGameLaunchMessageFuture()).thenReturn(completedFuture(gameLaunchMessage)); instance.startSearchMatchmaker().cancel(false); verify(notificationService, never()).addServerNotification(any()); } diff --git a/src/test/java/com/faforever/client/remote/ServerAccessorTest.java b/src/test/java/com/faforever/client/remote/ServerAccessorTest.java index 6042c59a66..054c9e552c 100644 --- a/src/test/java/com/faforever/client/remote/ServerAccessorTest.java +++ b/src/test/java/com/faforever/client/remote/ServerAccessorTest.java @@ -655,7 +655,7 @@ public void testOnGameLaunch() throws InterruptedException, JsonProcessingExcept .initMode(LobbyMode.AUTO_LOBBY) .get(); - instance.getGameLaunchMessage(); + instance.getGameLaunchMessageFuture(); sendFromServer(gameLaunchMessage); assertTrue(messageReceivedByClientLatch.await(TIMEOUT, TIMEOUT_UNIT)); assertThat(receivedMessage, is(gameLaunchMessage)); diff --git a/src/test/java/com/faforever/client/tournament/game/TournamentGameServiceTest.java b/src/test/java/com/faforever/client/tournament/game/TournamentGameServiceTest.java index b237924724..340f9a6c0e 100644 --- a/src/test/java/com/faforever/client/tournament/game/TournamentGameServiceTest.java +++ b/src/test/java/com/faforever/client/tournament/game/TournamentGameServiceTest.java @@ -63,6 +63,6 @@ public void testReceivingIsReadyMessageAndClickingReady() throws Exception { isReadyCallbackCapture.getValue().run(); verify(fafServerAccessor).sendIsReady("abc"); - verify(gameService).startListeningToTournamentGame("faf"); + verify(gameService).startListeningForTournamentGame("faf"); } } \ No newline at end of file