Address review

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<Void> startSearchMatchmaker() {
       return matchmakerFuture;
     }
 
-    matchmakerFuture =  listenToServerInitiatedGame(FAF.getTechnicalName());
+    matchmakerFuture =  listenForServerInitiatedGame(FAF.getTechnicalName());
     return matchmakerFuture;
   }
 
-  public CompletableFuture<Void> startListeningToTournamentGame(String featuredModTechnicalName) {
+  public CompletableFuture<Void> 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<Void> listenToServerInitiatedGame(String featuredModTechnicalName) {
+  private CompletableFuture<Void> listenForServerInitiatedGame(String featuredModTechnicalName) {
     if (!preferencesService.isGamePathValid()) {
       CompletableFuture<Path> 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<GameLaunchResponse> gameLaunchMessageFuture = fafServerAccessor.getGameLaunchMessageFuture();
+    final CompletableFuture<Void> matchFuture = modService.getFeaturedMod(featuredModTechnicalName)
         .thenAccept(featuredModBean -> updateGameIfNecessary(featuredModBean, Set.of()))
         .thenCompose(aVoid -> gameLaunchMessageFuture)
         .thenCompose(gameLaunchResponse -> downloadMapIfNecessary(gameLaunchResponse.getMapName())
@@ -563,7 +564,7 @@ private CompletableFuture<Void> 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");

src/main/java/com/faforever/client/remote/FafServerAccessor.java

@@ -212,7 +212,7 @@ public void requestMatchmakerInfo() {
     lobbyClient.requestMatchmakerInfo();
   }
 
-  public CompletableFuture<GameLaunchResponse> getGameLaunchMessage() {
+  public CompletableFuture<GameLaunchResponse> getGameLaunchMessageFuture() {
     return lobbyClient.getEvents()
         .filter(event -> event instanceof GameLaunchResponse)
         .next()

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<Parent> {
   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"));
   }
 }

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");
     }
   }

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 {

src/main/java/com/faforever/client/ui/progress/RingProgressIndicator.java

@@ -38,7 +38,7 @@
  * @author Andrea Vacondio
  */
 public class RingProgressIndicator extends ProgressCircleIndicator {
-  public ObjectProperty<StringConverter<Integer>> progressLableStringConverter = new SimpleObjectProperty<>(new StringConverter<>() {
+  public ObjectProperty<StringConverter<Integer>> progressLabelStringConverter = new SimpleObjectProperty<>(new StringConverter<>() {
     @Override
     public String toString(Integer object) {
       return String.format("%d%%", object);
@@ -121,15 +121,15 @@ public StyleableProperty<Number> getStyleableProperty(RingProgressIndicator n) {
     return StyleableProperties.STYLEABLES;
   }
 
-  public StringConverter<Integer> getProgressLableStringConverter() {
-    return progressLableStringConverter.get();
+  public StringConverter<Integer> getProgressLabelStringConverter() {
+    return progressLabelStringConverter.get();
   }
 
-  public ObjectProperty<StringConverter<Integer>> progressLableStringConverterProperty() {
-    return progressLableStringConverter;
+  public ObjectProperty<StringConverter<Integer>> progressLabelStringConverterProperty() {
+    return progressLabelStringConverter;
   }
 
-  public void setProgressLableStringConverter(StringConverter<Integer> progressLableStringConverter) {
-    this.progressLableStringConverter.set(progressLableStringConverter);
+  public void setProgressLabelStringConverter(StringConverter<Integer> progressLabelStringConverter) {
+    this.progressLabelStringConverter.set(progressLabelStringConverter);
   }
 }

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));
     }
   }
 

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<Void> 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());
   }

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));

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");
   }
 }