• Advertisement

Archived

This topic is now archived and is closed to further replies.

Bug in dice rolling

This topic is 5067 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I have this code to roll 4 dice, add them up, and take away the smallest. The value is then assigned to a variable such as strength, the roll is then repeated and the result assigned to dexterity, then repeated etc. But there seems to be a bug, it works ok most of the time, but sometimes in the last couple of rolls, the Smallest Value is not correct, eg it says 2 when in the rolls you can see that there was a 1. I have the sleep(3000); in there just so I can see each result before it fills up a small dos screen ;-)
   //FIRST IS THE STRENGTH CALCULATION


   int NUMBER_OF_SIDES =6;
   srand( (unsigned)time( NULL ) );
   int strengthroll[4];
   for (int i = 0; i < 4; ++i)  strengthroll[i] = 1 + rand() % NUMBER_OF_SIDES;
   
   int sum = 0;       // Start the total sum at 0.

    for (i=0; i<4; i++) {
sum+=strengthroll[i];
}
   
   system("cls");
	cout<<"\nThe die rolls for strength are: ";
for(int u=0; u < 4; u++)
cout<<strengthroll[u]<<" ";
	cout <<"\nThe total of the strength rolls is " <<sum;


int smallest;
smallest = 999999;
for(i=1; i<u; i++)        //check until condition i=n

   if(strengthroll[i] < smallest)
smallest = strengthroll[i];
strength = sum - smallest;
cout<<"\nSmallest value= "<<smallest;
cout<<"\n"<<charname<<"'s strength is "<<strength;


//SECOND IS THE DEXTERITY CALCULATION


int dexterityroll[4];
   for (i = 0; i < 4; ++i)  dexterityroll[i] = 1 + rand() % NUMBER_OF_SIDES;
   
    sum = 0;       // Start the total sum at 0.

    for (i=0; i<u; i++) {
sum+=dexterityroll[i];
}
   Sleep(3000);
   
	cout<<"\nThe die rolls for dexterity are: ";
for(u=0; u < 4; u++)
cout<<dexterityroll[u]<<" ";
	cout <<"\nThe total of the dexterity rolls is " <<sum;
int smalldex;
smalldex = 999999;
for(i=1; i<u; i++)        //check until condition i=n

   if(dexterityroll[i] < smalldex)
smalldex = dexterityroll[i];
dexterity = sum - smalldex;
cout<<"\nSmallest value= "<<smalldex;
cout<<"\n"<<charname<<"'s dexterity is "<<dexterity;


//THIRD IS THE CONSTITUTION CALCULATION



int constitutionroll[4];
   for (i = 0; i < 4; ++i)  constitutionroll[i] = 1 + rand() % NUMBER_OF_SIDES;
   
    sum = 0;       // Start the total sum at 0.

    for (i=0; i<u; i++) {
sum+=constitutionroll[i];
}
   Sleep(3000);
   
	cout<<"\nThe die rolls for constitution are: ";
for(u=0; u < 4; u++)
cout<<constitutionroll[u]<<" ";
	cout <<"\nThe total of the constitution rolls is " <<sum;
int smallcon;
smallcon = 999999;
for(i=1; i<u; i++)        //check until condition i=n

   if(constitutionroll[i] < smallcon)
smallcon = constitutionroll[i];
constitution = sum - smallcon;
cout<<"\nSmallest value= "<<smallcon;
cout<<"\n"<<charname<<"'s constitution is "<<constitution;




//FOURTH IS THE INTELLIGENCE CALCULATION



int intelligenceroll[4];
   for (i = 0; i < 4; ++i)  intelligenceroll[i] = 1 + rand() % NUMBER_OF_SIDES;
   
    sum = 0;       // Start the total sum at 0.

    for (i=0; i<u; i++) {
sum+=intelligenceroll[i];
}
   Sleep(3000);
   
	cout<<"\nThe die rolls for intelligence are: ";
for(u=0; u < 4; u++)
cout<<intelligenceroll[u]<<" ";
	cout <<"\nThe total of the intelligence rolls is " <<sum;
int smallint;
smallint = 999999;
for(i=1; i<u; i++)        //check until condition i=n

   if(intelligenceroll[i] < smallint)
smallint = intelligenceroll[i];
intelligence = sum - smallint;
cout<<"\nSmallest value= "<<smallint;
cout<<"\n"<<charname<<"'s intelligence is "<<intelligence;



//FIFTH IS THE WISDOM CALCULATION

 
int wisdomroll[4];
   for (i = 0; i < 4; ++i)  wisdomroll[i] = 1 + rand() % NUMBER_OF_SIDES;
   
    sum = 0;       // Start the total sum at 0.

    for (i=0; i<u; i++) {
sum+=wisdomroll[i];
}
   Sleep(3000);
   
	cout<<"\nThe die rolls for wisdom are: ";
for(u=0; u < 4; u++)
cout<<wisdomroll[u]<<" ";
	cout <<"\nThe total of the wisdom rolls is " <<sum;
int smallwis;
smallwis = 999999;
for(i=1; i<u; i++)        //check until condition i=n

   if(wisdomroll[i] < smallwis)
smallwis = wisdomroll[i];
wisdom = sum - smallwis;
cout<<"\nSmallest value= "<<smallwis;
cout<<"\n"<<charname<<"'s wisdom is "<<wisdom;


//SIXTH IS THE CHARISMA CALCULATION



int charismaroll[4];
   for (i = 0; i < 4; ++i)  charismaroll[i] = 1 + rand() % NUMBER_OF_SIDES;
   
    sum = 0;       // Start the total sum at 0.

    for (i=0; i<u; i++) {
sum+=charismaroll[i];
}
   Sleep(3000);
   
	cout<<"\nThe die rolls for charisma are: ";
for(u=0; u < 4; u++)
cout<<charismaroll[u]<<" ";
	cout <<"\nThe total of the charisma rolls is " <<sum;
int smallchar;
smallchar = 999999;
for(i=1; i<u; i++)        //check until condition i=n

   if(charismaroll[i] < smallchar)
smallchar = charismaroll[i];
charisma = sum - smallchar;
cout<<"\nSmallest value= "<<smallchar;
cout<<"\n"<<charname<<"'s charisma is "<<charisma;
strength, dexterity etc are declared earlier in the program. [edited by - unrealfragmaster on April 11, 2004 12:12:05 PM]

Share this post


Link to post
Share on other sites
Advertisement
I may be wrong but shouldnt


int smallest;
smallest = 999999;
for(i=1; i<u; i++)
if(strengthroll[i] < smallest)
smallest = strengthroll[i];

be

int smallest;
smallest = strengthroll[0];
for(i=1; i<u; i++)
if(strengthroll[i] < smallest)
smallest = strengthroll[i];

otherwise you never check to see if the first roll was the smallest.



[edited by - Drazgal on April 11, 2004 12:17:51 PM]

[edited by - Drazgal on April 11, 2004 12:18:33 PM]

Share this post


Link to post
Share on other sites
But doesnt

for(i=1; i<u; i++) if(strengthroll[i] < smallest) smallest = strengthroll[i];

check if the first one is smallest? Or should it be for i=0?

Share this post


Link to post
Share on other sites
As it stands it should be from i=0, and furthermore, if you''re going to flow loops/ifs onto multiple lines, use curly braces! It makes code so much clearer. Here''s my rewrite of it:


const int NUMBER_OF_SIDES = 6;

srand( (unsigned)time( NULL ));

int strengthRoll[4];
int i, sum=0;

for(i=0; i<4; ++i)
{
strengthRoll[i] = rand()%NUMBER_OF_SIDES + 1;
sum += strengthRoll[i];
}

system("cls");
cout<<"\nThe die rolls for strength are: ";
for(i=0; i<4; ++i) cout<<strengthRoll[i]<<" ";
cout<<"\nThe total of the strength rolls is "<<sum;

int smallest=strengthRoll[0];
for(i=1;i<4;++i)
{
if(strengthRoll[i] < smallest) smallest = strengthRoll[i];
}
sum -= smallest;

cout<<"\nSmallest value= "<<smallest;
cout<<"\n"<<charname<<"''s strength is "<<strength;


It starts from i=1 here because i=0 has already been used, when initializing smallest. If we started with 99999, like in your original post, then you''d need to loop from i=0 instead.

Share this post


Link to post
Share on other sites
Your code could be much simplified by creating a generic rollDice() function which returns a value. So, instead of having exactly the same code fragment copied and pasted 4 or 5 times, you would have something like:


int rollDice()
{
...
// Calculate your dice rolls here
return value;
}

...
int strength = rollDice();
int dexterity = rollDice();
int intelligence = rollDice();
int charisma = rollDice();
...

and so on.

Share this post


Link to post
Share on other sites
quote:
Original post by superpig
As it stands it should be from i=0, and furthermore, if you''re going to flow loops/ifs onto multiple lines, use curly braces! It makes code so much clearer. Here''s my rewrite of it:


const int NUMBER_OF_SIDES = 6;

srand( (unsigned)time( NULL ));

int strengthRoll[4];
int i, sum=0;

for(i=0; i<4; ++i)
{
strengthRoll[i] = rand()%NUMBER_OF_SIDES + 1;
sum += strengthRoll[i];
}

system("cls");
cout<<"\nThe die rolls for strength are: ";
for(i=0; i<4; ++i) cout<<strengthRoll[i]<<" ";
cout<<"\nThe total of the strength rolls is "<<sum;

int smallest=strengthRoll[0];
for(i=1;i<4;++i)
{
if(strengthRoll[i] < smallest) smallest = strengthRoll[i];
}
sum -= smallest;

cout<<"\nSmallest value= "<<smallest;
cout<<"\n"<<charname<<"''s strength is "<<strength;


It starts from i=1 here because i=0 has already been used, when initializing smallest. If we started with 99999, like in your original post, then you''d need to loop from i=0 instead.


Where is strength worked out in that?

Share this post


Link to post
Share on other sites
Im trying to shorten it by putting it into a function, but strength = rollDice()
dexterity = RollDice() etc all equal the same value


int rollDice()
{
const int NUMBER_OF_SIDES = 6;
srand( (unsigned)time( NULL ));
int Roll[4];
int i, sum=0;
for(i=0; i<4; ++i)
{ Roll[i] = rand()%NUMBER_OF_SIDES + 1;
sum += Roll[i];
int smallest=Roll[0];
for(i=1;i<4;++i)
{
if(Roll[i] < smallest) smallest = Roll[i];
}
sum -= smallest;
return sum;
}
}

Share this post


Link to post
Share on other sites
quote:
Original post by unrealfragmaster
quote:
Original post by superpig
As it stands it should be from i=0, and furthermore, if you''re going to flow loops/ifs onto multiple lines, use curly braces! It makes code so much clearer. Here''s my rewrite of it:


const int NUMBER_OF_SIDES = 6;

srand( (unsigned)time( NULL ));

int strengthRoll[4];
int i, sum=0;

for(i=0; i<4; ++i)
{
strengthRoll[i] = rand()%NUMBER_OF_SIDES + 1;
sum += strengthRoll[i];
}

system("cls");
cout<<"\nThe die rolls for strength are: ";
for(i=0; i<4; ++i) cout<<strengthRoll[i]<<" ";
cout<<"\nThe total of the strength rolls is "<<sum;

int smallest=strengthRoll[0];
for(i=1;i<4;++i)
{
if(strengthRoll[i] < smallest) smallest = strengthRoll[i];
}
sum -= smallest;

cout<<"\nSmallest value= "<<smallest;
cout<<"\n"<<charname<<"''s strength is "<<strength;


It starts from i=1 here because i=0 has already been used, when initializing smallest. If we started with 99999, like in your original post, then you''d need to loop from i=0 instead.


Where is strength worked out in that?



Whoops. I meant ''sum,'' not ''strength,'' at the end there.

The reason it''s always coming out as the same value is because you''re calling srand() each time, and time() will not have changed in the time it takes to move from one call to the next, so you''re effectively resetting the generator. Only call srand() once, at the beginning of your program.

Share this post


Link to post
Share on other sites
You're getting low values because you have the braces in the wrong place in your rollDice() function body.

Here's a version that works properly:


int rollDice()
{
const int NUMBER_OF_SIDES = 6;

// Summate 4 dice rolls

int Roll[4];
int i, sum=0;
for(i=0; i<4; ++i)
{
Roll[i] = rand()%NUMBER_OF_SIDES + 1;
sum += Roll[i];
}

// Find the smallest roll

int smallest=Roll[0];
for(i=1;i<4;++i)
{
if(Roll[i] < smallest)
{
smallest = Roll[i];
}
}

// Subtract the smallest roll from the sum

sum -= smallest;

return sum;
}


int main( )
{
srand( (unsigned)time(NULL) );

int strength = rollDice();
int dexterity = rollDice();
int charisma = rollDice();
int intelligence = rollDice();

return 0;
}


and here's a version that does away with the array allocation and the second loop looking for smallest:


unsigned int rollDice()
{
const int NUMBER_OF_SIDES = 6;

// Summate 4 dice rolls

unsigned int sum=0;
unsigned int smallest = NUMBER_OF_SIDES;
for( unsigned int i=0; i<4; i++)
{
unsigned int roll = rand() % NUMBER_OF_SIDES + 1;
sum += roll;
if ( roll < smallest )
{
smallest = roll;
}
}
// Subtract the lowest roll

sum -= smallest;
return sum;
}


[edited by - SDickson on April 11, 2004 3:06:27 PM]

Share this post


Link to post
Share on other sites

  • Advertisement