dealing the cards (c++)

Started by
9 comments, last by Grain 18 years, 8 months ago
hey! this is my first attempt to code a game on my own.. so wohw - exciting:) im making a crad game, as you might have guessed, and atm im sitting with the function that deals the cards to the players. here is what i got, so far:

void dealer()               //Deals the cards
{

srand(time(0))
random_shuffle(deck.begin(), deck.end()); //shuffles the deck (uses srand(time(0)) )

switch
case numPlayers == 2 //2 players
{
     for(int i = 0; i > 8; i++)
     {
             player1Hand.push_back((deck.begin() + deckNum));
             deckNum += 1;
     }//for loop
     for(int i = 0; i > 8; i++)
     {
             player2Hand.push_back((deck.begin() + deckNum));
             deckNum += 1;
     }//for loop
case numPlayers == 3
.
-> same code, just 3 loops
.
case nimPlayers == 4
.
-> same code just 4 loops
.
}//case

the player limit is 4. and i got to think that there gotta be a smarter/faster/better way to do this.. any comments, improvement idears etc?
Advertisement
I would use an array instead of the method you're using, so:

for(int i = 0; i < NUM_PLAYERS; i++)
{
playerHand.push_back((deck.begin() + deckNum));
deckNum += 1;
}

edit: well, i'd also include the "for(int i = 0; i < 8; i++)" so it'd be an embedded for loop.



full code:
void dealer() //Deals the cards
{

srand(time(0))
random_shuffle(deck.begin(), deck.end()); //shuffles the deck (uses srand(time(0)) )

for(int i = 0; i < NUM_PLAYERS; i++)
{
for(int j = 0; j < 8; j++)
{
playerHand.push_back((deck.begin() + deckNum));
deckNum += 1;
}//for loop
}
Or better yet, a vector of vectors!
SlimDX | Ventspace Blog | Twitter | Diverse teams make better games. I am currently hiring capable C++ engine developers in Baltimore, MD.
yeah.. .okay i get the point of using an array:)
well maybe ini j = 0; in 1 of the loops, think using 2xi will turn out quiet.. funny:)
thanks
-niux
Just an FYI to avoid weird bugs you may run into, you're using > instead of < in your for loops.
Assuming your player hands are standard containers, you may want to switch the series of push_back calls to an assign call. Ex:
for (int i = 0; i < numPlayers; ++i) {  player_hand.assign(deck.begin() + i * 8, deck.end() + i * 8 + 8);}
yup.. as far as i know push_back() isnt an array member function - correct me if im wrong. so i'll the .assign as u said SiCrane.

but what i allso need is a 2d array, they way i see it. cuz with:
const int MAX_PLAYERS = 4;
playerHand[MAX_PLAYERS]
i'll get an array that can contain 5 items. thats okay, cuz of the player number. but however, it allso needs to contain 7 cards, in every hand. so in declared it like:
playerHand[3, 6]; //4 players, 7 cards (including position 0
and i got this code:

void dealer() //Deals the cards
{

srand(time(0));
random_shuffle(deck.begin(), deck.end()); //shuffles the deck (uses srand(time(0)) )
int deckNum = 0;

for (int i = 0; i < numPlayers; i++)
{
for(int j = 0; j < 7; j++)
{
playerHand[j].assign(deck.begin() + deckNum);
deckNum += 1;
}//for loop
}//for loop

}//end func

how does that look?

EDIT: i ripped this part of code out to test it (and whatever it needed to run), but i get 1 compiler error:
C:\Documents and Settings\M\Dokumenter\C++ Game Programming\testing.cpp: In function `void dealer()':
C:\Documents and Settings\M\Dokumenter\C++ Game Programming\testing.cpp:106: error: no matching function for call to `std::basic_string<char, std::char_traits<char>, std::allocator<char> >::assign(__gnu_cxx::__normal_iterator<std::string*, std::vector<std::string, std::allocator<std::string> > >)'
C:/Dev-Cpp/Bin/../lib/gcc/mingw32/3.4.2/../../../../include/c++/3.4.2/bits/basic_string.tcc:246: note: candidates are: std::basic_string<_CharT, _Traits, _Alloc>& std::basic_string<_CharT, _Traits, _Alloc>::assign(const std::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT = char, _Traits = std::char_traits<char>, _Alloc = std::allocator<char>]
C:/Dev-Cpp/Bin/../lib/gcc/mingw32/3.4.2/../../../../include/c++/3.4.2/bits/basic_string.h:806: note: std::basic_string<_CharT, _Traits, _Alloc>& std::basic_string<_CharT, _Traits, _Alloc>::assign(const std::basic_string<_CharT, _Traits, _Alloc>&, typename _Alloc::size_type, typename _Alloc::size_type) [with _CharT = char, _Traits = std::char_traits<char>, _Alloc = std::allocator<char>]
C:/Dev-Cpp/Bin/../lib/gcc/mingw32/3.4.2/../../../../include/c++/3.4.2/bits/basic_string.tcc:262: note: std::basic_string<_CharT, _Traits, _Alloc>& std::basic_string<_CharT, _Traits, _Alloc>::assign(const _CharT*, typename _Alloc::size_type) [with _CharT = char, _Traits = std::char_traits<char>, _Alloc = std::allocator<char>]
C:/Dev-Cpp/Bin/../lib/gcc/mingw32/3.4.2/../../../../include/c++/3.4.2/bits/basic_string.h:834: note: std::basic_string<_CharT, _Traits, _Alloc>& std::basic_string<_CharT, _Traits, _Alloc>::assign(const _CharT*) [with _CharT = char, _Traits = std::char_traits<char>, _Alloc = std::allocator<char>]
C:/Dev-Cpp/Bin/../lib/gcc/mingw32/3.4.2/../../../../include/c++/3.4.2/bits/basic_string.h:850: note: std::basic_string<_CharT, _Traits, _Alloc>& std::basic_string<_CharT, _Traits, _Alloc>::assign(typename _Alloc::size_type, _CharT) [with _CharT = char, _Traits = std::char_traits<char>, _Alloc = std::allocator<char>]

and all i can come to think of is "yeeeaaahhh right". what does that mean? my guess is that im using the .assign call syntax wrong, but dunno

[Edited by - Niux on August 2, 2005 6:29:38 PM]
assign takes two parameters.

In case you're interested in that kind of things, here's how the error messages read, once you put on the C++ goggles:

Quote:error: no matching function for call to `std::basic_string<char, std::char_traits<char>, std::allocator<char> >::assign(__gnu_cxx::__normal_iterator<std::string*, std::vector<std::string, std::allocator<std::string> > >)'


std::basic_string<char, std::char_traits<char>, std::allocator<char> > means std::string

std::string is actually a typedef for a specialization of std::basic_string which uses char. There is also std::wstring which is a string of wchar_t "wide characters". That's the first template parameter. The second, char_traits, contains information on how to manipulate the character type. The third, allocator, controls what functions the string class should use to allocate and deallocate memory (i.e. malloc/free vs. new/delete or something custom-written).

I interpret __gnu_cxx::__normal_iterator<std::string*, std::vector<std::string, std::allocator<std::string> > > to mean std::vector<std::string>::iterator.

__gnu_cxx::__normal_iterator is an internal standard library class (take the hint from the leading pairs of underscores). std::vector also has an allocator template parameter, just like std::string and all other standard containers. I assume that the first template parameter of __gnu_cxx::__normal_iterator refers to the fact that std::vector uses pointers internally - and in fact in some library implementation, vector iterators are pointers. This makes me think that this class has been introduced for the explicit purpose of giving vector iterators a different type than naked pointers, which does have consequences in generic programming (in fact, I seem to remember that some standard library functions in VC6 failed to work due to the fact that vector iterators were undistinguishable from pointers).

So the error reads as:

error: no matching function for call to `std::string::assign(std::vector<std::string>::iterator)'

The compiler then follows with the list of member function overloads that match the name of the member function you wanted to call, but not the parameters. It does refer to them as member functions of the class template std::basic_string as well as telling you in detail what are the template parameters it uses to instanciate the class template : '[with _CharT = char, _Traits = std::char_traits<char>, _Alloc = std::allocator<char>]'

Those candidates are, in the order in which they are listed:

std::string::assign( const std::string& );
std::string::assign( const std::string, size_type, size_type );
std::string::assign( const char*, size_type );
std::string::assign( const char* );
std::string::assign( size_type, char );


There is a script that will decrypt those error messages for you. [smile]

[Edited by - Fruny on August 2, 2005 7:54:07 PM]
"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." — Brian W. Kernighan
Quote:Original post by Niux
const int MAX_PLAYERS = 4;
playerHand[MAX_PLAYERS]
i'll get an array that can contain 5 items. thats okay, cuz of the player number. but however, it allso needs to contain 7 cards, in every hand. so in declared it like:
playerHand[3, 6]; //4 players, 7 cards (including position 0


No!

Valid indices for an array[n] are 0 up to n-1 inclusive, which is n total items. The number indicates the number of slots, and when you index, it indicates an "offset" to that item (thus the first item is offset 0 from the beginning of the array, because it's right directly at the beginning).

Thus you can never validly dereference like "array[n]" for an array declared "type array[n];". That is trying to grab whatever is immediately after the array. However, you can certainly use this pointer; idiomatically, "ranges of items" in C++ include the beginning but not the end point. It makes the math simpler in the end - trust us.

Quote:
int deckNum = 0;

for (int i = 0; i < numPlayers; i++)
{
for(int j = 0; j < 7; j++)
{
playerHand[j].assign(deck.begin() + deckNum);
deckNum += 1;


You've missed the point. First off, from the errors you're getting, it appears as though you're representing individual cards as std::strings - wtf? Anyway, the idea of "assign" is to copy across a range of values, not a single value; thus you provide a beginning and end index (and note the above comment about including the begin value but not the end value), and the implementation of the "assign" function automatically iterates - it handles the inner for loop for you, so you only worry about the outer one.

const int HAND_SIZE = 7; // or whatever it needs to be - maybe a parameter eventypedef std::string Card; // fix this later!typedef Card[HAND_SIZE] Hand; // you might also consider std::vector<Card>,// especially if HAND_SIZE can't be a constantHand[numPlayers] allHands;typedef std::vector<Card> Deck;Deck deck;// and initialize it here// Keep track of the first undealt card.Deck::iterator topCard = deck.begin();// Now we assign a hand to each player.for (int i = 0; i < numPlayers; ++i) {  // Assign a range. Beginning point of the range needs to be the first card  // for this hand, and end point should be the next one after the last card -  // i.e., the first card for the next hand (ignoring that there is no  // "next hand" the last time around).  allHands.assign(topCard, topCard + HAND_SIZE);  topCard += HAND_SIZE; // indicate these cards have been dealt}


You might also consider doing it in a way that actually removes cards from the "deck" when dealt, rather than just marking a current position (thus implicitly indicating some of the cards are 'invalid'). It's actually possible to shuffle "on demand" with this approach. Visualization hint: Playing with a randomly shuffled deck, dealing cards in sequence, is really just dealing a random card out of a collection each time; thus we could also do it by leaving the deck sorted and pulling a card out randomly each time we need to deal. Now, with std::vector, the equivalent of "moving all the cards up to eliminate the space where the removed card was" is inefficient, but we can be even trickier: imagine dealing by taking the top card off of a not-necessarily-random deck, pushing it into the deck at a random spot in such a way as to displace a card, and handing out the displaced card. :) This still selects a random card (since out of the remaining cards, a random position is selected and the card there is dealt) regardless of shuffling, and works well with a "contiguous storage" type of container (array or vector, where the data is held in a single block of memory). Of course optimization isn't going to matter here, I just like this one because it's slick ;)

See also here
Quote:
No!

Valid indices for an array[n] are 0 up to n-1 inclusive, which is n total items. The number indicates the number of slots, and when you index, it indicates an "offset" to that item (thus the first item is offset 0 from the beginning of the array, because it's right directly at the beginning).


okay.. i wasnt quiet sure on that, thanks!

Quote:
it appears as though you're representing individual cards as std::strings - wtf?


yes they are strings in a vector - and its ulgy, i know, and you dont wanna look at it, trust me:)

okay i get how the assign works now, thanks Zahlman!

tho there is still one thing i dont get, in your code.
Quote:
allHands.assign(topCard, topCard + HAND_SIZE);

the way i see it, you put 7 cards in allHands, witch is inited with Hand[numPlayers] allHands;
witch gives it 0-1-2-3-4-5, 6 slots. how do you put 4x7 cards in it?

maybe its just me, that dont really understand this:
typedef Card[HAND_SIZE] Hand;
and what is does to Hand, and what Hand then does to allHands?!


Fruny - thanks alot for the script, diffently look useable. but i just got up, so ill have to look at your post a little more when my brain allso wakes up:)

thx all
niux

This topic is closed to new replies.

Advertisement