Remove several game list refresh broadcasts:

- never broadcast when a player joins a game (unless it's a new game)
 - removed a duplicate broadcast in this situation as well
- never broadcast when a player leaves a game (unless the game no longer exists)
- never broadcast when a game's options change (unless the state of it having or having not a password changes)
- DO broadcast when a game resets to the lobby state (we were broadcasting when a game started but not when it ended)

In an effort to reduce deadlocks, do not actually hold a lock over the list of players in a game and instead make a copy into an array for looping, in several locations.

WebContent/game.jsp

@@ -95,12 +95,14 @@ HttpSession hSession = request.getSession(true);
     If this is your first time playing, you may wish to read <a href="/">the changelog and list of
     known issues</a>.
   </p>
-  <p tabindex="0">Most recent update: 28 April 2013:</p>
+  <p tabindex="0">Most recent update: 7 August 2013:</p>
   <ul>
-    <li tabindex="0">Version 1.3 of the base Cards Against Humanity game.</li>
+    <li tabindex="0"><strong>The game list will not automatically update all the time now.</strong>
-    <li tabindex="0">Card sets are grouped by official or custom.</li>
+    You will need to start using the Refresh Games button. The game list will automatically update
-    <li tabindex="0">You can <a href="viewcards.jsp">view all of the cards in the game</a>,
+    for new games, removed games, when games become passworded, or when you leave a game.</li>
-    including searching by text and filtering card set.</li>
+    <li tabindex="0">A lot of custom card sets have been added.</li>
+    <li tabindex="0">Cleaned up some error handling.</li>
+    <li tabindex="0">Tried to fix some more of the server crashes.</li>
   </ul>
   <div id="nickbox">
     Nickname:

WebContent/index.jsp

@@ -54,6 +54,14 @@ to, for instance, display the number of connected players.
 </p>
 <p>Recent Changes:</p>
 <ul>
+  <li>7 August 2013:<ul>
+    <li tabindex="0"><strong>The game list will not automatically update all the time now.</strong>
+    You will need to start using the Refresh Games button. The game list will automatically update
+    for new games, removed games, when games become passworded, or when you leave a game.</li>
+    <li tabindex="0">A lot of custom card sets have been added.</li>
+    <li tabindex="0">Cleaned up some error handling.</li>
+    <li tabindex="0">Tried to fix some more of the server crashes.</li>
+  </ul></li>
   <li>28 April 2013:<ul>
     <li tabindex="0">Version 1.3 of the base Cards Against Humanity game.</li>
     <li tabindex="0">Card sets are grouped by official or custom.</li>

src/net/socialgamer/cah/data/Game.java

@@ -195,7 +195,8 @@ public class Game {
     data.put(LongPollResponse.NICKNAME, user.getNickname());
     broadcastToPlayers(MessageType.GAME_PLAYER_EVENT, data);
 
-    gameManager.broadcastGameListRefresh();
+    // Don't do this anymore, it was driving up a crazy amount of traffic.
+    // gameManager.broadcastGameListRefresh();
   }
 
   /**
@@ -263,7 +264,8 @@ public class Game {
       data.put(LongPollResponse.NICKNAME, user.getNickname());
       broadcastToPlayers(MessageType.GAME_PLAYER_EVENT, data);
 
-      gameManager.broadcastGameListRefresh();
+      // Don't do this anymore, it was driving up a crazy amount of traffic.
+      // gameManager.broadcastGameListRefresh();
 
       if (host == player) {
         if (players.size() > 0) {
@@ -428,13 +430,13 @@ public class Game {
       info.put(GameInfo.PASSWORD, password);
     }
     info.put(GameInfo.HAS_PASSWORD, password != null && !password.equals(""));
-    synchronized (players) {
+
-      final List<String> playerNames = new ArrayList<String>(players.size());
+    final Player[] playersCopy = players.toArray(new Player[players.size()]);
-      for (final Player player : players) {
+    final List<String> playerNames = new ArrayList<String>(playersCopy.length);
-        playerNames.add(player.toString());
+    for (final Player player : playersCopy) {
-      }
+      playerNames.add(player.toString());
-      info.put(GameInfo.PLAYERS, playerNames);
     }
+    info.put(GameInfo.PLAYERS, playerNames);
     return info;
   }
 
@@ -444,12 +446,11 @@ public class Game {
    */
   public List<Map<GamePlayerInfo, Object>> getAllPlayerInfo() {
     final List<Map<GamePlayerInfo, Object>> info;
-    synchronized (players) {
+    final Player[] playersCopy = players.toArray(new Player[players.size()]);
-      info = new ArrayList<Map<GamePlayerInfo, Object>>(players.size());
+    info = new ArrayList<Map<GamePlayerInfo, Object>>(playersCopy.length);
-      for (final Player player : players) {
+    for (final Player player : playersCopy) {
-        final Map<GamePlayerInfo, Object> playerInfo = getPlayerInfo(player);
+      final Map<GamePlayerInfo, Object> playerInfo = getPlayerInfo(player);
-        info.add(playerInfo);
+      info.add(playerInfo);
-      }
     }
     return info;
   }
@@ -549,14 +550,13 @@ public class Game {
       return false;
     }
     boolean started;
-    synchronized (players) {
+    final int numPlayers = players.size();
-      if (players.size() >= 3) {
+    if (numPlayers >= 3) {
-        // Pick a random start judge, though the "next" judge will actually go first.
+      // Pick a random start judge, though the "next" judge will actually go first.
-        judgeIndex = (int) (Math.random() * players.size());
+      judgeIndex = (int) (Math.random() * numPlayers);
-        started = true;
+      started = true;
-      } else {
+    } else {
-        started = false;
+      started = false;
-      }
     }
     if (started) {
       logger.info(String.format("Starting game %d.", id));
@@ -587,21 +587,19 @@ public class Game {
    * Move the game into the {@code DEALING} state, and deal cards. The game immediately then moves
    * into the {@code PLAYING} state.
    * <br/>
-   * Synchronizes on {@link #players}.
    */
   private void dealState() {
     state = GameState.DEALING;
-    synchronized (players) {
+    final Player[] playersCopy = players.toArray(new Player[players.size()]);
-      for (final Player player : players) {
+    for (final Player player : playersCopy) {
-        final List<WhiteCard> hand = player.getHand();
+      final List<WhiteCard> hand = player.getHand();
-        final List<WhiteCard> newCards = new LinkedList<WhiteCard>();
+      final List<WhiteCard> newCards = new LinkedList<WhiteCard>();
-        while (hand.size() < 10) {
+      while (hand.size() < 10) {
-          final WhiteCard card = getNextWhiteCard();
+        final WhiteCard card = getNextWhiteCard();
-          hand.add(card);
+        hand.add(card);
-          newCards.add(card);
+        newCards.add(card);
-        }
-        sendCardsToPlayer(player, newCards);
       }
+      sendCardsToPlayer(player, newCards);
     }
     playingState();
   }
@@ -761,15 +759,14 @@ public class Game {
           logger.info(String.format("Skipping idle player %s in game %d.",
               player.getUser().toString(), id));
           player.skipped();
-          final HashMap<ReturnableData, Object> data = getEventMap();
 
+          final HashMap<ReturnableData, Object> data = getEventMap();
+          data.put(LongPollResponse.NICKNAME, player.getUser().getNickname());
           if (player.getSkipCount() >= MAX_SKIPS_BEFORE_KICK || playedCards.size() < 2) {
             data.put(LongPollResponse.EVENT, LongPollEvent.GAME_PLAYER_KICKED_IDLE.toString());
-            data.put(LongPollResponse.NICKNAME, player.getUser().getNickname());
             playersToRemove.add(player.getUser());
           } else {
             data.put(LongPollResponse.EVENT, LongPollEvent.GAME_PLAYER_SKIPPED.toString());
-            data.put(LongPollResponse.NICKNAME, player.getUser().getNickname());
             playersToUpdateStatus.add(player);
           }
           broadcastToPlayers(MessageType.GAME_EVENT, data);
@@ -913,6 +910,8 @@ public class Game {
       data.put(LongPollResponse.PLAYER_INFO, getPlayerInfo(judge));
       broadcastToPlayers(MessageType.GAME_PLAYER_EVENT, data);
     }
+
+    gameManager.broadcastGameListRefresh();
   }
 
   /**
@@ -1015,19 +1014,16 @@ public class Game {
   /**
    * Get the {@code Player} object for a given {@code User} object.
    * 
-   * Synchronizes on {@link #players}.
-   * 
    * @param user
    * @return The {@code Player} object representing {@code user} in this game, or {@code null} if
    *         {@code user} is not in this game.
    */
   @Nullable
   private Player getPlayerForUser(final User user) {
-    synchronized (players) {
+    final Player[] playersCopy = players.toArray(new Player[players.size()]);
-      for (final Player player : players) {
+    for (final Player player : playersCopy) {
-        if (player.getUser() == user) {
+      if (player.getUser() == user) {
-          return player;
+        return player;
-        }
       }
     }
     return null;
@@ -1159,11 +1155,10 @@ public class Game {
    */
   private List<User> playersToUsers() {
     final List<User> users;
-    synchronized (players) {
+    final Player[] playersCopy = players.toArray(new Player[players.size()]);
-      users = new ArrayList<User>(players.size());
+    users = new ArrayList<User>(playersCopy.length);
-      for (final Player player : players) {
+    for (final Player player : playersCopy) {
-        users.add(player.getUser());
+      users.add(player.getUser());
-      }
     }
     return users;
   }

src/net/socialgamer/cah/handlers/ChangeGameOptionHandler.java

@@ -51,6 +51,7 @@ public class ChangeGameOptionHandler extends GameWithPlayerHandler {
                 Integer.parseInt(cardSetId)));
           }
         }
+        final String oldPassword = game.getPassword();
         String password = request.getParameter(AjaxRequest.PASSWORD);
         if (password == null) {
           password = "";
@@ -63,11 +64,16 @@ public class ChangeGameOptionHandler extends GameWithPlayerHandler {
           useTimer = Boolean.valueOf(useTimerString);
         }
         game.updateGameSettings(scoreLimit, playerLimit, cardSets, password, useTimer);
+
+        // only broadcast an update if the password state has changed, because it needs to change
+        // the text on the join button and the sort order
+        if (!password.equals(oldPassword)) {
+          gameManager.broadcastGameListRefresh();
+        }
       } catch (final NumberFormatException nfe) {
         return error(ErrorCode.BAD_REQUEST);
       }
 
-      gameManager.broadcastGameListRefresh();
       return data;
     }
   }

src/net/socialgamer/cah/handlers/JoinGameHandler.java

@@ -74,7 +74,6 @@ public class JoinGameHandler extends GameHandler {
     } catch (final TooManyPlayersException e) {
       return error(ErrorCode.GAME_FULL);
     }
-    gameManager.broadcastGameListRefresh();
     return data;
   }
 }