C++

Forum

 
Console game missues

Pages: 1234
seeplus (6591)
L212. No. You have it reversed!

//Initialize and set oldY to the mPosY.

Also, as has been said previously, you really need to design first and then code from the design. Things you are trying to figure out as you progress your code should have been thought out before a line of code is written. I know you said just messing around and practicing my programming skills. But programming skills include design, debugging and testing - not just coding. To be a good programmer you have to be able to analyse a problem, design classes/structs etc, devise algorithms etc and then code from the design. Typically, design can take up to about half (IMO typically around a third - depending upon the program) of the expected total time to develop a non-trivial program from scratch.
Last edited on
Ch1156 (1999)
Thank you, I made the correction. I also found a UML program called UMLet that I like and this is my attempt at a UML:

https://ibb.co/nP6RFHG

Its not done yet but its a start. Im not sure if im doing it correctly though.
Last edited on
seeplus (6591)
also found a UML program called UMLet that I like and this is my attempt at a UML


I used 2 different ones - pen & paper and wipe boards. Both with a selection of coloured pens. Both gave good service and always worked fine (just remember to have replacement pens...).
Ch1156 (1999)
Why does my collision code work in my switch statement but not the commented out one? Its exactly the same code just broken up into a function. Im just trying to test collision out, and i got it working but its not working with the Collision function in use. When i collide with the object, i can no longer move in any direction witht he collision function, but when i use the other switch statement with the collision built into it, i can and the collision works.

I did the collision in main first so i could make sure it worked then once it did i wanted to break it up into a function, but i dont want the collision part of the movement code, I want to be able to test against any object I want.

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
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
 
#include <iostream>
#include <string>
#include <vector>

class Tiles
{
    public:
        Tiles(char tile, int xPos, int yPos) : mTile(tile), mXpos(xPos), mYpos(yPos)
        {}

        char GetTile() const { return mTile; }
        int GetXPos() const { return mXpos; }
        int GetYPos() const { return mYpos; }

    private:
        char mTile{ 'T' };
        int mXpos{};
        int mYpos{};
};

//Not working?
bool Collision(int& playerX, int& playerY, std::vector<Tiles>& tiles)
{
    for (auto& i : tiles)
    {
        if (playerY - 1 == i.GetYPos() && playerX == i.GetXPos())
        {
            return true;
        }
        if (playerY + 1 == i.GetYPos() && playerX == i.GetXPos())
        {
            return true;
        }
        if (playerX - 1 == i.GetXPos() && playerY == i.GetYPos())
        {
            return true;
        }
        if (playerX + 1 == i.GetXPos() && playerY == i.GetYPos())
        {
            return true;
        }
    }
    return false;
}

int main()
{
    int mRows{ 15 };
    int mColumns{ 15 };

    char player{ 'O' };
    char mObject{ '#' };
    char mTile{ '.' };

    int playerX{ 5 };
    int playerY{ 5 };

    int objectX{ 5 };
    int objectY{ 8 };

    int choice{ 0 };

    Tiles Wall('@', 7, 9);

    std::vector<std::vector<char>> mMap;
    std::vector<Tiles> tiles;

    tiles.push_back(Wall);

    mMap.assign(mRows, std::vector<char>(mColumns, mTile));

    while (true)
    {
        for (int row{ }; row < mRows; ++row)
        {
            for (int col{ }; col < mColumns; ++col)
            {
                mMap[row][col] = mTile;
                mMap[playerY][playerX] = player;

                //Player visual collision field
                mMap[playerY -1][playerX] = '*'; //Top
                mMap[playerY + 1][playerX] = '*'; //Down
                mMap[playerY][playerX + 1] = '*'; //Right
                mMap[playerY][playerX - 1] = '*'; //Left
                mMap[playerY - 1][playerX - 1] = '*'; //Top Left
                mMap[playerY + 1 ][playerX - 1] = '*'; //Bottom Left
                mMap[playerY - 1][playerX + 1] = '*'; //Top Right
                mMap[playerY + 1][playerX + 1] = '*'; //Bottom Right
                //mMap[objectY][objectX] = mObject;
                mMap[Wall.GetYPos()][Wall.GetXPos()] = Wall.GetTile();

                std::cout << mMap[row][col];
            }
            std::cout << '\n';
        }

        std::cout << "What direction do you want to go in?\n\n";

        std::cout << "1) UP\n";
        std::cout << "2) DOWN\n";
        std::cout << "3) LEFT\n";
        std::cout << "4) RIGHT\n";

        std::cin >> choice;

        switch (choice)
        {
            case 1:
                if (playerY - 1 == Wall.GetYPos() && playerX == Wall.GetXPos())
                {
                    std::cout << "Cannot move Up.\n";
                }
                else
                {
                    --playerY;
                }
                break;
            case 2:
                if (playerY + 1 == Wall.GetYPos() && playerX == Wall.GetXPos())
                {
                    std::cout << "Cannot move Down.\n";
                }
                else
                {
                    ++playerY;
                }
                break;
            case 3:
                if (playerX - 1 == Wall.GetXPos() && playerY == Wall.GetYPos())
                {
                    std::cout << "Cannot move Left.\n";
                }
                else
                {
                    --playerX;
                }
                break;
            case 4:
                if (playerX + 1 == Wall.GetXPos() && playerY == Wall.GetYPos())
                {
                    std::cout << "Cannot move Right.\n";
                }
                else
                {
                    ++playerX;
                }
                break;
            default:
                std::cout << "Error\n";
        }

        /*Doesnt work
        * The issue is that the player touches the object, then will not
        * move in any direction after that.
        switch (choice)
        {
        case 1:
            if (Collision(playerX, playerY, tiles))
            {
                std::cout << "Cannot move Up.\n";
            }
            else
            {
                --playerY;
            }
            break;
        case 2:
            if (Collision(playerX, playerY, tiles))
            {
                std::cout << "Cannot move Down.\n";
            }
            else
            {
                ++playerY;
            }
            break;
        case 3:
            if (Collision(playerX, playerY, tiles))
            {
                std::cout << "Cannot move Left.\n";
            }
            else
            {
                --playerX;
            }
            break;
        case 4:
            if (Collision(playerX, playerY, tiles))
            {
                std::cout << "Cannot move Right.\n";
            }
            else
            {
                ++playerX;
            }
            break;
        default:
            std::cout << "Error\n";
        }*/
    }
}
Last edited on
mbozzi (3932)
The function Collision above tests whether the player is adjacent to a wall.

Being next to a wall doesn't mean the player is about to bump into it. That depends on their direction of travel. They could be moving parallel to it, or even away from it.

Instead, try moving the player first, then moving them back if that motion puts them inside the wall.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
 
bool Collision(int x, int y, std::vector<Tiles>& tiles)
{
  return std::ranges::any_of(tiles,
    [=](auto t) { return t.GetXPos() == x && t.GetYPos() == y; });
}

// ...

enum { up = 1, down, left, right };
if (choice == up)    --playerY;
if (choice == down)  ++playerY;
if (choice == left)  --playerX;
if (choice == right) ++playerX;

if (Collision(playerX, playerY, tiles))
{
  std::cout << "You can't move that direction!\n";
  if (choice == up)    ++playerY;
  if (choice == down)  --playerY;
  if (choice == left)  ++playerX;
  if (choice == right) --playerX;
}
Last edited on
salem c (3700)
A good way to keep your classes to the point is to use some 5"x3" library index cards (or similarly sized sticky notes).
- 1 line class name
- 1 line, one sentence summary.
Then divide the rest into two columns.
- left column for public methods
- right column for private data

If your class can't be succinctly summarised on a single card, then perhaps you need to look at simplification, say by moving some of it to another class.

If your class is too large, then
- the code will be large also
- it will take a long time to write
- it will take even longer to test and debug
- it will be harder to substitute for something much better later on.

seeplus (6591)
Why does my collision code work in my switch statement but not the commented out one?


Well what debugging have you done using the debugger? Have you traced through the code to see when it's execution isn't what is expected? Debugging is a programming skill which needs to be acquired. Now is a good time.

Also, for the purpose of the code in the function Collision() the definition could be:

 
 
bool Collision(int playerX, int playerY, const std::vector<Tiles>& tiles)


If you use the type std::vector<Tiles> in several places, it's easier if you define this as a new type and then use that type in the code:

1
2
3
 
using TileV = std::vector<Tiles>;
...
bool Collision(int playerX, int playerY, const TileV& tiles)


You might want to consider having a struct for a player position then you can just pass that struct around rather then x an y positions. You could then overload operators as required for that struct etc etc.

1
2
3
4
5
6
 
struct Pos {
   int x {};
   int y {};
};
...
bool Collision(const Pos& playerPos, const TileV& tiles)

Last edited on
Ch1156 (1999)
So i got collision working in a smaller file but im having trouble converting it to my larger multi file project. I converted it as best as i can but the player wont move so im working on solving that right now. Perhaps ill post it after a while if i cant solve it.
Last edited on
Ch1156 (1999)
I cant seem to figure out whats wrong with this. The collision function isnt working, the objects on the map are not being collided with the player. I will post a link to my github repo since i have like 15 files and posting them all here would span multiple posts. I stepped through it with the debugger but I didnt notice anything. I feel like either the collision function isnt working properly, or i need to do something with the map update function, or something related to that, but im unsure.:

https://github.com/ChayHawk/Console-Game/tree/development

But the problem functions:

Character.cpp

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
 
void Character::Movement(Character::Direction choice, Collision& collision, Map& map, const std::vector<Tiles>& tiles)
{
	//Working Master do not delete
	//switch (choice)
	//{
	//case Direction::UP:
	//	if (mPosY)
	//	{
	//		const auto oldY{ mPosY-- }; //This

	//		map.Update(mPosX, oldY, mPosX, mPosY, mCharacter);
	//		return;
	//	}
	//	break;

	//case Direction::DOWN:
	//	if (mPosY < map.GetMaxRows() - 1)
	//	{
	//		const auto oldY{ mPosY++ };

	//		map.Update(mPosX, oldY, mPosX, mPosY, mCharacter);
	//		return;
	//	}
	//	break;

	//case Direction::LEFT:
	//	if (mPosX)
	//	{
	//		const auto oldX{ mPosX-- };

	//		map.Update(oldX, mPosY, mPosX, mPosY, mCharacter);
	//		return;
	//	}
	//	break;

	//case Direction::RIGHT:
	//	if (mPosX < map.GetMaxColumns() - 1)
	//	{
	//		const auto oldX{ mPosX++ };

	//		map.Update(oldX, mPosY, mPosX, mPosY, mCharacter);
	//		return;
	//	}
	//	break;

	//default:
	//	std::cout << "Invalid Input\n";
	//	return;
	//}
	std::cout << "Inside Character::Movement Function\n";
	//Testing
	switch (choice)
	{
	case Direction::UP:
		if (collision.CheckCollision(mPosX, mPosY, tiles))
		{
			std::cout << "Cannot move Up.\n";
		}
		else
		{
			std::cout << "Inside Switch UP Case Function\n";
			const auto oldY{ mPosY-- };

			map.Update(mPosX, oldY, mPosX, mPosY, mCharacter);
			return;
		}
		break;
	case Direction::DOWN:
		if (collision.CheckCollision(mPosX, mPosY, tiles))
		{
			std::cout << "Cannot move Down.\n";
		}
		else
		{
			const auto oldY{ mPosY++ };

			map.Update(mPosX, oldY, mPosX, mPosY, mCharacter);
			return;
		}
		break;
	case Direction::LEFT:
		if (collision.CheckCollision(mPosX, mPosY, tiles))
		{
			std::cout << "Cannot move Left.\n";
		}
		else
		{
			const auto oldX{ mPosX-- };

			map.Update(oldX, mPosY, mPosX, mPosY, mCharacter);
			return;
		}
		break;
	case Direction::RIGHT:
		if (collision.CheckCollision(mPosX , mPosY, tiles))
		{
			std::cout << "Cannot move Right.\n";
		}
		else
		{
			const auto oldX{ mPosX++ };

			map.Update(oldX, mPosY, mPosX, mPosY, mCharacter);
			return;
		}
		break;
	default:
		std::cout << "Error\n";
	}
}


Collision.cpp

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
 
bool Collision::CheckCollision(int playerX, int playerY, const std::vector<Tiles>& tiles)
{
    std::cout << "DEBUG - Inside Collision::CheckCollision Function\n";
    for (auto& i : tiles)
    {
        std::cout << "DEBUG - Inside Collision::CheckCollision 1st For Loop\n";
        if (playerX == i.GetXPosition() || playerY == i.GetYPosition())
        {
            std::cout << "DEBUG - Inside Collision::CheckCollision 2nd For Loop\n";
            std::cout << "Collision Detected\n";
            return true;
        }
    }
    return false;
}


I made a smaller project to test the collision to make it less error prone and remove all unnecessary code so i can just focus on solving the collision issue, and this program works as i expect it to, however when i translate this code to my new project in the github link, it doe not work and im unsure why.

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
 
#include <iostream>
#include <string>
#include <vector>
#include <ranges>
#include <algorithm>
#include <chrono>
#include <random>

class Tiles
{
    public:
        Tiles(char tile, int xPos, int yPos) : mTile(tile), mXpos(xPos), mYpos(yPos)
        {}

        char GetTile() const { return mTile; }
        int GetXPos() const { return mXpos; }
        int GetYPos() const { return mYpos; }

    private:
        char mTile{ 'T' };
        int mXpos{ };
        int mYpos{ };
}; 


void GetDirection(int playerX, int playerY, int oldX, int oldY)
{
    if (playerX == oldX && playerY == oldY)
    {
        std::cout << "Player is not moving.\n";
    }
    else if (playerX == oldX && playerY < oldY)
    {
        std::cout << "Player is moving up.\n";
    }
    else if (playerX == oldX && playerY > oldY)
    {
        std::cout << "Player is moving down.\n";
    }
    else if (playerY == oldY && playerX < oldX)
    {
        std::cout << "Player is moving left.\n";
    }
    else if (playerY == oldY && playerX > oldX)
    {
        std::cout << "Player is moving right.\n";
    }
}

bool CheckCollision(int playerX, int playerY, std::vector<Tiles> tiles)
{
    for (auto& i : tiles)
    {
        if (playerX == i.GetXPos() && playerY == i.GetYPos())
        {
            std::cout << "Collision Detected\n";
            return true;
        }
    }
    return false;
}

int main()
{
    std::mt19937 mt{ static_cast<unsigned int>(std::chrono::steady_clock::now().time_since_epoch().count()) };

    int mRows{ 15 };
    int mColumns{ 25 };

    char player{ 'O' };
    char mTile{ '.' };

    int playerX{ 5 };
    int playerY{ 5 };

    int oldX{ playerX };
    int oldY{ playerY };

    int objectX{ 5 };
    int objectY{ 8 };

    int choice{ 0 };

    Tiles Rock('@', 7, 9);
    Tiles Tree('T', 3, 7);

    std::vector<std::vector<char>> mMap;
    std::vector<Tiles> tiles;

    tiles.push_back(Rock);
    tiles.push_back(Tree);

    mMap.assign(mRows, std::vector<char>(mColumns, mTile));

    while (true)
    {
        std::cout << "Player X: " << playerX << " Player Y: " << playerY << '\n';
        std::cout << "Player Old X: " << oldX << " Player Old Y: " << oldY << '\n';

        GetDirection(playerX, playerY, oldX, oldY);

        oldX = playerX;
        oldY = playerY;

        for (int row{ }; row < mRows; ++row)
        {
            for (int col{ }; col < mColumns; ++col)
            {
                mMap[row][col] = mTile;
                mMap[playerY][playerX] = player;
                mMap[Rock.GetYPos()][Rock.GetXPos()] = Rock.GetTile();
                mMap[Tree.GetYPos()][Tree.GetXPos()] = Tree.GetTile();

                std::cout << mMap[row][col];
            }
            std::cout << '\n';
        }

        std::cout << "What direction do you want to go in?\n\n";

        std::cout << "1) UP\n";
        std::cout << "2) DOWN\n";
        std::cout << "3) LEFT\n";
        std::cout << "4) RIGHT\n";

        std::cin >> choice;

        switch (choice)
        {
        case 1:
            if (CheckCollision(playerX, playerY - 1, tiles))
            {
                std::cout << "Cannot move Up.\n";
            }
            else
            {
                --playerY;
            }
            break;
        case 2:
            if (CheckCollision(playerX, playerY + 1, tiles))
            {
                std::cout << "Cannot move Down.\n";
            }
            else
            {
                ++playerY;
            }
            break;
        case 3:
            if (CheckCollision(playerX - 1, playerY, tiles))
            {
                std::cout << "Cannot move Left.\n";
            }
            else
            {
                --playerX;
            }
            break;
        case 4:
            if (CheckCollision(playerX + 1, playerY, tiles))
            {
                std::cout << "Cannot move Right.\n";
            }
            else
            {
                ++playerX;
            }
            break;
        default:
            std::cout << "Error\n";
        }
    }
}
seeplus (6591)
I stepped through it with the debugger but I didnt notice anything.


You've not therefore looking at the right things or not interpreting correctly. As the code isn't behaving as you expected, then tracing through the code will show where the execution isn't as expected. If you are expecting a collision and it doesn't happen then either you have a code error or a logic error. Tracing through with the debugger and looking at variable values will explain why - either the values are not what you expect or the conditions aren't right....

As I said, debugging is a skill that needs to be acquired.

The debug cout statements within CheckCollision() aren't that helpful. I'd be tempted to first display the contents of the vector tiles and playerX and PLayerY. A quick look at the output will then tell you if a collision is true or not. If it's not true from the shown data but is expected to be, then the problem is elsewhere - either playerX or PLayerY is wrong or the vector data isn't what is expected. If collision is true from looking at he data and the function reports it's not, then you know the issue is within the CheckCollision() function and you can then dive deeper into it. You narrow things down until you find the statement(s) that are causing the issue.
Last edited on
seeplus (6591)
 
 
if (playerX == i.GetXPos() && playerY == i.GetYPos())


Do you mean || instead of && ???

Also, the for loop can be replaced with any_of (include algorithm for C++20):

1
2
3
 
bool CheckCollision(int playerX, int playerY, const std::vector<Tiles>& tiles) {
	return std::ranges::any_of(tiles, [playerX, playerY](const auto& t) {return t.GetXPos() == playerX || t.GetYPos() == playerY; });
}

Ch1156 (1999)
My guess as to why the same code works in the complete program i posted and it doesnt in my program is something to do with the way the map is implemented. I'm going to investigate that now. I tried "hotwiring" the code and putting the switch statement in main and just directly using everything i could and it still didnt work, I tried a bunch of other stuff too. however where the code differs in the one project and this one is in the way the map is implemented. In the project where it works, the map is complete and is in one spot whereas in my project where im having problems its separated into an initialize, update and draw functions so my guess is the collision objects need to be updated on the map. The code you provided, i added a cout statement to it just to see and it always executes. I'll test more later. I think this is a logic error, and the code is working fine its just a situation where im just leaving something out, and i believe not updating the collision is the issue.
Last edited on
Ch1156 (1999)
I solved it. I wasnt getting the current x and y position of the objects drawn on the map so the game was always thinking their collisions for all of them was 0,0. So i was right, there was nothign wrong with the code i just left out information the game needed. I made a separate function to test 1 object, but i have a function that draws random objects all over the screen randomly so i have to find a way to get all their positions and check against them. How would i go about doing that?

Maybe i could write the locations of each randomly placed tile to a 2D vector and then either return it to the function, making it a 2D vector, and use the DrawRandomObjects function in my Cehck Collision function, but i dont like that solution. I think writing them to a 2D vector is a better idea, but idk.

Could also use a std::map or std::pair. Whats a good solution?

I updated the code on my github as well: https://github.com/ChayHawk/Console-Game/tree/development/Console%20Game

My check Collision code:

1
2
3
4
5
6
7
8
9
10
11
12
 
bool Collision::CheckCollision(int playerX, int playerY, const std::vector<Tiles>& tiles)
{
    for (auto& i : tiles)
    {
        if (playerX == i.GetXPosition() && playerY == i.GetYPosition())
        {
            std::cout << "Collision Detected\n";
            return true;
        }
    }
    return false;
}


My DrawRandomObjects Code

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
 
void Map::DrawRandomObjects(std::mt19937& mt, const std::vector<Tiles>& tiles, int amountToPlace, Character& character)
{

	std::uniform_int_distribution<> rows{ 0, mMapRows - 1 };
	std::uniform_int_distribution<> columns{ 0, mMapColumns - 1 };

	for (int i{ }; i < amountToPlace; ++i)
	{
		//if object is equal to the players position, do not draw it, we dont want an object
		//to be drawn over the player.
		if (rows(mt) != character.GetXPosition() && rows(mt) != character.GetYPosition() || columns(mt) != character.GetXPosition() && columns(mt) != character.GetYPosition())
		{
			for (auto& i : tiles)
			{
				mGameMap[rows(mt)][columns(mt)] = i.GetTile();
			}
		}
	}
}
Last edited on
Ch1156 (1999)
I got it working with the help of GPT-4 I had it scan my repository and suggest changes. But im the one who figured out what was happening. i got the collision working, the players X and Y were flipped in the CheckCollision function which caused the player to check for collision spots in areas where there were no objects, thus i would hit random impassable areas.
Last edited on
seeplus (6591)
When I use a rng (random number generator) I make this global so that everywhere it's needed it's available without having to keep passing it as a ref param in functions. It's one of the very few times I'd suggest using a non-const global variable.

1
2
3
4
 
#include <random>

std::mt19937 rng(std::random_device {}());
...


Also, why is CheckCollison() a member function of class Collision - when the function only uses it's arguments?
Last edited on
deleted account xyzzy (5768)
It is smart to use <random> in C++ code, the C pseudo random number generation methods are a bit lacking.

https://web.archive.org/web/20180123103235/http://cpp.indi.frih.net/blog/2014/12/the-bell-has-tolled-for-rand/

Leveraging the power of reuse in C++ why not create a simple toolkit you can then drop into a project when you want to generate random numbers?

https://open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3551.pdf

One of the biggest issue beginners run into when dealing with generating random numbers (especially in C) is seeding/reseeding the generators more than once and/or too frequently so the generator's internal state is unnecessarily reset.

Designing a simple toolkit that issue can be reduced.

https://github.com/GeorgePimpleton/misc_files/tree/main/Random%20Toolkit

A pre-C++20 non-module header and a C++20 module interface can be found there, along with an example of how to use either.
seeplus (6591)
Note that in DrawRandomObjects, i is used for both outer and inner loops! Whilst this is legal, it isn't the best practice!

Also note that L11 rows(mt) and columns(mt) do not usually give the same result as L15 rows(mt) and columns(mt) as the result from these are a random number.

L15 will always set the mGameMap to the last one in tiles.

What are trying to do in this function as the code is probably not doing what is expected?

Do you mean something like:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
void Map::DrawRandomObjects(std::mt19937& mt, const std::vector<Tiles>& tiles, int amountToPlace, const Character& character) {
	std::uniform_int_distribution<> rows { 0, mMapRows - 1 };
	std::uniform_int_distribution<> columns { 0, mMapColumns - 1 };
	std::uniform_int_distribution<> til { 0, tiles.size() - 1};

	for (int amt { }; amt < amountToPlace; ) {
		const auto r { rows(mt) }, c { columns(mt) };

		if (r != character.GetYPosition() || c != character.GetXPosition()) {
			mGameMap[r][c] = tiles[til(mt)].GetTile();
			++amt;
		}
	}
}


Note that has the issue that objects can overwrite other objects so although amountToPlace objects will have been placed, you could end up with actually fewer on the map. The if test should also test for an existing object as well as a character.

Last edited on
Ch1156 (1999)
Hello, I update my code base, development is the one I use the most and dont update master much unless all the features are working.

Here is my updated Draw Random Objects function:


I know this still doesnt test for drawing over other random objects but thats ok, its an easy enough implementation, just extend the if statement and do the same checks as i do for character except with objects, but its ok for right now.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
 
void Map::DrawRandomObjects(std::mt19937& mt, Tiles& tiles, int amountToPlace, Character& character)
{
    std::uniform_int_distribution<> rows{ 0, mMapRows - 1 };
    std::uniform_int_distribution<> columns{ 0, mMapColumns - 1 };

    for (int i{ }; i < amountToPlace; ++i)
    {
        int row = rows(mt);
        int col = columns(mt);

        // If the object is equal to a characters position, do not draw it, we don't want an object
        // to be drawn over characters.
        if ((row != character.GetXPosition() || col != character.GetYPosition()) &&
            (col != character.GetXPosition() || row != character.GetYPosition()))
        {
            // Store tile coordinates here so we can use them later
            mTileCoords.push_back(std::make_pair(row, col));

            mGameMap[row][col] = tiles.GetTile();
        }
    }
}


as for both loops having an i for inner and outer loops, that was an accident on my part, i usually use i and j for inner and outer loops.

When I use a rng (random number generator) I make this global so that everywhere it's needed it's available without having to keep passing it as a ref param in functions. It's one of the very few times I'd suggest using a non-const global variable.


Yeah it just feels wrong to me for doing it since its been hammered into my head that gobal variables are bad but i do know it is generally accepted by programmers to make an RNG gobal, but i think its ok for it to be used like it is for right now. I did however use a little composition and make collision part of the character class since a character has a collision, removing the need to pass character around in the parameters.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
 
class Character
{
	public:
		Character(const std::string& name, char character, int posX, int posY) : 
			mName(name), mCharacter(character), mPosX(posX), mPosY(posY) {}

		int GetXPosition() const { return mPosX; }
		int GetYPosition() const { return mPosY; }
		char GetCharacter() const { return mCharacter; }
		std::string GetName() const { return mName; }

		void Movement(char choice, Map& map);

		void GetDirection(int playerX, int playerY, int oldX, int oldY);

	private:
		std::string mName{ "Character" };
		const char mCharacter{ 'C' };
		int mPosX{ };
		int mPosY{ };
		Collision collision;
};



Also, why is CheckCollison() a member function of class Collision - when the function only uses it's arguments?

I probably could have just mad that a standalone function but i decided to make it part of a class just for futureproofing, I will probably add more to the class later.

@George P

Hmm, thats interesting, I'll take a look at the paper a little later, and the code on your github i can just add to my project and use? or do i need to build it?

Last edited on
Peter87 (11235)
Ch1156 wrote:
1
2
 
if ((row != character.GetXPosition() || col != character.GetYPosition()) &&
    (col != character.GetXPosition() || row != character.GetYPosition()))

Why compare row with an x-coordinate and col with a y-coordinate?

Note that col is an x-coordinate and row is a y-coordinate. You might want to decide on a consistent naming convention.

Either use x and y 
 
 
x != character.GetXPosition() || y != character.GetYPosition()
or row and column 
 
 
col != character.GetRow() || row != character.GetColumn()
not a mix.

Personally I prefer x and y for this sort of thing. But regardless or what you choose, being consistent will probably make it less confusing and easier to see what "mistake" you've made here.

Ch1156 wrote:
i usually use i and j for inner and outer loops

That is a relatively common convention when dealing with indices/integers.

Less common to see with other types of objects, especially when the letter has no relation.

If it's a Tile, why not just call it tile?
 
 
for (auto& tile : tiles)


Ch1156 wrote:
Yeah it just feels wrong to me for doing it since its been hammered into my head that gobal variables are bad but i do know it is generally accepted by programmers to make an RNG gobal, but i think its ok for it to be used like it is for right now.

There are pros and cons with both approaches.

Making it global makes it easy to access and you reduce the chance that you accidentally create copies of the RNG object (which would happen if you passed it by value instead of by reference). Copying might be a performance problem if you use a large generator such as std::mt19937 but the bigger issue is that it could lead to less random results.

Example:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
 
#include <iostream>
#include <random>

void print_random_number(std::mt19937 rng)
{
	std::cout << rng() << "\n";
}

int main()
{
	std::mt19937 rng(123);
	for (int i = 0; i < 5; ++i)
	{
		print_random_number(rng);
	}
}
Output:
2991312382
2991312382
2991312382
2991312382
2991312382

As you can see, it generates the same number each time.

Change the function to pass-by-reference and you'll get different numbers...

Modified line: 
 
 
void print_random_number(std::mt19937& rng)
Output:
2991312382
3062119789
1228959102
1840268610
974319580


However, passing the RNG as argument makes it more apparent that the function will not give the same result each time (unless the RNG state is the same). It could also be an advantage while testing and debugging because you could easily use a fixed seed in order to get a consistent result each time. You might for example have a function that works most of the time but that fails when you use seed 18621. By creating a new RNG using this seed each time before you pass it to the function you can easily reproduce the bug which makes debugging much easier. You might be able to do the same with a global RNG but then you need to be more careful that you don't call some other unrelated function that also use the same global RNG. Being able to reproduce stuff from a single seed is also useful in procedural generation. You might for example want to be able to regenerate a whole random map by just knowing a single seed (i.e. integer value).

Ch1156 wrote:
I probably could have just mad that a standalone function but i decided to make it part of a class just for futureproofing, I will probably add more to the class later.

Future proofing against what?

Ask yourself, what is a collision (what does it represent?), and does it make sense to ask a collision to check for a collision (collision.CheckCollision(...))?

If you make changes in the future you will probably have to update the code anyway so why not keep it simple?
https://en.wikipedia.org/wiki/KISS_principle

I could be wrong but to me it looks like the class has no purpose and therefore has no reason to exist.
Last edited on
seeplus (6591)
Re DrawRandomObjects(). I'd only inc i if the if test is true and not in the for statement. Inc-ing in the for statement means that say you want to place 4 objects and 2 of these come to the same position as a character you'd end up with only 2 objects rather than the 4 wanted.

Re global rng. An exception proves the rule! See
https://en.wikipedia.org/wiki/Exception_that_proves_the_rule

Also note that L17 can be simplified:

 
 
mTileCoords.emplace_back(row, col);


See https://cplusplus.com/reference/vector/vector/emplace_back/
Last edited on
Pages: 1234