feat(command-zone): add server-side support with tests

Implement server-side protocol and game logic for networked command
zone play.

Protocol changes:
- room_commands.proto: command zone messages and responses

Server components:
- ServerCounter: counter synchronization for commander tax
- ServerGame: command zone game state management
- ServerPlayer: player-level command zone handling
- Server/ServerProtocolHandler: command zone message routing
- Servatrice: command zone support in the main server

Design decisions:
- setCount() returns bool for event suppression (avoid duplicate events)
- Zone state synchronized across all connected clients
- Commander tax persists across zone transitions

Test coverage verifies setCount() return value behavior for proper
event suppression in networked scenarios.
This commit is contained in:
DawnFire42 2026-02-25 13:38:59 -05:00
parent 47fade7bcb
commit c2a10466db
13 changed files with 457 additions and 22 deletions

View file

@ -54,7 +54,7 @@ add_library(
add_dependencies(libcockatrice_network_server_remote libcockatrice_protocol)
target_include_directories(libcockatrice_network_server_remote PUBLIC .)
target_include_directories(libcockatrice_network_server_remote PUBLIC . ${CMAKE_CURRENT_SOURCE_DIR}/../../../../)
# Make cockatrice_server depend on cockatrice_protocol
target_link_libraries(

View file

@ -2,8 +2,15 @@
#include <libcockatrice/protocol/pb/serverinfo_counter.pb.h>
Server_Counter::Server_Counter(int _id, const QString &_name, const color &_counterColor, int _radius, int _count)
: id(_id), name(_name), counterColor(_counterColor), radius(_radius), count(_count)
Server_Counter::Server_Counter(int _id,
const QString &_name,
const color &_counterColor,
int _radius,
int _count,
int _minValue,
int _maxValue)
: id(_id), name(_name), counterColor(_counterColor), radius(_radius), count(qBound(_minValue, _count, _maxValue)),
minValue(_minValue), maxValue(_maxValue)
{
}

View file

@ -22,6 +22,7 @@
#include <QString>
#include <libcockatrice/protocol/pb/color.pb.h>
#include <limits>
class ServerInfo_Counter;
@ -33,9 +34,20 @@ protected:
color counterColor;
int radius;
int count;
int minValue;
int maxValue;
// Maximum reasonable counter value to prevent rendering/memory issues
static constexpr int DEFAULT_MAX_VALUE = 9999;
public:
Server_Counter(int _id, const QString &_name, const color &_counterColor, int _radius, int _count = 0);
Server_Counter(int _id,
const QString &_name,
const color &_counterColor,
int _radius,
int _count = 0,
int _minValue = std::numeric_limits<int>::min(),
int _maxValue = DEFAULT_MAX_VALUE);
~Server_Counter()
{
}
@ -59,9 +71,11 @@ public:
{
return count;
}
void setCount(int _count)
bool setCount(int _count)
{
count = _count;
int oldCount = count;
count = qBound(minValue, _count, maxValue);
return count != oldCount;
}
void getInfo(ServerInfo_Counter *info);

View file

@ -66,6 +66,9 @@ Server_Game::Server_Game(const ServerInfo_User &_creatorInfo,
bool _spectatorsSeeEverything,
int _startingLifeTotal,
bool _shareDecklistsOnLoad,
bool _enableCommandZone,
bool _enableCompanionZone,
bool _enableBackgroundZone,
Server_Room *_room)
: QObject(), room(_room), nextPlayerId(0), hostId(0), creatorInfo(new ServerInfo_User(_creatorInfo)),
gameStarted(false), gameClosed(false), gameId(_gameId), password(_password), maxPlayers(_maxPlayers),
@ -73,9 +76,10 @@ Server_Game::Server_Game(const ServerInfo_User &_creatorInfo,
onlyRegistered(_onlyRegistered), spectatorsAllowed(_spectatorsAllowed),
spectatorsNeedPassword(_spectatorsNeedPassword), spectatorsCanTalk(_spectatorsCanTalk),
spectatorsSeeEverything(_spectatorsSeeEverything), startingLifeTotal(_startingLifeTotal),
shareDecklistsOnLoad(_shareDecklistsOnLoad), inactivityCounter(0), startTimeOfThisGame(0), secondsElapsed(0),
firstGameStarted(false), turnOrderReversed(false), startTime(QDateTime::currentDateTime()), pingClock(nullptr),
gameMutex()
shareDecklistsOnLoad(_shareDecklistsOnLoad), enableCommandZone(_enableCommandZone),
enableCompanionZone(_enableCompanionZone), enableBackgroundZone(_enableBackgroundZone), inactivityCounter(0),
startTimeOfThisGame(0), secondsElapsed(0), firstGameStarted(false), turnOrderReversed(false),
startTime(QDateTime::currentDateTime()), pingClock(nullptr), gameMutex()
{
currentReplay = new GameReplay;
currentReplay->set_replay_id(room->getServer()->getDatabaseInterface()->getNextReplayId());

View file

@ -68,6 +68,9 @@ private:
bool spectatorsSeeEverything;
int startingLifeTotal;
bool shareDecklistsOnLoad;
bool enableCommandZone;
bool enableCompanionZone;
bool enableBackgroundZone;
int inactivityCounter;
int startTimeOfThisGame, secondsElapsed;
bool firstGameStarted;
@ -105,6 +108,9 @@ public:
bool _spectatorsSeeEverything,
int _startingLifeTotal,
bool _shareDecklistsOnLoad,
bool _enableCommandZone,
bool _enableCompanionZone,
bool _enableBackgroundZone,
Server_Room *parent);
~Server_Game() override;
Server_Room *getRoom() const
@ -172,6 +178,18 @@ public:
{
return shareDecklistsOnLoad;
}
bool getEnableCommandZone() const
{
return enableCommandZone;
}
bool getEnableCompanionZone() const
{
return enableCompanionZone;
}
bool getEnableBackgroundZone() const
{
return enableBackgroundZone;
}
Response::ResponseCode
checkJoin(ServerInfo_User *user, const QString &_password, bool spectator, bool overrideRestrictions, bool asJudge);
bool containsUser(const QString &userName) const;

View file

@ -13,6 +13,7 @@
#include <QDebug>
#include <QRegularExpression>
#include <algorithm>
#include <libcockatrice/common/counter_ids.h>
#include <libcockatrice/deck_list/deck_list.h>
#include <libcockatrice/deck_list/tree/deck_list_card_node.h>
#include <libcockatrice/protocol/pb/command_attach_card.pb.h>
@ -99,6 +100,24 @@ void Server_Player::setupZones()
addCounter(new Server_Counter(6, "x", makeColor(255, 255, 255), 20, 0));
addCounter(new Server_Counter(7, "storm", makeColor(255, 150, 30), 20, 0));
// Command zones for Commander format
if (game->getEnableCommandZone()) {
addZone(new Server_CardZone(this, "command", false, ServerInfo_Zone::PublicZone));
addCounter(new Server_Counter(CounterIds::CommanderTax, CounterNames::CommanderTax, makeColor(128, 128, 128),
20, 0, 0));
addZone(new Server_CardZone(this, "partner", false, ServerInfo_Zone::PublicZone));
addCounter(
new Server_Counter(CounterIds::PartnerTax, CounterNames::PartnerTax, makeColor(128, 128, 128), 20, 0, 0));
}
// Companion and Background zones are independent mechanics
if (game->getEnableCompanionZone()) {
addZone(new Server_CardZone(this, "companion", false, ServerInfo_Zone::PublicZone));
}
if (game->getEnableBackgroundZone()) {
addZone(new Server_CardZone(this, "background", false, ServerInfo_Zone::PublicZone));
}
// ------------------------------------------------------------------
// Assign card ids and create deck from deck list
@ -424,6 +443,10 @@ Server_Player::cmdUndoDraw(const Command_UndoDraw & /*cmd*/, ResponseContainer &
Response::ResponseCode
Server_Player::cmdIncCounter(const Command_IncCounter &cmd, ResponseContainer & /*rc*/, GameEventStorage &ges)
{
// Security note: Counter ownership is implicitly validated through command routing.
// Each participant's processGameCommand operates on their own counters map,
// so players can only modify their own counters.
if (!game->getGameStarted()) {
return Response::RespGameNotStarted;
}
@ -431,17 +454,27 @@ Server_Player::cmdIncCounter(const Command_IncCounter &cmd, ResponseContainer &
return Response::RespContextError;
}
Server_Counter *c = counters.value(cmd.counter_id(), 0);
const int counterId = cmd.counter_id();
// Defensive validation: Commander Tax counters require command zone to be enabled
if ((counterId == CounterIds::CommanderTax || counterId == CounterIds::PartnerTax) &&
!game->getEnableCommandZone()) {
return Response::RespContextError;
}
Server_Counter *c = counters.value(counterId, nullptr);
if (!c) {
return Response::RespNameNotFound;
}
c->setCount(c->getCount() + cmd.delta());
bool didChange = c->setCount(c->getCount() + cmd.delta());
Event_SetCounter event;
event.set_counter_id(c->getId());
event.set_value(c->getCount());
ges.enqueueGameEvent(event, playerId);
if (didChange) {
Event_SetCounter event;
event.set_counter_id(c->getId());
event.set_value(c->getCount());
ges.enqueueGameEvent(event, playerId);
}
return Response::RespOk;
}
@ -475,6 +508,8 @@ Server_Player::cmdCreateCounter(const Command_CreateCounter &cmd, ResponseContai
Response::ResponseCode
Server_Player::cmdSetCounter(const Command_SetCounter &cmd, ResponseContainer & /*rc*/, GameEventStorage &ges)
{
// Security note: Counter ownership is implicitly validated through command routing.
if (!game->getGameStarted()) {
return Response::RespGameNotStarted;
}
@ -482,17 +517,27 @@ Server_Player::cmdSetCounter(const Command_SetCounter &cmd, ResponseContainer &
return Response::RespContextError;
}
Server_Counter *c = counters.value(cmd.counter_id(), 0);
const int counterId = cmd.counter_id();
// Defensive validation: Commander Tax counters require command zone to be enabled
if ((counterId == CounterIds::CommanderTax || counterId == CounterIds::PartnerTax) &&
!game->getEnableCommandZone()) {
return Response::RespContextError;
}
Server_Counter *c = counters.value(counterId, nullptr);
if (!c) {
return Response::RespNameNotFound;
}
c->setCount(cmd.value());
bool didChange = c->setCount(cmd.value());
Event_SetCounter event;
event.set_counter_id(c->getId());
event.set_value(c->getCount());
ges.enqueueGameEvent(event, playerId);
if (didChange) {
Event_SetCounter event;
event.set_counter_id(c->getId());
event.set_value(c->getCount());
ges.enqueueGameEvent(event, playerId);
}
return Response::RespOk;
}

View file

@ -174,6 +174,10 @@ public:
{
return false;
}
virtual bool permitCommandZone() const
{
return true; // Default: allow command zones
}
Server_DatabaseInterface *getDatabaseInterface() const;
int getNextLocalGameId()

View file

@ -830,6 +830,12 @@ Server_ProtocolHandler::cmdCreateGame(const Command_CreateGame &cmd, Server_Room
int startingLifeTotal = cmd.has_starting_life_total() ? cmd.starting_life_total() : 20;
bool shareDecklistsOnLoad = cmd.has_share_decklists_on_load() ? cmd.share_decklists_on_load() : false;
// Only enable command zone if server permits it
bool enableCommandZone = cmd.has_enable_command_zone() && cmd.enable_command_zone() && server->permitCommandZone();
// Companion and Background zones are independent of command zone (explicit opt-in)
bool enableCompanionZone = cmd.has_enable_companion_zone() && cmd.enable_companion_zone();
bool enableBackgroundZone = cmd.has_enable_background_zone() && cmd.enable_background_zone();
const int gameId = databaseInterface->getNextGameId();
if (gameId == -1) {
@ -841,7 +847,8 @@ Server_ProtocolHandler::cmdCreateGame(const Command_CreateGame &cmd, Server_Room
auto *game = new Server_Game(copyUserInfo(false), gameId, description, QString::fromStdString(cmd.password()),
cmd.max_players(), gameTypes, cmd.only_buddies(), onlyRegisteredUsers,
cmd.spectators_allowed(), cmd.spectators_need_password(), cmd.spectators_can_talk(),
cmd.spectators_see_everything(), startingLifeTotal, shareDecklistsOnLoad, room);
cmd.spectators_see_everything(), startingLifeTotal, shareDecklistsOnLoad,
enableCommandZone, enableCompanionZone, enableBackgroundZone, room);
game->addPlayer(this, rc, asSpectator, asJudge, false);
room->addGame(game);

View file

@ -69,6 +69,15 @@ message Command_CreateGame {
// share decklists with all players when selected
optional bool share_decklists_on_load = 14;
// enable the command zone for Commander format
optional bool enable_command_zone = 15;
// enable companion zone for Ikoria mechanic
optional bool enable_companion_zone = 16;
// enable background zone for Baldur's Gate mechanic
optional bool enable_background_zone = 17;
}
message Command_JoinGame {

View file

@ -965,6 +965,11 @@ bool Servatrice::permitCreateGameAsJudge() const
return settingsCache->value("game/allow_create_as_judge", false).toBool();
}
bool Servatrice::permitCommandZone() const
{
return settingsCache->value("game/allow_command_zone", true).toBool();
}
QHostAddress Servatrice::getServerTCPHost() const
{
QString host = settingsCache->value("server/host", "any").toString();

View file

@ -265,6 +265,7 @@ public:
int getMaxCommandCountPerInterval() const override;
int getMaxUserTotal() const override;
bool permitCreateGameAsJudge() const override;
bool permitCommandZone() const override;
int getMaxTcpUserLimit() const;
int getMaxWebSocketUserLimit() const;
int getUsersWithAddress(const QHostAddress &address) const;

View file

@ -105,6 +105,21 @@ target_link_libraries(counter_visibility_test
add_test(NAME counter_visibility_test COMMAND counter_visibility_test)
# ------------------------
# Server Counter SetCount Test
# Tests setCount() return value for event suppression
# ------------------------
add_executable(server_counter_setcount_test
server_counter_setcount_test.cpp
)
target_link_libraries(server_counter_setcount_test
PRIVATE Threads::Threads
PRIVATE ${GTEST_BOTH_LIBRARIES}
)
add_test(NAME server_counter_setcount_test COMMAND server_counter_setcount_test)
# ------------------------
# Dependencies on gtest
# ------------------------
@ -114,4 +129,5 @@ if(NOT GTEST_FOUND)
add_dependencies(command_zone_state_test gtest)
add_dependencies(command_zone_integration_test gtest)
add_dependencies(counter_visibility_test gtest)
add_dependencies(server_counter_setcount_test gtest)
endif()

View file

@ -0,0 +1,305 @@
/**
* @file server_counter_setcount_test.cpp
* @brief Unit tests for Server_Counter::setCount() return value behavior
*
* Tests the change-detection contract:
* - Returns false when value unchanged (same value, clamped to same value)
* - Returns true when value actually changes (including when clamping occurs)
*
* This ensures that Event_SetCounter is only emitted when the counter
* value genuinely changes, preventing spurious log messages.
*/
#include <algorithm>
#include <gtest/gtest.h>
#include <limits>
/**
* @brief Testable implementation of Server_Counter::setCount() logic.
*
* Extracts the setCount logic for unit testing without requiring
* full Qt and server dependencies. Uses std::clamp to mirror qBound.
*
* @warning SYNC REQUIRED: This harness duplicates state logic from
* Server_Counter (server_counter.h). If that implementation changes,
* update this harness to match.
*/
class ServerCounterTestHarness
{
int count;
int minValue;
int maxValue;
public:
explicit ServerCounterTestHarness(int _count = 0,
int _minValue = std::numeric_limits<int>::min(),
int _maxValue = 9999)
: count(_count), minValue(_minValue), maxValue(_maxValue)
{
}
/**
* @brief Sets the counter value with bounds checking.
* @param _count The requested value
* @return true if the value actually changed, false otherwise
*
* Mirrors Server_Counter::setCount() exactly.
*/
bool setCount(int _count)
{
int oldCount = count;
count = std::clamp(_count, minValue, maxValue);
return count != oldCount;
}
[[nodiscard]] int getCount() const
{
return count;
}
[[nodiscard]] int getMinValue() const
{
return minValue;
}
[[nodiscard]] int getMaxValue() const
{
return maxValue;
}
};
class ServerCounterSetCountTest : public ::testing::Test
{
protected:
ServerCounterTestHarness *counter;
void SetUp() override
{
counter = nullptr;
}
void TearDown() override
{
delete counter;
}
void createCounterWithMinMax(int initial = 0, int minVal = 0, int maxVal = 9999)
{
delete counter;
counter = new ServerCounterTestHarness(initial, minVal, maxVal);
}
void createCommanderTaxCounter(int initial = 0)
{
createCounterWithMinMax(initial, 0, 9999);
}
};
// ============ Return Value Tests: Value Unchanged ============
TEST_F(ServerCounterSetCountTest, SetCount_ReturnsFalse_WhenClampedAtMin)
{
createCommanderTaxCounter(0);
bool result = counter->setCount(-5);
EXPECT_FALSE(result) << "setCount should return false when clamped to same value at min";
EXPECT_EQ(0, counter->getCount()) << "Value should remain at min bound";
}
TEST_F(ServerCounterSetCountTest, SetCount_ReturnsFalse_WhenClampedAtMax)
{
createCommanderTaxCounter(9999);
bool result = counter->setCount(10000);
EXPECT_FALSE(result) << "setCount should return false when clamped to same value at max";
EXPECT_EQ(9999, counter->getCount()) << "Value should remain at max bound";
}
TEST_F(ServerCounterSetCountTest, SetCount_ReturnsFalse_WhenSettingToSameValue)
{
createCommanderTaxCounter(5);
bool result = counter->setCount(5);
EXPECT_FALSE(result) << "setCount should return false when setting to same value";
EXPECT_EQ(5, counter->getCount()) << "Value should remain unchanged";
}
TEST_F(ServerCounterSetCountTest, SetCount_ReturnsFalse_WhenAlreadyAtMinAndDecrementingMore)
{
createCommanderTaxCounter(0);
bool result = counter->setCount(-1000);
EXPECT_FALSE(result) << "setCount should return false when already at min and trying to go lower";
EXPECT_EQ(0, counter->getCount()) << "Value should stay at min";
}
TEST_F(ServerCounterSetCountTest, SetCount_ReturnsFalse_WhenAlreadyAtMaxAndIncrementingMore)
{
createCommanderTaxCounter(9999);
bool result = counter->setCount(99999);
EXPECT_FALSE(result) << "setCount should return false when already at max and trying to go higher";
EXPECT_EQ(9999, counter->getCount()) << "Value should stay at max";
}
// ============ Return Value Tests: Value Changed ============
TEST_F(ServerCounterSetCountTest, SetCount_ReturnsTrue_WhenValueChanges)
{
createCommanderTaxCounter(0);
bool result = counter->setCount(1);
EXPECT_TRUE(result) << "setCount should return true when value changes";
EXPECT_EQ(1, counter->getCount()) << "Value should be updated";
}
TEST_F(ServerCounterSetCountTest, SetCount_ReturnsTrue_WhenValueChangesWithClamping)
{
createCommanderTaxCounter(5);
bool result = counter->setCount(-100);
EXPECT_TRUE(result) << "setCount should return true when value changes even if clamped";
EXPECT_EQ(0, counter->getCount()) << "Value should be clamped to min";
}
TEST_F(ServerCounterSetCountTest, SetCount_ReturnsTrue_WhenValueDecrements)
{
createCommanderTaxCounter(5);
bool result = counter->setCount(4);
EXPECT_TRUE(result) << "setCount should return true when value decrements";
EXPECT_EQ(4, counter->getCount()) << "Value should be decremented";
}
TEST_F(ServerCounterSetCountTest, SetCount_ReturnsTrue_WhenGoingToMin)
{
createCommanderTaxCounter(1);
bool result = counter->setCount(0);
EXPECT_TRUE(result) << "setCount should return true when reaching min from above";
EXPECT_EQ(0, counter->getCount()) << "Value should be at min";
}
TEST_F(ServerCounterSetCountTest, SetCount_ReturnsTrue_WhenGoingToMax)
{
createCommanderTaxCounter(9998);
bool result = counter->setCount(9999);
EXPECT_TRUE(result) << "setCount should return true when reaching max from below";
EXPECT_EQ(9999, counter->getCount()) << "Value should be at max";
}
TEST_F(ServerCounterSetCountTest, SetCount_ReturnsTrue_WhenValueChangesToMaxWithClamping)
{
createCommanderTaxCounter(100);
bool result = counter->setCount(100000);
EXPECT_TRUE(result) << "setCount should return true when value changes to max via clamping";
EXPECT_EQ(9999, counter->getCount()) << "Value should be clamped to max";
}
// ============ Command Handler Simulation Tests ============
// These simulate the cmdIncCounter/cmdSetCounter behavior
TEST_F(ServerCounterSetCountTest, IncrementAtMin_EventWouldBeEmitted)
{
createCommanderTaxCounter(0);
bool didChange = counter->setCount(counter->getCount() + 1);
EXPECT_TRUE(didChange) << "Increment from 0 to 1 should emit event";
}
TEST_F(ServerCounterSetCountTest, DecrementAtMin_NoEventEmitted)
{
createCommanderTaxCounter(0);
bool didChange = counter->setCount(counter->getCount() - 1);
EXPECT_FALSE(didChange) << "Decrement at 0 should NOT emit event";
}
TEST_F(ServerCounterSetCountTest, IncrementAtMax_NoEventEmitted)
{
createCommanderTaxCounter(9999);
bool didChange = counter->setCount(counter->getCount() + 1);
EXPECT_FALSE(didChange) << "Increment at max should NOT emit event";
}
TEST_F(ServerCounterSetCountTest, DecrementAtMax_EventWouldBeEmitted)
{
createCommanderTaxCounter(9999);
bool didChange = counter->setCount(counter->getCount() - 1);
EXPECT_TRUE(didChange) << "Decrement from max should emit event";
}
TEST_F(ServerCounterSetCountTest, SetToSameValue_NoEventEmitted)
{
createCommanderTaxCounter(5);
bool didChange = counter->setCount(5);
EXPECT_FALSE(didChange) << "Setting to same value should NOT emit event";
}
TEST_F(ServerCounterSetCountTest, SetToDifferentValue_EventWouldBeEmitted)
{
createCommanderTaxCounter(5);
bool didChange = counter->setCount(10);
EXPECT_TRUE(didChange) << "Setting to different value should emit event";
}
// ============ Edge Cases ============
TEST_F(ServerCounterSetCountTest, CounterWithNegativeMinValue)
{
createCounterWithMinMax(0, -10, 10);
bool result = counter->setCount(-5);
EXPECT_TRUE(result) << "Going to -5 from 0 with min=-10 should change";
EXPECT_EQ(-5, counter->getCount());
}
TEST_F(ServerCounterSetCountTest, CounterWithNegativeMinValue_ClampedAtMin)
{
createCounterWithMinMax(-10, -10, 10);
bool result = counter->setCount(-20);
EXPECT_FALSE(result) << "Already at min, should not change";
EXPECT_EQ(-10, counter->getCount());
}
TEST_F(ServerCounterSetCountTest, ZeroRangeCounter_CannotChange)
{
createCounterWithMinMax(5, 5, 5);
bool result = counter->setCount(10);
EXPECT_FALSE(result) << "Counter with min=max cannot change";
EXPECT_EQ(5, counter->getCount());
}
int main(int argc, char **argv)
{
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
}