• Advertisement
Sign in to follow this  

Fixing ugly code

This topic is 4380 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 am new to game programming, so please bear with me. I am very repulsed by the ugliness of the code that I have written - I probably broke every single rule that programmers follow. Without further ado, here is the code.
#include "SDL.h"
#include "SDL_ttf.h"
#include <stack>
#include <stdlib.h>
#include "SDL_Collide.h"


//Global constants

const int SCREEN_WIDTH = 640;
const int SCREEN_HEIGHT = 480;
const int SCREEN_BPP = 32;      //bits per pixel
const int FRAMES_PER_SECOND = 30;
const int FRAME_RATE = 1000/FRAMES_PER_SECOND;
const char* WM_TITLE = "A Pong Clone"; //window title

using namespace std;

void GameLogic();

//Game constants

const int PLAYER_PADDLE_X = 50;
const int ENEMY_PADDLE_X = SCREEN_WIDTH - PLAYER_PADDLE_X;
const int PADDLE_SPEED = 3;
int ball_speed = 1;

enum direction
{
        RIGHT = 0, UP_RIGHT, UP, UP_LEFT, LEFT, DOWN_LEFT, DOWN, DOWN_RIGHT
};

SDL_Surface* screen;            //screen buffer
bool isRunning = true;          //once this is false, the app stops
SDL_Event event;                //our event holding struct
SDLKey keyDown;                 //key event holder

struct StateStruct
{
  void (*StatePointer)();
};

stack<StateStruct> g_StateStack;

int g_Timer;

void Exit();
void Menu();

namespace Saketh
{
class Sprite
{
protected:
        SDL_Surface *image;
        int x;  //x position
        int y;  //y position
        Uint32 colorkey;
public:
        Sprite()
        {
                x = 0;
                y = 0;
        }
        ~Sprite()
        {
                SDL_FreeSurface(image);
        }
        void draw()
        {
                SDL_Rect source;
                SDL_Rect dest;

                source.x = 0;
                source.y = 0;
                source.w = image->w;
                source.h = image->h;

                dest.x = x;
                dest.y = y;
                dest.w = source.w;
                dest.h = source.h;

                SDL_BlitSurface(image, &source, screen, &dest);
        }

        void setCoordinates(int _x, int _y) {x = _x; y = _y;}
        void setX(int _x) {x = _x;}
        void setY(int _y) {y = _y;}
        void incrementCoordinates(int xIncrement, int yIncrement) {x+=xIncrement; y+=yIncrement;}
        int getX() {return x;}
        int getY() {return y;}
        SDL_Surface* getImage() {return image;}
        void setColorKey(Uint32 flags)
        {
                SDL_SetColorKey(image, flags, colorkey);
        }
        void mapRGB(int r, int g, int b)
        {
                colorkey = SDL_MapRGB(image->format, r, g, b);
        }
        void loadImage(const char* filename) { image = SDL_LoadBMP(filename); }
};


        int initialize()
        {
                SDL_WM_SetCaption(WM_TITLE, 0);
                if (SDL_Init(SDL_INIT_VIDEO) < 0)
                {
                        fprintf(stderr, "Could not initalize SDL: %s\n", SDL_GetError());
                        return -1;
                }
                atexit(SDL_Quit);       //call SDL_Quit once the app is done
                screen = SDL_SetVideoMode(SCREEN_WIDTH, SCREEN_HEIGHT, SCREEN_BPP, SDL_SWSURFACE | SDL_ANYFORMAT);
                if (!screen)    //if screen didn't get initalized properly
                {
                        fprintf(stderr, "Couldn't create a surface: %s\n", SDL_GetError());
                        return -1;
                }
		// We start by adding a pointer to our exit state, this way
    // it will be the last thing the player sees of the game.
    StateStruct state;
    state.StatePointer = Exit;
    g_StateStack.push(state);

    // Then we add a pointer to our menu state, this will
    // be the first thing the player sees of our game.
    state.StatePointer = Menu;
    g_StateStack.push(state);
		

		g_Timer = SDL_GetTicks();

		TTF_Init();
        }
        int shutdown()
        {
	    TTF_Quit();
        return 0;
        }

        bool checkbounds(Saketh::Sprite* s)
        {
                if (s->getX() <= 0)
                {
                        s->setX(1);
                        return false;
                }
                if (s->getY() <= 0)
                {
                        s->setY(1);
                        return false;
                }
                if ((s->getY() + s->getImage()->h) >= SCREEN_HEIGHT)
                {
                        s->setY(SCREEN_HEIGHT - s->getImage()->h - 1);
                        return false;
                }
                if (s->getX() >= SCREEN_WIDTH)
                {
                        s->setY(SCREEN_WIDTH);
                        return false;
                }
                //otherwise
                return true;
        }
void set_within_bounds(Saketh::Sprite* s)
{
        if (s->getX() <= 0)
        {
                s->setX(1);
        }
        if (s->getY() <= 0)
        {
                s->setY(1);
        }
        if ((s->getY() + s->getImage()->h) >= SCREEN_HEIGHT)
        {
                s->setY(SCREEN_HEIGHT - s->getImage()->h - 1);
        }
        if (s->getX() >= SCREEN_WIDTH)
        {
                s->setY(SCREEN_WIDTH);
        }
}

void drawText(SDL_Surface* screen,
              char* string,
              int size,
              int x, int y,
              int fR, int fG, int fB,
              int bR, int bG, int bB)
{
   TTF_Font* font = TTF_OpenFont("data\\Exocet2.ttf", size);

   SDL_Color foregroundColor = { fR, fG, fB };
   SDL_Color backgroundColor = { bR, bG, bB };

   SDL_Surface* textSurface = TTF_RenderText_Shaded(font, string,
      foregroundColor, backgroundColor);

   SDL_Rect textLocation = { x, y, 0, 0 };

   SDL_BlitSurface(textSurface, NULL, screen, &textLocation);

   SDL_FreeSurface(textSurface);

   TTF_CloseFont(font);
}

};

void ClearScreen()
{
    // This function just fills a surface with a given color. The
    // first 0 tells SDL to fill the whole surface. The second 0
    // is for black.
    SDL_FillRect(screen, 0, 0);
}

void DrawBackground()
{
    Saketh::Sprite background;
    background.loadImage("data\\background.bmp");
    // This just 'block-image transfers' our bitmap to our window.
    background.draw();
}

void Game();

void Menu()
{
 
    // Here we compare the difference between the current time and the last time we
    // handled a frame. If FRAME_RATE amount of time has, it's time for a new frame.
    if ( (SDL_GetTicks() - g_Timer) >= FRAME_RATE )
    {
        // Fill our event structure with event information.
    if ( SDL_PollEvent(&event) )
    {
        // Handle user manually closing game window
        if (event.type == SDL_QUIT)
        {
            // While state stack isn't empty, pop
            while (!g_StateStack.empty())
            {
                g_StateStack.pop();
            }

            return; // game is over, exit the function
        }

        // Handle keyboard input here
        if (event.type == SDL_KEYDOWN)
        {
            if (event.key.keysym.sym == SDLK_ESCAPE)
            {
                g_StateStack.pop();
                return; // this state is done, exit the function
            }
            // Quit
            if (event.key.keysym.sym == SDLK_q)
            {
                g_StateStack.pop();
                return; // game is over, exit the function
            }
            // Start Game
            if (event.key.keysym.sym == SDLK_g)
            {
	      GameLogic();
                exit(0); // This state is done, exit the function
            }
        }
    }
  

        // Make sure nothing from the last frame is still drawn.
        ClearScreen();

        
        Saketh::drawText(screen, "Start (G)ame\n(Q)uit Game", 50, 0, 0, 255, 0, 0, 0, 0, 0);
        // Tell SDL to display our backbuffer. The four 0's will make SDL display the whole screen.
        SDL_UpdateRect(screen, 0, 0, 0, 0);

        // We've processed a frame so we now need to record the time at which we did it.
        // This way we can compare this time with the next time our function gets called and
        // see if enough time has passed between calls.
        g_Timer = SDL_GetTicks();
    }
}

void Game()
{
  GameLogic();
}

void Exit()
{     
    // Here we compare the difference between the current time and the last time we
    // handled a frame. If FRAME_RATE amount of time has, it's time for a new frame.
    if ( (SDL_GetTicks() - g_Timer) >= FRAME_RATE )
    {
      // Fill our event structure with event information.
    if ( SDL_PollEvent(&event) )
    {
        // Handle user manually closing game window
        if (event.type == SDL_QUIT)
        {
            // While state stack isn't empty, pop
            while (!g_StateStack.empty())
            {
                g_StateStack.pop();
            }

            return; // game is over, exit the function
        }

        // Handle keyboard input here
        if (event.type == SDL_KEYDOWN)
        {
            if (event.key.keysym.sym == SDLK_ESCAPE)
            {
                g_StateStack.pop();

                return; // this state is done, exit the function
            }
            // Yes
            if (event.key.keysym.sym == SDLK_y)
            {
                g_StateStack.pop();
                return; // game is over, exit the function
            }
            // No
            if (event.key.keysym.sym == SDLK_n)
            {
                StateStruct temp;
                temp.StatePointer = Menu;
                g_StateStack.push(temp);
                return; // this state is done, exit the function
           }
        }
    }
        // Make sure nothing from the last frame is still drawn.
        ClearScreen();

        Saketh::drawText(screen, "Quit game? (Y/N)", 50, 0, 0, 255, 0, 0, 0, 0, 0);

        // Tell SDL to display our backbuffer. The four 0's will make SDL display the whole screen.
        SDL_UpdateRect(screen, 0, 0, 0, 0);

        // We've processed a frame so we now need to record the time at which we did it.
        // This way we can compare this time with the next time our function gets called and
        // see if enough time has passed between calls.
        g_Timer = SDL_GetTicks();
    }
}


struct Ball
{
        Saketh::Sprite image;
        direction d;
        int xspeed;
        int yspeed;
        void draw() { image.draw(); }
};

void stop(Ball ball)
{
        ball.xspeed = 0;
        ball.yspeed = 0;
}

void restart(Ball ball)
{
  ball.image.setX(SCREEN_WIDTH/2);
  ball.image.setY(SCREEN_HEIGHT/2);
  ball.d = UP_RIGHT;
}


        Saketh::Sprite player;
        Saketh::Sprite enemy;
        Ball ball;
        Saketh::Sprite background;

int main(int argc, char *argv[])
{
        Saketh::initialize();
        SDL_WM_SetCaption(WM_TITLE, 0);

        player.loadImage("data\\player.bmp");
        enemy.loadImage("data\\enemy.bmp");
        ball.image.loadImage("data\\ball.bmp");
        background.loadImage("data\\background.bmp");


        player.mapRGB(255, 255, 255);
        enemy.mapRGB(255, 255, 255);
        ball.image.mapRGB(255, 255, 255);

        player.setColorKey(SDL_SRCCOLORKEY);
        enemy.setColorKey(SDL_SRCCOLORKEY);
        ball.image.setColorKey(SDL_SRCCOLORKEY);

        player.setCoordinates(PLAYER_PADDLE_X, SCREEN_HEIGHT / 2);
        enemy.setCoordinates(ENEMY_PADDLE_X, SCREEN_HEIGHT / 2);
        ball.image.setCoordinates(SCREEN_HEIGHT / 2, SCREEN_WIDTH / 2);
	ball.d = DOWN_LEFT;
        ball.xspeed = ball_speed;
        ball.yspeed = ball_speed;
	Menu();
	GameLogic();
	Saketh::shutdown();
	return 0;
}
 void GameLogic()
	  {

        while(isRunning == true)
        {
                SDL_PollEvent(&event);

                if(event.type == SDL_QUIT)
		  {
		    while(!g_StateStack.empty()) g_StateStack.pop();
                        isRunning = false;
		  }
                if(event.type == SDL_KEYDOWN)
                {
                        keyDown = event.key.keysym.sym;
                        if(keyDown == SDLK_ESCAPE)
			  g_StateStack.pop();
                        if(keyDown == SDLK_UP)
                        {
                                if(Saketh::checkbounds(&player)) player.incrementCoordinates(0, -PADDLE_SPEED);
                        }
                        if(keyDown == SDLK_DOWN)
                        {
                                if(Saketh::checkbounds(&player)) player.incrementCoordinates(0, PADDLE_SPEED);
                        }
                }
                Saketh::set_within_bounds(&player);
                Saketh::set_within_bounds(&enemy);
                //Saketh::set_within_bounds(&ball.image);
                switch(ball.d)
                {
                        case RIGHT:
                                ball.image.incrementCoordinates(ball.xspeed, 0);                                break;
                        case UP_RIGHT:
                                ball.image.incrementCoordinates(ball.xspeed, ball.yspeed);
                                break;
                        case UP:
                                ball.image.incrementCoordinates(0, ball.yspeed);                                break;
                        case UP_LEFT:
                                ball.image.incrementCoordinates(-ball.xspeed, -ball.yspeed);
                                break;
                        case LEFT:
                                ball.image.incrementCoordinates(-ball.xspeed, 0);
                                break;
                        case DOWN_LEFT:
                                ball.image.incrementCoordinates(-ball.xspeed, -ball.yspeed);
                                break;
                        case DOWN:
                                ball.image.incrementCoordinates(0, -ball.yspeed);
                                break;
                        case DOWN_RIGHT:
                                ball.image.incrementCoordinates(ball.xspeed, -ball.yspeed);
                                break;
                }
                if(event.type == SDL_MOUSEBUTTONDOWN);
                if(event.type == SDL_MOUSEBUTTONUP);
                if(event.type == SDL_MOUSEMOTION);

                //background.draw(); //draw the background before the sprites, or it will cover them
                player.draw();
                enemy.draw();

                if((ball.image.getX() >=  SCREEN_WIDTH)) Saketh::drawText(screen, "You win!", 50, 0, 0, 255, 0, 0, 0, 0, 0);
		if((ball.image.getX() <= 0)) Saketh::drawText(screen, "You lose!", 50, 0, 0, 255, 0, 0, 0, 0, 0);;//TODO: Set up a score system
                if(((ball.image.getY()+ball.image.getImage()->h) >= SCREEN_HEIGHT)||((ball.image.getY()+ball.image.getImage()->h) <= 0))
                  {
		    ball.yspeed *= -1;
                  }


                ball.image.draw();

                if(SDL_BoundingCollide(player.getImage(), player.getX(), player.getY(), ball.image.getImage(), ball.image.getX(), ball.image.getY()) > 0)
                {
                switch(ball.d)
                {
                        case RIGHT:
                                ball.d = LEFT;
                                break;
                        case UP:
                                ball.d = DOWN;
                                break;
                        case UP_LEFT:
                                ball.d = UP_RIGHT;
                                break;
                        case LEFT:
                                ball.d = RIGHT;
                                break;
                        case DOWN_LEFT:
                                ball.d = DOWN_RIGHT;
                                break;
                        case DOWN:
                                ball.d = UP;
                                break;
                        case DOWN_RIGHT:
                                ball.d = UP_LEFT;
                                break;
                }
		ball.xspeed += 1;
		ball.yspeed += 1;
                }
                if(SDL_BoundingCollide(enemy.getImage(), enemy.getX(), enemy.getY(), ball.image.getImage(), ball.image.getX(), ball.image.getY()) > 0)
                {
                switch(ball.d)
                {
                        case RIGHT:
                                ball.d = LEFT;
                                break;
                        case UP_RIGHT:
                                ball.d = UP_LEFT;
                                break;
                        case UP:
                                ball.d = DOWN;
                                break;
                        case LEFT:
                                ball.d = RIGHT;
                                break;
                        case DOWN_LEFT:
                                ball.d = DOWN_RIGHT;
                                break;
                        case DOWN:
                                ball.d = UP;
                                break;
                        case DOWN_RIGHT:
                                ball.d = DOWN_LEFT;
                                break;
                }
		ball.xspeed +=1;
		ball.yspeed +=1;
                }
                if((ball.image.getY() > enemy.getY())) enemy.incrementCoordinates(0, PADDLE_SPEED);
                if((ball.image.getY() < enemy.getY())) enemy.incrementCoordinates(0, -PADDLE_SPEED);


                //enemy.setY(ball.image.getY() - (enemy.getImage()->h / 2));
                SDL_Flip(screen);
                SDL_FillRect(screen, NULL, SDL_MapRGB(screen->format, 0, 0, 0));        }
}

The biggest problem, in my opinion, is the collision and bouncing of the ball. It seems to be very hacked together. And incrementCoordinates should never have seen the light of day. However, I am very proud of my work because I can actually play it. I just want to know how to have good programming practices before I fall into an inescapable rut. Thank you for your help!

Share this post


Link to post
Share on other sites
Advertisement
CodeTitan,

I'm not a dedicated programmer, so I can't address the cleanliness of your code. But you may be interested in checking out an article that appeared in this month's issue of Game Developer (January, '06). It's called "Mature Optimization", and is featured in "The Inner Product" column by Mick West.

Again, I'm not a programmer, but it was a logical and very informative read.

Regards,

Share this post


Link to post
Share on other sites
I think it's best to tackle small bits of this problem at a time, so I'll point out the function which is causing the most grievance.

GameLogic()

void GameLogic()
{

while(isRunning == true)
{
SDL_PollEvent(&event);

if(event.type == SDL_QUIT)
{
while(!g_StateStack.empty()) g_StateStack.pop();
isRunning = false;
}
if(event.type == SDL_KEYDOWN)
{
keyDown = event.key.keysym.sym;
if(keyDown == SDLK_ESCAPE)
g_StateStack.pop();
if(keyDown == SDLK_UP)
{
if(Saketh::checkbounds(&player)) player.incrementCoordinates(0, -PADDLE_SPEED);
}
if(keyDown == SDLK_DOWN)
{
if(Saketh::checkbounds(&player)) player.incrementCoordinates(0, PADDLE_SPEED);
}
}
Saketh::set_within_bounds(&player);
Saketh::set_within_bounds(&enemy);
//Saketh::set_within_bounds(&ball.image);
switch(ball.d)
{
case RIGHT:
ball.image.incrementCoordinates(ball.xspeed, 0); break;
case UP_RIGHT:
ball.image.incrementCoordinates(ball.xspeed, ball.yspeed);
break;
case UP:
ball.image.incrementCoordinates(0, ball.yspeed); break;
case UP_LEFT:
ball.image.incrementCoordinates(-ball.xspeed, -ball.yspeed);
break;
case LEFT:
ball.image.incrementCoordinates(-ball.xspeed, 0);
break;
case DOWN_LEFT:
ball.image.incrementCoordinates(-ball.xspeed, -ball.yspeed);
break;
case DOWN:
ball.image.incrementCoordinates(0, -ball.yspeed);
break;
case DOWN_RIGHT:
ball.image.incrementCoordinates(ball.xspeed, -ball.yspeed);
break;
}
if(event.type == SDL_MOUSEBUTTONDOWN);
if(event.type == SDL_MOUSEBUTTONUP);
if(event.type == SDL_MOUSEMOTION);

//background.draw(); //draw the background before the sprites, or it will cover them
player.draw();
enemy.draw();

if((ball.image.getX() >= SCREEN_WIDTH)) Saketh::drawText(screen, "You win!", 50, 0, 0, 255, 0, 0, 0, 0, 0);
if((ball.image.getX() <= 0)) Saketh::drawText(screen, "You lose!", 50, 0, 0, 255, 0, 0, 0, 0, 0);;//TODO: Set up a score system
if(((ball.image.getY()+ball.image.getImage()->h) >= SCREEN_HEIGHT)||((ball.image.getY()+ball.image.getImage()->h) <= 0))
{
ball.yspeed *= -1;
}


ball.image.draw();

if(SDL_BoundingCollide(player.getImage(), player.getX(), player.getY(), ball.image.getImage(), ball.image.getX(), ball.image.getY()) > 0)
{
switch(ball.d)
{
case RIGHT:
ball.d = LEFT;
break;
case UP:
ball.d = DOWN;
break;
case UP_LEFT:
ball.d = UP_RIGHT;
break;
case LEFT:
ball.d = RIGHT;
break;
case DOWN_LEFT:
ball.d = DOWN_RIGHT;
break;
case DOWN:
ball.d = UP;
break;
case DOWN_RIGHT:
ball.d = UP_LEFT;
break;
}
ball.xspeed += 1;
ball.yspeed += 1;
}
if(SDL_BoundingCollide(enemy.getImage(), enemy.getX(), enemy.getY(), ball.image.getImage(), ball.image.getX(), ball.image.getY()) > 0)
{
switch(ball.d)
{
case RIGHT:
ball.d = LEFT;
break;
case UP_RIGHT:
ball.d = UP_LEFT;
break;
case UP:
ball.d = DOWN;
break;
case LEFT:
ball.d = RIGHT;
break;
case DOWN_LEFT:
ball.d = DOWN_RIGHT;
break;
case DOWN:
ball.d = UP;
break;
case DOWN_RIGHT:
ball.d = DOWN_LEFT;
break;
}
ball.xspeed +=1;
ball.yspeed +=1;
}
if((ball.image.getY() > enemy.getY())) enemy.incrementCoordinates(0, PADDLE_SPEED);
if((ball.image.getY() < enemy.getY())) enemy.incrementCoordinates(0, -PADDLE_SPEED);


//enemy.setY(ball.image.getY() - (enemy.getImage()->h / 2));
SDL_Flip(screen);
SDL_FillRect(screen, NULL, SDL_MapRGB(screen->format, 0, 0, 0)); }
}


Does anyone have a better method of checking if the ball has collided, and bouncing it off?

I was thinking of using vectors, by giving the ball a vector that describes the magnitude and direction of its velocity. This solution is appealing to me, but I do not know if it is the most efficient way.

Share this post


Link to post
Share on other sites
Add a "vector" member to the ball class. A vector describes the velocity and direction of movement, so you can get rid of the xspeed/yspeed and direction members.
struct Vector  // obviously has nothing to do with the STL vector
{
double x, y;
};
The sign (positive or negative) of the x and y indicates the direction. So you can replace that switch statement with:
ball.image.incrementCoordinates(ball.vec.x, ball.vec.y)
To "reflect" the ball, just multiply one (or both) of its component vectors (the x or the y) by a negative number. -1 will maintain the same speed. -2 will double it (you might want to go for something like -1.05, which is a 5% increase or just -1 which is no increase).

EDIT: And definitely split the code up into multiple files.

Share this post


Link to post
Share on other sites
Quote:
Original post by CodeTitan
I probably broke every single rule that programmers follow.


I see no goto.. so you missed one.

Share this post


Link to post
Share on other sites
Quote:
Original post by CodeTitan
I am new to game programming, so please bear with me. I am very repulsed by the ugliness of the code that I have written - I probably broke every single rule that programmers follow. Without further ado, here is the code.
*** Source Snippet Removed ***
The biggest problem, in my opinion, is the collision and bouncing of the ball. It seems to be very hacked together. And incrementCoordinates should never have seen the light of day.

However, I am very proud of my work because I can actually play it. I just want to know how to have good programming practices before I fall into an inescapable rut.

Thank you for your help!


Two parts: The code review about you breaking every signle rule, and second, the collision.

Code Review


You have a bunch of named constants at the top. That's good. Some of thouse should be moved to configuration files: screen dimensions could be adjusted without wanting to recompile.

I'd move your structure and enum declarations into a separate header file, but that's more for cleanup.

I don't see anything wrong with a cursory glance at the Sprite class, other than a complete lack of documentation.

Your menu function is trying to do both rendering and processing. It's generally a good idea to separate those out. The menu should adjust objects on the rendering side and adjust the menu state, but not do any actual rendering.

Also, the menu function is much too long for my tastes.

Game calling GameLogic might be a bit of an undersimplification, unless you plan on doing more work in there.

Exit has basically the same issues as Menu.

All the ball functions look like they should be inside a class.

Your main function could be cleaned up by adding comments, extracting all the initialization code over to the function that does the initialization, and adding a more elegant way to work with the menu. Otherwise, it's more or less good enough for this game.

Your game logic function looks like it should be broken into about 10 other functions. If you get a paddle-related event, send it on to the paddle motion handler. It looks like you are adjusting the ball position on *all* events, rather than on a regular timer. I'd put that into a separate set of functions (perhaps the ball class?) so it doesn't clutter up this function.

I'd cut the empty if statements, unless you have a reason for them.

Your ball collision code should end up in the ball class or some other world manipulation class.
Your rendering and screen flipping should also end up in a separate class.

Summary:

Add documentation about why you do the things you are doing. Your existing comments describe why you did it that way, which is very good.
Encapsulate the ball object and related content into a class.
Encapsulate all the rednering into a single location and class, and have your menu and other UI elements work with the rendering object rather than rendering their own content.
Call your physics (ball motion, paddle motion) at regular fixed intervals rather than at every event.

There are many style and structural changes I'd make in my own code, but that's more of personal preference than anything.

Collisions


The incrementCoordinates could easliy be handled with a simple delta vector (as was mentioned earlier). [delta means rate of change, just in case you don't know that.]

With a good vector library, it would look something like this:
/* motion is already set to +, -, or 0, depending on the input */
velocity += motion;
/* Check for collision */
if( (velocity+position).x > Screen.X ) /* do stuff, updating the velocity or position or whatever else you need */
...
/* Do the motion */
position += velocity;


There are both more elegant and more complex ways to do that, but that is all it really requires.

Share this post


Link to post
Share on other sites
Quote:
Original post by Maega
Quote:
Original post by CodeTitan
I probably broke every single rule that programmers follow.


I see no goto.. so you missed one.


I see no macros either... [attention]

Share this post


Link to post
Share on other sites
Quote:
Original post by Thevenin
Quote:
Original post by Maega
Quote:
Original post by CodeTitan
I probably broke every single rule that programmers follow.


I see no goto.. so you missed one.


I see no macros either... [attention]

I was exaggerating.

Share this post


Link to post
Share on other sites
I would recommend reading "Code Complete 2nd Edition" on how to design good code...that's what you're really trying to get at with your question. :)

As for your code, I'd definately break up that huge mass of code in your "GameLogic" section into smaller functions.

I'd put more white space between your lines of code, and perhaps a few more comments on what does what.
In my functions, I have this commenting standard:


int MyFunction(int Param1, int Param2, bool Param3)
{
/*
PURPOSE:
This function returns the sum of two numbers given as input parameters
INPUT:
Param1: The first number to add
Param2: The second number to add
Param3: If you think the wookies are going to be a part of the mars landing team
OUTPUT:
This function returns the sum of the first two input parameters
Notes:
If you set Param3 to "true" then the code in this function does something strange.
*/


//strange code for param3

return (Param1+Param2);
}




In team projects, people like to also add who created/last modified a function, and when the last change was made...probably not necessary in this project, unless you really feel like it. Might be a good habit?

Share this post


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

  • Advertisement