C# BlackJack - same card dealt to player & cpu?

Question

i'm working on a little black jack game for no apparent reason, and I've run into an issue and i can't figure out where i'm going wrong, the only thing i can imagine is that the 'new card' method is being called twice, too quickly...

The problem is that it's giving the same card to both players :/

Here is my code

Thank you! :)

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace BlackJack_Reworked
{
    public partial class BlackJack : Form
    {
        public BlackJack()
        {
            InitializeComponent();
        }
        class myVars
        {
            public static int cardsDrawn = 0;

            public static int playerX = 230;
            public static int playerY = 160;

            public static int cpuX = 230;
            public static int cpuY = 60;

            public static int playerCardValue = 0;
            public static int cpuCardValue = 0;
        }
        private PictureBox[] card = new PictureBox[100];
        private void makeCard(string pickedCard, int x)
        {
            card[myVars.cardsDrawn] = new PictureBox();

            if (x == 0)
            {
                card[myVars.cardsDrawn].Location = new Point(myVars.playerX, myVars.playerY);
                myVars.playerX += 15;
            }
            if (x == 1)
            {
                card[myVars.cardsDrawn].Location = new Point(myVars.cpuX, myVars.cpuY);
                myVars.cpuX += 15;
            }

            card[myVars.cardsDrawn].Image = (Image)Properties.Resources.ResourceManager.GetObject(pickedCard);
            card[myVars.cardsDrawn].Size = new Size(72, 96);
            card[myVars.cardsDrawn].Parent = this;
            card[myVars.cardsDrawn].BringToFront();
            card[myVars.cardsDrawn].Update();

            myVars.cardsDrawn++;

            checkScores(false);
        }
        private void newCard(int x)
        {
            Random cardPicker = new Random();

            int cardChoice = cardPicker.Next(1, 13);
            int houseChoice = cardPicker.Next(1, 4);

            string house = null;

            switch (houseChoice)
            {
                case 1:
                    house = "Hearts";
                    break;
                case 2:
                    house = "Diamonds";
                    break;
                case 3:
                    house = "Spades";
                    break;
                case 4:
                    house = "Clubs";
                    break;
            }

            if (x == 0)
            {
                makeCard(house + Convert.ToString(cardChoice), 0);
                myVars.playerCardValue += cardChoice;
            }
            if (x == 1)
            {
                makeCard(house + Convert.ToString(cardChoice), 1);
                myVars.cpuCardValue += cardChoice;
            }
        }
        private bool feelingLucky()
        {
            Random Dice = new Random();
            if (myVars.cpuCardValue >= 20) { return false; }
            if (myVars.cpuCardValue <= 16) { return true; }
            if (myVars.cpuCardValue >= 17 && myVars.cpuCardValue <= 18) if (Dice.Next(1, 5) == 1) { return true; }
            if (myVars.cpuCardValue == 19) if (Dice.Next(1, 10) == 1) { return true; }
            return false;
        }
        private void updateHandValues()
        {
            lblPlayerHand.Text = "Player: " + myVars.playerCardValue.ToString();
            lblCPUhand.Text = "CPU: " + myVars.cpuCardValue.ToString();
        }
        private void checkScores(bool stand)
        {
            if (stand == true)
            {
                if (myVars.playerCardValue <= 21 && myVars.playerCardValue > myVars.cpuCardValue)
                {
                    MessageBox.Show("Win!");
                    btnNewGame.Visible = true;
                }
                else if (myVars.cpuCardValue <= 21 && myVars.cpuCardValue > myVars.playerCardValue)
                {
                    btnNewGame.Visible = true;
                    MessageBox.Show("Lose!");
                }
           }
            else
            {
                if (myVars.playerCardValue > 21)
                {
                    MessageBox.Show("Bust!");
                    btnNewGame.Visible = true;
                }
                if (myVars.cpuCardValue > 21)
                {
                    MessageBox.Show("Win!");
                    btnNewGame.Visible = true;
                }
            }
        }
        private void newGame()
        {
            for(int x = 0; x < myVars.cardsDrawn; x++) { card[x].Dispose(); }

            myVars.cpuCardValue = 0;
            myVars.playerCardValue = 0;
            myVars.cpuX = 230;
            myVars.playerX = 230;

            btnNewGame.Visible = false;

            newCard(0); newCard(1);
        }
        private void btnNewGame_Click(object sender, EventArgs e)
        {
            newGame();
        }
        private void btnHit_Click(object sender, EventArgs e)
        {
            newCard(0); newCard(1);
            updateHandValues();
        }
        private void btnStand_Click(object sender, EventArgs e)
        {
            if (feelingLucky() == true) newCard(1);
            else checkScores(true);
        }
    }
}

screenshot

EDIT Here's the code to my new and working version with help from these nice guys below, just in case someone finds it useful, thanks everyone!

Here are the card picture files you'll need to add to your project's resources for this code to work.

I know my logic probably isn't great, but i feel like I've learnt from this little project, hopefully someone else might too, now, time to conjure up something new.. thanks stackoverflow.

Playing Card Pictures Download

Screenshot

using System;
using System.Collections.Generic;
using System.Drawing;
using System.Windows.Forms;
using System.Text.RegularExpressions;

namespace BlackJack_Reworked
{
    public partial class BlackJack : Form
    {
        public BlackJack()
        {
            InitializeComponent();
        }
        private PictureBox[] Card = new PictureBox[52];
        static List<string> Deck = new List<string>();
        class myVars
        {
            public static int playerX = 230;
            public static int cpuX = 230;
            public static int playerCardValue = 0;
            public static int cpuCardValue = 0;
            public static int cardsDrawn = 0;
        }
        private void newDeck()
        {
            Deck.Clear();

            for (int x = 0; x < myVars.cardsDrawn; x++)
            {
                Card[x].Dispose();
            }

            for (int x = 0; x < 52; x++)
            {
                int cardSuite = (x / 13) + 1;
                int faceValue = (x % 13) + 1;

                string Suite = null;

                switch (cardSuite)
                {
                    case 1:
                        Suite = "Hearts";
                        break;
                    case 2:
                        Suite = "Diamonds";
                        break;
                    case 3:
                        Suite = "Spades";
                        break;
                    case 4:
                        Suite = "Clubs";
                        break;
                }
                Deck.Add(Suite + Convert.ToString(faceValue));
            }
            Extensions.Shuffle(Deck);

            myVars.cardsDrawn = myVars.cpuCardValue = myVars.playerCardValue = 0;
            myVars.cpuX = myVars.playerX = 230;
        }
        private void handCard(string recipient)
        {
            Random Random = new Random(); Extensions.Shuffle(Deck);

            string pickedCard = Deck[Random.Next(Deck.Count)];
            int cardvalue = Convert.ToInt32(Regex.Replace(pickedCard, "[^0-9]", ""));

            Card[myVars.cardsDrawn] = new PictureBox();

            if (recipient == "player") {
                Card[myVars.cardsDrawn].Location = new Point(myVars.playerX, 160); myVars.playerX += 15;
                myVars.playerCardValue += cardvalue;
            }
            if (recipient == "cpu") {
                Card[myVars.cardsDrawn].Location = new Point(myVars.cpuX, 60); myVars.cpuX += 15;
                myVars.cpuCardValue += cardvalue;
            }

            Card[myVars.cardsDrawn].Image = (Image)Properties.Resources.ResourceManager.GetObject(pickedCard);
            Card[myVars.cardsDrawn].Size = new Size(72, 96);
            Card[myVars.cardsDrawn].Parent = this;
            Card[myVars.cardsDrawn].BringToFront();
            Card[myVars.cardsDrawn].Update();

            Deck.Remove(pickedCard); myVars.cardsDrawn++; updateHandValues();
        }
        private void updateHandValues()
        {
            lblPlayerHand.Text = "Player: " + myVars.playerCardValue.ToString();
            lblCPUhand.Text = "CPU: " + myVars.cpuCardValue.ToString();
        }
        private void newGame()
        {
            lblBlackJack.Text = "♠ Blackjack ♥";
            btnNewGame.Visible = false;
            newDeck(); handCard("player"); handCard("cpu");
        }
        private void checkCards()
        {
            if (playerWins() == true)
            {
                lblBlackJack.Text = "♠ You Win! ♥";
                btnNewGame.Visible = true;
            }
            else if (playerWins() == false)
            {
                lblBlackJack.Text = "♠ Dealer Wins! ♥";
                btnNewGame.Visible = true;
            }
        }
        private void tieBreak()
        {
            if (myVars.cpuCardValue == myVars.playerCardValue && myVars.cpuCardValue >= 17)
            {
                lblBlackJack.Text = "♠ Tie! ♥";
                btnNewGame.Visible = true;
            }
            else { checkCards(); }
        }
        private bool? playerWins()
        {
            if(myVars.cpuCardValue == 21 || myVars.playerCardValue > 21) { return false; }
            if(myVars.playerCardValue == 21 || myVars.cpuCardValue > 21) { return true; }
            else { return null; }
        }
        private bool cpuShouldPlay(bool stand)
        {
            Random Dice = new Random();

            if (stand == false)
            {
                if (myVars.cpuCardValue <= 15) { return true; }
                if (myVars.cpuCardValue >= 20 && myVars.cpuCardValue <= 21 && myVars.cpuCardValue > myVars.playerCardValue) { return false; }
                if (myVars.cpuCardValue == 19 && myVars.cpuCardValue < myVars.playerCardValue) { return true; } else { return false; }
            }
            else
            {
                if(myVars.cpuCardValue < myVars.playerCardValue)
                {
                    return true;
                }
                else { return false; }
            }
        }
        private void btnNewGame_Click(object sender, EventArgs e)
        {
            newGame();
        }
        private void btnHit_Click(object sender, EventArgs e)
        {
            handCard("player"); if(cpuShouldPlay(false) == true) handCard("cpu"); checkCards();
        }
        private void btnStand_Click(object sender, EventArgs e)
        {
            if (cpuShouldPlay(true) == true) handCard("cpu"); tieBreak();
        }
    }
    public static class Extensions
    {
        public static void Shuffle<T>(this IList<T> list)
        {
            Random rng = new Random();
            int n = list.Count;
            while (n > 1)
            {
                n--;
                int k = rng.Next(n + 1);
                T value = list[k];
                list[k] = list[n];
                list[n] = value;
            }
        }
    }
}

a card should be a single object with Suit and Value properties, a deck of these should be shuffled rather than cards picked randomly (and suit/value seperately as well!) [see this](http://stackoverflow.com/a/26532939/1070452) — Ňɏssa Pøngjǣrdenlarp, Feb 25 '15 at 15:41
You're constantly creating new instances of `Random`. Don't do that. Create just one per class or even thread. See http://stackoverflow.com/questions/767999/random-number-generator-only-generating-one-random-number -- also, you shouldn't randomly "generate" a card, but create a complete deck (`ICollection`) and randomly *select and remove* one card from the deck. — Corak, Feb 25 '15 at 15:43
You need to keep track of used cards so your new card method can select cards from unused cards, rather than from all cards. — horns, Feb 25 '15 at 15:43
possible duplicate of [Randomize a List in C#](http://stackoverflow.com/questions/273313/randomize-a-listt-in-c-sharp) — Tetsujin no Oni, Feb 25 '15 at 15:59
there is an entire Card, Deck, Shoe and shuffle method in the link above... — Ňɏssa Pøngjǣrdenlarp, Feb 25 '15 at 16:00
Those who are claiming this as duplicate are wrong. This problem is related to logic and not the language or Random class. — Yogee, Feb 26 '15 at 13:02

antiduh · Answer 1 · 2015-02-25T16:22:43.603

If you're playing from single deck rules, you must use a 'bag' or 'no replacement' model of random value source. Initially , fill the bag with all 52 possible cards. Then each iteration, pick one card at random from what remains in the bag, removing it from the bag. When the bag is empty, refill it.

Do note that when picking a random card from a bag that has n remaining items, your random value, being the index of the item in the bag to take, must be no larger than n-1 (assuming indicies run from 0 to n-1).

However, there are models of blackjack where multiple decks are shuffled together; many casinos play like this. From Wikipedia Blackjack, rules of play at casinos:

At a casino blackjack table, the dealer faces five to seven playing positions from behind a semicircular table. Between one and eight standard 52-card decks are shuffled together.

In this above model, it is certainly possible for the same card to be dealt twice in a row; however, depending on how many decks are shuffled together, this can happen only a limited number of times.

If I understand your code correctly, you currently model a blackjack shoe that has an infinite number of decks shuffled together.

score -1 · Answer 2 · answered Feb 25 '15 at 15:44

-1

You are declaring a new Random() within the new card method.

If the method is being called twice, and very quickly, the seed's that are automatically generated (Based on time I believe) will be so close that they will generate pretty much the same number.

The best thing to do would be to create your instance of random outside of the method, and pass the random in each time, that way you will not get the same number each call.

answered Feb 25 '15 at 15:44

David Watts

2,249
22
33

3

Their problem isn't how the random number is generated. Even by moving the random to global scope they problem would still persist occasionally as there's nothing stopping a random generating the same number twice in a row. – Ash Burlaczenko Feb 25 '15 at 15:50
this is only a fraction of what is wrong. the most important thing is to control that no more than 4 of any Value are ever selected / one card per deck for each shoe – Ňɏssa Pøngjǣrdenlarp Feb 25 '15 at 15:52
How so. He says that the method is called twice very quickly. Inside the method, he creates a new Random(). If no seed is provided, the seed is generated from the time. Now two method calls very quickly would lead to two almost identical seeds, and therefore, two identical numbers being generated. – David Watts Feb 25 '15 at 15:52
Using one random instance doesn't prevent the same card to be handed out twice. – CodeCaster Feb 25 '15 at 15:53
Thank you!!! now that i'm passing the random numbers into the method it's working as intended, as for you other guys that are pointing out that I need a way of managing which cards are available to be handed out or not, thank you too! can't believe that went over my head, lots to get busy with now so i'm a happy bunny :D – JustBenji Feb 25 '15 at 15:57
@JustBenji not reusing random just made the change of getting the same card twice in a row smaller, it can still happen. Don't go for the quick fix, try to understand the problem and then fix it. For example [shuffle the deck once](http://stackoverflow.com/questions/273313/randomize-a-listt-in-c-sharp), then draw cards from it. – CodeCaster Feb 25 '15 at 15:59

Yogee · Accepted Answer · 2015-02-25T16:17:34.140

Random.Next method generates random number from current time stamp and previously generated number (known as 'seed'). As you are create new object of Random everytime, it initializes with same seed. In your case, timestamp and seed doesn't change to 'Next' method of Random.

If you use single object of Random for all your operation, seed will change for each call of 'next'.

Second: You need to keep track of which cards are already drawn, so I've create a shoe list. I will remove the cards which are already used, just like real table.

NOTE: create a new shoe, when your number of cards are let's say 20.

 List<string> aShoe = new List<string>(); //shoe contains 4 to 8 decks. Depending upon blackjack version.
 int numberOfDeckPerShoe = 4;

private void CreateNewDeck()
 {
     for(int i =0;i <numberOfDeckPerShoe;i++)
 for(int j=0;j<52;j++)
 {
            int cardFace = (j%13)+1;
        int cardSuite = (j/13) + 1;

        string Suite = null;

        switch (cardSuite)
        {
        case 1:
            Suite = "Hearts";
            break;
        case 2:
            Suite = "Diamonds";
            break;
        case 3:
            Suite = "Spades";
            break;
        case 4:
            Suite = "Clubs";
            break;
        }
        aShoe.add(Suite + Convert.ToString(cardFace));
 }

 }

 Random cardPicker = new Random(); //This is change

 private void newCard(int x)
    {
        int cardChoice = 0;
        int houseChoice = 0;
    string cardDrawn = "";

    int cardToDraow = cardPicker.Next(0,aShoe.length);

    cardDrawn = card aShoe[cardToDraow];
    card.removeAt(cardToDraow);

        if (x == 0)
        {
            makeCard(cardDrawn, 0);
            myVars.playerCardValue += cardChoice;
        }
        if (x == 1)
        {
            makeCard(cardDrawn, 1);
            myVars.cpuCardValue += cardChoice;
        }
    }

this will not help: you cannot pick a random number over and over without constraints and have it emulate a deck of cards — Ňɏssa Pøngjǣrdenlarp, Feb 25 '15 at 15:53
Checking if a card has already been drawn like that will cause progressively worse performance with every card drawn. Probably not noticeable here, but in a real-world application it could dramatically hurt performance. Using a queue, or at least manually removing the card from the list as you draw, would be the way to go. — Joe Enos, Feb 25 '15 at 16:01
@JoeEnos, you are right. We should create a list of cards and we should take cards out of those. — Yogee, Feb 25 '15 at 16:06
Hey Yogee!!! thanks a lot for that code, works great, just trying to understand it, having trouble working out how the % operator is working here, still searching online to see if i can figure it out, excuse how slow i am, still new to this :) — JustBenji, Feb 25 '15 at 16:44
@JustBenji, you can see about modulus operator (%) here (http://stackoverflow.com/questions/2664301/how-does-modulus-divison-work). I am using that so that my counter resets to 1 when I encounter 13th card. — Yogee, Feb 26 '15 at 13:00

C# BlackJack - same card dealt to player & cpu?

3 Answers3