Jump to content
  • Advertisement
Sign in to follow this  
Raisor

Requesting Constructive Criticism

This topic is 3650 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

I just started getting back in to programming after taking a brief hiatus and to refresh my memory I decided to write a tetris like game. I was wondering if I could get some feedback from the community here. Please download the game itself and/or download the source and let me know what you think. Edit: I cleaned up the code a bit and cleared out some unecessary files in the source code download so it isnt so large now. [Edited by - Raisor on June 27, 2008 1:48:42 AM]

Share this post


Link to post
Share on other sites
Advertisement
I've taken a quick look and it looks pretty nice.

About the game:

I felt that the keyboard input wasn't as responsive as I wanted. Maybe it's a timing issue. I'm also used to rotating the shapes counter-clockwise. Of course, that's a personal preference, but it would be nice if I could choose which way they rotate.

About the code:

  • In C++, there's no need to put void in an empty argument list, just write empty parenthesis: void func();

  • In return statements, there's no need to put parenthesis around the return value: return 5;

  • To change the value of a boolean variable, you can just do this: cheat = !cheat;

  • Try to declare variables only when you need them, so that you can initialize them at the same time. For example, instead of this:


    int i;
    int temp;

    for (i = 0; ...)
    temp = ...;


    Do this:

    for (int i = 0; ...)
    int temp = ...;


    This makes the code more readable (less variables to remember in a given scope) and is less error-prone (you're less likely to use an uninitialized variable).

  • You should try to avoid duplicate code as much as possible. For example, in Game::DrawStatus(), you have:


    if( !cheat )
    {
    BitBlt( bmoMap, 16 * TILESIZE, 0, TILESIZE, TILESIZE, bmoBlocks, TILEGREY * TILESIZE, 0, SRCPAINT );
    }
    else
    {
    BitBlt( bmoMap, 16 * TILESIZE, 0, TILESIZE, TILESIZE, bmoBlocks, TILEORANGE * TILESIZE, 0, SRCPAINT );
    }


    Instead you can write:


    int tile;
    if (!cheat)
    tile = TILEGREY;
    else
    tile = TILEORANGE;

    BitBlt( bmoMap, 16 * TILESIZE, 0, TILESIZE, TILESIZE, bmoBlocks, tile * TILESIZE, 0, SRCPAINT );


    This way you only write the function call once.

  • Don't declare variables in header files at global scope (like in main.h). If you need global variables that are visible in several files, declare then as extern is the header file, and actually create them in one .cpp file, otherwise you'll get a multiple definitions error. See here for an excellent explanation of this.

    Or better yet, consider if the variables really need to be global.



Well, that's it for now. Hope this helps.

[Edited by - Gage64 on June 27, 2008 9:17:52 AM]

Share this post


Link to post
Share on other sites
Thanks for the pointers Gage64. I took your suggestions and made some changes to the code. Any suggestions on how to fix the timing issue?

Here is some information about the game:

Scoring:
1 - 3 lines cleared = 1 point per line
4 lines cleared = 5 points

Clear 10 * level lines and the level goes up
Block speed increases every 3 levels after the third level

Normal Controls:
Left Arrow - Move piece left
Right Arrow - Move piece right
Down Arrow - Move piece down 1 line
Up Arrow - Rotate piece clockwise
Space Bar - Rotate piece counter-clockwise
Pause - Pause or un-pause the game
Escape - Exit the game

Debug Controls:
F12 - Turn debug mode on
F1 - Change the current piece to the next piece
F2 to F9 - Change the current piece to a specific piece
Shift - Move piece up one line
Home - Increase the row count by 1
End - Decrease the row count by 1
Page Up - Increase the level by 1
Page Down - Decrease the level by 1

Cheat Controls:
Tab - Turn cheat mode on
F1 - Change the current piece to the next piece
Shift - Move piece up one line
While cheat mode is active the block does not automatically move down the screen

I am always open to more discussion so keep it coming everyone. :D

[Edited by - Raisor on June 30, 2008 8:57:53 AM]

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.

Participate in the game development conversation and more when you create an account on GameDev.net!

Sign me up!