Swap Cards in Hand (#280)

* card sorting

* Added refactor from @MeirGavish

---------

Co-authored-by: MathisMartin31 <mathis.martin31@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

source/game.c

@@ -125,6 +125,13 @@
 
 #define NEXT_ROUND_BTN_SEL_X 0
 
+#define GAME_PLAYING_HAND_SEL_Y        0
+#define GAME_PLAYING_BUTTONS_SEL_Y     1
+#define GAME_PLAYING_PLAY_BTN_SEL_X    0
+#define GAME_PLAYING_DISCARD_BTN_SEL_X 1
+#define GAME_PLAYING_NUM_SEL_ROWS      2
+#define GAME_PLAYING_NUM_BOTTOM_BTNS   2
+
 #define REROLL_BTN_FRAME_PAL_IDX 7
 #define REROLL_BTN_PAL_IDX       3
 
@@ -230,7 +237,7 @@ static void display_hands(int value);
 static void display_discards(int value);
 static void set_hand(void);
 static void hand_set_focus(int index);
-static bool hand_discard(void);
+static bool hand_can_discard(void);
 static int deck_get_size(void);
 static int deck_get_max_size(void);
 static void increment_blind(enum BlindState increment_reason);
@@ -1044,20 +1051,27 @@ void display_mult(void)
     check_flaming_score();
 }
 
+// idx_a and idx_b are assumed to be valid indexes within the hand array
+// no checks will be performed here for performance's sake
+static inline void swap_cards_in_hand(int idx_a, int idx_b)
+{
+    CardObject* temp = hand[idx_a];
+    hand[idx_a] = hand[idx_b];
+    hand[idx_b] = temp;
+}
+
 static inline void sort_hand_by_suit(void)
 {
-    for (int a = 0; a < hand_top; a++)
+    for (int idx_a = 0; idx_a < hand_top; idx_a++)
     {
-        for (int b = a + 1; b <= hand_top; b++)
+        for (int idx_b = idx_a + 1; idx_b <= hand_top; idx_b++)
         {
-            if (hand[a] == NULL ||
+            if (hand[idx_a] == NULL ||
-                (hand[b] != NULL && (hand[a]->card->suit > hand[b]->card->suit ||
+                (hand[idx_b] != NULL && (hand[idx_a]->card->suit > hand[idx_b]->card->suit ||
-                                     (hand[a]->card->suit == hand[b]->card->suit &&
+                                         (hand[idx_a]->card->suit == hand[idx_b]->card->suit &&
-                                      hand[a]->card->rank > hand[b]->card->rank))))
+                                          hand[idx_a]->card->rank > hand[idx_b]->card->rank))))
             {
-                CardObject* temp = hand[a];
+                swap_cards_in_hand(idx_a, idx_b);
-                hand[a] = hand[b];
-                hand[b] = temp;
             }
         }
     }
@@ -1065,20 +1079,88 @@ static inline void sort_hand_by_suit(void)
 
 static inline void sort_hand_by_rank(void)
 {
-    for (int a = 0; a < hand_top; a++)
+    for (int idx_a = 0; idx_a < hand_top; idx_a++)
     {
-        for (int b = a + 1; b <= hand_top; b++)
+        for (int idx_b = idx_a + 1; idx_b <= hand_top; idx_b++)
         {
-            if (hand[a] == NULL || (hand[b] != NULL && hand[a]->card->rank > hand[b]->card->rank))
+            if (hand[idx_a] == NULL ||
+                (hand[idx_b] != NULL && hand[idx_a]->card->rank > hand[idx_b]->card->rank))
             {
-                CardObject* temp = hand[a];
+                swap_cards_in_hand(idx_a, idx_b);
-                hand[a] = hand[b];
-                hand[b] = temp;
             }
         }
     }
 }
 
+static inline bool shift_null_card_to_end(int null_card_idx)
+{
+    // Start by searching any non NULL cards after the NULL one
+    // don't start at null_card_idx+1 to avoid potential illegal array access
+    int non_null_card_idx = null_card_idx;
+    for (; non_null_card_idx <= hand_top; non_null_card_idx++)
+    {
+        if (hand[non_null_card_idx] != NULL)
+        {
+            break;
+        }
+    }
+
+    // return false if there are no non-NULL cards left/there are no more sprites to destroy
+    if (non_null_card_idx > hand_top)
+    {
+        return false;
+    }
+
+    // If there is one, shift it and all the cards that follow forward
+    // This way we close the gap and ensure the next card is not NULL
+
+    // Iterating up to `hand_top - non_null_card_idx + 1` should end up out of bounds
+    // but for some reason it doesn't pose any issue, and taking out the +1 breaks
+    // the code, so I'll be elaving it here until someone figures it out ^^'
+    for (int j = 0; j <= hand_top - non_null_card_idx + 1; j++)
+    {
+        hand[null_card_idx + j] = hand[non_null_card_idx + j];
+    }
+
+    return true;
+}
+
+static void reorder_card_sprites_layers(void)
+{
+    // Update the sprites in the hand by destroying them and creating new ones in the correct order
+    // (This feels like a diabolical solution but like literally how else would you do this)
+    for (int i = 0; i <= hand_top; i++)
+    {
+        // a NULL card will only happen if we rearrange the sprites without having sorted them
+        // before. Any NULL CardObject will be sent to the end by shifting all elements forward
+        if (hand[i] == NULL)
+        {
+            if (!shift_null_card_to_end(i))
+            {
+                break;
+            }
+        }
+
+        // card_object_get_sprite() will not work here since we need the address
+        sprite_destroy(&(hand[i]->sprite_object->sprite));
+    }
+
+    // Recreate the sprites for the remaining non NULL cards, in order
+    for (int i = 0; i <= hand_top; i++)
+    {
+        if (hand[i] != NULL)
+        {
+            // Set the sprite for the card object
+            card_object_set_sprite(hand[i], i);
+            sprite_position(
+                card_object_get_sprite(hand[i]),
+                fx2int(hand[i]->sprite_object->x),
+                fx2int(hand[i]->sprite_object->y)
+            );
+        }
+    }
+}
+
 static void sort_cards(void)
 {
     if (sort_by_suit)
@@ -1090,31 +1172,7 @@ static void sort_cards(void)
         sort_hand_by_rank();
     }
 
-    // Update the sprites in the hand by destroying them and creating new ones in the correct order
+    reorder_card_sprites_layers();
-    // (This is feels like a diabolical solution but like literally how else would you do this)
-    for (int i = 0; i <= hand_top; i++)
-    {
-        if (hand[i] != NULL)
-        {
-            // card_object_get_sprite() will not work here since we need the address
-            sprite_destroy(&(hand[i]->sprite_object->sprite));
-        }
-    }
-
-    for (int i = 0; i <= hand_top; i++)
-    {
-        if (hand[i] != NULL)
-        {
-            // hand[i]->sprite = sprite_new(ATTR0_SQUARE | ATTR0_4BPP | ATTR0_AFF, ATTR1_SIZE_32,
-            // card_sprite_lut[hand[i]->card->suit][hand[i]->card->rank], 0, i);
-            card_object_set_sprite(hand[i], i); // Set the sprite for the card object
-            sprite_position(
-                card_object_get_sprite(hand[i]),
-                fx2int(hand[i]->sprite_object->x),
-                fx2int(hand[i]->sprite_object->y)
-            );
-        }
-    }
 }
 
 /* Copies the appropriate item into the top left panel (blind/shop icon)
@@ -1693,7 +1751,7 @@ static void hand_set_focus(int index)
     );
 }
 
-static bool hand_discard(void)
+static bool hand_can_discard(void)
 {
     if (hand_state != HAND_SELECT || hand_selections == 0)
         return false;
@@ -1888,49 +1946,150 @@ static inline void hand_change_sort(void)
     sort_cards();
 }
 
-static inline bool hand_play(void)
+static inline bool hand_can_play(void)
 {
     if (hand_state != HAND_SELECT || hand_selections == 0)
         return false;
     return true;
 }
 
+static inline void select_current_card(void)
+{
+    hand_toggle_card_selection();
+    set_hand();
+}
+
+// card moving logic
+
+// true if and only if we are currently moving a card around
+static bool moving_card = false;
+
+// This will prevent us from moving cards around if we selected one
+// by moving too fast after pressing the A button
+static bool card_moved_too_fast = false;
+static bool card_selected_instead_of_moved = false;
+
+// After pressing A, if we press Left/Right too fast, we should select the card
+// and change focus to the next one, instead of swapping them
+// This should fix inputs sometimes not registering when quickly selecting cards
+static const int card_swap_time_threshold = 6;
+static uint selection_hit_timer = TM_ZERO;
+
+static inline void game_playing_apply_card_movement_input(enum ScreenHorzDir move_dir)
+{
+    // The reason why this adds +1 (-SCREEN_LEFT) is because the hand is drawn from right to left.
+    // There is no particular reason for this, it's just how I did it.
+    int next_card = selection_x - move_dir;
+
+    // Do not use FRAMES(x) here as we are counting real frames ignoring game speed
+    card_moved_too_fast = (timer - selection_hit_timer) < card_swap_time_threshold;
+
+    // swap cards around if A is held down when pressing D-pad keys
+    if (key_is_down(SELECT_CARD) && !card_moved_too_fast && !card_selected_instead_of_moved)
+    {
+        bool selection_not_at_border =
+            (move_dir == SCREEN_LEFT) ? selection_x < hand_top : selection_x > 0;
+
+        if (selection_not_at_border)
+        {
+            swap_cards_in_hand(selection_x, next_card);
+            moving_card = true;
+            reorder_card_sprites_layers();
+            hand_set_focus(next_card);
+        }
+    }
+    else
+    {
+        // select current card if we tried moving it too fast
+        if (key_released(SELECT_CARD) || (card_moved_too_fast && !moving_card))
+        {
+            select_current_card();
+            card_selected_instead_of_moved = true;
+        }
+        hand_set_focus(next_card);
+    }
+}
+
+static inline void game_playing_highlight_play_btn(void)
+{
+    memset16(&pal_bg_mem[PLAY_HAND_BTN_BORDER_PID], HIGHLIGHT_COLOR, 1);
+    memcpy16(&pal_bg_mem[DISCARD_BTN_BORDER_PID], &pal_bg_mem[DISCARD_BTN_PID], 1);
+}
+
+static inline void game_playing_highlight_discard_btn(void)
+{
+    memcpy16(&pal_bg_mem[PLAY_HAND_BTN_BORDER_PID], &pal_bg_mem[PLAY_HAND_BTN_PID], 1);
+    memset16(&pal_bg_mem[DISCARD_BTN_BORDER_PID], HIGHLIGHT_COLOR, 1);
+}
+
+static inline void game_playing_unhighlight_buttons(void)
+{
+    memcpy16(&pal_bg_mem[PLAY_HAND_BTN_BORDER_PID], &pal_bg_mem[PLAY_HAND_BTN_PID], 1);
+    memcpy16(&pal_bg_mem[DISCARD_BTN_BORDER_PID], &pal_bg_mem[DISCARD_BTN_PID], 1);
+}
+
+static inline void game_playing_execute_hand_discard(void)
+{
+    play_sfx(SFX_BUTTON, MM_BASE_PITCH_RATE, BUTTON_SFX_VOLUME);
+
+    hand_state = HAND_DISCARD;
+    selection_x = 0;
+    selection_y = 0;
+    display_hands(--discards);
+    set_hand();
+    tte_printf(
+        "#{P:%d,%d; cx:0x%X000}%d",
+        DISCARDS_TEXT_RECT.left,
+        DISCARDS_TEXT_RECT.top,
+        TTE_RED_PB,
+        discards
+    );
+}
+
+static inline void game_playing_execute_hand_play(void)
+{
+    play_sfx(SFX_BUTTON, MM_BASE_PITCH_RATE, BUTTON_SFX_VOLUME);
+
+    hand_state = HAND_PLAY;
+    selection_x = 0;
+    selection_y = 0;
+    display_hands(--hands);
+}
+
 static inline void game_playing_process_hand_select_input(void)
 {
-    static bool discard_button_highlighted =
+    // true = play button highlighted, false = discard button highlighted
-        false; // true = play button highlighted, false = discard button highlighted
+    static bool discard_button_highlighted = false;
 
     if (key_hit(KEY_LEFT))
     {
-        if (selection_y == 0)
+        if (selection_y == GAME_PLAYING_HAND_SEL_Y)
         {
-            // The reason why this adds 1 is because the hand is drawn from right to left. There is
+            game_playing_apply_card_movement_input(SCREEN_LEFT);
-            // no particular reason for this, it's just how I did it.
-            hand_set_focus(selection_x + 1);
         }
-        else
+        else if (selection_y == GAME_PLAYING_BUTTONS_SEL_Y)
         {
-            discard_button_highlighted = false; // Play button
+            discard_button_highlighted = false;
         }
     }
     else if (key_hit(KEY_RIGHT))
     {
-        if (selection_y == 0)
+        if (selection_y == GAME_PLAYING_HAND_SEL_Y)
         {
-            hand_set_focus(selection_x - 1);
+            game_playing_apply_card_movement_input(SCREEN_RIGHT);
         }
-        else
+        else if (selection_y == GAME_PLAYING_BUTTONS_SEL_Y)
         {
-            discard_button_highlighted = true; // Discard button
+            discard_button_highlighted = true;
         }
     }
-    else if (key_hit(KEY_UP) && selection_y != 0)
+    else if (key_hit(KEY_UP) && selection_y == GAME_PLAYING_BUTTONS_SEL_Y)
     {
-        selection_y = 0;
+        selection_y = GAME_PLAYING_HAND_SEL_Y;
     }
-    else if (key_hit(KEY_DOWN) && selection_y != 1)
+    else if (key_hit(KEY_DOWN) && selection_y == GAME_PLAYING_HAND_SEL_Y)
     {
-        selection_y = 1;
+        selection_y = GAME_PLAYING_BUTTONS_SEL_Y;
 
         if (selection_x > hand_top / 2)
         {
@@ -1941,58 +2100,47 @@ static inline void game_playing_process_hand_select_input(void)
             discard_button_highlighted = true; // Discard button
         }
     }
-    else if (selection_y == 1) // On row of play/discard buttons
+    else if (selection_y == GAME_PLAYING_BUTTONS_SEL_Y)
     {
         if (discard_button_highlighted == false) // Play button logic
         {
-            memset16(&pal_bg_mem[PLAY_HAND_BTN_BORDER_PID], HIGHLIGHT_COLOR, 1);
+            game_playing_highlight_play_btn();
-            memcpy16(&pal_bg_mem[DISCARD_BTN_BORDER_PID], &pal_bg_mem[DISCARD_BTN_PID], 1);
+            if (key_hit(SELECT_CARD) && hands > 0 && hand_can_play())
-
-            if (key_hit(SELECT_CARD) && hands > 0 && hand_play())
             {
-                play_sfx(SFX_BUTTON, MM_BASE_PITCH_RATE, BUTTON_SFX_VOLUME);
+                game_playing_execute_hand_play();
-
-                hand_state = HAND_PLAY;
-                selection_x = 0;
-                selection_y = 0;
-                display_hands(--hands);
             }
         }
-        else // Discard button logic
+        else
         {
-            memcpy16(&pal_bg_mem[PLAY_HAND_BTN_BORDER_PID], &pal_bg_mem[PLAY_HAND_BTN_PID], 1);
+            game_playing_highlight_discard_btn();
-            memset16(&pal_bg_mem[DISCARD_BTN_BORDER_PID], HIGHLIGHT_COLOR, 1);
+            if (key_hit(SELECT_CARD) && discards > 0 && hand_can_discard())
-
-            if (key_hit(SELECT_CARD) && discards > 0 && hand_discard())
             {
-                play_sfx(SFX_BUTTON, MM_BASE_PITCH_RATE, BUTTON_SFX_VOLUME);
+                game_playing_execute_hand_discard();
-
-                hand_state = HAND_DISCARD;
-                selection_x = 0;
-                selection_y = 0;
-                display_hands(--discards);
-                set_hand();
-                tte_printf(
-                    "#{P:%d,%d; cx:0x%X000}%d",
-                    DISCARDS_TEXT_RECT.left,
-                    DISCARDS_TEXT_RECT.top,
-                    TTE_RED_PB,
-                    discards
-                );
             }
         }
     }
-    else if (selection_y == 0) // On row of cards
+    else if (selection_y == GAME_PLAYING_HAND_SEL_Y)
     {
-        // Play button highlight color
+        game_playing_unhighlight_buttons();
-        memcpy16(&pal_bg_mem[PLAY_HAND_BTN_BORDER_PID], &pal_bg_mem[PLAY_HAND_BTN_PID], 1);
-        // Discard button highlight color
-        memcpy16(&pal_bg_mem[DISCARD_BTN_BORDER_PID], &pal_bg_mem[DISCARD_BTN_PID], 1);
 
+        // Register timer when we hit A to pick a card
+        // If we try to move the picked card before card_swap_time_threshold frames,
+        // We will select it instead of moving it
         if (key_hit(SELECT_CARD))
         {
-            hand_toggle_card_selection();
+            selection_hit_timer = timer;
-            set_hand();
+        }
+        // select card if we were not moving it around
+        else if (key_released(SELECT_CARD))
+        {
+            if (!moving_card && !card_selected_instead_of_moved)
+            {
+                select_current_card();
+            }
+            moving_card = false;
+            card_moved_too_fast = false;
+            card_selected_instead_of_moved = false;
+            selection_hit_timer = TM_ZERO;
         }
 
         if (key_hit(DESELECT_CARDS))
@@ -2105,7 +2253,7 @@ static inline void card_in_hand_loop_handle_discard_and_shuffling(
             {
                 discard_push(hand[card_idx]->card);
                 card_object_destroy(&hand[card_idx]);
-                sort_cards();
+                reorder_card_sprites_layers();
 
                 hand_top--;
                 // This technically isn't drawing cards, I'm just reusing the variable
@@ -3013,7 +3161,7 @@ static inline void cards_in_hand_update_loop(void)
                         played_push(hand[i]);
                         sprite_destroy(&hand[i]->sprite_object->sprite);
                         hand[i] = NULL;
-                        sort_cards();
+                        reorder_card_sprites_layers();
 
                         play_sfx(
                             SFX_CARD_DRAW,