X-Git-Url: https://git.cworth.org/git?a=blobdiff_plain;f=kub.c;h=92cc533fdc74f6ae46c4e9879b9cb2a7928f3a09;hb=8a3a5d075300ac2fa9827f7c5a94906b10598863;hp=1a64c8d3f732c837a883f7ee1b983212df560d0b;hpb=8442454fc4bf8c73c9dc2b016f449784029a8961;p=kub

diff --git a/kub.c b/kub.c
index 1a64c8d..92cc533 100644
--- a/kub.c
+++ b/kub.c
@@ -77,33 +77,33 @@ static void player_init(player_t *player)
 
 static int card_compare(card_t *card_one, card_t *card_two)
 {
-    if (card_one->number < card_two->number)
-	return -1;
-    if (card_one->number == card_two->number)
-	return 0;
-    /* At this point, we know that this must be true:
-     * if (card_one->number > card_two->number)
-     */
-    return 1;
+    return card_one->number - card_two->number;
 }
 
 static int card_group_is_run(card_group_t *card_group)
 {
     int i;
     int lowest = 14, highest = 0;
-    if (card_group->num_cards > 13 || card_group->num_cards < 3)
+    color_t run_color;
+
+    /* By definition, a run must have at least 3 cards. Also, it's
+     * impossible for any group of cards with more than 13 cards to be
+     * a run, (there are only 13 unique numbers so a group with more
+     * than 13 cards must have some duplicates).
+     */
+    if (card_group->num_cards < 3 || card_group->num_cards > 13)
     {
 	return 0;
     }
-    for (i = 0; i < card_group->num_cards - 1; ++i)
+
+    /* Loop through all cards in the group, ensuring that they are all
+     * the same color and finding the highest and lowest number in the
+     * group. */
+    run_color = card_group->cards[i].color;
+    for (i = 0; i < card_group->num_cards; i++)
     {
-	if (card_group->cards[i].color != card_group->cards[i + 1].color)
-	{
+	if (card_group->cards[i].color != run_color)
 	    return 0;
-	}
-    }
-    for (i = 0; i < card_group->num_cards; ++i)
-    {
 	if (card_group->cards[i].number > highest)
 	{
 	    highest = card_group->cards[i].number;
@@ -113,10 +113,31 @@ static int card_group_is_run(card_group_t *card_group)
 	    lowest = card_group->cards[i].number;
 	}
     }
+
+    /* For a run, the difference between the highest and lowest cards
+     * will always be one less than the number of cards in the
+     * group. If not then we know it's not a run.
+     */
     if (highest - lowest != card_group->num_cards - 1)
     {
 	return 0;
     }
+
+    /* XXX: There's a bug here. We're guessing that at this point
+     * anything we're looking at must be a run. This would be correct
+     * if there were no duplicate cards, but since there are
+     * duplicates this us quite broken. For example consider two
+     * sequences of entirely red cards:
+     *
+     * This is a run:   1, 2, 3, 4
+     * But this is not: 1, 3, 4, 4
+     *
+     * As currently written, this function will consider both of these
+     * groups to be a run. One possible fix is to throw away the
+     * highest - lowest heuristic and instead simply sort the cards up
+     * front and ensure the difference between each adjacent pair is
+     * exactly 1.
+     */
     return 1;
 }