C++

Forum

 
Another rand() problem

Leppie (18)
Still trying to create a roll playing game, and using rand to decide a coin flip issue (if 1 attack, if 2 don't attack) then rand again to decide defensive move of enemy (if 1 then attack hits, if 2 attack misses). However on executing the code, all the time if my attack hits, then on enemy's turn his attack always hits? I know with such a small range that it should occasionally happen but not all the time. I've put "cout" in the code to show the numbers and when each hits. I presume that as rand is executing twice in short succession that the system time I'm using to seed rand does not move on so I end up with the same number generated. Any ideas on creating a more random way? (The code compiles and shows the results if this doesn't make too much sense).
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
 
#include <iostream>
#include <cstdlib>
#include <ctime>
using namespace std;

int main()
{
  int myHp = 20;
  int monsterHp = 20;
  int attack;
  int defence;
  
    time_t now;
    time(&now);
    srand(now);
    
   attack = rand() % 2 + 1;
   defence = rand() % 2 + 1;
   
cout << "You enter the arena" << endl;
do {
cout << attack << endl;
cout << defence << endl;
   if (myHp < 1) {
   cout << "Your brown bread (Dead!)" << endl;
   break;
   }   
   if (attack ==1) {
   cout << "You attack the monster" << endl;
   }
   else {
   cout << "You stand petrified" << endl;
   }
   if (attack ==1 && defence == 1){
   cout << "You lay into him" << endl;
   monsterHp -= 2;
   cout << "Monsters HP now: " << monsterHp << endl;
   }
   else if (attack == 1 && defence ==2) {
   cout << "The monster dodges your attack" << endl;
   }
   else {
   cout << "The monster stares at you" << endl;
   }
   
   attack = rand() % 2 + 1;
   defence = rand() % 2 + 1;
   
cout << attack << endl;
cout << defence << endl;
   
   if (monsterHp < 1) {
   cout << "You killed him (well done!)" << endl;
   break;
   }
   if (attack ==1) {
   cout << "The monster attacks" << endl;
   }
   else {
   cout << "The monster stares" << endl;
   }

   if (attack ==1 && defence == 1){
   cout << "Claws fly" << endl;
   myHp -= 2;
   cout << "Your HP now: " << myHp << endl;
   }
   else if (attack == 1 && defence ==2) {
   cout << "You dodge the attack" << endl;
   }
   else {
   cout << "You laugh with contempt" << endl;
   }
   }while( myHp != 0 || monsterHp != 0 );   
   
system ("Pause");
return 0;
}

Thanks in advance, Leppie
Disch (13742)
After trimming the fat, your game logic looks like this:

1
2
3
4
5
6
7
8
9
10
 
attack = rand();

do
{
   DidPlayerAttack();

   attack = rand();

   DidMonsterAttack();
}while(game_still_going);


Notice how you only set attack and defense once per loop iteration, but you use the value twice per loop iteration.

Essentially, you're not changing attack,defence after the monster's attack. You're using the monster's values to also determine the player's action.


Also, FWIW, you might want to consider breaking the code into functions. It really helps with code clarity. Instead of cramming everything into main, you can break it up into smaller functions and just call those functions. It makes logic problems like this easier to spot.
Leppie (18)
Thanks Disch, silly mistake, I just couldn't see it! Most impressed with your fat trimmed version (obviously I've still a long way to go)! I was/am going to break the code into functions but wanted to get it working properly first.
Thanks again, Leppie
Athar (4466)
Some notes:
Magic numbers (like attack and defence) make it hard to figure out what the code actually does. Using bool circumvents the problem nicely in this case.
And if you use magic numbers, you should use the one you actually mean when possible (i.e. 0 instead of 1 when you want to do something when the HP drops to 0).
For if/else constructs with just one statement you don't need a block. That just adds unnecessary visual clutter and can make even simple code hard to read when there are a lot of these.

Here's your code with some parts moved around to form something what I would consider a more obvious structure:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
 
#include <iostream>
#include <cstdlib>
#include <ctime>
using namespace std;

int main()
{
    int myHp=20;
    int monsterHp=20;

    srand(time(0));

    cout << "You enter the arena" << endl;
    do
    {
        //player's turn
        bool attack=rand()%2;
        bool defend=rand()%2;

        if (attack)
        {
          cout << "You attack the monster" << endl;
          if (defend)cout << "The monster dodges your attack" << endl;
          else
          {
            cout << "You lay into him" << endl;
            monsterHp-=2;
            if (monsterHp<=0)
            {
              cout << "You killed him (well done!)" << endl;
              break;
            }
            cout << "Monsters HP now: " << monsterHp << endl;
          }
        }
        else
        {
          cout << "You stand petrified" << endl;
          cout << "The monster stares at you" << endl;
        }

        //monster's turn
        attack=rand()%2;
        defend=rand()%2;

        if (attack)
        {
          cout << "The monster attacks" << endl;
          if (defend)cout << "You dodge the attack" << endl;
          else
          {
            cout << "Claws fly" << endl;
            myHp-=2;
            if (myHp<=0)
            {
               cout << "Your brown bread (Dead!)" << endl;
               break;
            }
            cout << "Your HP now: " << myHp << endl;
          }
        }
        else
        {
          cout << "The monster stares" << endl;
          cout << "You laugh with contempt" << endl;
        }
    }
    while (myHp>0 || monsterHp>0);
}
Last edited on
Engborg (10)
Here's my try for a rpg game. Feel free to comment!


1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
 
#include "stdafx.h"
#include <ctime>
#include <iostream>
#include <string>

using namespace std;

void start();
void stats(int hp, int lvl, int dmg, int exp, int money, int weapon);
void whattodo(int hp, int lvl, int dmg, int exp, int money, int weapon);
int battle(int hp, int lvl, int dmg, int exp, int weapon);
int attack(int dmg, int lvl, int exp, int weapon);
int shop(int hp, int lvl, int dmg, int exp, int money, int weapon);
int main()
{
	srand((unsigned)time(0));

	start();

	return 0;
}

void stats(int hp, int lvl, int dmg, int exp, int money, int weapon)
{
	string weaponname;

	lvl = exp / (lvl*100);

	int dmgmax = 0;
	int dmgmin = 0;
	
	if(weapon == 1){
		weaponname = "Sword";
		for(int i = 0; i < 100; i++){
			dmg = lvl * (rand() %35 + 2);
			if(dmg < dmgmin){
				dmgmin = dmg;
			}
			else if(dmg > dmgmax){
				dmgmax = dmg;
			}
		}
	}
	else if(weapon == 2){
			weaponname = "Mace";
		for(int i = 0; i < 100; i++){
			dmg = lvl * (rand() %40 + 2);
			if(dmg < dmgmin){
				dmgmin = dmg;
			}
			else if(dmg > dmgmax){
				dmgmax = dmg;
			}
		}
	}
	else if(weapon == 3){
			weaponname = "Axe";
		for(int i = 0; i < 100; i++){
			dmg = lvl * (rand() %50 + 2);
			if(dmg < dmgmin){
				dmgmin = dmg;
			}
			else if(dmg > dmgmax){
				dmgmax = dmg;
			}
		}
	}
	else{
		weaponname = "None";
		for(int i = 0; i < 100; i++){
			dmg = lvl * (rand() %30 + 2);
			if(dmg < dmgmin){
				dmgmin = dmg;
			}
			else if(dmg > dmgmax){
				dmgmax = dmg;
			}
		}
	}

	cout << "Your LVL is: " << lvl << endl;
	cout << "Your EXP is: " << exp - lvl*100 << endl;
	cout << "Your money is: " << money << endl;
	cout << "Your HP is: " << hp << endl;
	cout << "Your DMG is between: " << dmgmin << " - " << dmgmax << endl;
	cout << "Your weapon is: " << weaponname << endl << endl;
	whattodo(hp, lvl, dmg, exp, money, weapon);
}

void whattodo(int hp, int lvl, int dmg, int exp, int money, int weapon)
{
	int moneynexp;
	int what;
start:
	what = 0;
	cout << "What would you like to do?\n";
	cout << "1 = 'Show stats.'\n";
	cout << "2 = 'Battle.'\n";
	cout << "3 = 'Shop.'\n";
	cout << "4 = 'Quit.'\n";
	cin >> what;
	cout << endl;
	
	if(what != 1 && what != 2 && what != 3 && what != 4){
		cout << "Not a valid answer!\n\n";
		goto start;
	}
	switch(what){
		case 1:
			stats(hp, lvl, dmg, exp, money, weapon);
			break;
		case 2:
			moneynexp = battle(hp, lvl, dmg, exp, weapon);
			exp += moneynexp;
			money += moneynexp;
			whattodo(hp, lvl, dmg, exp, money, weapon);
			break;
		case 3:
			weapon = shop(hp, lvl, dmg, exp, money, weapon);
			whattodo(hp, lvl, dmg, exp, money, weapon);
			break;
		case 4:
			goto end;
			break;
		default:
			cout << "Not a valid answer!\n\n";
			goto start;
	}
end:
	cout << "\nGood bye!\n";
	exit(1);
}

int battle(int hp, int lvl, int dmg, int exp, int weapon)
{
	int expgaind;

	int monster = rand() %4 + 1;
	int mhp, mdmg, mlvl;
	string monstername;
	mlvl = lvl + (rand() %2 + 1) - (rand() %2 + 1);
	mhp = mlvl*20;
	mdmg = mlvl * (2 + rand() %10);

	switch(monster){
		case 1:
			cout << "A Ogre attacks you...\n";
			monstername = "Ogre";
			break;
		case 2:
			cout << "A Goblin attacks you...\n";
			monstername = "Goblin";
			break;
		case 3:
			cout << "A Murloc attacks you...\n";
			monstername = "Murloc";
			break;
		case 4:
			cout << "A Troll attacks you...\n";
			monstername = "Troll";
			break;
		default:
			cout << "Programm fel!\n";
			system("pause");
	}
attack:
	mdmg = mlvl * (2 + rand() %10);
	hp -= mdmg;
	cout << monstername << " did " << mdmg << " damage to you\n";
	cout << "You have " << hp << " HP left\n\n";
	if(hp <= 0){
		cout << "You died! Game over!\n\n";
		expgaind = 0;
		start();
	}
	dmg = attack(dmg, lvl, exp, weapon);
	mhp -= dmg;
	cout << endl << "You did " << dmg << " damage to " << monstername << endl;
	if(mhp <= 0){
		cout << "You won!\n\n";
		expgaind = 20;
	}
	else{
	cout << monstername << " now has " << mhp << " hp left!\n\n";
	goto attack;
	}
	return expgaind;
}

int attack(int dmg, int lvl, int exp, int weapon)
{
	int flee = 0;
	int choise;
start:
	choise = 0;
	cout << "What would you like to do?\n";
	cout << "1 = 'Attack'\n";
	cout << "2 = 'Try to flee'\n";
	cin >> choise;

	if(choise != 1 && choise != 2){
		cout << "Not a valid answer!\n\n";
		goto start;
	}
	else if(choise == 2){
		flee = rand() %100 + 1;
		if(flee > 65){
			cout << "You did'nt make it..\n\n";
			dmg = 0;
		}
	}
	else{
		if(weapon == 1){
			dmg = lvl * (2 + rand() %35);
		}
		else if(weapon == 2){
			dmg = lvl * (2 + rand() %40);
		}
		else if(weapon == 3){
			dmg = lvl * (2 + rand() %50);
		}
		else{
			dmg = lvl * (2 + rand() %30);
		}
	}

	return dmg;
}

void start()
{
	signed int hp, dmg, lvl, money, weapon;
	int exp = 100;
	string name;
	lvl = 1;
	lvl = exp / (lvl*100);
	hp = lvl*40;
	dmg = lvl * (2 + rand() %30);
	money = 0;
	weapon = 0;

	cout << "What do you want your characters name to be? ";
	cin >> name;

	cout << endl << "Hi " << name << "!\n";

	whattodo(hp, lvl, dmg, exp, money, weapon);
}

int shop(int hp, int lvl, int dmg, int exp, int money, int weapon)
{
start:
	int sell = 0;
	if(weapon != 0){
		cout << "\nYou already got a weapon.\n";
		cout << "Do you want to sell it?\n";
		cout << "1 = 'Yes'\n";
		cout << "2 = 'no'\n";
		cin >> sell;
		if(sell == 1){
			if(weapon == 1){
				money += 150;
			}
			else if(weapon == 2){
				money += 250;
			}
			else{
				money += 400;
			}
			weapon = 0;
		}
		else if(sell == 2){
			whattodo(hp, lvl, dmg, exp, money, weapon);
		}
		else{
			cout << "Not a valid answer!\n";
			goto start;
		}
	}
	cout << endl << "What would you like to buy?";
	cout << endl << "1 = 'Sword'. Cost 200.";
	cout << endl << "2 = 'Mace'. Cost 300";
	cout << endl << "3 = 'Axe'. Cost 500";
	cout << endl << "4 = 'Nothing'.\n";
	cin >> weapon;
	
	if(weapon == 1){
		if(money < 200){
			cout << "\nYou don't have that much money..\n\n";
			weapon = 0;
			goto start;
		}
		money -= 200;
	}
	else if(weapon == 2){
		if(money < 300){
			cout << "\nYou don't have that much money..\n\n";
			weapon = 0;
			goto start;
		}
		money -= 300;
	}
	else if(weapon == 3){
		if(money < 500){
			cout << "\nYou don't have that much money..\n\n";
			weapon = 0;
			goto start;
		}
		money -= 500;
	}
	else if(weapon == 4){
		cout << endl << "\nFine, dont buy anything and die then!\n\n";
		weapon = 0;
	}
	else{
		cout << "\nThat's not a valid answer! Try again.\n";
		weapon = 0;
		goto start;
	}
	return weapon;
}

Disch (13742)
You're avoiding globals which is great. Not many newbies know to do that. Good work.

On the other hand, you're using goto and recursion to simulate loops, which is bad. Instead, you should just use a loop.

Also, you're using functions as if they were gotos, which is bad.

whattodo is a good example. Here's my critique:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
 
void whattodo(int hp, int lvl, int dmg, int exp, int money, int weapon)
{
	int moneynexp;
	int what;
start:
	what = 0;
	cout << "What would you like to do?\n";
	cout << "1 = 'Show stats.'\n";
	cout << "2 = 'Battle.'\n";
	cout << "3 = 'Shop.'\n";
	cout << "4 = 'Quit.'\n";
	cin >> what;
	cout << endl;
	
	 // this if() is redundant -- you already check for valid 'what's in your switch
	if(what != 1 && what != 2 && what != 3 && what != 4){
		cout << "Not a valid answer!\n\n";
		goto start;  // goto to simulate a loop = bad
	}
	switch(what){
		case 1:
			stats(hp, lvl, dmg, exp, money, weapon);
			break;
		case 2:
			moneynexp = battle(hp, lvl, dmg, exp, weapon);
			exp += moneynexp;
			money += moneynexp;
			whattodo(hp, lvl, dmg, exp, money, weapon);  // recursion to simulate a loop = bad
			break;
		case 3:
			weapon = shop(hp, lvl, dmg, exp, money, weapon);
			whattodo(hp, lvl, dmg, exp, money, weapon); // more recursion
			break;
		case 4:
			goto end; // more goto
			break;
		default:
			cout << "Not a valid answer!\n\n";
			goto start;
	}
end:
	cout << "\nGood bye!\n";
	exit(1);  // exit is abrupt and unexpected -- instead you should just let the call stack unwind
   // naturally
}


Also, notice whattodo() calls stats(), which calls whattodo(), which calls stats() again, etc, etc. Every time the user decides to view their stats, you burn up more and more stack space by all the stacked up function calls. Eventually you could run out of stack space -- resulting in the program crashing.

Functions are meant to be returned from. When a function returns, program flow resumes at the point where the function was called.

Here's a better way to control the flow of the program:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
 
// renamed function to mainmenu(), as this more clearly describes its purpose
void mainmenu(int hp, int lvl, int dmg, int exp, int money, int weapon)
{
    bool rungame = true; // false when the game is over
    
    while(rungame)  // keep looping as long as the game is running
    {
        int what = 0;

        cout << "What would you like to do?\n";
        cout << "1 = 'Show stats.'\n";
        cout << "2 = 'Battle.'\n";
        cout << "3 = 'Shop.'\n";
        cout << "4 = 'Quit.'\n";
        cin >> what;
        cout << endl;

        // do your switch on 'what'
        switch(what)
        {
        case 1:
            showstats(hp,lvl,dmg,exp,money,weapon);
            break;
        case 2:
            int moneynexp = battle(hp, lvl, dmg, exp, weapon);
            exp += moneynexp;
            money += moneynexp;
            break;
        case 3:
            weapon = shop(hp, lvl, dmg, exp, money, weapon);
            break;
        case 4:
            rungame = false;  // user wants to quit
            break;
        default:
            cout << "Not a valid answer!" << endl;
            break;
        }
    }

    // program flow gets here when the user wanted to quit
    //   when 'rungame' is false
    cout << "Goodbye";

    // no need to exit(), just return from the function (returning to main)
    //  then main() can return thus closing the program.
}


Note that with this approach, only main should be calling mainmenu(). When other functions, like shop, battle, etc want to return to the main menu -- they should just return. Flow will then continue in mainmenu() and the game will keep running appropriately.
Engborg (10)
Thanks for the comments, really appreciate them!

In my defence i'm pretty knew to handling that big projects.
But I realise that your way of doing this is much better and will be working that way from now on.
Disch (13742)
In my defence i'm pretty knew to handling that big projects.


Everyone starts somewhere. I've seen code a lot worse than yours. I've even written much worse code myself.

The important thing is you're making the effort to improve and grow.

Keep up the good work!
Engborg (10)
Thanks alot!

I want to start programming with "win32", do you know any good place to start learning from?
Disch (13742)
If you want to make games, I wouldn't recommend going with Win32.

I would get a crossplatform gaming lib. Much, much easier, much better suited to do what you want, and much faster.

I recommend SFML:

http://www.sfml-dev.org/

There are tutorials on that site.
Engborg (10)
Thanks for the content and the quick reply. I will look at the website.
Engborg (10)
I dont want to make a fool of myself but.. what's SFML?
Disch (13742)
It's a library.

Basically it's prewritten code that is designed to do a specific task. Things like drawing graphics, getting input from keyboard/mouse/gamepad, playing sounds/music, etc, etc.

It does all the complicated stuff under the hood which makes writing a game easier.
Engborg (10)
Okey. I will start reading up on it this weekend, newyear is coming up and i got some party planning ahead.
Thanks yet again, you've been very helpful.

Btw, sorry if my english is a little bad, from sweden.
Disch (13742)
Your English is fine.

In fact you are typing better than a lot of native speakers I've seen. ;D
Engborg (10)
Haha, well it's 3 AM here so i'm going to watch a movie then i'm going to bed.

Really appreciate all your help!
Maby you'll find another of my comments someday. (;
Engborg (10)
I can't get the SFML to work, did everything it said on the tutorials but it didn't work.
Then i downloaded the new "Visual Studio C++ 2010" and now i cant find where i include the libs and so on...

Could use some help Disch.
Disch (13742)
It seems like many people have problems setting up SFML. Personally I didn't find it difficult.

Anyway I'm not online frequently enough to walk you through it. Can you post what you tried and what errors you're getting and/or describe how it's not working?
Topic archived. No new replies allowed.