Well my main() function has become a mess... What should I do next time to prevent this?

int main(int argc, char *args[])
{
	srand((unsigned)time(0));

	if(!init())
		return 1;

	atexit(cleanup); // to cleanup the surfaces
	
	// load images
	background = loadImage("background.jpg");
	message = loadImage("play_again.jpg");
	youWinMessage = loadImage("you_win.jpg");
	youLoseMessage = loadImage("you_lose.jpg");

start:
	// the ball
	SDL_Rect bounds; bounds.x = 0; bounds.y = 0; bounds.w = SCREEN_WIDTH; bounds.h = SCREEN_HEIGHT;
	Sprite ball(SCREEN_WIDTH / 2, SCREEN_HEIGHT / 2, loadImage("ball.gif"), bounds );
	ball.setStep(BALL_SPEED);
	randomizeDirections(ball);

	//Rectangles for the player and the computer
	Rect player(SCREEN_WIDTH / 2 - RECT_WIDTH / 2, SCREEN_HEIGHT - int(RECT_HEIGHT * 1.5), RECT_WIDTH, RECT_HEIGHT),
		computer(SCREEN_WIDTH / 2 - RECT_WIDTH / 2, int(RECT_HEIGHT * 1.5) - RECT_HEIGHT, RECT_WIDTH, RECT_HEIGHT);

	player.setColor(rand() % 256, rand() % 256, rand() % 256);
	computer.setColor(rand() % 256, rand() % 256, rand() % 256);
	
	bool playerWins = false, computerWins = false;

	if(SDL_Flip(screen) == -1)
		return 1;

	timer.start(); // start the timers
	bool quit = false; // the quit flag

	//**************************MAIN LOOP****************************************************
	while(!quit)
	{
		if(timer.getTicks() % 30 == 0)
		{
			//apply the background
			applySurface(0, 0, background, screen);

			// handle events
			while( SDL_PollEvent( &event ) )
			{
				//If the user has Xed out the window
				if(event.type == SDL_QUIT)
					quit = true;
				if(keystates[SDLK_ESCAPE])
					quit = true;

				if(keystates[SDLK_LEFT])
				{
					if(player.x >= 0)
						player.x -= STEP;
				}
				if(keystates[SDLK_RIGHT])
				{
					if(player.x + player.Width() <= SCREEN_WIDTH)
						player.x += STEP;
				}
				if(keystates[SDLK_UP])
				{
					if(player.y == Sint16(SCREEN_HEIGHT - int(RECT_HEIGHT) * 1.5))
						player.y -= STEP;
				}
				else
				{
					if(player.y < Sint16(SCREEN_HEIGHT - int(RECT_HEIGHT) * 1.5))
						player.y += STEP;
				}

				if(keystates[SDLK_RETURN])
				{
					if(playerWins || computerWins)
					{
						playerWins = false;
						computerWins = false;
						goto start;
					}
				}
			}
		
			if(timer.isStarted())
			{
				//calcuate the computer's move and motion of the ball
				AI(computer, ball);
				ball.move();

				if(ball.Y() + ball.H() + ball.getStep() - 2 >= SCREEN_HEIGHT)
				{
					computerWins = true;
					timer.stop();
				}
				if(ball.Y() - ball.getStep() + 5 <= 0)
				{
					playerWins = true;
					timer.stop();
				}
				
				//check for collision
				bool thereWasCollision = false;
				if(collision(ball.getOffset(), player.getSDL_Rect()))
				{
					ball.setDirectionY(Sprite::TO_ZERO);
					changeDirectionOnCollision(ball, player.getSDL_Rect());
					thereWasCollision = true;
				}
				if(collision(ball.getOffset(), computer.getSDL_Rect()))
				{		
					ball.setDirectionY(Sprite::TO_INFINITY);
					changeDirectionOnCollision(ball, computer.getSDL_Rect());
					thereWasCollision = true;
				}
				
				if(thereWasCollision)
					if(ball.getStep() < BALL_SPEED_MAX)
						ball.setStep(ball.getStep() + 1);
			}

			//draw
			player.draw();
			computer.draw();
			applySurface(ball.X(), ball.Y(), ball.getImage(), screen);

			if(playerWins || computerWins)
			{
				applySurface( (SCREEN_WIDTH - message->w) / 2, (SCREEN_HEIGHT + message->h + 10) / 2, message, screen);
				
				if(playerWins)
					applySurface( (SCREEN_WIDTH - youWinMessage->w) / 2, (SCREEN_HEIGHT - youWinMessage->h - 10) / 2, youWinMessage, screen);
				if(computerWins)
					applySurface( (SCREEN_WIDTH - youLoseMessage->w) / 2, (SCREEN_HEIGHT - youLoseMessage->h - 10) / 2, youLoseMessage, screen);
			}
			

			if(SDL_Flip(screen) == -1) return 1;
		}
	}

	SDL_Quit();
	return 0;
}

Dani AI

Generated

Short version: make main a tiny coordinator and move everything else into small, well-named modules. As suggested, start with a simple design; as pointed out, remove the goto and the one-big-function smell. Below are concrete, low-effort steps to refactor and some concrete alternatives for collision handling that answer ’s follow-up.

Split responsibilities

  • Keep main to: create Game object, call game.init(), game.run(), and return. Put init/cleanup in RAII-style objects rather than atexit.
  • Group code into clear subsystems: ResourceManager (images/sounds), InputManager, Physics/CollisionSystem, AI, Renderer, and a Game/Scene controller that owns the loop and game state.

Small API sketches (keep these in headers, not in main)

class Game {
public:
  bool init();
  void loadResources();
  void run();
  void resetRound();
  ~Game();
private:
  void handleEvents();
  void update(float dt);
  void render();
};

Collision design options

  • Simple: give each entity a Rect bounds() const and call a reusable bool intersects(a,b) free function. This keeps geometry tests isolated and easy to unit-test.
  • Component/Engine style: attach a Collider to entities and run a CollisionSystem that does broad-phase (AABB or spatial hash) then narrow-phase (circle/AABB/SAT). The system returns collision manifolds for response.
  • If you prefer OO, implement bool collidesWith(const Entity& other) const but keep the geometric tests in free functions or a small collision utilities module — don’t mix rendering/state changes into those functions.

Timing, loop and responsiveness
Use a fixed-step update + variable render pattern (accumulator) rather than timer.getTicks() % n == 0. Example:

const float STEP = 1.0f/60;
float acc = 0;
while(running) {
  acc += deltaTime();
  while(acc >= STEP){ handleInput(); update(STEP); acc -= STEP; }
  render();
}

Final tips: avoid globals for surfaces, use smart pointers or RAII to free SDL surfaces, test collision math separately, and keep functions short (if you can’t see a function’s braces on one screen, it’s probably doing too much).

Recommended Answers

All 4 Replies

> What should I do next time to prevent this?
Start with a design.

Even simple things like

do {
  if ( inputPresent ) {
    doInput();
  }
  moveEnemies();
  dead = checkGameOver();
  redrawScreen();
} while ( !dead );

Also, the moment you can no longer see the { and } of the whole of main (or any other function) on screen, then STOP and think about your code structure for a few minutes, rather than just carrying on typing.

One thing about collisions. At the moment I'm using functions that take 2 arguments. Would it be better if each object had a collision function so I could say:

Sprite s;
s.collision(some_rect_type rect); // returns bool

Is there a better alternative?

I dunno - you need to look at the overall design and implementation, before deciding whether specific local improvements are a good idea.

When you're into writing larger programs, there isn't a single "best way" of doing anything.

Except for not using goto's :P

edit: and using functions/methods is always a plus.

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.