In C++, my console game character will not stop when moving right inside the game field array

Question

Hi there, first time posting on SO, so please go gentle on me if I manage to fudge it up. I'm also pretty new to coding, so any push in the right direction to make me find the solution myself would be appreciated.

What should happen: There is a game field which is made from an array of arrays of strings. The player (P) should be able to move one field per loop.

What happens: For UP, DOWN and LEFT, it works. But when pressing RIGHT, the player travels as far as he can instead of only one space.

What it might be: I assume it has something to do with changing the position of P inside the array, but I can't find the reason why it would be wrong.

#include "iostream"
#include "conio.h"
#include "typeinfo"
#include <cstdlib>
#include <string>
#include <array>
#include <Windows.h>

void main()
{

    // Utilities 
    bool wants_to_exit = false;
    char move;

    // Block types
    std::string a = "   ";
    std::string W = "###";
    std::string P = " O ";

    // Map
    std::string fields[10][10] = {
                                 { W , W , W , W , W , W , W , W , W , W },
                                 { W , a , a , a , a , W , a , a , a , W },
                                 { W , a , P , a , a , W , a , a , a , W },
                                 { W , a , a , a , a , W , a , a , a , W },
                                 { W , a , a , a , a , W , a , a , a , W },
                                 { W , a , a , a , a , a , a , a , a , W },     
                                 { W , a , a , a , a , W , a , a , a , W },
                                 { W , a , a , a , a , W , a , a , a , W },
                                 { W , a , a , a , a , W , a , a , a , W },
                                 { W , W , W , W , W , W , W , W , W , W } 
                                 };

    // Main Loop
    while (!wants_to_exit)
    {
        // Display Map on Console
        for (int x = 0; x < 10; x++)
        {
            for (int y = 0; y < 10; y++) 
            {
                std::cout << fields[x][y];
            }
            printf("\n");
        }

        std::cout << std::endl;
        std::cout << _kbhit() << std::endl;
        std::cout << "\n\nUse the arrow Keys to move" << std::endl;


        move = _getch();

        // Movement
        if (move == 72) // UP
        {
            for (int x = 1; x < 10; x++)
            {
                for (int y = 1; y < 10; y++) 
                {
                    if (fields[y][x] == P) 
                    {
                        if (fields[y-1][x] == a)
                        { 
                            fields[y][x] = a;
                            fields[y-1][x] = P;
                            move = 0;
                            break;

                        }
                    }
                }
            }
        }

        else if (move == 80) // DOWN
        {
            for (int x = 1; x < 10; x++) 
            {
                for (int y = 1; y < 10; y++) 
                {
                    if (fields[y][x] == P) 
                    {
                        if (fields[y+1][x] == a)
                        { 
                            fields[y][x] = a;
                            fields[y+1][x] = P;
                            move = 0;
                            break;
                        }
                    }
                }
            }
        }

        else if (move == 75) // LEFT
        {
            for (int x = 1; x < 10; x++)
            {
                for (int y = 1; y < 10; y++)
                {
                    if (fields[y][x] == P)
                    {
                        if (fields[y][x-1] == a)
                        { 
                            fields[y][x] = a;
                            fields[y][x-1] = P;
                            move = 0;
                            break;

                        }
                    }
                }
            }
        }

        else if (move == 77) // RIGHT
        {
            for (int x = 1; x < 10; x++)
            {
                for (int y = 1; y < 10; y++)
                {
                    if (fields[y][x] == P) 
                    {
                        if (fields[y][x+1] == a)
                        { 
                            fields[y][x] = a;
                            fields[y][x+1] = P;
                            move = 0;
                            break;

                        }
                    }
                }
            }
        }

        else if (move == 113) // QUIT (press q)
        {
            wants_to_exit = true; 
        }

        Sleep(1);
        system("cls");

    }

}

Unrelated to problem: 1. [Declare `move` in the scope that you are using it (at `move = _getch();`), not at the function top.](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rnr-top) 2. [`void main()` is not correct C++](https://stackoverflow.com/questions/204476/what-should-main-return-in-c-and-c). — walnut, Sep 05 '19 at 15:07
`Sleep(1);` - *Why*? I'm fairly allergic to any kind of sleep since they are *usually* bugs. There are rare cases where a sleep is the right thing to do, sure, but they are *usually* misguided attempts at synchronization or attempts at making things happen at specific intervals etc. And in *most* cases, a sleep is *not* what you *actually* want to use. — Jesper Juhl, Sep 05 '19 at 17:04
`void main()` is not valid C++. `main` *always* returns `int`. `#include "conio.h"` - Wow, old-school. `_kbhit()` - *not* portable. `if (move == 72) // UP` - Why these magic numbers? Why not use a portable input library that will work across platforms and with different keyboard/locale settings etc? This feels like the way we wrote programs for DOS in the 80's ;) — Jesper Juhl, Sep 05 '19 at 17:09

score 1 · Accepted Answer · answered Sep 05 '19 at 15:12

Each of your break statements only break out of the inner for loop. The outer for loop continues, and in the case of rightwards movement keeps stumbling upon the player that was just nudged right by the previous iteration.

I suggest factoring out player movement into a local function from which you can return as soon as you have found and updated the player (untested code):

#include "iostream"
#include "conio.h"
#include "typeinfo"
#include <cstdlib>
#include <string>
#include <array>
#include <Windows.h>

void main()
{

    // Utilities 
    bool wants_to_exit = false;

    // Block types
    std::string a = "   ";
    std::string W = "###";
    std::string P = " O ";

    // Map
    std::string fields[10][10] = {
                                 { W , W , W , W , W , W , W , W , W , W },
                                 { W , a , a , a , a , W , a , a , a , W },
                                 { W , a , P , a , a , W , a , a , a , W },
                                 { W , a , a , a , a , W , a , a , a , W },
                                 { W , a , a , a , a , W , a , a , a , W },
                                 { W , a , a , a , a , a , a , a , a , W },     
                                 { W , a , a , a , a , W , a , a , a , W },
                                 { W , a , a , a , a , W , a , a , a , W },
                                 { W , a , a , a , a , W , a , a , a , W },
                                 { W , W , W , W , W , W , W , W , W , W } 
                                 };

    // New local function doing the moving logic
    auto const movePlayer = [&](int deltaX, int deltaY) {
        for (int x = 1; x < 10; x++) 
        {
            for (int y = 1; y < 10; y++) 
            {
                if (fields[y][x] == P) 
                {
                    if (fields[y + deltaY][x + deltaX] == a)
                    { 
                        fields[y][x] = a;
                        fields[y + deltaY][x + deltaX] = P;
                    }
                    return;
                }
            }
        }
    };

    // Main Loop
    while (!wants_to_exit)
    {
        // Display Map on Console
        for (int x = 0; x < 10; x++)
        {
            for (int y = 0; y < 10; y++) 
            {
                std::cout << fields[x][y];
            }
            printf("\n");
        }

        std::cout << std::endl;
        std::cout << _kbhit() << std::endl;
        std::cout << "\n\nUse the arrow Keys to move" << std::endl;


        // Definition moved to the point of actual use
        char const move = _getch();

        // Movement
        if (move == 72) // UP
        {
            movePlayer(-1, 0);
        }
        else if (move == 80) // DOWN
        {
            movePlayer(1, 0);
        }
        else if (move == 75) // LEFT
        {
            movePlayer(0, -1);
        }
        else if (move == 77) // RIGHT
        {
            movePlayer(0, 1);
        }
        else if (move == 113) // QUIT (press q)
        {
            wants_to_exit = true; 
        }

        Sleep(1);
        system("cls");
    }
}

Thanks for the help! With your input I managed to get it up and running. — cyberme0w, Sep 06 '19 at 06:38

score 0 · Answer 2 · answered Sep 05 '19 at 15:08

break will break only the inner loop, but it will not break out of both loops. Here:

       for (int x = 1; x < 10; x++)
        {
            for (int y = 1; y < 10; y++)
            {
                if (fields[y][x] == P) 
                {
                    if (fields[y][x+1] == a)
                    { 
                        fields[y][x] = a;
                        fields[y][x+1] = P;
                        move = 0;
                        break;

                    }
                }
        }

The break will break out of the y loop and the x loop continues with the next iteration. Actually you have the same problem in the other cases. It only appears here because for the x value where you find P you will write P to the position with x+1, so you hit it again in the next iteration. In the other cases the P will be placed either in a position you already visited or at [x,y+1] (and you do break out of the y loop).

Instead of fixing those double loops I would rather suggest you to store the position of the player

int player_position_x;
int player_position_y;

Then you do not need to search it on every move.

In C++, my console game character will not stop when moving right inside the game field array

2 Answers2