Great job! That's a pretty neat code for a beginner. And congrats on finishing it.
I recommend you to read this book, if you haven't already:
https://www.packtpub.com/game-development/sfml-game-developmentI think it will greatly improve your style and teach you a lot of really awesome patterns.
For now, I want to point out these things:
1) Make smaller functions. Right a lot of your game loop is in one big main function. You can separate it into three parts, at least: handleInput, update, render. I recommend you to make a "Game" class, which might make it easier to do. You can store a window, clock, pieces, etc. there
2) Give better names to some commonly used things. For example, you now have piece[0], piece[1], piece[2] everywhere. Why not do something like this?
auto& controlledPiece = piece[0];
3) Use enum classes instead of plain enums. Read about "enum class" to know why
4) Try to return references in your getters instead of returning by value. For example:
sf::RectangleShape Block::getRect()
{
return rect;
}
would better be:
const sf::RectangleShape& Block::getRect() const
{
return rect;
}
5) You don't have to call member functions with "this" pointer inside the class members. E.g.
void Block::blockRotate(sf::Vector2i piecePos)
{
...
this->setLocalPos(newPos, piecePos);
}
is the same as this:
void Block::blockRotate(sf::Vector2i piecePos)
{
...
setLocalPos(newPos, piecePos);
}
6) This might be a bit overkill for such a small project. But try to use less magic numbers. Define constants like border sizes, speed, colors, etc. in separate header or some txt/JSON file. This will make your code more readable, as the reader won't have to guess what one or another number means.
7) You can put more const everywhere, it'll make your code safer. Try to think about const-correctness all the time. Does the function change the argument? Make it const if not. Does the member function modify any data members? If not, make it const as well. You get the idea.
8 ) Use for loops instead of while loops, as they're more readable.
int loopvar=0;
while(loopvar<relativePos.size())
{
...
loopvar++;
}
should really be
for (int i = 0; i < relativePos.size(); ++i)
{
...
}
And I want to show how I would change one small loop to show some ideas:
Before:
int loopvar=0;
while(loopvar<relativePos.size())
{
rects.emplace_back();
rects[loopvar].setSize(sf::Vector2f(40, 40));
rects[loopvar].setFillColor(color);
rects[loopvar].setOutlineThickness(-5);
rects[loopvar].setOutlineColor(sf::Color(77, 77, 77, 255));
rects[loopvar].setPosition(relativePos[loopvar]+bgRect.getPosition());
loopvar++;
}
After:
Rect rect;
rect.setSize(sf::Vector2f(40, 40));
rect.setFillColor(color);
rect.setOutlineThicnkness(-5);
rect.setOutlineColor(sf::Color(77, 77, 77, 255)); // won't change for each rect in the loop
const auto& bgRectPos = bgRect.getPosition();
for (const auto& pos : relativePos)
{
rect.setPosition(pos + bgRectPos); // only this changes
rects.push_back(rect); // and here we copy our rect
}
That's it for now. Please, don't let the critique discourage you in any way. You're doing well, but can be doing even better, keep going!
