Jump to content
  • Advertisement
Sign in to follow this  
starfruit64

Tetris Clone Code Review Request

This topic is 437 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hello! I have made a clone of Tetris using SDL libraries for C++. I would love to get some input as to what I have done right and wrong. Also please tell me if im using Github correctly, it confuses me greatly. (I tried to add a new commit to remove the misplaced header file "DungeonCrawlV4.o" but it doesn't seem to have taken place.)

https://github.com/Starfruit64/Tetris-Clone

Share this post


Link to post
Share on other sites
Advertisement

Quick thoughts (some of which I think I mentioned to you last time we looked at this code):

  • there's no need to have a single, shared SDL_Event for everything to use - it can be a local object inside the function that polls for them.
  • it seems a bit pointless to abstract away the SDL input messages in a program this small, but if you prefer to do this, I'd suggest using enums rather than strings like "a_d" which are more error-prone and also less efficient to pass around. This also allows you to use switch/case instead of the uglier chains of if-elses.
  • I don't recommend wrapping your SDL_PollEvent call inside an input class because SDL events aren't all about player input.
  • Normally, instead of passing a graphics object (e.g. Renderer) to a game logic object (e.g. Board), you do it the other way around. This is because you don't usually want your logic objects to have to worry about details such as how they get rendered.
  • different gamestates are better represented with enums than with integers and comments explaining the integers.
  • you should consider making accessors like 'return_pos' const
  • you should probably add comments in your header files to say what each class does and what each member does
  • explicitly initialise variables when you create them, e.g. to nullptr
  • be consistent with your naming - you seem to use 'tetro' and 'tetri' interchangeably.
  • consider separating your game loop out into clear input/update/render functions. You might need to have some extra mechanism telling you when to render, since (unlike most games) you don't render every frame.

Share this post


Link to post
Share on other sites
1 hour ago, Kylotan said:

Quick thoughts (some of which I think I mentioned to you last time we looked at this code):

  • there's no need to have a single, shared SDL_Event for everything to use - it can be a local object inside the function that polls for them.
  • it seems a bit pointless to abstract away the SDL input messages in a program this small, but if you prefer to do this, I'd suggest using enums rather than strings like "a_d" which are more error-prone and also less efficient to pass around. This also allows you to use switch/case instead of the uglier chains of if-elses.
  • I don't recommend wrapping your SDL_PollEvent call inside an input class because SDL events aren't all about player input.
  • Normally, instead of passing a graphics object (e.g. Renderer) to a game logic object (e.g. Board), you do it the other way around. This is because you don't usually want your logic objects to have to worry about details such as how they get rendered.
  • different gamestates are better represented with enums than with integers and comments explaining the integers.
  • you should consider making accessors like 'return_pos' const
  • you should probably add comments in your header files to say what each class does and what each member does
  • explicitly initialise variables when you create them, e.g. to nullptr
  • be consistent with your naming - you seem to use 'tetro' and 'tetri' interchangeably.
  • consider separating your game loop out into clear input/update/render functions. You might need to have some extra mechanism telling you when to render, since (unlike most games) you don't render every frame.

About events: should I remove the Input class and have all events handles in Main in a function? Or should I have multiple times where I poll for events, one time for input, one time for window events, etc.?

Share this post


Link to post
Share on other sites

I recommend handling all the events in your main loop, and passing them on to whichever subsystem - e.g. Input - can handle them.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!