C++/SDL2 "Space Invaders"

Question

My biggest issue in this project was the structure of the code and using the necessary data structures. I organized it to the best of my ability but I am sure there much more room improvement and I would love some advice on this. The way I have my enemies firing feels a bit hacky and I'm not sure as to how to have that in a better way.

Also, I am not entirely sure why I have to have #include <vector> in a header or .cpp file at times. This is dependent in certain classes and I am unsure why.

main.cpp

#include "Display.h"
#include "Game.h"
#include "Media.h"
#include "Player.h"
#include <SDL.h>
#include <iostream>
int main(int argc, char* args[])
{
 Display display;
 Game game;
 Media media;
 Player player;
 game.start(display, media, player);
 printf("\n\n\nYou Win!");
 SDL_Delay(3000);
 return 0;
}

Display.h

#pragma once
#include <SDL.h>
#include <vector>
struct Laser;
struct Enemy;
class Player;
class Media;
class Display
{
public:
 Display();
 ~Display();
 bool init();
 void render(Media& media, Player& player, std::vector<Enemy>& enemies, std::vector<Laser>& playerLasers, std::vector<Laser>& enemyLasers);
 SDL_Surface* getWindowSurface() { return m_windowSurface; }
 SDL_Renderer* getRenderer() { return m_renderer; }
protected:
private:
 SDL_Window* m_window = nullptr;
 SDL_Surface* m_windowSurface = nullptr;
 SDL_Renderer* m_renderer = nullptr;
 //Screen resolution
 const int SCREEN_WIDTH = 800;
 const int SCREEN_HEIGHT = 800;
};

Display.cpp

#include "Display.h"
#include "Media.h"
#include "Player.h"
#include "Enemy.h"
#include "Laser.h"
#include <iostream>
#include <vector>
Display::Display()
{
 //init();
}
Display::~Display()
{
}
bool Display::init()
{
 bool success = true;
 //Initialize SDL Video
 if (SDL_Init(SDL_INIT_VIDEO) < 0)
 {
 success = false;
 printf("Video failed to initialize.", SDL_GetError());
 }
 else
 {
 //Create Window
 m_window = SDL_CreateWindow("Space Invaders", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, SCREEN_WIDTH, SCREEN_HEIGHT, SDL_WINDOW_SHOWN);
 if (m_window == nullptr)
 {
 success = false;
 printf("Failed to create window.", SDL_GetError());
 }
 else
 {
 //Get the window surface
 m_windowSurface = SDL_GetWindowSurface(m_window);
 if (m_windowSurface == nullptr)
 {
 success = false;
 printf("Failed to get the window surface.", SDL_GetError());
 }
 else
 {
 //Create the renderer
 m_renderer = SDL_CreateRenderer(m_window, -1, SDL_RENDERER_ACCELERATED);
 if (m_renderer == nullptr)
 {
 success = false;
 printf("Failed to create the renderer.", SDL_GetError());
 }
 }
 }
 }
 return success;
}
void Display::render(Media & media, Player & player, std::vector<Enemy>& enemies, std::vector<Laser>& playerLasers, std::vector<Laser>& enemyLasers)
{
 SDL_RenderClear(m_renderer);
 SDL_SetRenderDrawColor(m_renderer, 255, 255, 255, 0);
 //Render player
 SDL_RenderCopy(m_renderer, media.getPlayerTexture(), nullptr, &player.getPosition());
 //Render enemies
 for (auto &i : enemies)
 {
 SDL_RenderCopy(m_renderer, media.getEnemyTexture(), nullptr, &i.m_pos);
 }
 //Render Player Lasers
 for (auto &i : playerLasers)
 {
 SDL_RenderCopy(m_renderer, media.getLaserTexture(), nullptr, &i.m_pos);
 }
 for (auto &i : enemyLasers)
 {
 SDL_RenderCopy(m_renderer, media.getLaserTexture(), nullptr, &i.m_pos);
 }
 SDL_RenderPresent(m_renderer);
}

Media.h

#pragma once
#include <SDL.h>
#include <string>
class Display;
class Media
{
public:
 Media();
 ~Media();
 bool load(Display& display);
 SDL_Texture* getPlayerTexture() { return m_player; }
 SDL_Texture* getEnemyTexture() { return m_enemy; }
 SDL_Texture* getLaserTexture() { return m_laser; }
protected:
private:
 SDL_Surface* loadSurface(Display& display, std::string path);
 SDL_Texture* loadTexture(Display& display, std::string path);
 SDL_Texture* m_laser = nullptr;
 SDL_Texture* m_player = nullptr;
 SDL_Texture* m_enemy = nullptr;
};

Media.cpp

#include "Media.h"
#include "Display.h"
Media::Media()
{
}
Media::~Media()
{
}
bool Media::load(Display & display)
{
 bool success = true;
 m_laser = loadTexture(display, "laser.bmp");
 if (m_laser == nullptr)
 {
 printf("Failed to load laser.", SDL_GetError());
 success = false;
 }
 m_player = loadTexture(display, "player.bmp");
 if (m_player == nullptr)
 {
 printf("Failed to load player", SDL_GetError());
 success = false;
 }
 m_enemy = loadTexture(display, "enemy.bmp");
 if (m_enemy == nullptr)
 {
 printf("Failed to load enemy", SDL_GetError());
 success = false;
 }
 return success;
}
SDL_Surface * Media::loadSurface(Display & display, std::string path)
{
 SDL_Surface* optimizedSurface = nullptr;
 SDL_Surface* surface = SDL_LoadBMP(path.c_str());
 if(surface == nullptr)
 {
 printf("Unable to load", path.c_str(), SDL_GetError());
 }
 else
 {
 optimizedSurface = SDL_ConvertSurface(surface, display.getWindowSurface()->format, 0);
 }
 SDL_FreeSurface(surface);
 return optimizedSurface;
}
SDL_Texture * Media::loadTexture(Display& display, std::string path)
{
 SDL_Texture* texture = SDL_CreateTextureFromSurface(display.getRenderer(), loadSurface(display, path.c_str()));
 if (texture == nullptr)
 {
 printf("Failed to get texture.", SDL_GetError());
 }
 return texture;
}

Game.h

#pragma once
#include <SDL.h>
#include <vector>
struct Laser;
struct Enemy;
class Player;
class Media;
class Display;
class Game
{
public:
 Game();
 ~Game();
 void start(Display& display, Media& media, Player& player);
private:
 bool m_quit = false;
 void addEnemy(std::vector<Enemy>& enemies);
 void moveEnemies(std::vector<Enemy>& enemies);
 void moveLasers(std::vector<Laser>& playerLasers, std::vector<Laser>& enemyLasers);
 void checkCollisions(Player& player, std::vector<Enemy>& enemies, std::vector<Laser>& playerLasers, std::vector<Laser>& enemyLasers);
 bool findPlayerLaserCollision(Laser& laser, Enemy& enemy);
 bool findEnemyLaserCollisions(Laser& laser, Player& player);
 bool checkGameOver(std::vector<Enemy>& enemies);
 void enemyShoot(std::vector<Enemy>& enemies, std::vector<Laser>& enemyLasers);
};

Game.cpp

#include "Game.h"
#include "Display.h"
#include "Media.h"
#include "Player.h"
#include "Enemy.h"
#include "MoveDirection.h"
#include "Laser.h"
#include <iostream>
Game::Game()
{
}
Game::~Game()
{
}
void Game::start(Display& display, Media& media, Player& player)
{
 //Initialize SDL
 if (!display.init())
 {
 printf("Failed to initailize.", SDL_GetError());
 }
 else
 {
 //Load all media
 if (!media.load(display))
 {
 printf("Failed to load media", SDL_GetError());
 }
 else
 {
 SDL_Event e; 
 std::vector<Enemy> enemies; //Holds all enemies
 std::vector<Laser> playerLasers; //Holds all of player lasers
 std::vector<Laser> enemyLasers;
 addEnemy(enemies); //Adds enemies to the game
 //Begin game loop
 while (!m_quit)
 {
 //Event management
 while (SDL_PollEvent(&e) != 0)
 {
 if(e.type == SDL_KEYDOWN)
 { 
 player.movement(e);
 }
 if (e.type == SDL_MOUSEBUTTONDOWN)
 {
 player.shoot(playerLasers);
 }
 }
 //Shoot
 enemyShoot(enemies, enemyLasers);
 //Movement
 player.movementBounds();
 moveLasers(playerLasers, enemyLasers);
 moveEnemies(enemies);
 //Collision detection
 checkCollisions(player, enemies, playerLasers, enemyLasers);
 //Check game over
 m_quit = checkGameOver(enemies);
 //Render
 display.render(media, player, enemies, playerLasers, enemyLasers);
 //Simulate 60 fps - Read on tutorial, not entirely sure if this is ok. 
 SDL_Delay(16);
 }
 }
 }
}
void Game::addEnemy(std::vector<Enemy>& enemies)
{
 int spawnXPos = 100;
 int newSpawnXPos = 100; 
 int spawnYPos = 150;
 int enemyWidth = 50;
 int enemyHeight = 50;
 int numbOfEnemies = 6;
 int movementSpeed = 3;
 for (int i = 0; i < numbOfEnemies; i++)
 {
 enemies.push_back(Enemy({ spawnXPos, spawnYPos, enemyWidth, enemyHeight }, movementSpeed, MoveDirection::LEFT));
 spawnXPos += newSpawnXPos; //Spawns enemies along a row
 }
}
void Game::moveEnemies(std::vector<Enemy>& enemies)
{
 int newYPos = 20; //Move enemies down everytime they hit a wall
 for (auto &enemy : enemies)
 {
 //Move Left
 if (enemy.m_dir == MoveDirection::LEFT)
 {
 enemy.m_pos.x -= enemy.m_movementSpeed;
 //If an enemy has exceeded allowed bounds, change direction
 if (enemy.m_pos.x < enemy.MIN_X_POS)
 {
 for (auto &i : enemies)
 {
 i.m_dir = MoveDirection::RIGHT;
 i.m_pos.y += newYPos;
 }
 }
 }
 //Move Right
 if (enemy.m_dir == MoveDirection::RIGHT)
 {
 enemy.m_pos.x += enemy.m_movementSpeed;
 //If enemy exceeds allowed bounds, change direction
 if (enemy.m_pos.x > enemy.MAX_X_POS)
 {
 for(auto &i : enemies)
 { 
 i.m_dir = MoveDirection::LEFT;
 i.m_pos.y += newYPos;
 }
 }
 }
 }
}
void Game::moveLasers(std::vector<Laser>& playerLasers, std::vector<Laser>& enemyLasers)
{
 //Movement of player lasers
 for (auto &playerLaser : playerLasers)
 {
 if (playerLaser.m_dir == MoveDirection::UP)
 {
 playerLaser.m_pos.y -= playerLaser.m_movementSpeed;
 }
 }
 //Movement of enemy lasers
 for (auto &enemyLaser : enemyLasers)
 {
 if (enemyLaser.m_dir == MoveDirection::DOWN)
 {
 enemyLaser.m_pos.y += enemyLaser.m_movementSpeed;
 }
 }
}
void Game::checkCollisions(Player& player, std::vector<Enemy>& enemies, std::vector<Laser>& playerLasers, std::vector<Laser>& enemyLasers)
{
 //checkCollisions(player, enemies, playerLasers, enemyLasers);
 //Check for playerLaser/Enemy collision
 for (int playerLaser = 0; playerLaser < playerLasers.size(); playerLaser++)
 {
 for (int enemy = 0; enemy < enemies.size(); enemy++)
 {
 //If collision has been detected, delete both playerLaser and Enemy
 if (findPlayerLaserCollision(playerLasers[playerLaser], enemies[enemy]))
 {
 printf("Collision");
 playerLasers.erase(playerLasers.begin() + playerLaser);
 enemies.erase(enemies.begin() + enemy);
 return;
 }
 }
 }
 for (int enemyLaser = 0; enemyLaser < enemyLasers.size(); enemyLaser++)
 {
 if (findEnemyLaserCollisions(enemyLasers[enemyLaser], player))
 {
 printf("Player collision");
 enemyLasers.erase(enemyLasers.begin() + enemyLaser);
 return;
 }
 }
 //No collisions detected
}
bool Game::findPlayerLaserCollision(Laser& playerLaser, Enemy& enemy)
{
 //Player laser collisions
 int playerLaserLeft = playerLaser.m_pos.x;
 int playerLaserRight = playerLaser.m_pos.x + playerLaser.m_pos.w;
 int playerLaserTop = playerLaser.m_pos.y;
 int playerLaserBot = playerLaser.m_pos.y + playerLaser.m_pos.h;
 //Enemy collisions
 int enemyLeft = enemy.m_pos.x;
 int enemyRight = enemy.m_pos.x + enemy.m_pos.w;
 int enemyTop = enemy.m_pos.y;
 int enemyBot = enemy.m_pos.y + enemy.m_pos.h;
 //Check for collisions
 if (playerLaserRight < enemyLeft) {
 return false;
 }
 if (playerLaserLeft > enemyRight) {
 return false;
 }
 if (playerLaserTop > enemyBot) {
 return false;
 }
 if (playerLaserBot < enemyTop) {
 return false;
 }
 //If collision
 return true;
}
bool Game::findEnemyLaserCollisions(Laser & laser, Player & player)
{
 //Player Collisions
 int playerLeft = player.getPositionX();
 int playerRight = player.getPositionX() + player.getWidth();
 int playerTop = player.getPositionY();
 int playerBot = player.getPositionY() + player.getHeight();
 //EnemyLaser Collision
 int laserLeft = laser.m_pos.x;
 int laserRight = laser.m_pos.x + laser.m_pos.w;
 int laserTop = laser.m_pos.y;
 int laserBot = laser.m_pos.y + laser.m_pos.h;
 //Detect collisions
 if (playerLeft > laserRight) {
 return false;
 }
 if (playerRight < laserLeft) {
 return false;
 }
 if (playerTop > laserBot) {
 return false;
 }
 if (playerBot < laserTop) {
 return false;
 }
 //Collision
 return true;
}
bool Game::checkGameOver(std::vector<Enemy>& enemies)
{
 if (enemies.size() == 0)
 {
 printf("Game Over!");
 return true;
 }
 return false;
}
void Game::enemyShoot(std::vector<Enemy>& enemies, std::vector<Laser>& enemyLasers)
{
 //If any enemies are alive.
 if (enemies.size() > 0)
 {
 Uint32 startTime = SDL_GetTicks();
 int randEnemy = rand() % enemies.size();
 //Starting position of laser
 int startXPos = enemies[randEnemy].m_pos.x;
 int startYPos = enemies[randEnemy].m_pos.y;
 //Size of laser
 int sizeWidth = 25;
 int sizeHeight = 50;
 //Movement speed of laser
 int movementSpeed = 8;
 //Direction of laser
 MoveDirection dir = MoveDirection::DOWN;
 if (startTime % 125 == 0)
 {
 enemyLasers.push_back(Laser({ startXPos, startYPos, sizeWidth, sizeHeight }, movementSpeed, dir));
 }
 }
}

Player.h

#pragma once
#include <SDL.h>
#include <vector>
struct Enemy;
struct Laser;
class Player
{
public:
 Player();
 ~Player();
 void movement(SDL_Event& e);
 void movementBounds();
 void init(); //Initialize
 SDL_Rect getPosition() { return m_pos; }
 int getPositionY() { return m_pos.y; }
 int getPositionX() { return m_pos.x; }
 int getWidth() { return SIZE_WIDTH; }
 int getHeight() { return SIZE_HEIGHT; }
 void shoot(std::vector<Laser>& playerLasers);
private:
 //position of player
 SDL_Rect m_pos;
 //Size of player
 const int SIZE_WIDTH = 50;
 const int SIZE_HEIGHT = 50;
 //Starting position of player
 const int START_XPOS = 350;
 const int START_YPOS = 700;
 //Movement speed of player
 const int MOVEMENT_SPEED = 10;
 //MovementBoundaries
 const int xMinDistance = 0;
 const int xMaxDistance = 775;
};

Player.cpp

#include "Player.h"
#include "Laser.h"
Player::Player()
{
 init();
}
Player::~Player()
{
}
void Player::movement(SDL_Event& e)
{
 switch (e.key.keysym.sym)
 {
 //Move right
 case SDLK_d: m_pos.x += MOVEMENT_SPEED;
 break;
 //Move left
 case SDLK_a: m_pos.x -= MOVEMENT_SPEED;
 break;
 }
}
void Player::movementBounds()
{
 if (m_pos.x <= xMinDistance) {
 m_pos.x = xMinDistance;
 }
 if (m_pos.x >= xMaxDistance) {
 m_pos.x = xMaxDistance;
 }
}
void Player::init()
{
 //Size of player
 m_pos.h = SIZE_HEIGHT;
 m_pos.w = SIZE_WIDTH;
 //starting position
 m_pos.x = START_XPOS;
 m_pos.y = START_YPOS;
}
void Player::shoot(std::vector<Laser>& playerLasers)
{
 int startXPos = m_pos.x + 10;
 int startYPos = m_pos.y - 35;
 int movementSpeed = 8;
 int sizeWidth = 25;
 int sizeHeight = 50;
 MoveDirection dir = MoveDirection::UP;
 playerLasers.push_back(Laser({ startXPos, startYPos, sizeWidth, sizeHeight }, movementSpeed, dir));
}

Enemy.h

#pragma once
#include <SDL.h>
#include "MoveDirection.h"
struct Enemy
{
 Enemy(SDL_Rect pos, int movementSpeed, MoveDirection dir)
 {
 m_pos = pos;
 m_movementSpeed = movementSpeed;
 m_dir = dir;
 }
 SDL_Rect m_pos; //Position of enemy
 int m_movementSpeed = 0; //Movement speed of enemy
 MoveDirection m_dir; //Movement direction of enemy
 int MIN_X_POS = 0;
 int MAX_X_POS = 750;
};

MoveDirection.h

#pragma once
//Movement directions of players, enemies and lasers
enum class MoveDirection
{
 LEFT,
 RIGHT,
 UP,
 DOWN
};

Laser.h

#pragma once
#include <SDL.h>
#include "MoveDirection.h"
struct Laser
{
 Laser(SDL_Rect pos, int movementSpeed, MoveDirection dir)
 {
 m_pos = pos;
 m_movementSpeed = movementSpeed;
 m_dir = dir;
 }
 SDL_Rect m_pos; //Position of enemy
 int m_movementSpeed = 0; //Movement speed of enemy
 MoveDirection m_dir; //Movement direction of enemy
};

• R Sahu: That's too much code for one post, IMO.
• Quill: @RSahu That's how we do things on Code Review, splitting up the code would only cause issues

Answer 1

This is a fairly long question, so this is by no means a thorough review. I'll be focusing on the more obvious things that we can get out of the way right now :)

main

Your main function is clean and I specially like to see that you're passing dependencies through function parameters.

The printf+delay seems a bit misplaced though, it could be done inside the game class. Eventually you will probably want to draw the message to screen instead of printing to the console, so if you move that into the game, main can be left unchanged.

return 0 at the end of main is optional. This is a special rule that only applies to the main function. If the program reaches the end of main without encountering a return statement, it is the same as returning zero.

Display class

You could clean it up a bit, get rid of unnecessary stuff. The default empty constructor doesn't have to be provided when there's no work to be done in it. Let the compiler supply a default for you.

The protected section is also empty, so omit that tag. Same for the Media class.

SDL shutdown: Where is it happening? I would expect the destructor of Display to be destroying the SDL objects it created in init().

Also, the work done by init should be done by the constructor, to avoid having an invalid or partly constructed object. I suppose you created init() to be able to return an error from the function, but you could have the constructor throw an exception instead. Having explicit init/shutdown methods in a class is usually not a great design, because you can have a partially constructed object. Instead, take advantage of constructors and destructors and use the RAII style whenever possible.

---

These two constants should be marked static, alternatively, use static constexpr if your compiler has descent C++11 support:

const int SCREEN_WIDTH = 800;
const int SCREEN_HEIGHT = 800;

They are still taking unnecessary space in the class (the size of an int each). If you mark them as statics, then the compiler will optimize them to compile-time constants and they will take no extra space in the objects.

static constexpr int SCREEN_WIDTH = 800;
static constexpr int SCREEN_HEIGHT = 800;

There are other such instances in the Player class.

---

Make thorough use of const for parameters. E.g.:

void Display::render(Media & media, Player & player, std::vector<Enemy>& enemies, std::vector<Laser>& playerLasers, std::vector<Laser>& enemyLasers)

When you take read-only parameters by pointer of reference, be sure to mark them as const. This will prevent you from accidentally modifying stuff that shouldn't be and will also convey that info to the caller, which will know the object is not changed by the function.

Display::render() won't change any of the vectors nor the player, so the expected signature would be:

void Display::render(Media & media, const Player& player, 
 const std::vector<Enemy>& enemies, 
 const std::vector<Laser>& playerLasers, 
 const std::vector<Laser>& enemyLasers)

Media class

Has the same issue of not using RAII and not releasing the resources it allocates. You must free the textures in the destructor to avoid leaking them!

Miscellaneous

• Use stderr (std::cerr) to report errors. Using std::cout/printf outputs to stdout, which is not the expected place for them. Being consistent with the convention of using stderr for errors allows users to filter the normal output of a program from the errors.

• Use of rand() is discouraged. It has a very bad rep because implementations are usually very trivial, making the pseudo-random numbers it generates very predictable. Even for something as simple as a game it might not be suitable. It could cause easily predictable behavior in the game where it should not have. Take a look at the new <random> API from C++11 as a future exercise.

• This is not simulating 60 frame-per-second, it merely slows down the game loop:

   //Simulate 60 fps - Read on tutorial, not entirely sure if this is ok. 
   SDL_Delay(16);
  

  On a fast machine, you might not even notice it, but on a slow one, it will just freeze for 16 milliseconds, potentially bring the frame-rate dangerously below the golden 30fps. I don't see a lot of reasons for doing that, but if you'd like to limit the frame-rate to at most 60 frame-per-second, you have to measure the time taken to render the frame and then sleep for the precise amount to prevent rendering too fast. Something along the lines of:

  // 60fps => 16ms in a frame
  constexpr int MIN_MILLISECONDS_PER_FRAME = 16;
  while (true)
  {
   const int startTime = getTimeMilliseconds();
   // do game logic, rendering, etc...
   const int endTime = getTimeMilliseconds();
   const int millisecondsThisFrame = endTime - startTime;
   if (millisecondsThisFrame < MIN_MILLISECONDS_PER_FRAME)
   {
   // This frame was done in less than 16ms, so we stall for a
   // while to ensure we meet the 16ms minimum. E.g. if we took
   // just 10ms to render a frame: 16 - 10 = 6, sleep for 6ms.
   SDL_Delay(MIN_MILLISECONDS_PER_FRAME - millisecondsThisFrame);
   }
  }
  

• Ryan Swann: Thank you for this! I think I need to read up on exceptions and learn how to implement them into my projects.

Answer 2

I see a number of things that may help you improve your code.

Fix the bug

In a number of places in the code, the success of some SDL operation is checked for error. That's good and appropriate. However, if there's an error detected, the code includes lines like this:

printf("Video failed to initialize.", SDL_GetError());

The problem is that there are too many arguments passed to printf and the SDL_GetError() string is essentially thrown away. Instead, the code should look like this:

printf("Video failed to initialize: %s\n", SDL_GetError());

Eliminate unused arguments

In main, argc and argv are unused and can be omitted.

Be aware of signed vs. unsigned

In a few places within Game there are lines like this:

for (int playerLaser = 0; playerLaser < playerLasers.size(); playerLaser++)

This might be fine for your particular compiler, but some compilers return an unsigned value. There are a few different ways to address this. One way would be to declare playerLaser as size_t. Another method is detailed in the following suggestion.

Use C++11 "range for"

Instead of using an index variable, C++11 provides for a convenient method of iterating through collections of objects using the "range for". Instead of using this:

for (int playerLaser = 0; playerLaser < playerLasers.size(); playerLaser++)

the loop could be written like this:

for (auto &laser : playerLasers)

Then within the loop, one can simply use laser instead of playerLasers[playerLaser].

Use const where practical

A number of member functions and parameters could and should be made const. For example, we have this code:

bool Game::checkGameOver(std::vector<Enemy>& enemies)
{
 if (enemies.size() == 0)
 {
 printf("Game Over!");
 return true;
 }
 return false;
}

However, this alters neither the underlying Game object nor the passed enemies vector, so it should instead be declared like this:

bool Game::checkGameOver(const std::vector<Enemy>& enemies) const 

Let the compiler do its work

The program include functions like these:

Game::Game()
{
}
Game::~Game()
{
}

They are essentially the same as would have been automatically generated, so there's no reason to explicitly include them.

Beware of temporary objects

The code currently contains this line in Display::render():

SDL_RenderCopy(m_renderer, media.getPlayerTexture(), nullptr, &player.getPosition());

However, that last argument is a problem. The player.getPosition() returns an SDL_Rect and what you are attempting to do is to take its address to pass to SDL_RenderCopy, but this is a very dubious way to do so. The reason is that the compiler could very well have returned the value in a register in which case there may be no way to take its address. To avoid this problem, a straightforward way to redo that would be to write it like this:

auto player_pos = player.getPosition();
SDL_RenderCopy(m_renderer, media.getPlayerTexture(), nullptr, &player_pos);

However, I might be inclined instead to refactor it like this:

SDL_RenderCopy(m_renderer, media.getPlayerTexture(), nullptr, player.getPositionPtr());

And then change Player to include this replacement function:

const SDL_Rect *getPositionPtr() const { return &m_pos; }

Don't delegate construction

There's not really much point to the current implementation of the Player constructor delegating to an init member. Instead, the constructor could be written like this:

Player::Player()
 : m_pos{START_XPOS, START_YPOS, SIZE_WIDTH, SIZE_HEIGHT}
{ }

Consolidate important constants

It's good that you seem to have isolated file names such as "player.bmp" in a single file (Media.cpp). However, if you wanted to change it from that file to some other, you'd have to hunt through the file for those file names. Better would be to create named constants for those file names and have them all together at the top of the cpp file.

---

There's more, but I lack the time to add it right now.

• Ryan Swann: Thank you for all of that it was very helpful! Sorry about my code being very long.
• Edward: No problems with long code here! It just takes a while to review.