From aac2488d5c8ac3119c17db06776c41c3649372b6 Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Sat, 20 Aug 2022 20:21:33 +0100 Subject: [PATCH] LibCards+Games: Replace card "value" int with a Rank enum Because `card->value() == 11` is a lot less clear than `card->rank() == Cards::Rank::Queen`, and also safer. Put this, along with the `Suit` enum, in the `Cards` namespace directly instead of inside `Cards::Card`. Slightly less typing that way. --- Userland/Games/Hearts/Game.cpp | 34 ++++----- Userland/Games/Hearts/Helpers.h | 8 +- Userland/Games/Hearts/Player.cpp | 10 +-- Userland/Games/Hearts/Player.h | 6 +- Userland/Games/Solitaire/Game.cpp | 10 +-- Userland/Games/Spider/Game.cpp | 15 ++-- Userland/Libraries/LibCards/Card.cpp | 9 +-- Userland/Libraries/LibCards/Card.h | 92 +++++++++++++++++------ Userland/Libraries/LibCards/CardStack.cpp | 12 +-- 9 files changed, 121 insertions(+), 75 deletions(-) diff --git a/Userland/Games/Hearts/Game.cpp b/Userland/Games/Hearts/Game.cpp index 348c24967a..a792c0647c 100644 --- a/Userland/Games/Hearts/Game.cpp +++ b/Userland/Games/Hearts/Game.cpp @@ -203,10 +203,10 @@ void Game::setup(String player_name, int hand_number) deck.ensure_capacity(Card::card_count * 4); for (int i = 0; i < Card::card_count; ++i) { - deck.append(Card::construct(Card::Suit::Clubs, i)); - deck.append(Card::construct(Card::Suit::Spades, i)); - deck.append(Card::construct(Card::Suit::Hearts, i)); - deck.append(Card::construct(Card::Suit::Diamonds, i)); + deck.append(Card::construct(Cards::Suit::Clubs, static_cast(i))); + deck.append(Card::construct(Cards::Suit::Spades, static_cast(i))); + deck.append(Card::construct(Cards::Suit::Hearts, static_cast(i))); + deck.append(Card::construct(Cards::Suit::Diamonds, static_cast(i))); } for (auto& player : m_players) { @@ -314,7 +314,7 @@ bool Game::other_player_has_queen_of_spades(Player& player) for (auto& other_player : m_players) { if (&player != &other_player) { for (auto& other_card : other_player.hand) { - if (other_card && other_card->suit() == Card::Suit::Spades && hearts_card_value(*other_card) == CardValue::Queen) + if (other_card && other_card->suit() == Cards::Suit::Spades && hearts_card_value(*other_card) == CardValue::Queen) return true; } } @@ -335,7 +335,7 @@ size_t Game::pick_card(Player& player) bool is_first_trick = m_trick_number == 0; if (is_leading_player) { if (is_first_trick) { - auto clubs_2 = player.pick_specific_card(Card::Suit::Clubs, CardValue::Number_2); + auto clubs_2 = player.pick_specific_card(Cards::Suit::Clubs, CardValue::Number_2); VERIFY(clubs_2.has_value()); return clubs_2.value(); } else { @@ -352,8 +352,8 @@ size_t Game::pick_card(Player& player) for (auto& card : m_trick) if (high_card->suit() == card.suit() && hearts_card_value(card) > hearts_card_value(*high_card)) high_card = &card; - if (high_card->suit() == Card::Suit::Spades && hearts_card_value(*high_card) > CardValue::Queen) - RETURN_CARD_IF_VALID(player.pick_specific_card(Card::Suit::Spades, CardValue::Queen)); + if (high_card->suit() == Cards::Suit::Spades && hearts_card_value(*high_card) > CardValue::Queen) + RETURN_CARD_IF_VALID(player.pick_specific_card(Cards::Suit::Spades, CardValue::Queen)); auto card_has_points = [](Card& card) { return hearts_card_points(card) > 0; }; auto trick_has_points = m_trick.first_matching(card_has_points).has_value(); bool is_trailing_player = m_trick.size() == 3; @@ -376,7 +376,7 @@ size_t Game::pick_card(Player& player) if (is_third_player && !trick_has_points) { play_highest_value_card = true; - if (high_card->suit() == Card::Suit::Spades && other_player_has_queen_of_spades(player)) { + if (high_card->suit() == Cards::Suit::Spades && other_player_has_queen_of_spades(player)) { Optional chosen_card_index = player.pick_low_points_high_value_card(high_card->suit()); if (chosen_card_index.has_value()) { auto& card = player.hand[chosen_card_index.value()]; @@ -518,7 +518,7 @@ void Game::advance_game() // Find whoever has 2 of Clubs, they get to play the first card for (auto& player : m_players) { auto clubs_2_card = player.hand.first_matching([](auto& card) { - return card->suit() == Card::Suit::Clubs && hearts_card_value(*card) == CardValue::Number_2; + return card->suit() == Cards::Suit::Clubs && hearts_card_value(*card) == CardValue::Number_2; }); if (clubs_2_card.has_value()) { m_leading_player = &player; @@ -632,7 +632,7 @@ bool Game::is_valid_play(Player& player, Card& card, String* explanation) const if (m_trick_number == 0 && m_trick.is_empty()) { if (explanation) *explanation = "The first card must be Two of Clubs."; - return card.suit() == Card::Suit::Clubs && hearts_card_value(card) == CardValue::Number_2; + return card.suit() == Cards::Suit::Clubs && hearts_card_value(card) == CardValue::Number_2; } // Can't play hearts or The Queen in the first trick. @@ -646,7 +646,7 @@ bool Game::is_valid_play(Player& player, Card& card, String* explanation) const } // ... unless the player only has points cards (e.g. all Hearts or // 12 Hearts + Queen of Spades), in which case they're allowed to play Hearts. - if (all_points_cards && card.suit() == Card::Suit::Hearts) + if (all_points_cards && card.suit() == Cards::Suit::Hearts) return true; if (explanation) *explanation = "You can't play a card worth points in the first trick."; @@ -656,10 +656,10 @@ bool Game::is_valid_play(Player& player, Card& card, String* explanation) const // Leading card can't be hearts until hearts are broken // unless the player only has hearts cards. if (m_trick.is_empty()) { - if (are_hearts_broken() || card.suit() != Card::Suit::Hearts) + if (are_hearts_broken() || card.suit() != Cards::Suit::Hearts) return true; auto non_hearts_card = player.hand.first_matching([](auto const& other_card) { - return !other_card.is_null() && other_card->suit() != Card::Suit::Hearts; + return !other_card.is_null() && other_card->suit() != Cards::Suit::Hearts; }); auto only_has_hearts = !non_hearts_card.has_value(); if (!only_has_hearts && explanation) @@ -681,7 +681,7 @@ bool Game::are_hearts_broken() const { for (auto& player : m_players) for (auto& card : player.cards_taken) - if (card->suit() == Card::Suit::Hearts) + if (card->suit() == Cards::Suit::Hearts) return true; return false; } @@ -750,9 +750,9 @@ int Game::calculate_score(Player& player) for (auto& other_player : m_players) { int score = 0; for (auto& card : other_player.cards_taken) - if (card->suit() == Card::Suit::Spades && card->value() == 11) + if (card->suit() == Cards::Suit::Spades && card->rank() == Cards::Rank::Queen) score += 13; - else if (card->suit() == Card::Suit::Hearts) + else if (card->suit() == Cards::Suit::Hearts) score++; if (!min_score.has_value() || score < min_score.value()) min_score = score; diff --git a/Userland/Games/Hearts/Helpers.h b/Userland/Games/Hearts/Helpers.h index 9e4065e009..61273be5f0 100644 --- a/Userland/Games/Hearts/Helpers.h +++ b/Userland/Games/Hearts/Helpers.h @@ -31,17 +31,17 @@ enum class CardValue : uint8_t { inline CardValue hearts_card_value(Card const& card) { // Ace has a higher value than all other cards in Hearts - if (card.value() == 0) + if (card.rank() == Cards::Rank::Ace) return CardValue::Ace; else - return static_cast(card.value() - 1); + return static_cast(to_underlying(card.rank()) - 1); } inline uint8_t hearts_card_points(Card const& card) { - if (card.suit() == Card::Suit::Hearts) + if (card.suit() == Cards::Suit::Hearts) return 1; - else if (card.suit() == Card::Suit::Spades && hearts_card_value(card) == CardValue::Queen) + else if (card.suit() == Cards::Suit::Spades && hearts_card_value(card) == CardValue::Queen) return 13; else return 0; diff --git a/Userland/Games/Hearts/Player.cpp b/Userland/Games/Hearts/Player.cpp index 9094122d38..80a3c3729c 100644 --- a/Userland/Games/Hearts/Player.cpp +++ b/Userland/Games/Hearts/Player.cpp @@ -71,7 +71,7 @@ size_t Player::pick_lead_card(Function valid_play, Function Player::pick_low_points_high_value_card(Optional suit) +Optional Player::pick_low_points_high_value_card(Optional suit) { auto sorted_hand = hand_sorted_by_fn(compare_card_value); int min_points = -1; @@ -115,10 +115,10 @@ Optional Player::pick_slightly_higher_value_card(Card& other_card) size_t Player::pick_max_points_card(Function ignore_card) { - auto queen_of_spades_maybe = pick_specific_card(Card::Suit::Spades, CardValue::Queen); + auto queen_of_spades_maybe = pick_specific_card(Cards::Suit::Spades, CardValue::Queen); if (queen_of_spades_maybe.has_value()) return queen_of_spades_maybe.value(); - if (has_card_of_suit(Card::Suit::Hearts)) { + if (has_card_of_suit(Cards::Suit::Hearts)) { auto highest_hearts_card_index = pick_last_card(); auto& card = hand[highest_hearts_card_index]; if (!ignore_card(*card)) @@ -127,7 +127,7 @@ size_t Player::pick_max_points_card(Function ignore_card) return pick_low_points_high_value_card().value(); } -Optional Player::pick_specific_card(Card::Suit suit, CardValue value) +Optional Player::pick_specific_card(Cards::Suit suit, CardValue value) { for (size_t i = 0; i < hand.size(); i++) { auto& card = hand[i]; @@ -150,7 +150,7 @@ size_t Player::pick_last_card() VERIFY_NOT_REACHED(); } -bool Player::has_card_of_suit(Card::Suit suit) +bool Player::has_card_of_suit(Cards::Suit suit) { auto matching_card = hand.first_matching([&](auto const& other_card) { return !other_card.is_null() && other_card->suit() == suit; diff --git a/Userland/Games/Hearts/Player.h b/Userland/Games/Hearts/Player.h index 27d31f79bc..4d573eba52 100644 --- a/Userland/Games/Hearts/Player.h +++ b/Userland/Games/Hearts/Player.h @@ -35,13 +35,13 @@ public: NonnullRefPtrVector pick_cards_to_pass(PassingDirection); size_t pick_lead_card(Function, Function); - Optional pick_low_points_high_value_card(Optional suit = {}); + Optional pick_low_points_high_value_card(Optional suit = {}); Optional pick_lower_value_card(Card& other_card); Optional pick_slightly_higher_value_card(Card& other_card); size_t pick_max_points_card(Function); - Optional pick_specific_card(Card::Suit suit, CardValue value); + Optional pick_specific_card(Cards::Suit suit, CardValue value); size_t pick_last_card(); - bool has_card_of_suit(Card::Suit suit); + bool has_card_of_suit(Cards::Suit suit); Vector hand_sorted_by_fn(bool (*)(CardWithIndex&, CardWithIndex&)) const; void sort_hand() { quick_sort(hand, hearts_card_less); } diff --git a/Userland/Games/Solitaire/Game.cpp b/Userland/Games/Solitaire/Game.cpp index 14e4ce5631..7a7b1ff3dc 100644 --- a/Userland/Games/Solitaire/Game.cpp +++ b/Userland/Games/Solitaire/Game.cpp @@ -90,7 +90,7 @@ void Game::timer_event(Core::TimerEvent&) void Game::create_new_animation_card() { - auto card = Card::construct(static_cast(get_random_uniform(to_underlying(Card::Suit::__Count))), get_random_uniform(Card::card_count)); + auto card = Card::construct(static_cast(get_random_uniform(to_underlying(Cards::Suit::__Count))), static_cast(get_random_uniform(to_underlying(Cards::Rank::__Count)))); card->set_position({ get_random_uniform(Game::width - Card::width), get_random_uniform(Game::height / 8) }); int x_sgn = card->position().x() > (Game::width / 2) ? -1 : 1; @@ -163,10 +163,10 @@ void Game::setup(Mode mode) on_undo_availability_change(false); for (int i = 0; i < Card::card_count; ++i) { - m_new_deck.append(Card::construct(Card::Suit::Clubs, i)); - m_new_deck.append(Card::construct(Card::Suit::Spades, i)); - m_new_deck.append(Card::construct(Card::Suit::Hearts, i)); - m_new_deck.append(Card::construct(Card::Suit::Diamonds, i)); + m_new_deck.append(Card::construct(Cards::Suit::Clubs, static_cast(i))); + m_new_deck.append(Card::construct(Cards::Suit::Spades, static_cast(i))); + m_new_deck.append(Card::construct(Cards::Suit::Hearts, static_cast(i))); + m_new_deck.append(Card::construct(Cards::Suit::Diamonds, static_cast(i))); } for (uint8_t i = 0; i < 200; ++i) diff --git a/Userland/Games/Spider/Game.cpp b/Userland/Games/Spider/Game.cpp index 7d57d0b98d..dd1cd3517e 100644 --- a/Userland/Games/Spider/Game.cpp +++ b/Userland/Games/Spider/Game.cpp @@ -55,13 +55,13 @@ void Game::setup(Mode mode) switch (m_mode) { case Mode::SingleSuit: for (int j = 0; j < 8; j++) { - deck.append(Card::construct(Card::Suit::Spades, i)); + deck.append(Card::construct(Cards::Suit::Spades, static_cast(i))); } break; case Mode::TwoSuit: for (int j = 0; j < 4; j++) { - deck.append(Card::construct(Card::Suit::Spades, i)); - deck.append(Card::construct(Card::Suit::Hearts, i)); + deck.append(Card::construct(Cards::Suit::Spades, static_cast(i))); + deck.append(Card::construct(Cards::Suit::Hearts, static_cast(i))); } break; default: @@ -150,15 +150,14 @@ void Game::detect_full_stacks() break; if (!started) { - if (card.value() != 0) { + if (card.rank() != Cards::Rank::Ace) break; - } started = true; color = card.color(); - } else if (card.value() != last_value + 1 || card.color() != color) { + } else if (to_underlying(card.rank()) != last_value + 1 || card.color() != color) { break; - } else if (card.value() == Card::card_count - 1) { + } else if (card.rank() == Cards::Rank::King) { // we have a full set auto original_current_rect = current_pile.bounding_box(); @@ -174,7 +173,7 @@ void Game::detect_full_stacks() update_score(101); } - last_value = card.value(); + last_value = to_underlying(card.rank()); } } diff --git a/Userland/Libraries/LibCards/Card.cpp b/Userland/Libraries/LibCards/Card.cpp index 25c0ff3556..3a0dd9e4cb 100644 --- a/Userland/Libraries/LibCards/Card.cpp +++ b/Userland/Libraries/LibCards/Card.cpp @@ -67,13 +67,13 @@ static constexpr Gfx::CharacterBitmap s_club { static RefPtr s_background; static RefPtr s_background_inverted; -Card::Card(Suit suit, uint8_t value) +Card::Card(Suit suit, Rank rank) : m_rect(Gfx::IntRect({}, { width, height })) , m_front(Gfx::Bitmap::try_create(Gfx::BitmapFormat::BGRA8888, { width, height }).release_value_but_fixme_should_propagate_errors()) , m_suit(suit) - , m_value(value) + , m_rank(rank) { - VERIFY(value < card_count); + VERIFY(to_underlying(rank) < card_count); Gfx::IntRect paint_rect({ 0, 0 }, { width, height }); if (s_background.is_null()) { @@ -99,7 +99,6 @@ Card::Card(Suit suit, uint8_t value) Gfx::Painter painter(m_front); auto& font = Gfx::FontDatabase::default_font().bold_variant(); - auto label = labels[value]; painter.fill_rect_with_rounded_corners(paint_rect, Color::Black, card_radius); paint_rect.shrink(2, 2); painter.fill_rect_with_rounded_corners(paint_rect, Color::White, card_radius - 1); @@ -108,7 +107,7 @@ Card::Card(Suit suit, uint8_t value) paint_rect.shrink(10, 6); auto text_rect = Gfx::IntRect { 4, 6, font.width("10"sv), font.glyph_height() }; - painter.draw_text(text_rect, label, font, Gfx::TextAlignment::Center, color()); + painter.draw_text(text_rect, card_rank_label(m_rank), font, Gfx::TextAlignment::Center, color()); auto const& symbol = [&]() -> Gfx::CharacterBitmap const& { switch (m_suit) { diff --git a/Userland/Libraries/LibCards/Card.h b/Userland/Libraries/LibCards/Card.h index 6577ed3c1e..6a4c4eaaf3 100644 --- a/Userland/Libraries/LibCards/Card.h +++ b/Userland/Libraries/LibCards/Card.h @@ -1,48 +1,96 @@ /* * Copyright (c) 2020, Till Mayer * Copyright (c) 2022, the SerenityOS developers. + * Copyright (c) 2022, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ #pragma once -#include #include #include #include #include #include #include -#include namespace Cards { +enum class Rank : u8 { + Ace, + Two, + Three, + Four, + Five, + Six, + Seven, + Eight, + Nine, + Ten, + Jack, + Queen, + King, + __Count +}; + +constexpr StringView card_rank_label(Rank rank) +{ + switch (rank) { + case Rank::Ace: + return "A"sv; + case Rank::Two: + return "2"sv; + case Rank::Three: + return "3"sv; + case Rank::Four: + return "4"sv; + case Rank::Five: + return "5"sv; + case Rank::Six: + return "6"sv; + case Rank::Seven: + return "7"sv; + case Rank::Eight: + return "8"sv; + case Rank::Nine: + return "9"sv; + case Rank::Ten: + return "10"sv; + case Rank::Jack: + return "J"sv; + case Rank::Queen: + return "Q"sv; + case Rank::King: + return "K"sv; + case Rank::__Count: + VERIFY_NOT_REACHED(); + } + VERIFY_NOT_REACHED(); +} + +enum class Suit : u8 { + Clubs, + Diamonds, + Spades, + Hearts, + __Count +}; + class Card final : public Core::Object { C_OBJECT(Card) public: static constexpr int width = 80; static constexpr int height = 100; - static constexpr int card_count = 13; + static constexpr int card_count = to_underlying(Rank::__Count); static constexpr int card_radius = 5; - static constexpr Array labels = { - "A"sv, "2"sv, "3"sv, "4"sv, "5"sv, "6"sv, "7"sv, "8"sv, "9"sv, "10"sv, "J"sv, "Q"sv, "K"sv - }; - - enum class Suit { - Clubs, - Diamonds, - Spades, - Hearts, - __Count - }; virtual ~Card() override = default; Gfx::IntRect& rect() { return m_rect; } Gfx::IntPoint position() const { return m_rect.location(); } Gfx::IntPoint const& old_position() const { return m_old_position; } - uint8_t value() const { return m_value; }; + Rank rank() const { return m_rank; }; Suit suit() const { return m_suit; } bool is_old_position_valid() const { return m_old_position_valid; } @@ -63,7 +111,7 @@ public: void clear_and_draw(GUI::Painter&, Color const& background_color); private: - Card(Suit suit, uint8_t value); + Card(Suit, Rank); static NonnullRefPtr invert_bitmap(Gfx::Bitmap&); @@ -72,7 +120,7 @@ private: RefPtr m_front_inverted; Gfx::IntPoint m_old_position; Suit m_suit; - uint8_t m_value; + Rank m_rank; bool m_old_position_valid { false }; bool m_moving { false }; bool m_upside_down { false }; @@ -88,22 +136,22 @@ struct AK::Formatter : Formatter { StringView suit; switch (card.suit()) { - case Cards::Card::Suit::Clubs: + case Cards::Suit::Clubs: suit = "C"sv; break; - case Cards::Card::Suit::Diamonds: + case Cards::Suit::Diamonds: suit = "D"sv; break; - case Cards::Card::Suit::Hearts: + case Cards::Suit::Hearts: suit = "H"sv; break; - case Cards::Card::Suit::Spades: + case Cards::Suit::Spades: suit = "S"sv; break; default: VERIFY_NOT_REACHED(); } - return Formatter::format(builder, "{:>2}{}"sv, Cards::Card::labels[card.value()], suit); + return Formatter::format(builder, "{:>2}{}"sv, Cards::card_rank_label(card.rank()), suit); } }; diff --git a/Userland/Libraries/LibCards/CardStack.cpp b/Userland/Libraries/LibCards/CardStack.cpp index 7826a3938c..591df4f054 100644 --- a/Userland/Libraries/LibCards/CardStack.cpp +++ b/Userland/Libraries/LibCards/CardStack.cpp @@ -170,12 +170,12 @@ void CardStack::add_all_grabbed_cards(Gfx::IntPoint const& click_location, Nonnu break; } - if (!color_match || card.value() != last_value - 1) { + if (!color_match || to_underlying(card.rank()) != last_value - 1) { valid_stack = false; break; } } - last_value = card.value(); + last_value = to_underlying(card.rank()); last_color = card.color(); } @@ -195,13 +195,13 @@ bool CardStack::is_allowed_to_push(Card const& card, size_t stack_size, Movement if (m_type == Type::Normal && is_empty()) { // FIXME: proper solution for this if (movement_rule == MovementRule::Alternating) { - return card.value() == 12; + return card.rank() == Rank::King; } return true; } if (m_type == Type::Foundation && is_empty()) - return card.value() == 0; + return card.rank() == Rank::Ace; if (!is_empty()) { auto& top_card = peek(); @@ -212,7 +212,7 @@ bool CardStack::is_allowed_to_push(Card const& card, size_t stack_size, Movement // Prevent player from dragging an entire stack of cards to the foundation stack if (stack_size > 1) return false; - return top_card.suit() == card.suit() && m_stack.size() == card.value(); + return top_card.suit() == card.suit() && m_stack.size() == to_underlying(card.rank()); } else if (m_type == Type::Normal) { bool color_match; switch (movement_rule) { @@ -227,7 +227,7 @@ bool CardStack::is_allowed_to_push(Card const& card, size_t stack_size, Movement break; } - return color_match && top_card.value() == card.value() + 1; + return color_match && to_underlying(top_card.rank()) == to_underlying(card.rank()) + 1; } VERIFY_NOT_REACHED();