   My first C++/SDL game: Tic Tac Toe
  Hi! I'm not sure if this is the right category for this post but since I'm beginner in gamedev I suppose this is the right place...I have been studying cpp for about a week now and SDL for few days. I have taken a sneak peak on cpp before few times but never got further than simple console applications (calculator, guess the number -game, etc.). I also took 2 basic classes at high school 2 years ago. But now my interest in cpp and game making has rised again. So here's my first game with graphics, Graphic's "Hello World", if you will: Tic Tac Toe :)I would like to get some feedback on my source, what could I do differently, if something is good, gameplay, etc. I'm not a much of an artist but would appreciate feedback on graphics too.You place the marks with numpad on your keyboard as it is your board so 7 | 8 | 9 --------- 4 | 5 | 6 --------- 1 | 2 | 3Just realized it doesn't work so well for the laptops that don't have numpad :)I'm planning on adding it possible to use mouse as soon as I learn the mouse functions.EDIT: Guess I could give you some sort of link to download the game :D-- >Download the game <--And here's the source for the game:NOTE: For the comments, I have them on my original source file but they are in finnish so I thought it would be better not to post them, so this is un-commented version.[source lang=cpp]#include <SDL/SDL.h>#include <SDL/SDL_image.h>const int SCREEN_WIDTH = 640;const int SCREEN_HEIGHT = 480;const int SCREEN_BPP = 32;void DrawImage(SDL_Surface *img, SDL_Surface *screen, int x, int y){    SDL_Rect alue;    alue.x = x;    alue.y = y;    SDL_BlitSurface(img, NULL, screen, &alue);}void DrawMarks(int *Places, SDL_Surface *imgX, SDL_Surface *imgO, SDL_Surface *screen){    if (Places[0] == 1)    {   DrawImage(imgX, screen, 120, 43);    }    else if (Places[0] == 2)    {   DrawImage(imgO, screen, 120, 43);    }    if (Places[1] == 1)    {   DrawImage(imgX, screen, 273, 43);    }    else if (Places[1] == 2)    {   DrawImage(imgO, screen, 273, 43);    }    if (Places[2] == 1)    {   DrawImage(imgX, screen, 423, 43);}    else if (Places[2] == 2)    {   DrawImage(imgO, screen, 423, 43);    }    if (Places[3] == 1)    {   DrawImage(imgX, screen, 120, 188);    }    else if (Places[3] == 2)    {   DrawImage(imgO, screen, 120, 188);    }    if (Places[4] == 1)    {   DrawImage(imgX, screen, 273, 188);}    else if (Places[4] == 2)    {   DrawImage(imgO, screen, 273, 188);}    if (Places[5] == 1)    {   DrawImage(imgX, screen, 423, 188);}    else if (Places[5] == 2)    {   DrawImage(imgO, screen, 423, 188);    }    if (Places[6] == 1)    {   DrawImage(imgX, screen, 120, 338);    }    else if (Places[6] == 2)    {   DrawImage(imgO, screen, 120, 338);    }    if (Places[7] == 1)    {   DrawImage(imgX, screen, 273, 338);    }    else if (Places[7] == 2)    {   DrawImage(imgO, screen, 273, 338);    }    if (Places[8] == 1)    {   DrawImage(imgX, screen, 423, 338);    }    else if (Places[8] == 2)    {   DrawImage(imgO, screen, 423, 338);    }}int main(int argc, char *argv[]){    SDL_Init(SDL_INIT_VIDEO);    SDL_WM_SetCaption("Tic Tac Toe", NULL);    SDL_Surface *screen = NULL;SDL_Surface *board = NULL;SDL_Surface *imgX = NULL;    SDL_Surface *imgO = NULL;    SDL_Surface *winX = NULL;SDL_Surface *winO = NULL;SDL_Surface *xTurn = NULL;    SDL_Surface *oTurn = NULL;    SDL_Surface *draw = NULL;    screen = SDL_SetVideoMode(SCREEN_WIDTH, SCREEN_HEIGHT, SCREEN_BPP, SDL_HWSURFACE | SDL_DOUBLEBUF);board = IMG_Load("board.png");imgX = IMG_Load("imgX.png");imgO = IMG_Load("imgO.png");winX = IMG_Load("xwin.png");    winO = IMG_Load("owin.png");    xTurn = IMG_Load("xturn.png");    oTurn = IMG_Load("oturn.png");    draw = IMG_Load("draw.png");    start:    bool EndGameLoop = false;    bool EndGame = false;    int winner = 0;int turn = 1;    int Places[9] = {NULL};    SDL_Event event;    while(EndGameLoop != true)    {    DrawImage(board, screen, 0, 0);if(!(turn % 2) == 1)    {   DrawImage(xTurn, screen, 15, 43);    }    else if (!(turn % 2) == 0)    {   DrawImage(oTurn, screen, 13, 43);    }   SDL_PollEvent(&event);   if(event.type == SDL_QUIT)   { EndGameLoop = true; EndGame = true; goto endgame;  }   if(event.type == SDL_KEYDOWN)  { if(event.key.keysym.sym == SDLK_ESCAPE)  {EndGameLoop = true;EndGame = true;goto endgame; }   if(event.key.keysym.sym == SDLK_KP7)   {  if(!(turn % 2) == 1)  { if(Places[0] == 0) { Places[0] = 1; turn++; }  }  if(!(turn % 2) == 0)  { if(Places[0] == 0) { Places[0] = 2; turn++; }  }   }   if(event.key.keysym.sym == SDLK_KP8)   {  if(!(turn % 2) == 1)  { if(Places[1] == 0) { Places[1] = 1; turn++; }  }  if(!(turn % 2) == 0)  { if(Places[1] == 0) { Places[1] = 2; turn++; }  }   }   if(event.key.keysym.sym == SDLK_KP9)   {  if(!(turn % 2) == 1)  { if(Places[2] == 0) { Places[2] = 1; turn++; }  }  if(!(turn % 2) == 0)  { if(Places[2] == 0) { Places[2] = 2; turn++; }  }   }   if(event.key.keysym.sym == SDLK_KP4)   {  if(!(turn % 2) == 1)  { if(Places[3] == 0) { Places[3] = 1; turn++; }  }  if(!(turn % 2) == 0)  { if(Places[3] == 0) { Places[3] = 2; turn++; }  }   }   if(event.key.keysym.sym == SDLK_KP5)   {  if(!(turn % 2) == 1)  { if(Places[4] == 0) { Places[4] = 1; turn++; }  }  if(!(turn % 2) == 0)  { if(Places[4] == 0) { Places[4] = 2; turn++; }  }   }   if(event.key.keysym.sym == SDLK_KP6)   {  if(!(turn % 2) == 1)  { if(Places[5] == 0) { Places[5] = 1; turn++; }  }  if(!(turn % 2) == 0)  { if(Places[5] == 0) { Places[5] = 2; turn++; }  }   }   if(event.key.keysym.sym == SDLK_KP1)   {  if(!(turn % 2) == 1)  { if(Places[6] == 0) { Places[6] = 1; turn++; }  }  if(!(turn % 2) == 0)  { if(Places[6] == 0) { Places[6] = 2; turn++; }  }   }   if(event.key.keysym.sym == SDLK_KP2)   {  if(!(turn % 2) == 1)  { if(Places[7] == 0) { Places[7] = 1; turn++; }  }  if(!(turn % 2) == 0)  { if(Places[7] == 0) { Places[7] = 2; turn++; }  }   }   if(event.key.keysym.sym == SDLK_KP3)   {  if(!(turn % 2) == 1)  { if(Places[8] == 0) { Places[8] = 1; turn++; }  }  if(!(turn % 2) == 0)  { if(Places[8] == 0) { Places[8] = 2; turn++; }  }   }    }    DrawMarks(Places, imgX, imgO, screen);  if (Places[0] == 1 && Places[1] == 1 && Places[2] == 1)    {   winner = 1;   EndGameLoop = true;    }    else if (Places[0] == 2 && Places[1] == 2 && Places[2] == 2)    {   winner = 2;   EndGameLoop = true;    }    else if (Places[3] == 1 && Places[4] == 1 && Places[5] == 1)    {   winner = 1;   EndGameLoop = true;    }    else if (Places[3] == 2 && Places[4] == 2 && Places[5] == 2)    {   winner = 2;   EndGameLoop = true;    }    else if (Places[6] == 1 && Places[7] == 1 && Places[8] == 1)    {   winner = 1;   EndGameLoop = true;    }    else if (Places[6] == 2 && Places[7] == 2 && Places[8] == 2)    {   winner = 2;   EndGameLoop = true;    }    else if (Places[0] == 1 && Places[3] == 1 && Places[6] == 1)    {   winner = 1;   EndGameLoop = true;    }    else if (Places[0] == 2 && Places[3] == 2 && Places[6] == 2)    {   winner = 2;   EndGameLoop = true;    }    else if (Places[1] == 1 && Places[4] == 1 && Places[7] == 1)    {   winner = 1;   EndGameLoop = true;    }    else if (Places[1] == 2 && Places[4] == 2 && Places[7] == 2)    {   winner = 2;   EndGameLoop = true;    }    else if (Places[2] == 1 && Places[5] == 1 && Places[8] == 1)    {   winner = 1;   EndGameLoop = true;    }    else if (Places[2] == 2 && Places[5] == 2 && Places[8] == 2)    {   winner = 2;   EndGameLoop = true;    }    else if (Places[0] == 1 && Places[4] == 1 && Places[8] == 1)    {   winner = 1;   EndGameLoop = true;    }    else if (Places[0] == 2 && Places[4] == 2 && Places[8] == 2)    {   winner = 2;   EndGameLoop = true;    }    else if (Places[2] == 1 && Places[4] == 1 && Places[6] == 1)    {   winner = 1;   EndGameLoop = true;    }    else if (Places[2] == 2 && Places[4] == 2 && Places[6] == 2)    {   winner = 2;   EndGameLoop = true;    }    SDL_Flip(screen);    if(turn == 10)    {   EndGameLoop = true;    }    }    while (EndGame != true)    {   if (winner == 1)   {  DrawImage(winX, screen, 0, 0);  SDL_Flip(screen);  SDL_Delay(2000);  EndGame = true;   }   else if (winner == 2)   {  DrawImage(winO, screen, 0, 0);  SDL_Flip(screen);  SDL_Delay(2000);  EndGame = true;   }   else   {  DrawImage(draw, screen, 0, 0);  SDL_Flip(screen);  SDL_Delay(2000);  EndGame = true;   }    }    goto start;    endgame:    SDL_FreeSurface(screen);    SDL_FreeSurface(imgX);    SDL_FreeSurface(imgO);    SDL_FreeSurface(board);    SDL_FreeSurface(winX);    SDL_FreeSurface(winO);    SDL_FreeSurface(xTurn);    SDL_FreeSurface(oTurn);    SDL_FreeSurface(draw);    SDL_Quit();    return 0;} [Edited by - Tobiski on June 4, 2010 7:13:08 AM]
  Your game seems nice! You could add a "vs CPU" mode or timer so that each player has a limited amount of time to think...   ;  I love how you used "marking pens" for the presentation. :)   ;  Very nice stuff:For the code: I don't think commenting is so important. If the code is self explaining, you don't need comments.Just a little thing: you could calculate "turn" to have 0-1 directly, so you don't always have to check "%2"For example: turn ^= 1;or turn = (turn+1)%2;EDIT: I say you could make a pretty cool Gomoku (5 in-a-raw) game from it (perhaps with a real exercise book background). TIC-TAC-TOE is so useless.With mouse  of course...   ;  Very nice, I loved it :)As for the source, I'll just note a few things that seemed off while skimming through it. It's nice that you've made a few functions to simplify drawing, but how about refactoring even more? For example, you could include your game logic (placing x'ses and o's) or the code that determines who wins into separate functions. That wouldn't really accomplish much for a game this size, as those functions would only be used once, but it would make code a bit more readable.Also, I see you used gotos. This practice is generally discouraged and you'd be wise to rething some of the flow logic and try replacing those with appropriate loops. I know it seems to work now and people can get whats going on from the source code, but as programs get bigger you'll run into a lot of problems concerning readability and debugging.(Not to understate this, gotos are very VERY heavily discouraged to the point that you'd be hard pressed to find a sane programmer who uses them anywhere and most people consider them just plain evil ;))   ;Quote:Original post by AndreiVictorI love how you used "marking pens" for the presentation. :)Thanks! :) I drew graphics into paper with marker, scanned them and then colored on my pcQuote:Original post by szecsJust a little thing: you could calculate "turn" to have 0-1 directly, so you don't always have to check "%2"For example: turn ^= 1;or turn = (turn+1)%2;EDIT: I say you could make a pretty cool Gomoku (5 in-a-raw) game from it (perhaps with a real exercise book background). TIC-TAC-TOE is so useless.With mouse of course...I tried at first that turn = 1 and turn = 2 but for some reason I couldn't get it work, maybe it was some other piece of code which wasn't working at the time.I'll try to expand this source for the Gomoku game and with mouse of course :)Quote:Original post by Also, I see you used gotos.Yes, that was fast solution, tried one way and couldn't make it work at first try so I used gotos, will not use those in future projects :)Thanks for your feedback!   ;Quote:Original post by TobiskiQuote:Original post by AndreiVictorI love how you used "marking pens" for the presentation. :)Thanks! :) I drew graphics into paper with marker, scanned them and then colored on my pcYeah, really like 'em too! :)Quote:I would like to get some feedback on my sourceFirst thing I noticed was the repetitive code in certain areas, for instance:    if (Places[0] == 1)    {   DrawImage(imgX, screen, 120, 43);    }    else if (Places[0] == 2)    {   DrawImage(imgO, screen, 120, 43);    }    if (Places[1] == 1)    {   DrawImage(imgX, screen, 273, 43);    }    else if (Places[1] == 2)    {   DrawImage(imgO, screen, 273, 43);    }    if (Places[2] == 1)    {   DrawImage(imgX, screen, 423, 43);}    else if (Places[2] == 2)    {   DrawImage(imgO, screen, 423, 43);    }    if (Places[3] == 1)    {   DrawImage(imgX, screen, 120, 188);    }    else if (Places[3] == 2)    {   DrawImage(imgO, screen, 120, 188);    }    if (Places[4] == 1)    {   DrawImage(imgX, screen, 273, 188);}    else if (Places[4] == 2)    {   DrawImage(imgO, screen, 273, 188);}    if (Places[5] == 1)    {   DrawImage(imgX, screen, 423, 188);}    else if (Places[5] == 2)    {   DrawImage(imgO, screen, 423, 188);    }    if (Places[6] == 1)    {   DrawImage(imgX, screen, 120, 338);    }    else if (Places[6] == 2)    {   DrawImage(imgO, screen, 120, 338);    }    if (Places[7] == 1)    {   DrawImage(imgX, screen, 273, 338);    }    else if (Places[7] == 2)    {   DrawImage(imgO, screen, 273, 338);    }    if (Places[8] == 1)    {   DrawImage(imgX, screen, 423, 338);    }    else if (Places[8] == 2)    {   DrawImage(imgO, screen, 423, 338);    }If you recognize the pattern, we can shorten this to a simple loop:    const int CELL_WIDTH = 150;    const int CELL_HEIGHT = 150;    const int CELL_NUM_X = 3;    const int CELL_NUM_Y = 3;    const int GRID_MARGIN_X = 120;    const int GRID_MARGIN_Y = 43;    for (int y = 0; y < CELL_NUM_Y; y++)    { for (int x = 0; x < CELL_NUM_X; x++) {   int i = y * CELL_NUM_X + x;  // array index  if (Places == 1)DrawImage(imgX, screen, x * CELL_WIDTH + GRID_MARGIN_X, y * CELL_HEIGHT + GRID_MARGIN_Y);   else if (Places == 2)DrawImage(imgO, screen, x * CELL_WIDTH + GRID_MARGIN_X, y * CELL_HEIGHT + GRID_MARGIN_Y); }    }I realize that it doesn't quite work, because the images aren't strictly aligned to the grid, so it would require either some extra code or a small chance in the graphics. There are other ways to write this loop, this is just one example. Besides reducing code size, it has the advantage that will work for grids of any size, not just 3 x 3.There are more places where we can reduce code size, just by recognizing a pattern:    if (Places[0] == 1 && Places[1] == 1 && Places[2] == 1)    { winner = 1; EndGameLoop = true;    }    else if (Places[0] == 2 && Places[1] == 2 && Places[2] == 2)    { winner = 2; EndGameLoop = true;    }We could also write this:    if (Places[0] > 0 && Places[0] == Places[1] && Places[1] == Places[2])    { winner = Places[0]; EndGameLoop = true;    }Or generalize it to check ever row:    for (int i = 0; i < 9; i += 3)    {   if (Places > 0 && Places == Places &amp;&amp; Places == Places)<br>   {<br> winner = Places<span style="font-weight:bold;">;<br> EndGameLoop = <span class="cpp-keyword">true</span>;<br>   }<br>    }<br><br><br></pre></div><!–ENDSCRIPT–><br>None of my rewrites are the &#111;nly or even the best way of doing things, just showing you some possibilities. :);  Actually here:if (Places == 1) DrawImage(imgX, screen, x * CELL_WIDTH + GRID_MARGIN_X, y * CELL_HEIGHT + GRID_MARGIN_Y);else if (Places == 2) DrawImage(imgO, screen, x * CELL_WIDTH + GRID_MARGIN_X, y * CELL_HEIGHT + GRID_MARGIN_Y);you can reduce the duplication even further:SDL_Surface *img;if (Places == 1)    img = imgX;else if (Places == 2)    img = imgO;DrawImage(img, screen, x * CELL_WIDTH + GRID_MARGIN_X, y * CELL_HEIGHT + GRID_MARGIN_Y);;Quote:Original post by Gage64Actually here:*** Source Snippet Removed ***you can reduce the duplication even further:*** Source Snippet Removed ***Or even further:if (Places > 0) //and maybe && Places < 3    DrawImage( (Places == 1 ? imgX : imgO), screen, x * CELL_WIDTH + GRID_MARGIN_X, y * CELL_HEIGHT + GRID_MARGIN_Y);;  Heh, I knew there was a way to reduce the duplication, but I just couldn't figure it out at the time.  I looked at his code, and something at the back of my head was screaming at me to fix the duplications.  Sadly, my lack of experience couldn't figure it out.Thank you guys for providing suggestions, as you've helped both me and (I'm sure) the OP.   