First I want to give you a few general comments on the style and aesthetics of your code, then I'll provide you with a detailed code sample applying a slightly different approach to the problem, that you can use as comparison or baseline.
Avoiding unnecessary dependencies
You don't need <Windows.h> for this program. It is the only thing preventing it from compiling on Mac or Linux. You can remove that include file.
<time.h> is also not necessary. You are not currently using any timing routines, that can also be stripped. I also don't see any uses of functions defined in <algorithm>.
Indenting
Is very important to denote nesting. Make sure to always add one level of indentation for code that lives inside a function. The contents of main should be indented to convey that nesting.
Use of "magic numbers"
There are quite a few literal constants in your code that are not clear about their meaning from a first glance. In particular, the several comparisons in the main while loop seem very obscure. I can spot that some of them are actually testing for collision with a trap or the end of the map. You should avoid using raw literal numbers and strings in your program when they can be replaced by named values. We can use const and enum in C++ to define immutable values that have a descriptive name that is much easier to remember than the number 15 or whatever.
But ultimately, for the checks that you're performing, I think you are going about it the wrong way. Instead of testing specific indexes in the map grid, you should just verify what is the symbol in the map for the current position of the player. By doing so, you can get rid of all those magic numbers. You will see in the following sample that I do so and also use constants and enums to name some of the other literal values.
Use a 2D matrix to represent the grid
You've opted for a std::string to represent the map grid, which is fine, but complicates things a little, since the
characters are stored linearly in the string. A more straightforward way of defining the map would be with an actual square matrix of N rows by M columns (or width x height). Like so:
char world_map[MapWidth][MapHeight];
Where the X direction is the width (from the top-left corner to the top-right corner), and Y is the height (from the top-left corner to the bottom-left):
(x=0, y=0)
+----------+
|0000000000|
|0000000000|
|0000000000|
|0000000000|
|0000000000|
|0000000000|
|0000000000|
+----------+ (x=width-1, y=height-1)
Also note that we can use a constant for the width (columns) and height (rows) of the map. This allows us to easily change the size of the map without having to hunt down all over the code for occurrences of literal numbers.
Avoid repetition
Your code has some degree of repetition in the bodies of the if tests. The code for moving the player and checking if she has hit a trap, wall or exit can be generalized into a function that you can call once for each direction of
movement.
In the following example you will see that the movement and map update logic has been moved into a move_player() function, which returns a value indicating if we are free to KeepLooking for the exit or if we have HitTrap and died or FoundExit and are home free.
if (key == Right)
{
gameState = move_player(world_map, playerPosX, playerPosY, playerPosX + 1, playerPosY);
}
else if (key == Left)
{
gameState = move_player(world_map, playerPosX, playerPosY, playerPosX - 1, playerPosY);
}
else if (key == Up)
{
gameState = move_player(world_map, playerPosX, playerPosY, playerPosX, playerPosY - 1);
}
else if (key == Down)
{
gameState = move_player(world_map, playerPosX, playerPosY, playerPosX, playerPosY + 1);
}
else if (key == Quit)
{
gameState = QuitGame;
}
Note that the current position of the player in the map it taken by reference (&) so we can modify it inside the function and have the result visible outside in main (see the code below). This is necessary to ensure the player doesn't move outside the map bounds. I don't think you performed that correctly
in your attempt.
Use functions to separate unrelated logic and reuse code
Sometimes it is helpful to split unrelated logic into separate functions. Notice that I have introduced clear_map() and print_map(). This cleans up main and also makes those functions reusable if you decide the expand the code and find yourself needing to clear the map several times, for instance.
Printing the map is already done in two places, so that by itself already warrants it a function.
Putting it all together:
Following is my suggested solution to the exercise, applying the above discussed points. I hope you find it readable, but feel free to ask if you are in doubt about something. I've also added more message printing and a check for invalid keys, so it is slightly longer than your original code.
#include <iostream>
#include <string>
const int MapWidth = 10;
const int MapHeight = 7;
// These make our lives easier if we ever decide to change the symbols:
const char PlayerSymbol = 'G';
const char TrapSymbol = 'T';
const char ExitSymbol = 'X';
const char EmptySymbol = '0';
enum Keys
{
Right = 'd',
Left = 'a',
Up = 'w',
Down = 's',
Quit = 'q'
};
enum GameState
{
HitTrap,
FoundExit,
KeepLooking,
QuitGame
};
void print_map(const char map[MapWidth][MapHeight])
{
for (int y = 0; y < MapHeight; ++y)
{
for (int x = 0; x < MapWidth; ++x)
{
std::cout << map[x][y];
}
std::cout << "\n";
}
}
void clear_map(char map[MapWidth][MapHeight])
{
for (int y = 0; y < MapHeight; ++y)
{
for (int x = 0; x < MapWidth; ++x)
{
map[x][y] = EmptySymbol;
}
}
}
void place_props(char map[MapWidth][MapHeight])
{
// The initial player position (0,0):
map[0][0] = PlayerSymbol;
// Traps:
map[4][1] = TrapSymbol;
map[2][3] = TrapSymbol;
map[1][4] = TrapSymbol;
map[1][6] = TrapSymbol;
// Exit is at the right end of the map.
map[MapWidth - 1][MapHeight - 1] = ExitSymbol;
}
GameState move_player(char map[MapWidth][MapHeight],
int & currPlayerPosX, int & currPlayerPosY,
int desiredPlayerX, int desiredPlayerY)
{
// Clamp the new desired position to stay within the map bounds:
if (desiredPlayerX < 0) { desiredPlayerX = 0; }
if (desiredPlayerY < 0) { desiredPlayerY = 0; }
if (desiredPlayerX >= MapWidth) { desiredPlayerX = MapWidth - 1; }
if (desiredPlayerY >= MapHeight) { desiredPlayerY = MapHeight - 1; }
GameState newState;
// Check victory condition or if we hit a trap:
if (map[desiredPlayerX][desiredPlayerY] == TrapSymbol)
{
std::cout << "You hit a trap an got decapitated! Better luck next time.\n";
newState = HitTrap;
}
else if (map[desiredPlayerX][desiredPlayerY] == ExitSymbol)
{
std::cout << "You find the exit and live another day to tell the tales of your adventures!\n";
newState = FoundExit;
}
else
{
newState = KeepLooking;
}
// Clear current position:
map[currPlayerPosX][currPlayerPosY] = EmptySymbol;
// Place the player symbol:
map[desiredPlayerX][desiredPlayerY] = PlayerSymbol;
// Return the new clamped position, avoiding the player from leaving the map.
// Since we are using a reference to int, this change will be visible outside.
currPlayerPosX = desiredPlayerX;
currPlayerPosY = desiredPlayerY;
return newState;
}
int main()
{
int playerPosX = 0;
int playerPosY = 0;
GameState gameState = KeepLooking;
char world_map[MapWidth][MapHeight];
clear_map(world_map);
place_props(world_map);
print_map(world_map);
while (gameState == KeepLooking)
{
char key;
std::cin >> key;
if (key == Right)
{
gameState = move_player(world_map, playerPosX, playerPosY, playerPosX + 1, playerPosY);
}
else if (key == Left)
{
gameState = move_player(world_map, playerPosX, playerPosY, playerPosX - 1, playerPosY);
}
else if (key == Up)
{
gameState = move_player(world_map, playerPosX, playerPosY, playerPosX, playerPosY - 1);
}
else if (key == Down)
{
gameState = move_player(world_map, playerPosX, playerPosY, playerPosX, playerPosY + 1);
}
else if (key == Quit)
{
gameState = QuitGame;
}
else
{
std::cout << "Invalid direction, use W,S,A,D to move up, down, left, right. Q to quit.\n";
}
print_map(world_map);
}
}
