Can't get Collision to work

[Lark_sh]

I simply want my player to jump on collision with the floor. I can’t get SDL_HasIntersection() to work.
Here’s my Game.cpp file:

#include "Game.h"
#include "TextureManager.h"
#include <iostream>
#include <map>
using namespace std;

SDL_Renderer* gRenderer = nullptr; // ~ this is a definition

TextureManager* manager = nullptr;

// HORIZONTAL MOVEMENT
const float speed = 2;
float right_accel = 0.0;
float left_accel = 0.0;
float x_shift = 0;

int c = 2;
int var = 0;
int y_shift = 0;

map<string, SDL_Rect> texture_list{};

map<string, SDL_Rect> wall_list{};

void Game::init(const char* title, int xpos, int ypos, int width, int height, int flags) {

	if (SDL_Init(SDL_INIT_EVERYTHING) == 0) {

		mWindow = SDL_CreateWindow(title, xpos, ypos, width, height, flags);

		if (mWindow != NULL) {

			gRenderer = SDL_CreateRenderer(mWindow, -1, 0);

			isRunning = true;
		}
	}
	manager = new TextureManager();
}

void Game::handleEvents() {

	SDL_Event event; // SDL_Event class can only ever have one object

	while (SDL_PollEvent(&event)) {

		switch (event.type) {

		case SDL_QUIT:
			isRunning = false;
			break;
		}
	}
}

void Game::update() {

	manager->Update();

	if (SDL_GetKeyboardState(0)[SDL_GetScancodeFromKey(SDLK_a)]) {
		right_accel = 0;
		x_shift -= speed + left_accel;
		left_accel += 2;
	}
	else {
		if (left_accel > 0) {
			left_accel -= 0.5;
		}
	}

	if (SDL_GetKeyboardState(0)[SDL_GetScancodeFromKey(SDLK_d)]) {
		left_accel = 0;
		x_shift += speed + right_accel;
		right_accel += 2;
	}
	else {
		if (right_accel > 0) {
			right_accel -= 0.5;
		}
	}
}

void Game::render() {
	SDL_RenderClear(gRenderer);
	// --------------------------

	manager->drawTexture("assets/doux/doux_00.png", 350, 265, "player");
	manager->drawRect(250, 320, 400, 50, "floor");
	manager->drawRect(200, 250, 50, 70, "left");
	manager->drawRect(650, 250, 50, 70, "right");

	if (SDL_HasIntersection(&texture_list["player"], &wall_list["floor"])) {
		texture_list["player"].y -= 40;
	}

	// --------------------------
	SDL_SetRenderDrawColor(gRenderer, 64, 64, 64, 1);
	SDL_RenderPresent(gRenderer);
}

void Game::clean() {
	SDL_DestroyWindow(mWindow);
	SDL_DestroyRenderer(gRenderer);
	SDL_Quit();
}

SDL_HasIntersection() doesn’t detect collision in my case, what did I do wrong?

Thnak you for your attention!

[Peter87]

What is the value of texture_list["player"] and wall_list["floor"]?

How do you make use of texture_list["player"].y after updating it?

You might want to test printing something from within the body of the if statement to see if it really is the collision detection, and not the response, that is the problem (it might be both).

[Lark_sh]

@Peter87

What is the value of texture_list["player"] and wall_list["floor"]?

The values are the memory locations of the player and the floor respectively.

I didn’t understand your 2nd question, my bad.

@Peter87

You might want to test printing something from within the body of the if statement to see if it really is the collision detection

Oh, It is not the collision detection because the loop doesn’t even print hello if there’s a collision.

[Peter87]

@Lark_sh

The values are the memory locations of the player and the floor respectively.

But texture_list and wall_list don’t store memory addresses (pointers). They store actual SDL_Rect objects. If you assigned a SDL_Rect to texture_list["player"] it would copy it. Modifying the copy won’t affect the original.

@Lark_sh

I didn’t understand your 2nd question, my bad.

I didn’t see texture_list["player"].y being used anywhere, directly or indirectly, other than in the collision code. If that is the case then you will obviously not see it affect anything. I don’t know how drawTexture is implemented, maybe it gets used in there?

@Lark_sh

Oh, It is not the collision detection because the loop doesn’t even print hello if there’s a collision.

Well, it’s not “detecting” a collision but I doubt it’s SDL_HasIntersection that is buggy. It’s more likely that texture_list["player"] and wall_list["floor"] don’t contain the values that you expect. You might want to print the x, y, w and h values just before the if statement to make sure.

[Lark_sh]

@Peter87

But texture_list and wall_list don’t store memory addresses (pointers). They store actual SDL_Rect objects

Well they were originally pointers to rects but since it didn’t work I was trying out different things and ended up having them store actual SDL_Rect objects.

Here’s my modified code:

#include "Game.h"
#include "TextureManager.h"
#include <iostream>
#include <map>
using namespace std;

SDL_Renderer* gRenderer = nullptr; // ~ this is a definition

TextureManager* manager = nullptr;

// HORIZONTAL MOVEMENT
const float speed = 2;
float right_accel = 0.0;
float left_accel = 0.0;
float x_shift = 0;

int c = 2;
int var = 0;
int y_shift = 0;

map<string, SDL_Rect*> texture_list{};

map<string, SDL_Rect*> wall_list{};

void Game::init(const char* title, int xpos, int ypos, int width, int height, int flags) {

	if (SDL_Init(SDL_INIT_EVERYTHING) == 0) {

		mWindow = SDL_CreateWindow(title, xpos, ypos, width, height, flags);

		if (mWindow != NULL) {

			gRenderer = SDL_CreateRenderer(mWindow, -1, 0);

			isRunning = true;
		}
	}
	manager = new TextureManager();
}

void Game::handleEvents() {

	SDL_Event event; // SDL_Event class can only ever have one object

	while (SDL_PollEvent(&event)) {

		switch (event.type) {

		case SDL_QUIT:
			isRunning = false;
			break;
		}
	}
}

void Game::update() {

	manager->Update();

	if (SDL_GetKeyboardState(0)[SDL_GetScancodeFromKey(SDLK_a)]) {
		right_accel = 0;
		x_shift -= speed + left_accel;
		left_accel += 2;
	}
	else {
		if (left_accel > 0) {
			left_accel -= 0.5;
		}
	}

	if (SDL_GetKeyboardState(0)[SDL_GetScancodeFromKey(SDLK_d)]) {
		left_accel = 0;
		x_shift += speed + right_accel;
		right_accel += 2;
	}
	else {
		if (right_accel > 0) {
			right_accel -= 0.5;
		}
	}
}

void Game::render() {
	SDL_RenderClear(gRenderer);
	// --------------------------

	manager->drawTexture("assets/doux/doux_00.png", 350, 265, "player");
	manager->drawRect(250, 320, 400, 50, "floor");
	manager->drawRect(200, 250, 50, 70, "left");
	manager->drawRect(650, 250, 50, 70, "right");

	cout << texture_list["player"]->x << " " << texture_list["player"]->y << " " << texture_list["player"]->w << " " << texture_list["player"]->h << endl;
	
	if (SDL_HasIntersection(texture_list["player"], wall_list["floor"])) {
		cout << "hello";
	}

	// --------------------------
	SDL_SetRenderDrawColor(gRenderer, 64, 64, 64, 1);
	SDL_RenderPresent(gRenderer);
}

void Game::clean() {
	SDL_DestroyWindow(mWindow);
	SDL_DestroyRenderer(gRenderer);
	SDL_Quit();
}

and since you asked me to check whether it outputs the x, y, w, h of the player appropriately, I wanted to tell you it doesn’t. I can’t figure out what the numbers on the console are. Here’s the image:

[Peter87]

Without a complete program I can only guess…

Maybe you haven’t actually assigned any SDL_Rect* to texture_list["player"] in which case it will return a null pointer. Trying to read the x, y, w or h would then be UB. Passing it to SDL_HasIntersection is not UB but would always return false.

If texture_list["player"] is not null, are you sure that the SDL_Rect object that it points is still alive? It would for example be a problem if you have assigned the address of a local variable that has since went out of scope.

[Lark_sh]

You’re right. I’ll try modifying the code so the rect is not limited to the function’s scope.

[Lark_sh]

@Peter87

You might want to print the x, y, w and h values just before the if statement to make sure.

So, now it prints all the values you mentioned perfectly since I made sure the rect stored in the map doesn’t go out of scope, but SDL_HasIntersection() still doesn’t for me.
Here’s how I simplified my code, This is my Game.cpp file:

#include "Game.h"

SDL_Renderer* gRenderer = nullptr; // ~ this is a definition

Body* player = nullptr;
Body* tile = nullptr;

map<string, SDL_Rect*> list{};

// The player and floor's rect lived for the entire life of the function that created them only, this is a problem the rect infact needs to be a private variable of the class.

void Game::init(const char* title, int xpos, int ypos, int width, int height, int flags) {

	if (SDL_Init(SDL_INIT_EVERYTHING) == 0) {

		mWindow = SDL_CreateWindow(title, xpos, ypos, width, height, flags);

		if (mWindow != NULL) {

			gRenderer = SDL_CreateRenderer(mWindow, -1, 0);

			isRunning = true;
		}
	}
	player = new Body;
	tile = new Body;
}

void Game::handleEvents() {

	SDL_Event event; // SDL_Event class can only ever have one object

	while (SDL_PollEvent(&event)) {

		switch (event.type) {

		case SDL_QUIT:
			isRunning = false;
			break;
		}
	}
}

void Game::update() {
	player->loadTexture("assets/tard/tard_00.png");
	tile->loadTexture("assets/tile.jpg");
}

void Game::render() {
	int x_increment = 320;

	SDL_RenderClear(gRenderer);
	// --------------------------

	player->drawTexture(320, 300, 4, "player");

	cout << list["player"]->w << " ";
	cout << list["player"]->h << " ";
	cout << list["player"]->x << " ";
	cout << list["player"]->y << "\n";

	for (int count = 0; count < 5; count++) {
		tile->drawTexture(x_increment, 300, 2, "tile");
		x_increment += (24 * 2);
	}

	if (SDL_HasIntersection(list["player"], list["tile"])) {
		cout << "done" << endl;
	}

	// --------------------------
	SDL_SetRenderDrawColor(gRenderer, 64, 64, 64, 1);
	SDL_RenderPresent(gRenderer);
}

void Game::clean() {
	SDL_DestroyWindow(mWindow);
	SDL_DestroyRenderer(gRenderer);
	SDL_Quit();
}

This is my Body.cpp:

#include "Body.h"

void Body::loadTexture(const char* path) {
	texture = IMG_LoadTexture(gRenderer, path);
}

void Body::drawTexture(int x_pos, int y_pos, int magnify, string id) {
	src.x = 0;
	src.y = 0;

	SDL_QueryTexture(texture, NULL, NULL, &src.w, &src.h);

	dest.x = x_pos;
	dest.y = y_pos;
	dest.w = src.w * magnify;
	dest.h = src.h * magnify;

	list[id] = &dest;

	SDL_RenderCopy(gRenderer, texture, &src, &dest);
}

So I cut off extra weight off my code just to understand this has-intersection issue better. What am I doing wrong?

[rtrussell]

@Lark_sh

What am I doing wrong?

Shouldn’t you be calling SDL_HasIntersection() something like this:

if (SDL_HasIntersection(&list["player"]->x, &list["tile"]->x)) {
	cout << "done" << endl;

[Peter87]

It seems like you’re using the variable tile to draw 5 tiles.
list["tile"] is updated each time you call tile->drawTexture(...).
So I guess you’re only doing collision detection against the last (rightmost) tile.

[Lark_sh]

@rtrussell

&list["player"]->x, &list["tile"]->x

I don’t get why you’ve passed those arguments, my bad…

[Lark_sh]

@Peter87

you’re only doing collision detection against the last (rightmost) tile.

Ohh yeah you’re right, I do get the output “done” in the console if I place the player on the rightmost tile. I tried making a new map just for tiles but even then collision is detected only for rightmost. I guess I’ll have to name each tile distinctly and seperately look for collision. My game’s small right now so checking collision for each tile is fine but it’s gonna be a problem once I start making entire levels. I think using arrays/vectors should do the trick. That way all can be checked at once.

[Peter87]

I don’t want to tell you how you should write your code but I feel you might benefit from some general advice. Feel free to ignore if you want. There are many ways to do things and what you have done is not “wrong”. I’m just trying to help.

• I think you should keep updates and drawing code separate. First update, then draw. Right now you are doing a lot of updates in the drawing code and I think it really complicates the code. I mean for example drawTexture shouldn’t implicitly modifying some global object that you later use to check for collisions. Instead I think it would have been better if the SDL_Rects got updated in the update function, and after they have been updated you might want to use them for detecting collisions but that is also something that should be done in the update function.

• To me the global “list” seems a bit error prone because 1) you need to make sure to spell the names correctly, and 2) it deals with pointers. My philosophy is that pointers should not be used unless there is a reason, otherwise it just complicates the code. You did experiment without pointers earlier, and maybe you have a reason to use them here, but if you just use them because it happened to be how your code looked like when you got it to work then maybe you should reconsider. Maybe you should remove the “list” completely? Can’t you store the rects in the player/tile objects instead?

• You might want to take advantage of the fact that it is “tile-based” to avoid having to do collision detection against all tiles every time. If you store the tiles in some kind of “grid” (based on arrays/vector(s)) where you can look up tiles based on their position then you only need to check against the tiles that are closest to the player. You might want to search for some “tile-based game” tutorials to get a better idea of how this can be handled.

• Reducing the amount of (redundant) data that you store makes it easier to keep things consistent and simple. You might not need to store everything all the time. Some objects could be constructed on the fly when necessary. I’m thinking mainly about all the SDL_Rects. If you store the tiles in a “grid”, like I suggested above, then the position for each tile could be implicitly deduced from its location in the grid. The size of each tile would always be the same. That means you could just construct the SDL_Rect when/if you need it, e.g. to have something to pass to SDL_HasIntersection.

• Removing redundancies in the code is also a good thing (less that can go wrong). Do you really need to pass “player” as argument on this line player->drawTexture(320, 300, 4, "player");? If it still needs to know the name then can’t the player object keep track of it so that it doesn’t need to be passed as argument each time? Maybe you could pass the name to the constructor when creating the object?

[Lark_sh]

Thank you for your advice. I actually got SDL_HasIntersection() to work now. I’m storing instances of the Body class so, the tiles essentially in a dynamically increasing vector, that way I can check collision against all tiles. I’ll modify it further to match your advice @Peter87 , thank you again.

[Lark_sh]

This is a different project. And It keeps throwing this exception that I can’t figure out. Here’s the error:

And here’s Entity.cpp:

#include "Entity.h"

void Entity::loadTexture(const char* path){
	texture = IMG_LoadTexture(gRenderer, path);
}

void Entity::drawTexture(int x, int y) {
	src.x = 0;
	src.y = 0;
	src.w = 24;
	src.h = 24;

	dest.x = x;
	dest.y = y;
	dest.w = src.w * 4;
	dest.h = src.h * 4;

	SDL_RenderCopy(gRenderer, texture, &src, &dest);
}

Here’s Game.cpp:

#include "Game.h"

SDL_Window* gWindow = nullptr;
SDL_Renderer* gRenderer = nullptr;

// HORIZONTAL
float speedX = 2.0f;
float rt_runStr = 8.5f;
float lt_runStr = 8.5f;
float maxX = 15.5f;

// VERTICAL
float speedY = 0.0f;
float gravity = 2.2f;
float jumpStr = 16.2f;
int ground = 500;

// flags
int rightK = 0;
int leftK = 0;
int onGround = 0;
int up = 0;

Entity* player = nullptr;

void Game::init(const char* title ,int x, int y, int w, int h, int flags) {
	if (SDL_Init(SDL_INIT_EVERYTHING) == 0) {

		// create window
		gWindow = SDL_CreateWindow(title, x, y, w, h, flags);

		if (gWindow != NULL) {

			// create renderer
			gRenderer = SDL_CreateRenderer(gWindow, -1, 0);

			isRunning = true;
		}
		else {
			isRunning = false;
		}
	}
	else {
		isRunning = false;
	}
	player = new Entity;
}

void Game::handleEvent() {

	SDL_Event event;

	while (SDL_PollEvent(&event)) {

		switch (event.type) {

		case SDL_QUIT:
			isRunning = false;
			break;
		}
	}
}

void Game::update() {
	if (SDL_GetKeyboardState(0)[SDL_GetScancodeFromKey(SDLK_RIGHT)]) {
		rightK = 1;
	}
	else {
		rightK = 0;
	}

	if (SDL_GetKeyboardState(0)[SDL_GetScancodeFromKey(SDLK_LEFT)]) {
		leftK = 1;
	}
	else {
		leftK = 0;
	}

	if (SDL_GetKeyboardState(0)[SDL_GetScancodeFromKey(SDLK_SPACE)]) {
		up = 1;
	}
	else {
		up = 0;
	}

	if (up == 1) {
		gravity = 0;
	}

	if (rightK == 1) {
		lt_runStr = 0.0f;
		rt_runStr += 1.2f;
	}

	if (leftK == 1) {
		rt_runStr = 0.0f;
		lt_runStr += 1.2f;
	}

	if (rt_runStr >= maxX) {
		rt_runStr = maxX;
		lt_runStr = 0.0f;
	}

	if (lt_runStr >= maxX) {
		lt_runStr = maxX;
		rt_runStr = 0.0f;
	}

	speedX += (rightK * rt_runStr) - (leftK * lt_runStr);
	gravity += 0.6f;
	speedY += gravity - (up * jumpStr);

	player->loadTexture("assets/doux_00.png");
}

void Game::render() {
	SDL_RenderClear(gRenderer);
	// ------------------------------------------
	
	player->drawTexture(250, 200);

	// ------------------------------------------
	SDL_SetRenderDrawColor(gRenderer, 0, 0, 0, 1);
	SDL_RenderPresent(gRenderer);
}

void Game::clean() {
	SDL_DestroyWindow(gWindow);
	SDL_DestroyRenderer(gRenderer);
	SDL_Quit();
}

[Daniel1985]

The application breaks/crashes in the drawTexture() function in the Entity class, which indicates that there might be some issue with the created texture, the image file that you try to load might not be found, which makes the SDL_Texture invalid etc. Make sure that the image file are present in the directory that the application expects the image file to be in.

Also, don’t load the image file/create the SDL_Texture every frame (in the update() function) like you’re doing right now.
Move the texture loading/creation to the application startup, so move player->loadTexture("assets/doux_00.png"); into the init() function of the Game class.

[Lark_sh]

@Daniel1985

move player->loadTexture("assets/doux_00.png"); into the init() function of the Game class.

I did just that and It works now. I’m using the same file path “assets/doux_00.png” so I think it’s able to load the image properly. It’s just that player->loadTexture() in update() func throws an error while it doesn’t in the init() func. Is there any specific reason for that?

[Daniel1985]

I can only speculate regarding the crash, but my guess is that some memory corruption occured.

What you were basically doing was allocating/creating an SDL_Texture every frame. So if your application were, for example, running at 60 frames per second, you were allocating/creating 60 SDL_Textures every second. If your application were running at 120 frames per second, you were allocating/creating 120 SDL_Textures every second, and so on. Note also that none of those textures were ever deallocated/destroyed (with SDL_DestroyTexture()) during the lifetime of the application, which means the memory after a while might get full. The application will then probably crash.

So in short - my guess is that the SDL_Texture pointer texture inside the player entity got corrupt after a few of those SDL_Texture allocations/creations that were happening every frame.
Or the crash can be the cause of the computer/device running out of memory.

[Daniel1985]

Also, in your clean() function inside the Game class, don’t forget to call delete player; to clean up the previously created player entity.

You should also, inside the player entity, call SDL_DestroyTexture() to clean up the previously created texture. You can destroy the texture either inside the Entity’s destructor, or have a deinit() / destroy() function inside the Entity class that gets called before the player object is destroyed (so before you call delete player;.

Example:

void Game::clean() {
	
	// Calls SDL_DestroyTexture(texture); but you can optionally call it in the destructor instead, if wanted
	player->deinit(); 
	delete player;
	
	SDL_DestroyWindow(gWindow);
	SDL_DestroyRenderer(gRenderer);
	SDL_Quit();
}

For every object being created with new, you need a corresponding delete for the created object.

[Lark_sh]

Thank you, I’ll make sure to delete the texture and new objects.