🎉 Celebrating 25 Years of GameDev.net! 🎉

Not many can claim 25 years on the Internet! Join us in celebrating this milestone. Learn more about our history, and thank you for being a part of our community!

Code review. C++, OpenGL (glut)

Started by
2 comments, last by frob 2 years ago

Hi, I'm making game and want for someone to review my code if it's any good, any bad begginer mistakes etc?

https://github.com/Povilas-Jablonskis/Space-Shooter

Advertisement

void Animation::loadFromFile → void Animation::LoadFromFile usually functions are capitalized.
Your indentation for the namespace is annoying. Functions usually align to the left. The way you have it being indented, it looks like everything is already indented inside of a function.
{
Animation::Animation()

Always comment your code. Player::update() what does it do? Take input and move, or anything else?

bool Enemy::update(float dt) , didn't check all your code but never abbreviate any variables even though dt is common for "deltaTime". I have seen code that is very old in production games where variables are ar, b, sx that have no meaning and everytime you read it you have to re-train yourself what that variable is because it has no meaning.

only browsed a few files.

NBA2K, Madden, Maneater, Killing Floor, Sims http://www.pawlowskipinball.com/pinballeternal

I notice right away you've published items with incompatible licenses. You're releasing your project as GPL, but a few of the libraries you're incorporating aren't compatible. If your project ever succeeds (unlikely) that's going to bite you.

It looks like a typical beginner project overall. If it works for you building your own game then go for it, but it's definitely beginner. Learn about abstractions, learn about SOLID, learn to think in terms of systems. Learn to think about batch processing rather than individual items. Learn to suitably encapsulate systems so you aren't passing around a bunch of extra, strange, or unnecessary data. As one example of many, creating an enemy takes a width and height (assuming it's square), color, position, and speed. You've got no way to make other types of enemies, no way to give different models with different sizes, different materials, or other options. You've hard-coded data names, (addAnimation("explosion", animations["explosion"])) angles (setRotationAngle(3.14159265358979323846f)), positions ((9.0f, 20.0f, glm::vec2(position.x + (width / 2.0f), position.y - 4.5f), glm::vec2(0.0f, -200.0f), glm::vec4(255.0f, 69.0f, 0.0f, 1.0f))), and more. For a single person, small, beginner project those are fine, but they're brittle and difficult to work with, and almost always considered defects in large projects and mature code.

You're mixing behavior in many classes and making common beginner mistakes about resource management. Not a problem, just shows the maturity of the developer. Again as one example of many, loading an animation from file looks like it interferes with more than just loading data from a file, has no way to deal with errors, for example, what do you do if a file isn't found, or is corrupt or unloadable?

If you can build your game from it and it mostly works for you individually, great.

------

Since you're probably looking for something that you can improve, my recommendation is to start programming defensively and look at error conditions. For a lot of code error handling outpaces the ideal path by several times. In some systems my error handling represents about 80% of the code, in some it's closer to 60%, very rarely it's about 40% of the code.

Taking Animation::LoadFromFile() as an example because it's near the beginning. I've already touched on it not having a single responsibility and on the functionality not matching the name, so no more on that. glGenTextures() is doing a single texture, consider batching work since the game is going to need more than one texture at a time. A system could generate a large pool and then allocate from the pool. It also has unchecked error conditions. Then you call glBindTexture with the results from getSpriteSheetTexture(). What happens if there's an error in getting the sprite sheet texture? glBindTexture has three error conditions that you didn't check for. You have a series of glTexParameter calls, with no error checking. You create and load an image, what happens if the file doesn't exist, or there is some other error loading and parsing the image? You call glTexImage2D to create an image, what if the underlying image is not an RGBA image due to earlier errors, or the width and height are not allowed by the device, or any of the other long list of errors that glTexImage2D can generate?

For a beginner project that's typical and expected. You assume that things work all the time and if there is an issue you'll crash in the debugger and be able to reason through it. In a more mature project you need to account for all the possible error conditions in the code. If something can potentially have an error, sooner or later in production the error will occur and your code must handle it gracefully.

This topic is closed to new replies.

Advertisement