Click here to Skip to main content
14,695,085 members
Please Sign up or sign in to vote.
0.00/5 (No votes)
See more:
hi..can someone explain me why if i press s button or d button it throws me segmetation fault but with a button and w button it works fine.it is just a function to move player p in a square just like snake.

What I have tried:

void move(){

int i;
int j;
char choice;
scanf("%s",&choice);

for (i = 0; i < rows; ++i) {
	for (j = 0; j < cols; ++j) {
		if(maze[i][j] == 'p' && choice == 'a'){
                   char tmp =  maze[i][j];
                    maze[i][j] = maze[i][j-1];
                    maze[i][j-1] = tmp;
		}

                 if(maze[i][j] == 'p' && choice == 'w'){
                         char tmp =  maze[i][j];
                        maze[i][j] = maze[i-1][j];
                        maze[i-1][j] = tmp;
                }

                if(maze[i][j] == 'p' && choice == 'd'){
                        char tmp = maze[i][j];
                        maze[i][j] = maze[i][j+1];
                        maze[i][j+1] = tmp;
                }

                  if(maze[i][j] == 'p' && choice == 's'){
                        char tmp =  maze[i][j];
                        maze[i][j] = maze[i+1][j];
                        maze[i+1][j] = tmp;
                }
	}
 }

}
Posted
Updated 8-May-20 1:40am
v2
Comments
jeron1 7-May-20 13:42pm
   
maze[i][j-1] = tmp;


What happens if j = 0?
Member 14825085 7-May-20 14:04pm
   
for this its ok cause i create a bound for the player(so he cant go there).the problem is with d(right) and s(down).
jeron1 7-May-20 14:10pm
   
Not sure I understand that, but...
a similar thing might occur here
maze[i+1][j] = tmp; 

where [i+1] might be out of range.

The first thing you need to do is make yourself a swap function. This sequence of code :
char tmp = maze[i][j];
maze[i][j] = maze[i][j+1];
maze[i][j+1] = tmp;
is repeated four times with varying arguments and that's is an invitation for errors. Make a swap function for yourself that takes four arguments: destination x, destination y, source x, and source y. It should take the maze itself as an argument but apparently that is a global variable.

This swap function will serve two purposes : it will put common functionality in one place and it will give you the opportunity to do verification in one place. In this function you can make sure all indexes are valid. It is apparent that they are not valid in certain circumstances and this will give you one place to check and report an error when found.
   
Comments
Patrice T 7-May-20 22:57pm
   
Hi Rick, S2 is a message for you.
Try to simplify your code in this manner
for (i = 0; i < rows; ++i) {
	for (j = 0; j < cols; ++j) {
		if(maze[i][j] == 'p') 
			continue;

		switch( choice )  {
		  case 'a':
                   swapMaze(i, j, i, j - 1);
                   break;
                   
                   //further cases
		}
	}
 }
When you have problem than make some output in the swapMaze function.
   
Advice: Learn to indent properly your code, it show its structure and it helps reading and understanding. It also helps spotting structures mistakes.
void move(){

    int i;
    int j;
    char choice;
    scanf("%s",&choice);

    for (i = 0; i < rows; ++i) {
        for (j = 0; j < cols; ++j) {
            if(maze[i][j] == 'p' && choice == 'a'){
                char tmp =  maze[i][j];
                maze[i][j] = maze[i][j-1];
                maze[i][j-1] = tmp;
            }

            if(maze[i][j] == 'p' && choice == 'w'){
                char tmp =  maze[i][j];
                maze[i][j] = maze[i-1][j];
                maze[i-1][j] = tmp;
            }

            if(maze[i][j] == 'p' && choice == 'd'){
                char tmp = maze[i][j];
                maze[i][j] = maze[i][j+1];
                maze[i][j+1] = tmp;
            }

            if(maze[i][j] == 'p' && choice == 's'){
                char tmp =  maze[i][j];
                maze[i][j] = maze[i+1][j];
                maze[i+1][j] = tmp;
            }
        }
    }

}

Indentation style - Wikipedia[^]

Professional programmer's editors have this feature and others ones such as parenthesis matching and syntax highlighting.
Notepad++ Home[^]
ultraedit[^]
-----
Your code is very simple minded, this leads to repeating mostly same code.
First thing you repeat same thing everywhere in the loop:
void move(){

	int i;
	int j;
	char choice;
	scanf("%s",&choice);

	for (i = 0; i < rows; ++i) {
		for (j = 0; j < cols; ++j) {
			if(maze[i][j] == 'p'){
				if(choice == 'a'){
					char tmp =  maze[i][j];
					maze[i][j] = maze[i][j-1];
					maze[i][j-1] = tmp;
				}

				if(choice == 'w'){
					char tmp =  maze[i][j];
					maze[i][j] = maze[i-1][j];
					maze[i-1][j] = tmp;
				}

				if(choice == 'd'){
					char tmp = maze[i][j];
					maze[i][j] = maze[i][j+1];
					maze[i][j+1] = tmp;
				}

				if(choice == 's'){
					char tmp =  maze[i][j];
					maze[i][j] = maze[i+1][j];
					maze[i+1][j] = tmp;
				}
			}
		}
	}

}

This code is faster because it test for 'P' once per loop instead of 4 times.
other optimizations are possible.
-----
My guess is that 'p' is the player.
Problem, your code do not check for walls.
Problem, your code do not check if player will stay in maze.
Problem, when moving down or right, p is moved to a new position that will be visited by loops and thus moved again, until moved after the end of maze.
Advice keep track of position of p with 2 variables px and py, it avoid the need of 2 loops when moving p.
   
Hey rick york. How can i make this its a little complicated
   
Comments
Patrice T 7-May-20 23:00pm
   
To discuss with author if a solution, use "Have a Question or comment?" button at bottom of solution.
Rick York 8-May-20 2:44am
   
No, it is most definitely not complicated. The source x and y are always going to be i and j. The destination depends on which way it is moving. The example I showed was to j+1 in columns. This means the arguments to the function will be (i, j, i, j+1) to move right. Now you need to associate those four arguments with the logic in the example and you will have your function. The key here is those four parameters can be destx, desty, srcx, and srcy so then look how j+1 was used in the example snipped and substitute it with argument srcy that you will pass to it. Then do that with the other three arguments. The first i, paired with (j) will be destx and that lone j will be dest y. The second i which is paired with j+1 becomes srcx.

I am NOT going to write this for you because YOU need to do it.
Member 14825085 8-May-20 4:22am
   
//my update still not works


void move(){

int i;
int j;
char choice;
scanf("%c",&choice);


for (i = 0; i < rows-1; ++i) {
for (j = 0; j < cols-1; ++j) {





if(maze[i][j] == 'p' && choice == 'a'){

move2(i,j,i,j-1);

}

if(maze[i][j] == 'p' && choice == 'w'){

move2(i,j,i-1,j);

}

if(maze[i][j] == 'p' && choice == 'd'){


move2(i,j,i,j+1);

}

if(maze[i][j] == 'p' && choice == 's'){


move2(i,j,i+1,j);



}



}
}
}

void move2(int srcx,int srcy,int destx,int desty){


char tmp = maze[srcx][srcy];
maze[srcx][srcy] = maze[destx][desty];
maze[destx][desty] = tmp;





}

/////i update my code like this as you say but s and d button still doesnt work
Rick York 8-May-20 11:03am
   
That's not all I wrote. I also wrote about checking for errors in the move function. Make sure neither goes below zero or above the size of the maze. That was the other purpose for putting things in one place.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)




CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900