C++

Forum

 
Standard Programming

lorilew (12)
Hi, I just built my first game. I've had no teacher, I'm just learning by myself. I know there are certain ways to do some things.. standardized programming. Is there anything I need to improve on here?

It's a kind of dungeon crawl game, where the user has to reach the treasure 'X', without hitting a trap 'T'. And the traps keep multiplying.

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
 
#include <iostream>

using namespace std;

//GLOBAL VARIABLES
char board[8][10];
int x, y;
int m = 0, n = 0;
int trapx, trapy;
int win = 0, lose = 0;


//FUNCTIONS
void buildboard();
void initialize();
void moveplayer(char userinput);
int checkwin(int n, int m);
void settrap();
int checklose(int n, int m);


int main()
{ 
	//INITIALIZE
	char userinput;
	initialize();
	settrap();
	
	//DO WHILE NO WINNER OR LOSER
	while(win==0 && lose==0){
	
		buildboard();
		settrap();
		
		//ONLY CONTINUES IF VALID INPUT
		while(userinput != 'w' && userinput != 's' && userinput != 'a' && userinput != 'd')
		{
			cout << "\t\nPlayer move up, down, left, right (w,s,a,d)?";
			cin >> userinput; 
		}
		
		moveplayer(userinput);
		userinput = '.'; //RESET
	}
	
	//FINALLY OUTPUT RESULT
	buildboard();
	if (win == 1){
		cout << "\n\n\t***WINNER!!!***\n";
	}
	else
	{
		cout << "\n\n\t***YOU LOSE!!!***\n";
	}
}

void initialize()
{	 
	//GIVES ALL SPACES NULL VALUE
	for(int i=0; i<8; i++)
	{
		for(int j=0; j<10; j++)
		{
			board[i][j] = '.';
		}
	}	 
	
	//TREASURE RANDOM
	while(x+y == 0){
		srand(time(0)); 					
		x = rand() % 10;
		y = rand() % 8;
		board[y][x] = 'X';
	}
	
	//PLAYER AT 0,0.
	board[m][n] = 'G';
		
}

void buildboard()
{
	system ("cls");
	cout << "\n\tDUNGEON CRAWL \n";
	cout << "\n\tHurry they're multiplying... \n\n";
	
 	for(int i=0; i<8; i++)
	{
		cout << "\t ";
		for(int j=0; j<10; j++)
		{
			cout << board[i][j];
		}
		cout << endl;
	}	 
}

void moveplayer(char userinput)
{
	//W UP, S DOWN, A LEFT, D RIGHT;
	//DOES NOT LET PLAYER MOVE OFF BOARD
	board[n][m] = '.';
	if(userinput =='w' && n!=0){
		n = n - 1;
	}
	else if(userinput =='s' && n!=7){
		n = n + 1;
	}	 	 
	else if(userinput =='a' && m!=0){
		m = m - 1;
	}	 	 
	else if(userinput =='d' && m!=9){
		m = m + 1;
	}
	//before reassigning G check for treasure or trap!!******
	//if win
	win = checkwin(n,m);
	//if lose
	lose = checklose(n,m);
	//else	 
	board[n][m] ='G'; 	  	        	  
}

void settrap()
{
	//TRAP RANDOM
	bool go = false;
	while(go == false){
		trapx = rand() % 10;
		trapy = rand() % 8;
		if(board[trapy][trapx]=='.'){
			board[trapy][trapx] = 'T';
			go=true;
		}
	}
}

int checkwin(int n, int m)
{
	//IF PLAYER G FINDS TREASURE X
	if(board[n][m] == 'X'){
		return 1;
		buildboard();
		cout << "\n\n\t***WINNER!!!***\n";
	}
	else
	{
		return 0;
	}	
}


int checklose(int n, int m)
{
	//IF PLAYER G FINDS TRAP
	if(board[n][m] == 'T'){
		return 1;
		buildboard();
		cout << "LOSER";
	}
	else
	{
		return 0;
	}	
}
TheIdeasMan (6798)
Hi,

First of all your code is a pretty good effort for someone who is self taught - so well done!

Here are some tips:

Line 3:

Try not to bring in the whole std namespace into the global namespace. It can cause problems - there are a number of functions in std that might conflict with your variables - such as std::left and std::right plus many more for example. If you have a look at the code from advanced members of this forum you will see they prefix everything from std with std:: as in std::cout. You can do using std::cout; and similar for things that are used frequently if you want. Technically you should put your own stuff in your own namespace - read for how to do this.

Lines 6 - 10:
Really avoid using global variables - there can be confusion between the global & local variables. They generally lead to messy code.

Line 36:
I personally hate dislike :) constructs like this. IMO they are non scalable & messy. A much better option is to use a switch with a quit option, it might be more code - but it is much clearer & maintainable IMO. Sometimes it is worth it to have longer code instead of a nasty one liner.

bool variables lines 29 - 48:

I prefer to use these with true / false rather than 0 or 1. Technically no difference, just easier IMO. Although you have used them later on in your program.

i or j variables:

This is really nit picking, but I don't like these either!! I have seen before where someone had a mass of i and j variables for matrix processing & there was an error. So we did a find/replace changing them to row & col and the error was obvious straight away !

system calls

These are not preferred because they are not portable & pose a security threat.

If you format your IDE to use 4 spaces instead of tabs, the indenting might display better on this web page, as it always uses 8 spaces for tabs, whereas 4 spaces is 4 spaces.

Finally, there are heaps of resources in the reference section at the top left of this page + articles & tutorial (Wasn't sure if you were aware of these)

Hope all goes well :+)
lorilew (12)
Thanks a lot TheIdeasMan, I didn't know half of this stuff, I'll get to work. :D
lorilew (12)
Do you think the board[][] should not be a global variable even though it's used in every function?
MaxterTheTurtle (62)
lorilew:
Do you think the board[][] should not be a global variable even though it's used in every function?


This is an interesting question, because it depends on what you intend to do. For something of this size, having board as a global doesn't really hurt anything. However, if you intend to increase the size of your game and have multiple levels, then having board has a global can complicate things.

That's a pretty good program, especially for someone who is new and self taught. It seems to me that looking at your code, you haven't learned about classes or pointers/references yet. Those would really help you with your program.

It's futile to explain how learning about those things would help you with your program. (There are many others who can tell you much more than I can and in a better way. Learning those topics will make it apparent where you can improve your program.)

It's still a good program though! I remember when I wrote something like that when I was starting out. We all start from somewhere, and grow into something awesome.
lorilew (12)
This is the new version. I couldn't figure out how to verify the users input, but I'm still trying to interpret some of these articles :) And I go rid of system "cls" but it seems a little messy now, but it's ok. 

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
 
#include <iostream>
#include <string>


//GLOBAL VARIABLES
char board[8][10];


//FUNCTIONS
void buildboard();
void initialize();
int moveplayer(char userinput);
bool checkwin(int row, int col);
void settrap();
bool checklose(int row, int col);
void ClearScreen();


int main()
{ 
	//INITIALIZE
	char userinput;
	initialize();
	settrap();
	bool stop = false;
	
	//DO WHILE NO WINNER OR LOSER
	while(stop == false){
	
		buildboard();
		settrap();
	
		std::cout << "\n        Player move up, down, left, right (w,s,a,d)?\n\n\n\n";	  
		std:: cout << "        ";
		std::cin >> userinput; 
		stop = moveplayer(userinput);
		userinput = '.'; //RESET
	}
}

void initialize()
{	 
	int col = 0,row = 0;
	//GIVES ALL SPACES NULL VALUE
	for(int i=0; i<8; i++)
	{
		for(int j=0; j<10; j++)
		{
			board[i][j] = '.';
		}
	}	 
	
	//TREASURE RANDOM
	while(col+row == 0){
		srand(time(0)); 					
		col = rand() % 10;
		row = rand() % 8;
		board[row][col] = 'X';
	}
	
	//PLAYER AT 0,0.
	board[0][0] = 'G';
		
}

void buildboard()
{
	ClearScreen();
	std::cout << "\n        DUNGEON CRAWL \n";
	std::cout << "\n        Hurry they're multiplying... \n\n";
	
 	for(int row=0; row<8; row++)
	{
		std::cout << "\t ";
		for(int col=0; col<10; col++)
		{
			std::cout << board[row][col];
		}
		std::cout << "\n";
	}	 
}

int moveplayer(char userinput)
{
	int col,row;
	//need to set m,n to player's position
	for(int i=0; i<10; i++){
		for(int j=0; j<8; j++){
			if(board[j][i] == 'G'){
				col = i;
				row = j;
				break;
			}
		}
	}
	
	//W UP, S DOWN, A LEFT, D RIGHT;
	//DOES NOT LET PLAYER MOVE OFF BOARD
	board[row][col] = '.';
	if(userinput =='w' && row!=0){
		row--;
	}
	else if(userinput =='s' && row!=7){
		row++;
	}	 	 
	else if(userinput =='a' && col!=0){
		col--;
	}	 	 
	else if(userinput =='d' && col!=9){
		col++;
	}
	//before reassigning G check for treasure or trap!!******
	//if win
	if (checkwin(row,col) == true){
		std::cout << "\n        WINNER!\n\n\n";
		return true;
	}
	//if lose
	else if(checklose(row,col) == true){
		std::cout << "\n        Sorry you LOSE!\n\n\n";
		return true;
	}
	else{	
		board[row][col] ='G';
		return false;
	} 	  	        	  
}

void settrap()
{
	//TRAP RANDOM
	int col, row;
	bool ok = false;
	while(ok == false){
		col = rand() % 10;
		row = rand() % 8;
		if(board[row][col]=='.'){
			board[row][col] = 'T';
			ok=true;
		}
	}
}

bool checkwin(int row, int col)
{
	//IF PLAYER G FINDS TREASURE X
	if(board[row][col] == 'X'){
		return true;
		buildboard();
	}
	else
	{
		return false;
	}	
}


bool checklose(int row, int col)
{
	//IF PLAYER G FINDS TRAP
	if(board[row][col] == 'T'){
		return true;
		buildboard();
	}
	else
	{
		return false;
	}	
}
void ClearScreen()
{
    std::cout << std::string( 100, '\n' );
}
closed account (j3Rz8vqX)
Hey, I hope you don't mind.

I've decided to complicate the code, as well as simply:
(Alterations we're implemented with comments)

Edited-Exported Code: Based on the original post: http://pastie.org/8684192#11,18,30,48,51,117,135,143-148

Hopefully the above helps motivate new ideas.

Nice program, good job.

Edit: The above should not have impacted your game... hopefully not =D
Last edited on
TheIdeasMan (6798)
As Dput has alluded, the conditional statements always evaluate to true or false, so this:

while(stop == false){

can be simplified to:

while(!stop){

You have a boolean variable here, so that is easy to understand. It also works for integers - when it's 0 it's false. This can be useful when one has a loop that decrements - so it ends when the variable gets to 0.


In your original program you had:

while(userinput != 'w' && userinput != 's' && userinput != 'a' && userinput != 'd')

You may not have got around to changing it yet - but it would be better using a switch. It's important to do error checking on user input, so we can't leave that out. Here is a switch inside a while:

http://www.cplusplus.com/forum/beginner/92070/#msg499037

Here's a slightly different version with some of the game logic in:

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
 
bool Quit = false;

while (!Quit) {
     // get user input

     switch (userinput) {
         case 'w' :
            if (row) {  // row not zero
                row--;
            }
            else {
                //print error msg and let it loop again
            }
            break;

         case 's' :
               if (row!=7) {
                   //print error msg and let it loop again
               }
            break;
         // etc for other options
         case 'q' :  // user wants to quit
            Quit = true;
            break;

         default: // process errors
                     // doing nothing here makes it loop again
          break;

     }
}


One should use a std::endl at key points, because it flushes the buffer. Use \n as you have for the rest of the time.


@Dput

As I mentioned to the OP, I prefer Row & Col for variable names, rather than n & m, for the same reason I explained to the OP. Btw n = n - 1 is more concisely written as --n or n--. The prefix version decrements the value before it is used.

Any way I hope you are all having good fun - I am, anyone would think it was the wet season at this end, so I have time to reply to you guys 8>D

EDIT:

I saw you earlier post about the problems 10's and 8's. The usual thing to do there is make them const variables, and just use the variable name from there on.

1
2
3
4
5
6
7
8
9
10
 
const int RowSize = 8;
const int ColSize = 10;

char board[RowSize ][ColSize ];

for(int Row =0; Row < RowSize ; ++Row) {
		for(int Col=0; Col <ColSize ; ++Col){
			board[Row][Col] = '.';
		}
}
Last edited on
lorilew (12)
Hey ThedIdeasMan, MaxterTheTurtle and Dput, thanks for all the detailed feedback, I've actually learnt a lot today!

This is it so far now:
http://pastie.org/8684687

I think I'm gonna have a look at learning classes next. And I just don't understand pointers... I'm hoping to just absorb them unconsciously.
Last edited on
TheIdeasMan (6798)
Hi lorilew,

Thanks for your feedback :)

With pointers - they are just a variable that holds the memory address of another variable. However in C++ these days, we hardly ever use ordinary pointers, for now prefer to use references instead. They behave in a similar fashion but are better because they must refer to an object, whereas a pointer can be made to point at anything (even invalid things). There can be problems with references when they go out of scope, but they are much less error prone than ordinary pointers. A more advanced topic is smart pointers.

With classes, start with something simple & Google what others have written about them - there have been plenty of topics on this forum. They seem like a simple concept at first, but they do get complicated.

Another thing for you to look at is the STL, there are containers that hold any type of object, & algorithms that manipulate containers. A std::vector is similar to an array, but it can resize itself & all the containers have properties (like size for example) that one can inquire about. Other containers worth looking at are std::string & std::map. Simple algorithms might include std::sort or std::find

Anyway have fun
Topic archived. No new replies allowed.