Problem using threads

[Hydro]

Hello.

I'm making a Breakout game and I'm getting some problems using sf::Threads. I'm trying to launch a thread per block to check if the ball collided with the block. I created in the class Blocks a method with a template.

I think that showing my code is the best way to understand what I'm saying.

The class:

Code: [Select]

class Block
{
public:
// Methods
Block(sf::Vector2f dimension, sf::Vector2f position, sf::Color color);
~Block();
template <class Obj>
void Collision(const Obj &object);

// Atributes
sf::RectangleShape blockShape;
sf::Vector2f blockPosition;
sf::SoundBuffer hitBuffer;
sf::Sound hitSound;
float blockLeft, blockRight, blockTop, blockBottom;
};

The method:

Code: [Select]

template <class Obj> void Block::Collision(const Obj &object)
{
// Collision variable
bool getCollision = false;

// Object borders
float objLeft   = object.getPosition().x - object.getRadius() / 2;
float objRight  = object.getPosition().x + object.getRadius() / 2;
float objTop    = object.getPosition().y - object.getRadius() / 2;
float objBottom = object.getPosition().y + object.getRadius() / 2;

if (objLeft <= blockRight)
getCollision = true;
else if (objRight >= blockLeft)
getCollision = true;
else if (objTop <= blockBottom)
getCollision = true;
else if (objBottom >= blockTop)
getCollision = true;

// Destroy the object in case of collision
if (getCollision)
this->~Block();
}

And the piece of code where I instantiate the blocks:

Code: [Select]

// "Draw" the blocks (vector)
std::vector<Block> blocks;
int row    = 4;
int colums = 15;

float spacement = 2.5f;
for (int i = 0; i < colums; i++)
{
Block block(sf::Vector2f(window.getSize().x / colums, 25), sf::Vector2f(i * window.getSize().x / colums + spacement, 100.f), sf::Color::Blue);
spacement += 2.5f;
blocks.push_back(block);

// Collision threads
sf::Thread blockThread(&Block::Collision<sf::CircleShape>ball, &blocks);
blockThread.launch();
}

The compiler says (line 114, where I create the sf::Thread):
"error: expected primary-expression before '<' token"
"error: expected primary-expression before '>' token"

Grateful for help.

[Mario]

Code: [Select]

// Destroy the object in case of collision
if (getCollision)
    this->~Block();

This won't work as expected (at all). You're just calling the destructor, but you're not actually destroying the object (freeing its memory). You'll most likely have to use delete instead (but freeing this is usually a bad idea unless you're really careful.

Code: [Select]

sf::Thread blockThread(&Block::Collision<sf::CircleShape>ball, &blocks);

I think this line is your culprit using a rather weird definition. What are you trying to define here? Is this supposed to be a cast? Then you're lacking brackets and your & is in the wrong position as well.

If this isn't the line it's complaining about, tell us which line is line 114.

[Hydro]

So at first I must create the block as pointers. Then I can use the delete. So what I did on the block class was just put in the destructor "delete this". In the main code I must declare the object as a pointer. But now I'm getting confused: how do I change the 3rd block of code I showed to you?

The line 144 is the sf::Thread blockThread(&Block::Collision<sf::CircleShape>ball, &blocks);

[eXpl0it3r]

Without really digging into the code I can tell you, that you've go a wrong understanding of threads. They don't make everything magically better and they should not be used for short tasks etc.
Threads introduce quite a bit of overhead regarding the performance, thus if you don't have an heavy task, you'll not gain anything, but rather lose performance.
The bigger problem is the amount of work and care that needs to be taken when using code in parallel with shared resources, because you have to make sure that not two thread will read or write at the same time on the same memory address. And if you manage that, you'll have to make sure that it truely runs in parallel and not just seriel with threads and locks, etc. And there are numberous other things to take into account.
Personally I suggest to use a different design, without threads and rather just check around the ball what it might hit instead of checking for each block.
I can also advise you to read more on C++, since you sewm to lack a bit of understanding regarding pointers, manual memory management (new & delete) and RAII.

[Nexus]

Then I can use the delete. So what I did on the block class was just put in the destructor "delete this".

No, no. Don't use delete -- use smart pointers instead. Even better, use automatic objects if you don't need pointers. And especially don't use delete this in destructors, it shows almost always a bigger design problem. And it prevents from using a class with other storage classes than dynamic (e.g. automatic or static). Also, your class lacks copy constructor and assignment operator -- another problem you wouldn't have if you used proper RAII and value semantics.

Furthermore, I can fully agree with eXpl0it3r, you should really only use threads if you know exactly what you do (which you apparently don't at the moment). Otherwise, they introduce a lot of unnecessary complexity. For the Break-Out functionality you described here, threads are definitely the wrong approach.

See also my recent post here.