patterncppMinor
Collision-detection for an SDL game
Viewed 0 times
sdlcollisiongamefordetection
Problem
I am working on a simple SDL game in C++. I want to make my collision detection implementation better.
Here's the complete project:
```
#include
#include
#include
#include
const int SCREEN_WIDTH = 512;
const int SCREEN_HEIGHT = 512;
bool initialize_game();
bool load_game();
void close_game();
SDL_Texture* load_texture(std::string path);
bool check_collision(SDL_Rect a, SDL_Rect b);
SDL_Texture* playerOneTexture = NULL;
SDL_Window* globalWindow = NULL;
SDL_Renderer* globalRenderer = NULL;
class Timer
{
public:
Timer();
void start_timer();
void stop_timer();
bool is_started();
Uint32 get_ticks();
private:
Uint32 startTicks;
Uint32 pausedTicks;
bool isPaused;
bool isStarted;
};
class Player
{
public:
const int PLAYER_WIDTH = 32;
const int PLAYER_HEIGHT = 32;
const int PLAYER_VELOCITY = 160;
Player();
void move_player(float timeStep, SDL_Rect objectWeHit);
void render_player(float positionX, float positionY, SDL_Texture texture, SDL_Rect clip = NULL, double angle = 0.0, SDL_Point* center = NULL, SDL_RendererFlip flip = SDL_FLIP_NONE);
void handle_player_event(SDL_Event &event);
float positionX, positionY;
float velocityX, velocityY;
private:
SDL_Rect playerCollider;
};
Timer::Timer()
{
startTicks = 0;
pausedTicks = 0;
isPaused = false;
isStarted = false;
}
void Timer::start_timer()
{
isStarted = true;
isPaused = false;
startTicks = SDL_GetTicks();
pausedTicks = 0;
}
void Timer::stop_timer()
{
isStarted = false;
isPaused = false;
startTicks = 0;
pausedTicks = 0;
}
Uint32 Timer::get_ticks()
{
Uint32 time = 0;
if(isStarted){
time = SDL_GetTicks() - startTicks;
}
return time;
}
bool Timer::is_started()
{
return isStarted;
}
Player::Player()
{
positionX = 0.0;
positionY = 0.0;
velocityX = 0.
Here's the complete project:
```
#include
#include
#include
#include
const int SCREEN_WIDTH = 512;
const int SCREEN_HEIGHT = 512;
bool initialize_game();
bool load_game();
void close_game();
SDL_Texture* load_texture(std::string path);
bool check_collision(SDL_Rect a, SDL_Rect b);
SDL_Texture* playerOneTexture = NULL;
SDL_Window* globalWindow = NULL;
SDL_Renderer* globalRenderer = NULL;
class Timer
{
public:
Timer();
void start_timer();
void stop_timer();
bool is_started();
Uint32 get_ticks();
private:
Uint32 startTicks;
Uint32 pausedTicks;
bool isPaused;
bool isStarted;
};
class Player
{
public:
const int PLAYER_WIDTH = 32;
const int PLAYER_HEIGHT = 32;
const int PLAYER_VELOCITY = 160;
Player();
void move_player(float timeStep, SDL_Rect objectWeHit);
void render_player(float positionX, float positionY, SDL_Texture texture, SDL_Rect clip = NULL, double angle = 0.0, SDL_Point* center = NULL, SDL_RendererFlip flip = SDL_FLIP_NONE);
void handle_player_event(SDL_Event &event);
float positionX, positionY;
float velocityX, velocityY;
private:
SDL_Rect playerCollider;
};
Timer::Timer()
{
startTicks = 0;
pausedTicks = 0;
isPaused = false;
isStarted = false;
}
void Timer::start_timer()
{
isStarted = true;
isPaused = false;
startTicks = SDL_GetTicks();
pausedTicks = 0;
}
void Timer::stop_timer()
{
isStarted = false;
isPaused = false;
startTicks = 0;
pausedTicks = 0;
}
Uint32 Timer::get_ticks()
{
Uint32 time = 0;
if(isStarted){
time = SDL_GetTicks() - startTicks;
}
return time;
}
bool Timer::is_started()
{
return isStarted;
}
Player::Player()
{
positionX = 0.0;
positionY = 0.0;
velocityX = 0.
Solution
You have implemented your own rectangle collision check routine when you really didn't have to. SDL provides the function
Now lets talk about coding practices a little bit.
It looks like you are an adept of the single return statement. In the
Prefer to return early and avoid deep nesting. It makes logic easier to follow. The single return technique is hardly applicable to modern C++. The RAII idiom covers dynamic resource management inside functions.
Other minor things:
-
-
Did you code everything up in the same file? If so, it is probably time to move each class to it's own module.
-
Give
SDL_IntersectRect() which tests two SDL_Rects for intersection and also returns the intersection rect of A and B.Now lets talk about coding practices a little bit.
It looks like you are an adept of the single return statement. In the
initialize_game() function, for instance, it has lead to a lot of juggling and flag checking to ensure the single return. In that case, it only made the code more complex. You should have just done it like so:if(SDL_Init(SDL_INIT_VIDEO) < 0) {
return false;
}
... stuff ...
if(globalWindow == NULL) {
return false;
}Prefer to return early and avoid deep nesting. It makes logic easier to follow. The single return technique is hardly applicable to modern C++. The RAII idiom covers dynamic resource management inside functions.
Other minor things:
-
NULL is not C++! NULL is a C library macro. Modern C++ should use nullptr.-
Did you code everything up in the same file? If so, it is probably time to move each class to it's own module.
-
Give
std::cout a try. You might end up liking it ;).Code Snippets
if(SDL_Init(SDL_INIT_VIDEO) < 0) {
return false;
}
... stuff ...
if(globalWindow == NULL) {
return false;
}Context
StackExchange Code Review Q#67887, answer score: 7
Revisions (0)
No revisions yet.