Don't load card sets until the game starts. Loading every card for every selected card set every time any option changed was a stupid idea.

This commit is contained in:
Andy Janata 2014-02-22 19:03:27 -08:00
parent 93203d3029
commit de65ad2383
6 changed files with 83 additions and 71 deletions

View File

@ -24,6 +24,7 @@
package net.socialgamer.cah.data;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
@ -47,7 +48,7 @@ public class BlackDeck {
/**
* Create a new black card deck, loading the cards from the database and shuffling them.
*/
public BlackDeck(final Set<CardSet> cardSets) {
public BlackDeck(final Collection<CardSet> cardSets) {
final Set<BlackCard> allCards = new HashSet<BlackCard>();
for (final CardSet cardSet : cardSets) {
allCards.addAll(cardSet.getBlackCards());

View File

@ -25,6 +25,7 @@ package net.socialgamer.cah.data;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
@ -58,8 +59,10 @@ import net.socialgamer.cah.db.CardSet;
import net.socialgamer.cah.db.WhiteCard;
import org.apache.log4j.Logger;
import org.hibernate.Session;
import com.google.inject.Inject;
import com.google.inject.Provider;
/**
@ -98,6 +101,7 @@ public class Game {
private final ConnectedUsers connectedUsers;
private final GameManager gameManager;
private Player host;
private final Provider<Session> sessionProvider;
private BlackDeck blackDeck;
private BlackCard blackCard;
private final Object blackCardLock = new Object();
@ -152,7 +156,7 @@ public class Game {
private final ScheduledThreadPoolExecutor globalTimer;
private int scoreGoal = 8;
private final Set<CardSet> cardSets = new HashSet<CardSet>();
private final Set<Integer> cardSetIds = new HashSet<Integer>();
private String password = "";
private boolean useIdleTimer = true;
@ -171,11 +175,14 @@ public class Game {
*/
@Inject
public Game(@GameId final Integer id, final ConnectedUsers connectedUsers,
final GameManager gameManager, final ScheduledThreadPoolExecutor globalTimer) {
final GameManager gameManager, final ScheduledThreadPoolExecutor globalTimer,
final Provider<Session> sessionProvider) {
this.id = id;
this.connectedUsers = connectedUsers;
this.gameManager = gameManager;
this.globalTimer = globalTimer;
this.sessionProvider = sessionProvider;
state = GameState.LOBBY;
}
@ -438,26 +445,15 @@ public class Game {
return password;
}
/**
*
* @param newScoreGoal
* @param newMaxPlayers
* @param newMaxSpectators
* @param newCardSets Card sets to use in this game. These should be fully loaded from the
* database, as there might not be a {@link org.hibernate.Session} from which to load the cards.
* @param newMaxBlanks
* @param newPassword
* @param newUseTimer
*/
public void updateGameSettings(final int newScoreGoal, final int newMaxPlayers,
final int newMaxSpectators, final List<CardSet> newCardSets, final int newMaxBlanks,
final int newMaxSpectators, final Collection<Integer> newCardSetIds, final int newMaxBlanks,
final String newPassword, final boolean newUseTimer) {
this.scoreGoal = newScoreGoal;
this.playerLimit = newMaxPlayers;
this.spectatorLimit = newMaxSpectators;
synchronized (this.cardSets) {
this.cardSets.clear();
this.cardSets.addAll(newCardSets);
synchronized (this.cardSetIds) {
this.cardSetIds.clear();
this.cardSetIds.addAll(newCardSetIds);
}
this.blanksInDeck = newMaxBlanks;
this.password = newPassword;
@ -500,14 +496,11 @@ public class Game {
}
info.put(GameInfo.HOST, host.toString());
info.put(GameInfo.STATE, state.toString());
final List<Integer> cardSetIds;
synchronized (cardSets) {
cardSetIds = new ArrayList<Integer>(cardSets.size());
for (final CardSet cardSet : cardSets) {
cardSetIds.add(cardSet.getId());
final List<Integer> cardSetIdsCopy;
synchronized (this.cardSetIds) {
cardSetIdsCopy = new ArrayList<Integer>(this.cardSetIds);
}
}
info.put(GameInfo.CARD_SETS, cardSetIds);
info.put(GameInfo.CARD_SETS, cardSetIdsCopy);
info.put(GameInfo.BLANKS_LIMIT, blanksInDeck);
info.put(GameInfo.PLAYER_LIMIT, playerLimit);
info.put(GameInfo.SPECTATOR_LIMIT, spectatorLimit);
@ -657,9 +650,24 @@ public class Game {
logger.info(String.format("Starting game %d.", id));
// do this stuff outside the players lock; they will lock players again later for much less
// time, and not at the same time as trying to lock users, which has caused deadlocks
synchronized (cardSets) {
synchronized (cardSetIds) {
Session session = null;
try {
session = sessionProvider.get();
@SuppressWarnings("unchecked")
final List<CardSet> cardSets = session.createQuery("from CardSet where id in (:ids)")
.setParameterList("ids", cardSetIds).list();
blackDeck = new BlackDeck(cardSets);
whiteDeck = new WhiteDeck(cardSets, blanksInDeck);
} catch (final Exception e) {
logger.error(String.format("Unable to load cards to start game %d", id), e);
return false;
} finally {
if (null != session) {
session.close();
}
}
}
startNextRound();
gameManager.broadcastGameListRefresh();
@ -668,16 +676,30 @@ public class Game {
}
public boolean hasBaseDeck() {
synchronized (cardSets) {
for (final CardSet cardSet : cardSets) {
if (cardSet.isBaseDeck()) {
return true;
}
}
}
synchronized (cardSetIds) {
if (cardSetIds.isEmpty()) {
return false;
}
Session session = null;
try {
session = sessionProvider.get();
final Number baseDeckCount = (Number) session
.createQuery("select count(*) from CardSet where id in (:ids) and base_deck = true")
.setParameterList("ids", cardSetIds).uniqueResult();
return baseDeckCount.intValue() > 0;
} catch (final Exception e) {
logger.error(String.format("Unable to determine if game %d has base deck", id), e);
return false;
} finally {
if (null != session) {
session.close();
}
}
}
}
/**
* Move the game into the {@code DEALING} state, and deal cards. The game immediately then moves
* into the {@code PLAYING} state.

View File

@ -24,6 +24,7 @@
package net.socialgamer.cah.data;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
@ -48,7 +49,7 @@ public class WhiteDeck {
/**
* Create a new white card deck, loading the cards from the database and shuffling them.
*/
public WhiteDeck(final Set<CardSet> cardSets, final int numBlanks) {
public WhiteDeck(final Collection<CardSet> cardSets, final int numBlanks) {
final Set<WhiteCard> allCards = new HashSet<WhiteCard>();
for (final CardSet cardSet : cardSets) {
allCards.addAll(cardSet.getWhiteCards());

View File

@ -1,10 +1,7 @@
package net.socialgamer.cah.handlers;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@ -19,12 +16,6 @@ import net.socialgamer.cah.RequestWrapper;
import net.socialgamer.cah.data.Game;
import net.socialgamer.cah.data.GameManager;
import net.socialgamer.cah.data.User;
import net.socialgamer.cah.db.CardSet;
import org.hibernate.FetchMode;
import org.hibernate.Session;
import org.hibernate.criterion.CriteriaSpecification;
import org.hibernate.criterion.Restrictions;
import com.google.inject.Inject;
@ -33,12 +24,9 @@ public class ChangeGameOptionHandler extends GameWithPlayerHandler {
public static final String OP = AjaxOperation.CHANGE_GAME_OPTIONS.toString();
private final Session hibernateSession;
@Inject
public ChangeGameOptionHandler(final GameManager gameManager, final Session hibernateSession) {
public ChangeGameOptionHandler(final GameManager gameManager) {
super(gameManager);
this.hibernateSession = hibernateSession;
}
@Override
@ -64,7 +52,6 @@ public class ChangeGameOptionHandler extends GameWithPlayerHandler {
cardSetIds.add(Integer.parseInt(cardSetId));
}
}
final List<CardSet> cardSets = getCardSets(cardSetIds);
final int blanksLimit = Integer.parseInt(request.getParameter(AjaxRequest.BLANKS_LIMIT));
final String oldPassword = game.getPassword();
@ -79,7 +66,7 @@ public class ChangeGameOptionHandler extends GameWithPlayerHandler {
if (null != useTimerString && !"".equals(useTimerString)) {
useTimer = Boolean.valueOf(useTimerString);
}
game.updateGameSettings(scoreLimit, playerLimit, spectatorLimit, cardSets, blanksLimit,
game.updateGameSettings(scoreLimit, playerLimit, spectatorLimit, cardSetIds, blanksLimit,
password, useTimer);
// only broadcast an update if the password state has changed, because it needs to change
@ -89,25 +76,9 @@ public class ChangeGameOptionHandler extends GameWithPlayerHandler {
}
} catch (final NumberFormatException nfe) {
return error(ErrorCode.BAD_REQUEST);
} finally {
hibernateSession.close();
}
return data;
}
}
@SuppressWarnings("unchecked")
private final List<CardSet> getCardSets(final Collection<Integer> cardSetIds) {
if (cardSetIds.isEmpty()) {
return Collections.emptyList();
} else {
return hibernateSession.createCriteria(CardSet.class)
.add(Restrictions.in("id", cardSetIds))
.setFetchMode("whiteCards", FetchMode.JOIN)
.setFetchMode("blackCards", FetchMode.JOIN)
.setResultTransformer(CriteriaSpecification.DISTINCT_ROOT_ENTITY)
.list();
}
}
}

View File

@ -36,6 +36,8 @@ import static org.junit.Assert.assertNull;
import java.util.Collection;
import java.util.HashMap;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.atomic.AtomicInteger;
import net.socialgamer.cah.HibernateUtil;
import net.socialgamer.cah.data.GameManager.GameId;
@ -76,6 +78,21 @@ public class GameManagerTest {
@Override
protected void configure() {
bind(ConnectedUsers.class).toInstance(cuMock);
final ScheduledThreadPoolExecutor threadPool =
new ScheduledThreadPoolExecutor(1,
new ThreadFactory() {
final AtomicInteger threadCount = new AtomicInteger();
@Override
public Thread newThread(final Runnable r) {
final Thread t = new Thread(r);
t.setDaemon(true);
t.setName("timer-task-" + threadCount.incrementAndGet());
return t;
}
});
bind(ScheduledThreadPoolExecutor.class).toInstance(threadPool);
}
@SuppressWarnings("unused")
@ -118,11 +135,11 @@ public class GameManagerTest {
// fill it up with 3 games
assertEquals(0, gameManager.get().intValue());
gameManager.getGames().put(0, new Game(0, cuMock, gameManager, timer));
gameManager.getGames().put(0, new Game(0, cuMock, gameManager, timer, null));
assertEquals(1, gameManager.get().intValue());
gameManager.getGames().put(1, new Game(1, cuMock, gameManager, timer));
gameManager.getGames().put(1, new Game(1, cuMock, gameManager, timer, null));
assertEquals(2, gameManager.get().intValue());
gameManager.getGames().put(2, new Game(2, cuMock, gameManager, timer));
gameManager.getGames().put(2, new Game(2, cuMock, gameManager, timer, null));
// make sure it says it can't make any more
assertEquals(-1, gameManager.get().intValue());
@ -130,13 +147,13 @@ public class GameManagerTest {
gameManager.destroyGame(1);
// make sure it re-uses that id
assertEquals(1, gameManager.get().intValue());
gameManager.getGames().put(1, new Game(1, cuMock, gameManager, timer));
gameManager.getGames().put(1, new Game(1, cuMock, gameManager, timer, null));
assertEquals(-1, gameManager.get().intValue());
// remove game 1 out from under it, to make sure it'll fix itself
gameManager.getGames().remove(1);
assertEquals(1, gameManager.get().intValue());
gameManager.getGames().put(1, new Game(1, cuMock, gameManager, timer));
gameManager.getGames().put(1, new Game(1, cuMock, gameManager, timer, null));
assertEquals(-1, gameManager.get().intValue());
gameManager.destroyGame(2);

View File

@ -61,7 +61,7 @@ public class GameTest {
public void setUp() throws Exception {
cuMock = createMock(ConnectedUsers.class);
gmMock = createMock(GameManager.class);
game = new Game(0, cuMock, gmMock, timer);
game = new Game(0, cuMock, gmMock, timer, null);
}
@SuppressWarnings("unchecked")