From 554368967d3aeac77c6d0b033db0b960bf68e87c Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Sun, 8 Nov 2026 16:22:30 +0100 Subject: [PATCH] [Refactor] Extract GameCreationOptions from DlgCreateGame (#7800) * Extract GameCreationOptions from DlgCreateGame * Address review findings --- cockatrice/CMakeLists.txt | 1 + .../widgets/dialogs/dlg_create_game.cpp | 41 ++++---- .../game_hosting/game_creation_options.cpp | 24 +++++ .../game_hosting/game_creation_options.h | 44 +++++++++ tests/CMakeLists.txt | 11 +++ tests/game_creation_options_test.cpp | 93 +++++++++++++++++++ 6 files changed, 197 insertions(+), 17 deletions(-) create mode 100644 cockatrice/src/interface/widgets/game_hosting/game_creation_options.cpp create mode 100644 cockatrice/src/interface/widgets/game_hosting/game_creation_options.h create mode 100644 tests/game_creation_options_test.cpp diff --git a/cockatrice/CMakeLists.txt b/cockatrice/CMakeLists.txt index 984784161..46e6702d9 100644 --- a/cockatrice/CMakeLists.txt +++ b/cockatrice/CMakeLists.txt @@ -216,6 +216,7 @@ set(cockatrice_SOURCES src/interface/widgets/deck_editor/deck_state_manager.cpp src/interface/widgets/deck_editor/deck_zone_dialog.cpp src/interface/widgets/deck_editor/printing_disabled_info_widget.cpp + src/interface/widgets/game_hosting/game_creation_options.cpp src/interface/widgets/general/background_sources.cpp src/interface/widgets/general/display/background_plate_widget.cpp src/interface/widgets/general/display/banner_widget.cpp diff --git a/cockatrice/src/interface/widgets/dialogs/dlg_create_game.cpp b/cockatrice/src/interface/widgets/dialogs/dlg_create_game.cpp index ada4237f7..2030882fd 100644 --- a/cockatrice/src/interface/widgets/dialogs/dlg_create_game.cpp +++ b/cockatrice/src/interface/widgets/dialogs/dlg_create_game.cpp @@ -1,6 +1,7 @@ #include "dlg_create_game.h" #include "../../../client/settings/cache_settings.h" +#include "../interface/widgets/game_hosting/game_creation_options.h" #include "../interface/widgets/tabs/tab_room.h" #include "libcockatrice/protocol/pb/response.pb.h" #include "libcockatrice/protocol/pb/room_commands.pb.h" @@ -14,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -287,32 +289,37 @@ void DlgCreateGame::actReset() void DlgCreateGame::actOK() { - Command_CreateGame cmd; - cmd.set_description(descriptionEdit->text().simplified().toStdString()); - cmd.set_password(passwordEdit->text().toStdString()); - cmd.set_max_players(maxPlayersEdit->value()); - cmd.set_only_buddies(onlyBuddiesCheckBox->isChecked()); - cmd.set_only_registered(onlyRegisteredCheckBox->isChecked()); - cmd.set_spectators_allowed(spectatorsAllowedCheckBox->isChecked()); - cmd.set_spectators_need_password(spectatorsNeedPasswordCheckBox->isChecked()); - cmd.set_spectators_can_talk(spectatorsCanTalkCheckBox->isChecked()); - cmd.set_spectators_see_everything(spectatorsSeeEverythingCheckBox->isChecked()); - cmd.set_join_as_judge(createGameAsJudgeCheckBox->isChecked()); - cmd.set_join_as_spectator(createGameAsSpectatorCheckBox->isChecked()); - cmd.set_starting_life_total(startingLifeTotalEdit->value()); - cmd.set_share_decklists_on_load(shareDecklistsOnLoadCheckBox->isChecked()); - cmd.set_shuffle_players(shufflePlayersCheckBox->isChecked()); - auto _gameTypes = QString(); + QList gameTypeIds; QMapIterator gameTypeCheckBoxIterator(gameTypeCheckBoxes); while (gameTypeCheckBoxIterator.hasNext()) { gameTypeCheckBoxIterator.next(); if (gameTypeCheckBoxIterator.value()->isChecked()) { - cmd.add_game_type_ids(gameTypeCheckBoxIterator.key()); + gameTypeIds.append(gameTypeCheckBoxIterator.key()); _gameTypes += gameTypeCheckBoxIterator.value()->text() + ", "; } } + GameCreationOptions options{ + .description = descriptionEdit->text(), + .password = passwordEdit->text(), + .maxPlayers = maxPlayersEdit->value(), + .onlyBuddies = onlyBuddiesCheckBox->isChecked(), + .onlyRegistered = onlyRegisteredCheckBox->isChecked(), + .spectatorsAllowed = spectatorsAllowedCheckBox->isChecked(), + .spectatorsNeedPassword = spectatorsNeedPasswordCheckBox->isChecked(), + .spectatorsCanTalk = spectatorsCanTalkCheckBox->isChecked(), + .spectatorsSeeEverything = spectatorsSeeEverythingCheckBox->isChecked(), + .joinAsJudge = createGameAsJudgeCheckBox->isChecked(), + .joinAsSpectator = createGameAsSpectatorCheckBox->isChecked(), + .startingLifeTotal = startingLifeTotalEdit->value(), + .shareDecklistsOnLoad = shareDecklistsOnLoadCheckBox->isChecked(), + .shufflePlayers = shufflePlayersCheckBox->isChecked(), + .gameTypeIds = gameTypeIds, + }; + + Command_CreateGame cmd = options.toCreateGameCommand(); + SettingsCache::instance().game().setRememberGameSettings(rememberGameSettings->isChecked()); if (rememberGameSettings->isChecked()) { SettingsCache::instance().game().setGameDescription(descriptionEdit->text()); diff --git a/cockatrice/src/interface/widgets/game_hosting/game_creation_options.cpp b/cockatrice/src/interface/widgets/game_hosting/game_creation_options.cpp new file mode 100644 index 000000000..33e30adb1 --- /dev/null +++ b/cockatrice/src/interface/widgets/game_hosting/game_creation_options.cpp @@ -0,0 +1,24 @@ +#include "game_creation_options.h" + +Command_CreateGame GameCreationOptions::toCreateGameCommand() const +{ + Command_CreateGame cmd; + cmd.set_description(description.simplified().toStdString()); + cmd.set_password(password.toStdString()); + cmd.set_max_players(maxPlayers); + cmd.set_only_buddies(onlyBuddies); + cmd.set_only_registered(onlyRegistered); + cmd.set_spectators_allowed(spectatorsAllowed); + cmd.set_spectators_need_password(spectatorsNeedPassword); + cmd.set_spectators_can_talk(spectatorsCanTalk); + cmd.set_spectators_see_everything(spectatorsSeeEverything); + cmd.set_join_as_judge(joinAsJudge); + cmd.set_join_as_spectator(joinAsSpectator); + cmd.set_starting_life_total(startingLifeTotal); + cmd.set_share_decklists_on_load(shareDecklistsOnLoad); + cmd.set_shuffle_players(shufflePlayers); + for (int gameTypeId : gameTypeIds) { + cmd.add_game_type_ids(gameTypeId); + } + return cmd; +} diff --git a/cockatrice/src/interface/widgets/game_hosting/game_creation_options.h b/cockatrice/src/interface/widgets/game_hosting/game_creation_options.h new file mode 100644 index 000000000..15284eee0 --- /dev/null +++ b/cockatrice/src/interface/widgets/game_hosting/game_creation_options.h @@ -0,0 +1,44 @@ +/** + * @file game_creation_options.h + * @ingroup RoomWidgets + */ + +#ifndef GAME_CREATION_OPTIONS_H +#define GAME_CREATION_OPTIONS_H + +#include "libcockatrice/protocol/pb/room_commands.pb.h" + +#include +#include + +/** + * @brief The values needed to create a game, independent of the widgets that edit them. + * + * Pure value object: it holds no widget or settings dependencies, so every host entry + * point can build a @ref Command_CreateGame through the same conversion. + */ +struct GameCreationOptions +{ + QString description; + QString password; + int maxPlayers = 2; + bool onlyBuddies = false; + bool onlyRegistered = false; + bool spectatorsAllowed = true; + bool spectatorsNeedPassword = false; + bool spectatorsCanTalk = false; + bool spectatorsSeeEverything = false; + bool joinAsJudge = false; + bool joinAsSpectator = false; + int startingLifeTotal = 20; + bool shareDecklistsOnLoad = false; + bool shufflePlayers = false; + QList gameTypeIds; + + /** + * @brief Builds the room command, simplifying the description's whitespace. + */ + [[nodiscard]] Command_CreateGame toCreateGameCommand() const; +}; + +#endif diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 0450e5b8a..7595706b2 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -462,6 +462,17 @@ cockatrice_add_test( INCLUDE_DIRS ${CMAKE_SOURCE_DIR}/cockatrice/src ) +cockatrice_add_test( + NAME + game_creation_options_test + SOURCES + ${CMAKE_SOURCE_DIR}/cockatrice/src/interface/widgets/game_hosting/game_creation_options.cpp + game_creation_options_test.cpp + LIBS + libcockatrice_protocol + INCLUDE_DIRS + ${CMAKE_SOURCE_DIR}/cockatrice/src +) cockatrice_add_test( NAME deck_hash_performance_test diff --git a/tests/game_creation_options_test.cpp b/tests/game_creation_options_test.cpp new file mode 100644 index 000000000..e182e4e91 --- /dev/null +++ b/tests/game_creation_options_test.cpp @@ -0,0 +1,93 @@ +#include "interface/widgets/game_hosting/game_creation_options.h" + +#include +#include + +TEST(GameCreationOptions, DefaultsMatchTheDialog) +{ + const GameCreationOptions options; + + EXPECT_TRUE(options.description.isEmpty()); + EXPECT_TRUE(options.password.isEmpty()); + EXPECT_EQ(options.maxPlayers, 2); + EXPECT_FALSE(options.onlyBuddies); + EXPECT_FALSE(options.onlyRegistered); + EXPECT_TRUE(options.spectatorsAllowed); + EXPECT_FALSE(options.spectatorsNeedPassword); + EXPECT_FALSE(options.spectatorsCanTalk); + EXPECT_FALSE(options.spectatorsSeeEverything); + EXPECT_FALSE(options.joinAsJudge); + EXPECT_FALSE(options.joinAsSpectator); + EXPECT_EQ(options.startingLifeTotal, 20); + EXPECT_FALSE(options.shareDecklistsOnLoad); + EXPECT_FALSE(options.shufflePlayers); + EXPECT_TRUE(options.gameTypeIds.isEmpty()); +} + +TEST(GameCreationOptions, EveryFieldMapsToTheCommand) +{ + GameCreationOptions options; + options.description = "Some game"; + options.password = "hunter2"; + options.maxPlayers = 4; + options.onlyBuddies = true; + options.onlyRegistered = true; + options.spectatorsAllowed = false; + options.spectatorsNeedPassword = true; + options.spectatorsCanTalk = true; + options.spectatorsSeeEverything = true; + options.joinAsJudge = true; + options.joinAsSpectator = true; + options.startingLifeTotal = 40; + options.shareDecklistsOnLoad = true; + options.shufflePlayers = true; + + const Command_CreateGame cmd = options.toCreateGameCommand(); + + EXPECT_EQ(cmd.description(), "Some game"); + EXPECT_EQ(cmd.password(), "hunter2"); + EXPECT_EQ(cmd.max_players(), 4); + EXPECT_TRUE(cmd.only_buddies()); + EXPECT_TRUE(cmd.only_registered()); + EXPECT_FALSE(cmd.spectators_allowed()); + EXPECT_TRUE(cmd.spectators_need_password()); + EXPECT_TRUE(cmd.spectators_can_talk()); + EXPECT_TRUE(cmd.spectators_see_everything()); + EXPECT_TRUE(cmd.join_as_judge()); + EXPECT_TRUE(cmd.join_as_spectator()); + EXPECT_EQ(cmd.starting_life_total(), 40); + EXPECT_TRUE(cmd.share_decklists_on_load()); + EXPECT_TRUE(cmd.shuffle_players()); +} + +TEST(GameCreationOptions, GameTypeIdsKeepTheirOrder) +{ + GameCreationOptions options; + options.gameTypeIds = {7, 2, 5}; + + const Command_CreateGame cmd = options.toCreateGameCommand(); + + ASSERT_EQ(cmd.game_type_ids_size(), 3); + EXPECT_EQ(cmd.game_type_ids(0), 7); + EXPECT_EQ(cmd.game_type_ids(1), 2); + EXPECT_EQ(cmd.game_type_ids(2), 5); +} + +TEST(GameCreationOptions, EmptyGameTypeIdsProduceAnEmptyRepeatedField) +{ + GameCreationOptions options; + + const Command_CreateGame cmd = options.toCreateGameCommand(); + + EXPECT_EQ(cmd.game_type_ids_size(), 0); +} + +TEST(GameCreationOptions, DescriptionWhitespaceIsSimplified) +{ + GameCreationOptions options; + options.description = " a b "; + + const Command_CreateGame cmd = options.toCreateGameCommand(); + + EXPECT_EQ(cmd.description(), "a b"); +}