Jump to content
  • Advertisement
Sign in to follow this  

Programming Techniques

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

Well recently, I figured I'd try to change the way I've been programming. I learned how to spread out data through multiple .cXX and .h files as opposed to just using a bunch of headers. My post here isn't geared towards an API or working with something really, I just want to know if the way I'm setting things up is garbage or what. Basically I've got a small SDL\OpenGL application that displays an image on the screen. I myself was simply surprised that any of it worked as usually when I try something new, everything fails.Right now, all I'm really worried about is the way I'm including header files and the way I'm handling variables a little bit differently. One thing I'm interested in is that for certain functions that need an integer or a struct I do &nameOfVariable so I don't have to return it and I can keep as many local variables as possible.

Another thing that I'm doing is making an int Game() loop, and what I do in my int main() is return Game() at the end of it. Alright, I've talked about my major concerns, I'll put up my code. I know it's quite a bit of code, I'm just asking for a skim over it and tell me if there's any major concern that pop out at you. I'm not looking for my code to be fixed, I just want to make sure that what I am doing isn't bad. Any suggestions are greatly appreciated.

Main.cpp


#include <SDL.h>
#include <SDL_audio.h>
#include <SDL_opengl.h>
#include "Init.h"
#include "Game.h"
int main(int argc, char* argv[])
{
Screen screen;
if (Initialize(0, screen, 0, 0, 0, SDL_HWSURFACE | SDL_HWACCEL | SDL_OPENGL | SDL_FULLSCREEN) < 0)
{
printf("Error: %s\n", SDL_GetError());
return -1;
}
return Game(screen);
}


Init.h:


#ifndef SCREEN_H
#define SCREEN_H
#include "Screen.h"
#endif
int Initialize(int toggleCursor, Screen &screen, int Width, int Height, int BitsPerPixel, Uint32 Flags);


Init.cpp:


#include <SDL.h>
#include <SDL_opengl.h>
#include "Init.h"
int Initialize(int toggleCursor, Screen &screen, int Width, int Height, int BitsPerPixel, Uint32 Flags)
{
if (SDL_Init(SDL_INIT_EVERYTHING) < 0)
return -1;
atexit(SDL_Quit);
SDL_ShowCursor(toggleCursor);
if (Width == 0 || Height == 0 || BitsPerPixel == 0)
{
screen.videoInfo = (SDL_VideoInfo*)SDL_GetVideoInfo();
if (Width != 0 && Height != 0)
{
screen.Width = Width;
screen.Height = Height;
}
else
{
screen.Width = screen.videoInfo->current_w;
screen.Height = screen.videoInfo->current_h;
}
if (BitsPerPixel == 0)
{
screen.BitsPerPixel = screen.videoInfo->vfmt->BitsPerPixel;
screen.BytesPerPixel = screen.videoInfo->vfmt->BytesPerPixel;
}
else
{
screen.BitsPerPixel = BitsPerPixel;
screen.BytesPerPixel = BitsPerPixel / 8;
}
screen.Flags = Flags;
}
else
{
screen.Width = Width;
screen.Height = Height;
screen.BitsPerPixel = BitsPerPixel;
screen.Flags = Flags;
screen.BytesPerPixel = BitsPerPixel / 8;
}
if (!(SDL_SetVideoMode(screen.Width, screen.Height, screen.BitsPerPixel, screen.Flags)))
return -1;
glMatrixMode(GL_PROJECTION);
glLoadIdentity();
glOrtho(0, screen.Width, screen.Height, 0, -1, 1);
glMatrixMode(GL_MODELVIEW);
glLoadIdentity();
glClearColor(0, 0, 0, 0);
glClear(GL_COLOR_BUFFER_BIT);

return 0;
}


Game.h:


#ifndef SCREEN_H
#define SCREEN_H
#include "Screen.h"
#endif
int Game(Screen &screen);


Game.cpp:


#include <SDL.h>
#include <SDL_opengl.h>
#include "Game.h"
#include "Rendering.h"
void EventHandle(int &quit, SDL_Event &event)
{
switch (event.type)
{
case SDL_QUIT:
quit = 1;
break;
case SDL_KEYDOWN:
switch (event.key.keysym.sym)
{
case SDLK_ESCAPE:
quit = 1;
break;
default:
break;
}
break;
default:
break;
}
}
int Game(Screen &screen)
{
SDL_Event event;
int quit = 0;
GLuint IMAGE;
IMAGE = LoadTexture("Image.jpg");
if (!IMAGE)
return -1;
while (!quit)
{
Render2D(0, 0, screen.Width, screen.Height, IMAGE);
if (SDL_PollEvent(&event))
{
EventHandle(quit, event);
}
SDL_GL_SwapBuffers();
}
glDeleteTextures(1, &IMAGE);
return 0;
}


Rendering.h:


GLuint LoadTexture(const char* fileName);
void Render2D(float X, float Y, float Width, float Height, GLuint texture);


Rendering.cpp:


#include <SDL.h>
#include <SDL_image.h>
#include <SDL_opengl.h>
#include "Rendering.h"
GLuint LoadTexture(const char* fileName)
{
SDL_Surface* Image = IMG_Load(fileName);
Image = SDL_DisplayFormatAlpha(Image);
if (!Image)
return NULL;
GLuint Texture;
GLenum TextureFormat;
switch(Image->format->BytesPerPixel)
{
case 3:
if (Image->format->Rmask == 0x000000ff)
TextureFormat = GL_RGB;
else
TextureFormat = GL_BGR;
break;
case 4:
if (Image->format->Rmask == 0x000000ff)
TextureFormat = GL_RGBA;
else
TextureFormat = GL_BGRA;
break;
default:
return NULL;
break;
}
glGenTextures(1, &Texture);
glBindTexture(GL_TEXTURE_2D, Texture);
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
glTexImage2D(GL_TEXTURE_2D, 0, Image->format->BytesPerPixel, Image->w, Image->h, 0, TextureFormat, GL_UNSIGNED_BYTE, Image->pixels);
SDL_FreeSurface(Image);
return Texture;
}
void Render2D(float X, float Y, float Width, float Height, GLuint texture)
{
glEnable(GL_TEXTURE_2D);
glPushMatrix();
if (texture) glBindTexture(GL_TEXTURE_2D, texture);
glBegin(GL_QUADS);
glTranslatef(X, Y, 0.0f);
if (texture) glTexCoord2i(0, 0); glVertex3f(0.0f, 0.0f, 0.0f);
if (texture) glTexCoord2i(1, 0); glVertex3f(Width, 0.0f, 0.0f);
if (texture) glTexCoord2i(1, 1); glVertex3f(Width, Height, 0.0f);
if (texture) glTexCoord2i(0, 1); glVertex3f(0.0f, Height, 0.0f);
glEnd();
glPopMatrix();
glDisable(GL_TEXTURE_2D);
}


Screen.h:


typedef struct Screen
{
unsigned int Width, Height, BitsPerPixel, BytesPerPixel;
Uint32 Flags;
SDL_VideoInfo* videoInfo;
} Screen;

Share this post


Link to post
Share on other sites
Advertisement
int Game(Screen &screen) doesn't modify screen, so you'd better pass screen as reference to const (const Screen & screen).
Also a function should be better a verb than a noun because a function does some action.
So UdateGame may be better than Game?

Share this post


Link to post
Share on other sites
I don't see anything major wrong. You are doing procedural programming, which is fine. I have only couple suggestions (nitpicking really)

- Why not change[font=monospace] [/font]void EventHandle(int &quit, SDL_Event &event) to int HandleEvent(SDL_Event &event)
- Rename Game() to RunGame()
- Try to be consistent in naming local variables (quit, IMAGE, Texture)
- Return 0 instead of NULL in LoadTexture()
- Declare local variables as late as possible
- Drop the typedef from struct declaration (just struct Screen {...}; )

Share this post


Link to post
Share on other sites
Unless you mean to change a primitive type (char, short, int, long,float and their unsigned versions) it makes no sense to pass them as a reference as they are going on the stack or passed through a register of the same size as the reference.

Also if a function doesn't change it's input variables and those are passed by reference pass them as const references so the caller knows that his data won't be changed. Don't mark primitive types as const parameters for functions as it doesn't matter the function can take a copy of them and change the values all the same.
Once you move to classes make sure that methods that aren't changing the internal state of the object are marked as const as well, this allows you to call these functions on const instances of that class.

Share this post


Link to post
Share on other sites
There are a couple of things that I've found really improve maintainability and readability (I tend towards OO and generics, but these should apply equally to Procedural):

Decompose functions. Every function should be short (2-10 lines) and every statement should be at the same level of abstraction. This will do wonders to avoid duplication and (if you name your functions well) dramatically improve readability. If you have a long function, and you're not sure how to split it up, first just chunk it into blocks, then look at what each of those blocks does. Whenever you end up with two short functions that are similar, try to make them identical and then remove one.

Maintain invariants, preconditions, and post conditions. I am not as inflexible about this as some (I generally don't assert them, etc.) but basically, if your function claims to do something, it should either do that thing or very clearly not do that thing. If you don't like to use exceptions, this can be tricky, but if you define (even just in your head) the input domain and output range and then stick to that, you will have far fewer edge cases. A simple way to do this if you don't want to deal with exceptions is to wrap primitive data types (such as the uints that opengl uses for everything) in validating structs. The struct might take a uint in the constructor, but will signal an error if that value is not valid.

If you are interested in programming style, the book Clean Code by Robert Martin is excellent, but is definitely aimed more at OO design :/

Share this post


Link to post
Share on other sites
Well right now I'm working with a procedural design. As a matter of fact I'd rather not use C++ if at all possible, right now. I've been working on ripping apart everything to see how it works myself, so I can understand it better. I don't like building off of someone else's code. I mean like if I wanted a game engine, I would rather make it myself so I know how it's supposed to work and I don't have to worry about screwing things up because a tutorial or documentation is out of date, or because I simply missed a step. It's why I'm moving closer to Assembly language and eventually creating my own OS, so I know how it works.

Eventually I'll go back to at least a semi object oriented design. I guess with all that said, I'd be more on the end of a software developer than a game developer. I can put all of the fancy math together, but I couldn't make a good 3D model or a 2D image to save my life.

Again thanks, but I ask again, why would I remove the typedef from the struct?

Share this post


Link to post
Share on other sites

Again thanks, but I ask again, why would I remove the typedef from the struct?

You can leave the typedef there.
But nowadays in C++ it's common to use "struct A" instead of "typedef struct A". The later is more C style.
There is difference of them in C, but not in C++.
http://stackoverflow.com/questions/612328/difference-between-struct-and-typedef-struct-in-c

Share this post


Link to post
Share on other sites

There are a couple of things that I've found really improve maintainability and readability (I tend towards OO and generics, but these should apply equally to Procedural):

Decompose functions. Every function should be short (2-10 lines) and every statement should be at the same level of abstraction. This will do wonders to avoid duplication and (if you name your functions well) dramatically improve readability. If you have a long function, and you're not sure how to split it up, first just chunk it into blocks, then look at what each of those blocks does. Whenever you end up with two short functions that are similar, try to make them identical and then remove one.


Ten lines seems very small. Some of my functions have gotten smaller as I've worked on them, but the average is more than ten lines. I asked for advice about the code here a few months ago and no one said anything about the functions being too long.

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!