Sign in to follow this  
Josheir

2nd Code Review, Please - Asteroids

Recommended Posts

Various comments typed in your code, so if you apply the patch, make a branch first so it doesn't mess up the code

Despite being called ".diff", it's a plain text-file you can open it with any text editor.

 

In all, it looks nice!

comments.diff

 

Edit: Link to another post with references to the timestep solution

 

Edited by Alberth
Added link to "fix-your-timestep" post

Share this post


Link to post
Share on other sites

Alberth already caught most of the things I saw, but I want to give some more general advice. If you look at the movableObject class, you'll see that it only has some member variables and some setters and getters for them, but no functions that have any behavior that's unique to that class. I would expect this class to have a move() function that then updates the position using the direction (deltaX, deltaY?). Right now this is spread out and duplicated all over the place, in asteroid::moveAsteroid and the various move*** function in main.cpp. 

SFML also has the sf::Vector2f type that you could use instead of splitting every position in an X and Y variable. (If it needs to be integers you can also use sf::Vector2i)

Also regarding the Sleep(10), what I recommand doing right now is enabling vsync (SFML uses OpenGL by default right?) It's more reliable than the Sleep function and has the added benefit of preventing screen-tearing. Later on you can decouple the render-loop from the simulation-loop by following the fix-your-timestep article.
 

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this