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

@@ -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<Cards::Rank>(i)));
+        deck.append(Card::construct(Cards::Suit::Spades, static_cast<Cards::Rank>(i)));
+        deck.append(Card::construct(Cards::Suit::Hearts, static_cast<Cards::Rank>(i)));
+        deck.append(Card::construct(Cards::Suit::Diamonds, static_cast<Cards::Rank>(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<size_t> 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;

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<CardValue>(card.value() - 1);
+        return static_cast<CardValue>(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;

Userland/Games/Hearts/Player.cpp

@@ -71,7 +71,7 @@ size_t Player::pick_lead_card(Function<bool(Card&)> valid_play, Function<bool(Ca
     return last_index;
 }
 
-Optional<size_t> Player::pick_low_points_high_value_card(Optional<Card::Suit> suit)
+Optional<size_t> Player::pick_low_points_high_value_card(Optional<Cards::Suit> suit)
 {
     auto sorted_hand = hand_sorted_by_fn(compare_card_value);
     int min_points = -1;
@@ -115,10 +115,10 @@ Optional<size_t> Player::pick_slightly_higher_value_card(Card& other_card)
 
 size_t Player::pick_max_points_card(Function<bool(Card&)> 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<bool(Card&)> ignore_card)
     return pick_low_points_high_value_card().value();
 }
 
-Optional<size_t> Player::pick_specific_card(Card::Suit suit, CardValue value)
+Optional<size_t> 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;

Userland/Games/Hearts/Player.h

@@ -35,13 +35,13 @@ public:
 
     NonnullRefPtrVector<Card> pick_cards_to_pass(PassingDirection);
     size_t pick_lead_card(Function<bool(Card&)>, Function<bool(Card&)>);
-    Optional<size_t> pick_low_points_high_value_card(Optional<Card::Suit> suit = {});
+    Optional<size_t> pick_low_points_high_value_card(Optional<Cards::Suit> suit = {});
     Optional<size_t> pick_lower_value_card(Card& other_card);
     Optional<size_t> pick_slightly_higher_value_card(Card& other_card);
     size_t pick_max_points_card(Function<bool(Card&)>);
-    Optional<size_t> pick_specific_card(Card::Suit suit, CardValue value);
+    Optional<size_t> 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<CardWithIndex> hand_sorted_by_fn(bool (*)(CardWithIndex&, CardWithIndex&)) const;
 
     void sort_hand() { quick_sort(hand, hearts_card_less); }

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<Card::Suit>(get_random_uniform(to_underlying(Card::Suit::__Count))), get_random_uniform(Card::card_count));
+    auto card = Card::construct(static_cast<Cards::Suit>(get_random_uniform(to_underlying(Cards::Suit::__Count))), static_cast<Cards::Rank>(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<Cards::Rank>(i)));
+        m_new_deck.append(Card::construct(Cards::Suit::Spades, static_cast<Cards::Rank>(i)));
+        m_new_deck.append(Card::construct(Cards::Suit::Hearts, static_cast<Cards::Rank>(i)));
+        m_new_deck.append(Card::construct(Cards::Suit::Diamonds, static_cast<Cards::Rank>(i)));
     }
 
     for (uint8_t i = 0; i < 200; ++i)

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<Cards::Rank>(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<Cards::Rank>(i)));
+                deck.append(Card::construct(Cards::Suit::Hearts, static_cast<Cards::Rank>(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());
         }
     }
 

Userland/Libraries/LibCards/Card.cpp

@@ -67,13 +67,13 @@ static constexpr Gfx::CharacterBitmap s_club {
 static RefPtr<Gfx::Bitmap> s_background;
 static RefPtr<Gfx::Bitmap> 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) {

Userland/Libraries/LibCards/Card.h

@@ -1,48 +1,96 @@
 /*
  * Copyright (c) 2020, Till Mayer <till.mayer@web.de>
  * Copyright (c) 2022, the SerenityOS developers.
+ * Copyright (c) 2022, Sam Atkins <atkinssj@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
 #pragma once
 
-#include <AK/Array.h>
 #include <AK/Format.h>
 #include <LibCore/Object.h>
 #include <LibGUI/Painter.h>
 #include <LibGfx/Bitmap.h>
 #include <LibGfx/CharacterBitmap.h>
 #include <LibGfx/Rect.h>
-#include <ctype.h>
 
 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<StringView, card_count> 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<Gfx::Bitmap> invert_bitmap(Gfx::Bitmap&);
 
@@ -72,7 +120,7 @@ private:
     RefPtr<Gfx::Bitmap> 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<Cards::Card> : Formatter<FormatString> {
         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<FormatString>::format(builder, "{:>2}{}"sv, Cards::Card::labels[card.value()], suit);
+        return Formatter<FormatString>::format(builder, "{:>2}{}"sv, Cards::card_rank_label(card.rank()), suit);
     }
 };

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();