Two random cards, random but there the same.

Only call srand() once in your program, not every time you create a card.

To expand on SiCrane's answer, rand() is psuedo-random, it uses a fixed algorithm to generate a sequence of numbers. The "randomness" is the seed, which causes different sequences for different seeds. The seed is varied using the time usually. Your program keeps re-seeding the sequence in a tight loop, because the loop completes so quickly all the cards appear to have the same value. If the loop were slower the values would be different.

However, in a real card game you don't want to generate random cards. You want to generate one or more complete decks, and then randomly shuffle them. The c++ standard library provides the std::random_shuffle() algorithm, which does a good job at this.

Note that you can replace the switch in Card's constructor with the expression: cardValue = cardType + 1. Alternatively just don't store the value, you can compute it on demand from the type. In applyCard() the most elegant solution is to use the card value to look up an array of strings, and only have the drawing code written once.

Haha thanks I knew it was something stupid like that.

EDIT: rip-off, that's a good idea so that I don't get two cards after the deck already used them but I have never heard of this thing and I am going to get confused doing so, so I think I have a easier way of making sure I don't get a card that isn't taken.

ok came across another problem, I am getting the same cards twice occasionally, heres what I redid to try to fix the problem

From this


case 1:
cardValue = 2;
break;
case 2:
cardValue = 3;
break;
case 3:
cardValue = 4;
break;
case 4:
cardValue = 5;
break;
case 5:
cardValue = 6;
break;
case 6:
cardValue = 7;
break;
case 7:
cardValue = 8;
break;
case 8:
cardValue = 9;
break;
case 9:
cardValue = 10;
break;
case 10:
cardValue = 10;
break;
case 11:
cardValue = 10;
break;
case 12:
cardValue = 10;
break;
case 13:
cardValue = 11;
break;


to this,



case 0:
if (Deck[0] = 0)
{
Deck[0] = 1;
cardValue = 2;
}
else
rerand();
break;
case 1:
if (Deck[1] = 0)
{
Deck[1] = 1;
cardValue = 3;
}
else
rerand();
break;
case 2:
if (Deck[2] = 0)
{
Deck[2] = 1;
cardValue = 4;
}
else
rerand();
break;
case 3:
if (Deck[3] = 0)
{
Deck[3] = 1;
cardValue = 5;
}
else
rerand();
break;
case 4:
if (Deck[4] = 0)
{
Deck[4] = 1;
cardValue = 6;
}
else
rerand();
break;
case 5:
if (Deck[5] = 0)
{
Deck[5] = 1;
cardValue = 7;
}
else
rerand();
break;
case 6:
if (Deck[6] = 0)
{
Deck[6] = 1;
cardValue = 8;
}
else
rerand();
break;
case 7:
if (Deck[7] = 0)
{
Deck[7] = 1;
cardValue = 9;
}
else
rerand();
break;
case 8:
if (Deck[8] = 0)
{
Deck[8] = 1;
cardValue = 10;
}
else
rerand();
break;
case 9:
if (Deck[9] = 0)
{
Deck[9] = 1;
cardValue = 10;
}
else
rerand();
break;
case 10:
if (Deck[10] = 0)
{
Deck[10] = 1;
cardValue = 10;
}
else
rerand();
break;
case 11:
if (Deck[11] = 0)
{
Deck[11] = 1;
cardValue = 10;
}
else
rerand();
break;
case 12:
if (Deck[12] = 0)
{
Deck[12] = 1;
cardValue = 10;
}
else
rerand();
break;
case 13:
if (Deck[13] = 0)
{
Deck[13] = 1;
cardValue = 11;
}
else
rerand();
break;


I also created a global array of 13 cards, and the rerand() is just this,



void Card::rerand()
{
cardType = (rand() % 13);
cardValue = 0;
}


My advice is not to have the cards randomize themselves. Change the constructor to accept an initial value and have logic outside the card class determine which cards to create.

Having two of the same card isn't exactly a failure condition (consider that there are 4 of each type of card in a standard deck corresponding to the 4 different suits), though that said, you're not handling the cards as a deck, you seem to be plucking random numbers out of the air for each card value.

The simplest way to approach this is to consider how a real blackjack deal works. The dealer doesn't randomly select a card from the entire stack, he/she take the top card from an already shuffled deck and turns it over.

So essentially what you need to do (in pseudo code):

Initial setup:

1. Create an array with 52 elements.
2. Populate the array with each card in a standard deck.
3. Shuffle the array.

Then to deal a card:

1. Select the top card
2. Increment a counter so we now point to the next card in the array.

An important thing to note about the shuffling of the array is that is easy to inadvertently create a biased shuffling algorithm, which means certain cards will appear in a certain order more frequently than other combinations. Your best bet is to use a standard library function such as std::random_shuffle() as suggested by rip-off, or if you're determined to roll your own look at something like the Fisher-Yates shuffle as mentioned in the coding horror post I linked.

A really simple way, which should be just as random (from the view of the player) is to populate an array of 52 cards. Instead of shuffling, pick a random integer 0 to 51, return that card to the player. When you do this, you take the last card from the set of remaining cards, and move that to the position the player took from:



card* take_card(deck* deck)
{
if ( !deck || (deck->num_cards == 0) )
{
return NULL; //out of cards!
}

int i = rand() % (deck->num_cards); //pick a card
card* the_card = deck->cards; //take the card
deck->cards = deck->cards[ deck->num_cards -1 ]; //move last card from where card was taken from
deck->num_cards--; //reduce card count

return the_card;

}



This fulfills the two key requirements: 1) once a card is taken, you can't get it again 2) cards are returned in a random order

Also, if you don't trust rand() to give good random numbers, you can add real randomless, such as the number of microseconds since the user last pressed a key, or moved the mouse, stuff like that.

Also, if you don't trust rand() to give good random numbers, you can add real randomless, such as the number of microseconds since the user last pressed a key, or moved the mouse, stuff like that.
[/quote]
Don't hack this though. If you want non psuedo random values, you can ask the system to generate them for you. On Unix systems that might involve reading from /dev/random, on Windows you might make calls like CryptGenRandom.

But for such a game a psuedo random generator will suffice. You could use a different version, something like the Mersenne Twister.

Also, if you don't trust rand() to give good random numbers, you can add real randomless, such as the number of microseconds since the user last pressed a key, or moved the mouse, stuff like that.

"you can add real randomless, such as , stuff like that."

While I appreciate that you are attempting to suggest methods to 'increase the randomness' of the random numbers from the random number generator, it is NOT a good idea to chose things that the user has direct control over (whether or not they are aware of it). I would also recommend not representing it as random, any way you look at it. Random number generators are generally sufficient, especially in this case, which seems to be a fairly simple independent blackjack program.

All of your if statements are assigning a value...not doing a comparison.


if (Deck[0] = 0)


Change it to

if (Deck[0] == 0)


ok I fixed it but, then I went to go add some other things and I came back and now its giving me 3,5,9 every time D:




class Card
{
public:
Card();
int cardValue;
int cardType;
void applyCard(int x,int y);
void rerand();
};

void Card::rerand()
{
cardType = (rand() % 13);
cardValue = 0;
}

Card::Card()
{
cardType = (rand() % 13);
cardValue = 0;
switch (cardType)
{

case 0:
if (Deck[0] == 0)
{
Deck[0] = 1;
cardValue = 2;
}
else
rerand();
break;
case 1:
if (Deck[1] == 0)
{
Deck[1] = 1;
cardValue = 3;
}
else
rerand();
break;
case 2:
if (Deck[2] == 0)
{
Deck[2] = 1;
cardValue = 4;
}
else
rerand();
break;
case 3:
if (Deck[3] == 0)
{
Deck[3] = 1;
cardValue = 5;
}
else
rerand();
break;
case 4:
if (Deck[4] == 0)
{
Deck[4] = 1;
cardValue = 6;
}
else
rerand();
break;
case 5:
if (Deck[5] == 0)
{
Deck[5] = 1;
cardValue = 7;
}
else
rerand();
break;
case 6:
if (Deck[6] == 0)
{
Deck[6] = 1;
cardValue = 8;
}
else
rerand();
break;
case 7:
if (Deck[7] == 0)
{
Deck[7] = 1;
cardValue = 9;
}
else
rerand();
break;
case 8:
if (Deck[8] == 0)
{
Deck[8] = 1;
cardValue = 10;
}
else
rerand();
break;
case 9:
if (Deck[9] == 0)
{
Deck[9] = 1;
cardValue = 10;
}
else
rerand();
break;
case 10:
if (Deck[10] == 0)
{
Deck[10] = 1;
cardValue = 10;
}
else
rerand();
break;
case 11:
if (Deck[11] == 0)
{
Deck[11] = 1;
cardValue = 10;
}
else
rerand();
break;
case 12:
if (Deck[12] == 0)
{
Deck[12] = 1;
cardValue = 10;
}
else
rerand();
break;
case 13:
if (Deck[13] == 0)
{
Deck[13] = 1;
cardValue = 11;
}
else
rerand();
break;
}
}

void Card::applyCard(int x, int y)
{
SDL_Surface *Card_image = NULL;
switch(cardType)
{
case 0:
Card_image = clarify_image("Art/2Club.png");
apply_surface(x,y,Card_image,screen);
break;
case 1:
Card_image = clarify_image("Art/3Club.png");
apply_surface(x,y,Card_image,screen);
break;
case 2:
Card_image = clarify_image("Art/4Club.png");
apply_surface(x,y,Card_image,screen);
break;
case 3:
Card_image = clarify_image("Art/5Club.png");
apply_surface(x,y,Card_image,screen);
break;
case 4:
Card_image = clarify_image("Art/6Club.png");
apply_surface(x,y,Card_image,screen);
break;
case 5:
Card_image = clarify_image("Art/7Club.png");
apply_surface(x,y,Card_image,screen);
break;
case 6:
Card_image = clarify_image("Art/8Club.png");
apply_surface(x,y,Card_image,screen);
break;
case 7:
Card_image = clarify_image("Art/9Club.png");
apply_surface(x,y,Card_image,screen);
break;
case 8:
Card_image = clarify_image("Art/10Club.png");
apply_surface(x,y,Card_image,screen);
break;
case 9:
Card_image = clarify_image("Art/JClub.png");
apply_surface(x,y,Card_image,screen);
break;
case 10:
Card_image = clarify_image("Art/QClub.png");
apply_surface(x,y,Card_image,screen);
break;
case 11:
Card_image = clarify_image("Art/KClub.png");
apply_surface(x,y,Card_image,screen);
break;
case 12:
Card_image = clarify_image("Art/AClub.png");
apply_surface(x,y,Card_image,screen);
break;
}
}


I can't really see (with the snippet provided) why it would generate the same numbers all the time other than that you are calling srand at the Cards initialization. You are going to run into many problems with this approach that can be solved (and your current problem most likely will be too) solved with pulling your randomization outside of the Cards initialization. You should have a shuffler class that knows it needs to create 52 cards, and each number can be repeated 4 times (different suites). In your shuffler class you would track what numbers have been used and what suites have been used for that number and than they cannot be generated again.

I cannot provide source because I am at work...but even if I wasn't it wouldn't help you because I have only done it in C#, but the same principal applies. You should be able to figure it out. Break it down into logic chunks and its not that difficult.

Let me know how it turns out.

As SiCrane has said, the easiest way to do this is to generate a known, complete deck, and then randomly shuffle that deck. If you want to write a blackjack program that works, that is how to do it.

Something like this:

#include <iostream>
#include <string>
#include <vector>
#include <algorithm>
#include <stdexcept>
#include <cassert>

enum Suit
{
Heart, Club, Diamond, Spade
};

std::ostream &operator<<(std::ostream &out, Suit suit)
{
static const std::string lookup[] = { "Hearts", "Clubs", "Diamonds", "Spades" };
assert(suit >= 0 && suit < 4);
return out << lookup[suit];
}

enum CardType
{
One, Two, Three, Four, Five, Six, Seven, Eight, Nine, Ten, Jack, Queen, King
};

std::ostream &operator<<(std::ostream &out, CardType cardType)
{
assert(cardType >= 0 && cardType <= King);
switch(cardType)
{
case Jack: out << "Jack"; break;
case Queen: out << "Queen"; break;
case King: out << "King"; break;
default:
out << static_cast<int>(cardType + 1);
break;
}
return out;
}

class Card
{
public:
Card(CardType type, Suit suit)
:
suit(suit),
type(type)
{
}

// ...

friend std::ostream &operator<<(std::ostream &out, const Card &card)
{
return out << card.type << " of " << card.suit;
}

private:
Suit suit;
CardType type;
};

class Deck
{
public:
Deck();

void shuffle();

Card takeCard();

bool isEmpty() const;

private:
std::vector<Card> cards;
};

Deck::Deck()
{
for(int suit = Heart ; suit <= Spade ; ++suit)
{
for(int type = One ; type <= King ; ++type)
{
Card card = Card(static_cast<CardType>(type), static_cast<Suit>(suit));
cards.push_back(card);
}
}
}

void Deck::shuffle()
{
std::random_shuffle(cards.begin(), cards.end());
}

bool Deck::isEmpty() const
{
return cards.empty();
}

Card Deck::takeCard()
{
if(isEmpty())
{
throw std::runtime_error("Cannot take a card from an empty deck!");
}
Card card = cards.back();
cards.pop_back();
return card;
}

int main()
{
Deck deck;
deck.shuffle();

std::string input;
while(!deck.isEmpty() && std::cout << "Press enter to be dealt a card!" && std::getline(std::cin, input))
{
std::cout << "Dealing card: " << deck.takeCard() << '\n';
}

if(deck.isEmpty())
{
std::cout << "Ran out of cards!\n";
}
else
{
std::cout << "Bye...";
}
}

Notice especially how I avoided writing massive switch statements to cover all possible card values. You should be able to do the same thing when trying to draw them, if you put all the pictures in an array you can index with the values to get the correct image.

Rip-off has provided exactly what I was talking about. I didn't really want to provide source anyway because I find most people will just copy the source and not study it but what Rip-off has provided is the best (IMO) way to do it and it will eliminate future bugs and most likely your current bugs using this approach.

I am sure that whole shuffling thing with vectors and arrays and stuff is a way better way, but its all way to advanced I have know idea what assert,ostream,&operator stuff is, I like to keep it dumb, but I don't know anything about this. My psuedo-random thingy was working so now I just need that to be fixed and I will be on my way.


EDIT: I did fix it so that srand() is only called once, now its just giving me the same numbers srand() is in main now.

Show us your code.

Gladly,


main



int main ( int argc, char *args[] )
{
srand(time(0));
bool quit = false;
if( init() == false ) { return 1;}
if( load_files() == false ) { return 2;}
apply_surface( 0,0,Table,screen);
apply_surface(370,513,Border,screen);
Card1.applyCard(100,280);
Card2.applyCard(140,280);
Card3.applyCard(420,50);
while (quit == false)
{
option();
while ( SDL_PollEvent( &event ) )
{
if( event.type == SDL_QUIT )
{
quit = true;
}
}
SDL_Flip( screen );
}
cleanup();
return 0;
}


Class and all the member functions


class Card
{
public:
Card();
int cardValue;
int cardType;
void applyCard(int x,int y);
void rerand();
};

void Card::rerand()
{
cardType = (rand() % 13);
cardValue = 0;
}

Card::Card()
{
cardType = (rand() % 13);
cardValue = 0;
switch (cardType)
{

case 0:
if (Deck[0] == 0)
{
Deck[0] = 1;
cardValue = 2;
}
else
rerand();
break;
case 1:
if (Deck[1] == 0)
{
Deck[1] = 1;
cardValue = 3;
}
else
rerand();
break;
case 2:
if (Deck[2] == 0)
{
Deck[2] = 1;
cardValue = 4;
}
else
rerand();
break;
case 3:
if (Deck[3] == 0)
{
Deck[3] = 1;
cardValue = 5;
}
else
rerand();
break;
case 4:
if (Deck[4] == 0)
{
Deck[4] = 1;
cardValue = 6;
}
else
rerand();
break;
case 5:
if (Deck[5] == 0)
{
Deck[5] = 1;
cardValue = 7;
}
else
rerand();
break;
case 6:
if (Deck[6] == 0)
{
Deck[6] = 1;
cardValue = 8;
}
else
rerand();
break;
case 7:
if (Deck[7] == 0)
{
Deck[7] = 1;
cardValue = 9;
}
else
rerand();
break;
case 8:
if (Deck[8] == 0)
{
Deck[8] = 1;
cardValue = 10;
}
else
rerand();
break;
case 9:
if (Deck[9] == 0)
{
Deck[9] = 1;
cardValue = 10;
}
else
rerand();
break;
case 10:
if (Deck[10] == 0)
{
Deck[10] = 1;
cardValue = 10;
}
else
rerand();
break;
case 11:
if (Deck[11] == 0)
{
Deck[11] = 1;
cardValue = 10;
}
else
rerand();
break;
case 12:
if (Deck[12] == 0)
{
Deck[12] = 1;
cardValue = 10;
}
else
rerand();
break;
case 13:
if (Deck[13] == 0)
{
Deck[13] = 1;
cardValue = 11;
}
else
rerand();
break;
}
}

void Card::applyCard(int x, int y)
{
SDL_Surface *Card_image = NULL;
switch(cardType)
{
case 0:
Card_image = clarify_image("Art/2Club.png");
apply_surface(x,y,Card_image,screen);
break;
case 1:
Card_image = clarify_image("Art/3Club.png");
apply_surface(x,y,Card_image,screen);
break;
case 2:
Card_image = clarify_image("Art/4Club.png");
apply_surface(x,y,Card_image,screen);
break;
case 3:
Card_image = clarify_image("Art/5Club.png");
apply_surface(x,y,Card_image,screen);
break;
case 4:
Card_image = clarify_image("Art/6Club.png");
apply_surface(x,y,Card_image,screen);
break;
case 5:
Card_image = clarify_image("Art/7Club.png");
apply_surface(x,y,Card_image,screen);
break;
case 6:
Card_image = clarify_image("Art/8Club.png");
apply_surface(x,y,Card_image,screen);
break;
case 7:
Card_image = clarify_image("Art/9Club.png");
apply_surface(x,y,Card_image,screen);
break;
case 8:
Card_image = clarify_image("Art/10Club.png");
apply_surface(x,y,Card_image,screen);
break;
case 9:
Card_image = clarify_image("Art/JClub.png");
apply_surface(x,y,Card_image,screen);
break;
case 10:
Card_image = clarify_image("Art/QClub.png");
apply_surface(x,y,Card_image,screen);
break;
case 11:
Card_image = clarify_image("Art/KClub.png");
apply_surface(x,y,Card_image,screen);
break;
case 12:
Card_image = clarify_image("Art/AClub.png");
apply_surface(x,y,Card_image,screen);
break;
}
}
Card Card1;
Card Card2;
Card Card3;


I don't see any srand() calls in your code now.

Whoops I was trying to find a different place to put it in but I put it back at the top of main, still same problem.