fix(sdk): handle responses cleanly in LobbyClient and don't track rooms

Author: xeruf

sdk/src/server-api/sc/networking/clients/LobbyClient.java

@@ -17,9 +17,7 @@
 
 import java.io.IOException;
 import java.net.Socket;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
+import java.util.*;
 
 /**
  * This class is used to handle all communication with a server.
@@ -31,7 +29,6 @@
  */
 public final class LobbyClient extends XStreamClient implements IPollsHistory {
   private static final Logger logger = LoggerFactory.getLogger(LobbyClient.class);
-  private final List<String> rooms = new ArrayList<>();
   private final AsyncResultManager asyncManager = new AsyncResultManager();
   private final List<ILobbyClientListener> listeners = new ArrayList<>();
   private final List<IHistoryListener> historyListeners = new ArrayList<>();
@@ -52,10 +49,6 @@ private static INetworkInterface createTcpNetwork(String host, int port) throws
     return new TcpNetwork(new Socket(host, port));
   }
 
-  public List<String> getRooms() {
-    return Collections.unmodifiableList(this.rooms);
-  }
-
   @Override
   protected final void onObject(ProtocolMessage o) {
     if (o == null) {
@@ -83,26 +76,21 @@ protected final void onObject(ProtocolMessage o) {
         onRoomMessage(roomId, data);
       }
     } else if (o instanceof GamePreparedResponse) {
-      GamePreparedResponse preparation = (GamePreparedResponse) o;
-      onGamePrepared(preparation);
+      onGamePrepared((GamePreparedResponse) o);
     } else if (o instanceof JoinedRoomResponse) {
-      String roomId = ((JoinedRoomResponse) o).getRoomId();
-      this.rooms.add(roomId);
-      onGameJoined(roomId);
+      onGameJoined(((JoinedRoomResponse) o).getRoomId());
+    } else if (o instanceof RoomWasJoinedEvent) {
+      onGameJoined(((RoomWasJoinedEvent) o).getRoomId());
     } else if (o instanceof LeftGameEvent) {
-      String roomId = ((LeftGameEvent) o).getRoomId();
-      this.rooms.remove(roomId);
-      onGameLeft(roomId);
-    } else if (o instanceof ProtocolErrorMessage) {
-      ProtocolErrorMessage response = (ProtocolErrorMessage) o;
-
-      onError(response.getMessage(), response);
+      onGameLeft(((LeftGameEvent) o).getRoomId());
     } else if (o instanceof ObservationResponse) {
-      String roomId = ((ObservationResponse) o).getRoomId();
-      onGameObserved(roomId);
-    } else if (o instanceof TestModeResponse) { // for handling testing
+      onGameObserved(((ObservationResponse) o).getRoomId());
+    } else if (o instanceof TestModeResponse) {
       boolean testMode = (((TestModeResponse) o).getTestMode());
       logger.info("TestMode was set to {} ", testMode);
+    } else if (o instanceof ProtocolErrorMessage) {
+      ProtocolErrorMessage response = (ProtocolErrorMessage) o;
+      onError(response.getMessage(), response);
     } else {
       onCustomObject(o);
     }
@@ -259,8 +247,8 @@ protected void request(ProtocolMessage request, Class<? extends ProtocolMessage>
   protected RequestResult blockingRequest(ProtocolMessage request,
                                           Class<? extends ProtocolMessage> response) throws InterruptedException {
     // TODO return a proper future here
-    // This is really old async code, so the variable needs to be final but still manipulatable - IDEA suggested to
-    // use an array and we'll stay with that until we reimplement it properly.
+    // This is really old async code, so the variable needs to be final but still mutable
+    // IDEA suggested to use an array and we'll stay with that until we reimplement it properly.
     final RequestResult[] requestResult = {null};
     final Object beacon = new Object();
     synchronized(beacon) {

server/test/sc/server/network/LobbyTest.kt

@@ -37,13 +37,11 @@ class LobbyTest: RealServerTest() {
         player1.joinRoomRequest(TestPlugin.TEST_PLUGIN_UUID)
         player2.joinRoomRequest(TestPlugin.TEST_PLUGIN_UUID)
 
-        TestHelper.assertEqualsWithTimeout(1, { player1.rooms.size })
-        TestHelper.assertEqualsWithTimeout(1, { player2.rooms.size })
+        // TODO Listen for RoomJoinedResponse directly instead
+        //TestHelper.assertEqualsWithTimeout(1, { player1.rooms.size })
+        //TestHelper.assertEqualsWithTimeout(1, { player2.rooms.size })
         TestHelper.assertEqualsWithTimeout(1, { this@LobbyTest.gameMgr.games.size })
 
-        Assert.assertEquals(1, this.gameMgr.games.size)
-        Assert.assertEquals(player1.rooms[0], player2.rooms[0])
-        
         val theRoom = this@LobbyTest.gameMgr.games.iterator().next()
 
         Assert.assertEquals(false, theRoom.isOver)