How do I move in a square and what am I doing wrong

Question

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:

C++

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 7-May-20 7:26am

Member 14825085

Updated 8-May-20 0:40am

KarstenK

v2

Add a Solution

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.

4 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

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

Rick York · Answer 1 · 2020-05-07T09:42:00

The first thing you need to do is make yourself a swap function. This sequence of code :

C++

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.

KarstenK · Answer 2 · 2020-05-07T23:07:00

Try to simplify your code in this manner

C++

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.

Patrice T · Answer 3 · 2020-05-08T00:40:00

Advice: Learn to indent properly your code, it show its structure and it helps reading and understanding. It also helps spotting structures mistakes.

C++

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:

C++

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.

Member 14825085 · Answer 4 · 2020-05-07T11:42:00

Solution 2

Hey rick york. How can i make this its a little complicated

Posted 7-May-20 11:42am

Member 14825085

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.

How do I move in a square and what am I doing wrong

4 solutions

Solution 1

Solution 3

Solution 4

Solution 2

Add your solution here

Preview 0