0
\$\begingroup\$

Im trying to make a retro game of Breakout using an SDL library. Im still trying to learn developing and OOP. I got to the part where i want to check for collision between paddle and the ball, and later on with the bricks. I have a general idea on how it suppose to be done, but Im not quite sure how to do it in the OOP. Should I make a vector for objects and run the collision between those objects in vectors or is there another way I should consider doing.

Entity.h

#pragma once
#include "SDL.h"
#include <string>
#include "../include/Transform.h"

struct Properties
{
public:
    Properties(std::string textureID, float x, float y, int width, int heigth)
    {
        _x = x;
        _y = y;
        _width = width;
        _heigth = heigth;
        _textureID = textureID;
    }

    float _x, _y;
    int _width, _heigth;
    std::string _textureID;
};

class Entity
{
public:
    Entity() {}

    Entity(Properties* props) : _textureID(props->_textureID), _width(props->_width), _heigth(props->_heigth)
    {
        _transform = new Transform(props->_x, props->_y);
    }

    virtual void Draw() = 0;
    virtual void Update() = 0;
    virtual void Clean() = 0;

protected:
    Transform* _transform = nullptr;

    int _width = 50;
    int _heigth = 50;
    std::string _textureID;
};

Ball.h

#pragma once
#include "Entity.h"
#include "Vector2D.h"
#include "Transform.h"
#include <math.h>

class Ball : public Entity
{
public:
    Ball(Properties* props);

    virtual void Draw();
    virtual void Update();
    virtual void Clean();

private:
    void CheckBounds();
    void Reset();
    void InitRandomSpeed();

    Vector2D _ballSpeed;
};

Ball.cpp

#include "../include/Ball.h"
#include "../include/TextureManager.h"
#include "../include/Engine.h"

Ball::Ball(Properties* props) : Entity(props)
{   
    Reset();
}

void Ball::Draw()
{
    TextureManager::GetInstance()->Draw(_textureID, _transform->X, _transform->Y, _width, _heigth);
}

void Ball::Update()
{
    _transform->TranslateX(_ballSpeed.X);
    _transform->TranslateY(_ballSpeed.Y);

    CheckBounds();
}

void Ball::Clean()
{
    TextureManager::GetInstance()->Clean();
}

void Ball::CheckBounds()
{
    if (_transform->X <= 0)
    {       
        _transform->X = 0;      
        _ballSpeed.X *= -1;
    }

    else if (_transform->X + _width >= SCREEN_WIDTH)
    {
        _transform->X = SCREEN_WIDTH - _width;      
        _ballSpeed.X *= -1;
    }   

    else if (_transform->Y <= 0)
    {       
        _transform->Y = 0;
        _ballSpeed.Y *= -1;
    }

    else if (_transform->Y + _heigth >= SCREEN_HEIGHT)
    {       
        Reset();
    }
}

void Ball::Reset()
{
    _transform->X = SCREEN_WIDTH  / static_cast<float>(2) - _width / static_cast<float>(2);
    _transform->Y = SCREEN_HEIGHT / static_cast<float>(2) - _heigth / static_cast<float>(2);

    InitRandomSpeed();
}

void Ball::InitRandomSpeed()
{
    srand(time(NULL));

    switch (rand() * 10 % 3)
    {
    case 0:
        _ballSpeed.X = 0.05f;
        _ballSpeed.Y = 0.15f;
        break;
    case 1:
        _ballSpeed.X = -0.1f;
        _ballSpeed.Y = -0.2f;
        break;
    case 2:
        _ballSpeed.X = -0.1f;
        _ballSpeed.Y = 0.15f;
        break;
    default:
        break;
    }
}

Paddle.h

#pragma once
#include "Entity.h"
constexpr float PADDLE_SPEED = 0.15f;

class Paddle : public Entity
{
public: 
    Paddle(Properties* props);

    virtual void Draw();
    virtual void Update();
    virtual void Clean();

private:
    bool CheckBounds();

    float _lastSafePosition = 0;
};

Paddle.cpp

#include "../include/Paddle.h"
#include "../include/TextureManager.h"
#include "../include/Input.h"
#include "../include/Engine.h"

Paddle::Paddle(Properties* props) : Entity(props)
{
}

void Paddle::Draw()
{       
    TextureManager::GetInstance()->Draw(_textureID, _transform->X, _transform->Y, _width, _heigth); 
}

void Paddle::Update()
{
    _lastSafePosition = _transform->X;

    if (Input::GetInstance()->GetKeyDown(SDL_SCANCODE_A) || Input::GetInstance()->GetKeyDown(SDL_SCANCODE_LEFT))
    {
        _transform->X -= PADDLE_SPEED;
    }

    if (Input::GetInstance()->GetKeyDown(SDL_SCANCODE_D) || Input::GetInstance()->GetKeyDown(SDL_SCANCODE_RIGHT))
    {
        _transform->X += PADDLE_SPEED;
    }       

    if (CheckBounds())  
        _transform->X = _lastSafePosition;      
}

void Paddle::Clean()
{
    TextureManager::GetInstance()->Clean();
}

bool Paddle::CheckBounds()
{
    if (_transform->X <= 0 || _transform->X + _width >= SCREEN_WIDTH)       
        return true;
    
    return false;
}

Engine.h

#pragma once
#include <iostream>
#include <SDL.h>

constexpr int SCREEN_WIDTH = 1024;
constexpr int SCREEN_HEIGHT = 1024;

class Engine
{
public:
    static Engine* GetInstance() { return _instance = (_instance != nullptr) ? _instance : new Engine(); }
    SDL_Renderer* GetRenderer() { return _renderer; }
    inline bool IsRunning() { return _isRunning; }

    void Init();
    void Clean();
    void Quit();

    void Render();
    void Update();
    void Events();

private:
    Engine() {}
    static Engine* _instance;

    bool _isRunning = false;

    SDL_Window* _window = nullptr;
    SDL_Renderer* _renderer = nullptr;
};

Engine.cpp

#include "../include/Engine.h"
#include "../include/TextureManager.h"
#include "../include/Paddle.h"
#include "../include/Input.h"
#include "../include/Ball.h"
#include "../include/CollisionHandler.h"

Engine* Engine::_instance = nullptr;
Paddle* player = nullptr;
Ball* ball = nullptr;

void Engine::Init()
{
    _isRunning = true;

    if (SDL_Init(SDL_INIT_VIDEO) > 0)
        std::cout << "SDL_Init has failed: " << SDL_GetError() << std::endl;

    if (!(IMG_Init(IMG_INIT_PNG | IMG_INIT_JPG)))
        std::cout << "IMG_Init has failed: " << SDL_GetError() << std::endl;

    _window = SDL_CreateWindow("Breakout", SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED, SCREEN_WIDTH, SCREEN_HEIGHT, 0);

    if (_window == nullptr)
        std::cout << "Window has failed to initialize! Error: " << SDL_GetError() << std::endl;

    _renderer = SDL_CreateRenderer(_window, -1, SDL_RENDERER_ACCELERATED);

    if (_renderer == nullptr)
        std::cout << "Renderer has failed to initialize! Error: " << SDL_GetError() << std::endl;

    TextureManager::GetInstance()->LoadTexture("background", "res\\img\\space_background.png");
    TextureManager::GetInstance()->LoadTexture("player", "res\\img\\paddle.png");
    TextureManager::GetInstance()->LoadTexture("ball", "res\\img\\ball_sprite.png");

    player = new Paddle(new Properties("player", 433, 980, 158, 20));
    ball = new Ball(new Properties("ball", SCREEN_WIDTH / 2, SCREEN_HEIGHT / 2, 16, 16 ));
}

void Engine::Clean()
{
    TextureManager::GetInstance()->Clean();

    delete player;

    SDL_DestroyRenderer(_renderer);
    SDL_DestroyWindow(_window);
}

void Engine::Quit()
{
}

void Engine::Render()
{
    SDL_SetRenderDrawColor(_renderer, 200, 0, 100, SDL_ALPHA_OPAQUE);
    SDL_RenderClear(_renderer);

    TextureManager::GetInstance()->Draw("background", 0, 0, SCREEN_WIDTH, SCREEN_HEIGHT);   
    player->Draw();
    ball->Draw();

    SDL_RenderPresent(_renderer);
}

void Engine::Update()
{
    player->Update();   
    ball->Update(); 
}

void Engine::Events()
{
    Input::GetInstance()->HandleEvents();
}

CollisionHandler.h

#pragma once
#include "SDL.h"
#include <vector>
#include "Engine.h"
#include "Entity.h"

class CollisionHandler
{
public:
    static CollisionHandler* GetInstance() { return _instance = (_instance != nullptr) ? _instance : new CollisionHandler(); }
    bool CheckCollision(SDL_Rect a, SDL_Rect b);
    bool CheckCollision(Entity& a, Entity& b);

private:
    CollisionHandler() {}
    
    static CollisionHandler* _instance;
};

CollisionHandler.cpp

#include "../include/CollisionHandler.h"

bool CollisionHandler::CheckCollision(SDL_Rect a, SDL_Rect b)
{
    bool x_overlaps = (a.x < b.x + b.w) && (a.x + a.w > b.x);
    bool y_overlaps = (a.y < b.y + b.h) && (a.y + a.h > b.y);

    return (x_overlaps && y_overlaps);
}

bool CollisionHandler::CheckCollision(Entity& a, Entity& b)
{
    return false;
}

This is the relevant code that I have so far, if you think there is something else I need to change, I'm open to suggestions.

\$\endgroup\$
3
  • 3
    \$\begingroup\$ Welcome to the Code Review Community. Our goal here is to help you improve your coding ability by making insightful observations about the code. The code must be working as intended. We don't answer How to ... questions because that indicates the code isn't working as intended. Is this code complete and working as intended? Please read How do I ask a good question? and Is my question on topic? \$\endgroup\$
    – pacmaninbw
    Commented Mar 7, 2023 at 13:58
  • 1
    \$\begingroup\$ Welcome! Does "I have a general idea on how it suppose to be done, but Im not quite sure how to do it in the OOP." mean that the code is not fully working as intended yet? \$\endgroup\$ Commented Mar 7, 2023 at 17:10
  • \$\begingroup\$ I am really looking forward to reviewing this when the code is complete! \$\endgroup\$
    – pacmaninbw
    Commented Mar 8, 2023 at 16:23

1 Answer 1

2
\$\begingroup\$

Although the comments are right that the code should be working before submitting here, there is still one comment that can be done: do not use raw owning pointers (CPP Core guidelines, Stackoverflow). You should use std::unique_ptr for all owning pointers, raw pointers for non-owning pointers, and in some cases where you need to share the pointer you should use a std::shared_ptr (note: it is trickier to use smart pointers for the pointers that are returned by SDL, you need to set the deleter manually as you cannot use delete).

There are several reasons for this. One reason is that you need to manually manage the memory when using raw owning pointers, which means that every new should be accompanied with its delete, which is not the case here (I counted 7 new and only 1 delete, this would only be correct if your delete freed the data allocated by each new, which is not the case as it only frees the player). Another reason is that it is way simpler to understand which pointer is owning when using smart pointers. In this code for example, the transform pointer is an owning pointer, so it should be a unique pointer. Even better, I don't even think it needs to be a pointer at all.

\$\endgroup\$
1
  • \$\begingroup\$ Good observation and review, but we generally don't answer questions that are off-topic. In this case the original poster can improve the question once the code is working as intended, but your answer may not be valid after they improve the code. \$\endgroup\$
    – pacmaninbw
    Commented Mar 8, 2023 at 14:24

Not the answer you're looking for? Browse other questions tagged or ask your own question.