Only send vetoes when player updates them

Author: Sheikah45

src/main/java/com/faforever/client/preferences/VetoKey.java

@@ -1,6 +1,14 @@
 package com.faforever.client.preferences;
 
+import com.faforever.client.domain.api.MapPoolAssignment;
+
 public record VetoKey(
     int matchmakerQueueMapPoolId,
     int mapPoolMapVersionId
-) {}
+) {
+
+  public static VetoKey of(MapPoolAssignment assignment) {
+    return new VetoKey(assignment.mapPool().mapPool().id(), assignment.id());
+  }
+
+}

src/main/java/com/faforever/client/teammatchmaking/TeamMatchmakingMapTileController.java

@@ -4,9 +4,9 @@
 import com.faforever.client.domain.api.MapPoolAssignment;
 import com.faforever.client.domain.api.MapVersion;
 import com.faforever.client.fx.ImageViewHelper;
+import com.faforever.client.fx.NodeController;
 import com.faforever.client.i18n.I18n;
 import com.faforever.client.map.MapService;
-import com.faforever.client.fx.NodeController;
 import com.faforever.client.map.MapService.PreviewSize;
 import com.faforever.client.map.generator.MapGeneratorService;
 import com.faforever.client.preferences.MatchmakerPrefs;
@@ -25,13 +25,14 @@
 import javafx.scene.image.ImageView;
 import javafx.scene.layout.HBox;
 import javafx.scene.layout.Pane;
-import javafx.scene.layout.VBox;
 import javafx.scene.layout.Region;
+import javafx.scene.layout.VBox;
 import lombok.RequiredArgsConstructor;
 import lombok.Setter;
 import org.springframework.beans.factory.config.ConfigurableBeanFactory;
 import org.springframework.context.annotation.Scope;
 import org.springframework.stereotype.Component;
+
 import java.util.function.Consumer;
 
 /**
@@ -101,9 +102,10 @@ void setVetoModeEnabled(BooleanProperty vetoModeEnabled) {
   public void bindVetoModeDependentProperties() {
     vetoesBox.mouseTransparentProperty().bind(vetoModeEnabled.not());
     vetoesBox.visibleProperty().bind(vetoModeEnabled.or(tokenCount.greaterThan(0)));
-    root.cursorProperty().bind(Bindings.when(
-        Bindings.createBooleanBinding(() -> isGeneratedMap.getValue() || vetoModeEnabled.get(), isGeneratedMap, vetoModeEnabled)
-    ).then(Cursor.DEFAULT).otherwise(Cursor.HAND));
+    root.cursorProperty()
+        .bind(Bindings.when(
+            Bindings.createBooleanBinding(() -> isGeneratedMap.getValue() || vetoModeEnabled.get(), isGeneratedMap,
+                                          vetoModeEnabled)).then(Cursor.DEFAULT).otherwise(Cursor.HAND));
   }
 
   @Override
@@ -112,9 +114,9 @@ protected void onInitialize() {
     isGeneratedMap = mapObservable.map(map -> mapGeneratorService.isGeneratedMap(map.displayName())).orElse(false);
 
     thumbnailImageView.imageProperty()
-                      .bind(assignment
-                                .map(assignmentBean -> mapService.loadPreview(assignmentBean.mapVersion(), PreviewSize.SMALL))
-                                .flatMap(imageViewHelper::createPlaceholderImageOnErrorObservable));
+                      .bind(assignment.map(
+                                          assignmentBean -> mapService.loadPreview(assignmentBean.mapVersion(), PreviewSize.SMALL))
+                                      .flatMap(imageViewHelper::createPlaceholderImageOnErrorObservable));
 
     nameLabel.textProperty().bind(mapObservable.map(map -> {
       if (isGeneratedMap.getValue()) {
@@ -123,9 +125,7 @@ protected void onInitialize() {
       return map.displayName();
     }));
 
-    authorBox.visibleProperty()
-             .bind(mapObservable.map(
-                 map -> (map.author() != null) || isGeneratedMap.getValue()));
+    authorBox.visibleProperty().bind(mapObservable.map(map -> (map.author() != null) || isGeneratedMap.getValue()));
     authorBox.managedProperty().bind(authorBox.visibleProperty());
 
     authorLabel.textProperty().bind(mapObservable.map(map -> {
@@ -165,27 +165,26 @@ protected void onInitialize() {
     });
 
     vetoButton.setOnAction(event -> {
-      if (assignment.getValue() == null) {
+      MapPoolAssignment value = assignment.getValue();
+      if (value == null) {
         return;
       }
       int currentTokenCount = tokenCount.get();
       int currentMaxPerMap = maxPerMap.get();
       if ((isMaxPerMapDynamic.get() || currentTokenCount < currentMaxPerMap) && currentTokenCount < vetoTokensMax.get() && vetoTokensLeft.getValue() > 0) {
-        teamMatchmakingService.setTokensForMap(
-            new VetoKey(assignment.getValue().mapPool().mapPool().id(), assignment.getValue().id()),
-            currentTokenCount + 1);
+        teamMatchmakingService.setTokensForMap(VetoKey.of(value), currentTokenCount + 1);
       }
       event.consume();
     });
 
     minusButton.setOnAction(event -> {
-      if (assignment.getValue() == null) {
+      MapPoolAssignment value = assignment.getValue();
+      if (value == null) {
         return;
       }
       int current = tokenCount.get();
       if (current > 0) {
-        teamMatchmakingService.setTokensForMap(
-            new VetoKey(assignment.getValue().mapPool().mapPool().id(), assignment.getValue().id()), current - 1);
+        teamMatchmakingService.setTokensForMap(VetoKey.of(value), current - 1);
       }
       event.consume();
     });
@@ -213,11 +212,12 @@ private void updateBannedState() {
   }
 
   private void updateVetoes() {
-    if (assignment.getValue() == null) {
+    MapPoolAssignment value = assignment.getValue();
+    if (value == null) {
       tokenCount.set(0);
       return;
     }
-    VetoKey key = new VetoKey(assignment.getValue().mapPool().mapPool().id(), assignment.getValue().id());
+    VetoKey key = VetoKey.of(value);
     int usedTokens = matchmakerPrefs.getAppliedVetoes().getOrDefault(key, 0);
     tokenCount.set(usedTokens);
   }

src/main/java/com/faforever/client/teammatchmaking/TeamMatchmakingService.java

@@ -246,12 +246,6 @@ public void afterPropertiesSet() throws Exception {
       }
     });
 
-    matchmakerPrefs.getAppliedVetoes().subscribe(() -> {
-      if (fafServerAccessor.getConnectionState() == ConnectionState.CONNECTED) {
-        sendVetoes();
-      }
-    });
-
     party.ownerProperty().subscribe((oldValue, newValue) -> {
       if (oldValue != null) {
         chatService.leaveChannel("#" + oldValue.getUsername() + PARTY_CHANNEL_SUFFIX);
@@ -275,21 +269,31 @@ private void sendVetoes() {
     fafServerAccessor.setPlayerVetoes(getVetoesAsList());
   }
 
-  public void setTokensForMap(VetoKey vetoKey, Integer vetoTokensApplied) {
-    matchmakerPrefs.getAppliedVetoes().put(vetoKey, vetoTokensApplied);
+  public void setTokensForMap(VetoKey key, int vetoTokensApplied) {
+    Integer previousValue;
+    if (vetoTokensApplied == 0) {
+      previousValue = matchmakerPrefs.getAppliedVetoes().remove(key);
+    } else {
+      previousValue = matchmakerPrefs.getAppliedVetoes().put(key, vetoTokensApplied);
+    }
+
+    if ((previousValue == null && vetoTokensApplied > 0) || (previousValue != null && previousValue != vetoTokensApplied)) {
+      sendVetoes();
+    }
   }
 
-  public void setAllVetoes(List<VetoData> vetoes) {
+  public void updateVetoes(List<VetoData> vetoes) {
     Map<VetoKey, Integer> vetosMap = vetoes.stream()
                                            .collect(Collectors.toMap(
                                                vetoData -> new VetoKey(vetoData.getMatchmakerQueueMapPoolId(),
                                                                        vetoData.getMapPoolMapVersionId()),
                                                VetoData::getVetoTokensApplied));
+    matchmakerPrefs.getAppliedVetoes().clear();
     matchmakerPrefs.getAppliedVetoes().putAll(vetosMap);
   }
 
   private void onVetoesChanged(VetoesChangedInfo vetoesChangedInfo) {
-    setAllVetoes(vetoesChangedInfo.getVetoes());
+    updateVetoes(vetoesChangedInfo.getVetoes());
 
     if (vetoesChangedInfo.getForced()) {
       notificationService.addNotification(new ImmediateNotification(i18n.get("teammatchmaking.vetoes.forced.title"),

src/test/java/com/faforever/client/teammatchmaking/TeamMatchmakingMapTileControllerTest.java

@@ -34,7 +34,6 @@
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
-import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;

src/test/java/com/faforever/client/teammatchmaking/TeamMatchmakingServiceTest.java

@@ -638,10 +638,8 @@ public void testSendVetoesOnConnection() {
   public void testSendVetoesOnVetoesChange() {
     connectionState.set(ConnectionState.CONNECTED);
     when(fafServerAccessor.getConnectionState()).thenReturn(ConnectionState.CONNECTED);
-    // Clear invocations from connection state change
-    org.mockito.Mockito.clearInvocations(fafServerAccessor);
 
-    matchmakerPrefs.getAppliedVetoes().put(new VetoKey(1, 1), 1);
+    instance.setTokensForMap(new VetoKey(1, 1), 1);
 
     verify(fafServerAccessor, times(1)).setPlayerVetoes(anyList());
   }