Monochrome Programming: Structs and Wrappers

PROGRAMMING PART 2

/*Once again wordpress has issues with tabs, ignore the complete lack of horizontal spacing for all these programming posts. This is frustrating, since it makes the code mostly unreadable here, which defeats much of the purpose.*/

Another big mistake I made with the game was not wrapping things up into more readable forms. This post is basically going to be massively heavy on examples, and less on theory. Honestly I much prefer it that way.

WRAPPERS

Here’s an example of some code I was using for loading in sound effects, and is an example of a “wrapper” done fairly well:

///Load in Mix_Chunk, returns false if error
bool LoadSFX(std::string path, Mix_Chunk** ppChunk)
{
Mix_Chunk* pTemp = NULL;
pTemp = Mix_LoadWAV(path.c_str());
if (pTemp == NULL){
printf(“Failed to load sound effect at %s\n”, path.c_str());
return false;
}

*ppChunk = pTemp;
return true;
}

It is much preferable to call:

Mix_Chunk* pSFX=NULL;
if(!LoadSFX(tdcDirectory+”\\music\\musicClipNameHere.wav”, &pSFX)) return -1;

Than:

Mix_Chunk* pSFX=NULL;
std::string path = tdcDirectory+”\\music\\musicClipNameHere.wav”;
pSFX=Mix_LoadWAV(path.c_str());
if(pSFX==NULL){
printf(“Failed to load in sample\n”);
return -1;
}

For every time that we have to load in a sound effect. The second example is actually worse because we have the path string hanging around and being overwritten if we have to load multiple different SFX. Loading in 20 different sound effects with the first code is going to be a few lines of declaring the pointers, and then 20 lines to load them all in. Loading in 20 different sound effects with the second code is going to be 20 lines to declare the Mic_Chunks (which need to be 20 lines, because they all need to be explicitly NULL), and then 100 lines to load them all in. That’s at least 100 more lines of code, and probably more like 118 more lines of code, for the exact same functionality.

That’s a massive waste of vertical real estate, and it’s something that I didn’t think was going to be a problem until I had 172 lines of code in my BATTLE game state file, just for loading in the textures and doing error checking. Even that was a lot better than it could have been, since I had a poorly designed wrapper already in place.

Instead of doing this every time we want to load in a texture.:

//BTW this also assumes pure white is alpha, and treats it as transparent.
SDL_Texture* pBlt = NULL;
SDL_Surface* pImg = NULL;
std::string path = “blahblah.png”;
pImg = IMG_Load(path.c_str());
if (pImg == NULL){
printf(“PNG not loaded. IMG Error: %s\n”, IMG_GetError());
return -1;
}
SDL_SetColorKey(pImg, SDL_TRUE, SDL_MapRGB(pImg->format, 255, 255, 255));
pBlt = SDL_CreateTextureFromSurface(renderer, pImg);    //renderer already part of code
if (pBlt == NULL){
printf(“Surface to texture conversion failed. Error: %s\n”, SDL_GetError());
return -1;
}
SDL_FreeSurface(pImg);

We write our wrapper function and stick in some image loading file:

///load in a PNG and convert it to a texture.///load in a PNG and convert it to a texture.
bool LoadTexture(std::string path, SDL_Texture** ppTextImg, SDL_Renderer* pRend)
{
SDL_Surface* pImg = NULL;
SDL_Texture* pTemp = NULL;
pImg = IMG_Load(path.c_str());
if (pImg == NULL){
printf(“PNG not loaded. IMG Error: %s\n”, IMG_GetError());
return false;
}
SDL_SetColorKey(pImg, SDL_TRUE, SDL_MapRGB(pImg->format, 255, 255, 255));
pTemp = SDL_CreateTextureFromSurface(pRend, pImg);
if (pTemp == NULL){
printf(“Surface to texture conversion failed. Error: %s\n”, SDL_GetError());
return false;
}
//we again need to free the surface
SDL_FreeSurface(pImg);

*ppTextImg = pTemp;
return true;
}

And then in our code we just write:

SDL_Texture* pBlt=NULL;
if (!LoadTexture(tdcDirectoryPath+”\\bolt.png”, &pBlt, pFrame)){
std::cout << “Error loading projectile image\n”;
return 1;
}

Which is much, much better, even if there’s only a few images that need to get loaded. BATTLE game mode had 35 images it needed to load, and while this was good, we can do much better.

If we can accept not explicitly making the SDL_Texture*’s explicitly NULL, and we can accept error messages to be written as (Cant load “fileName”) instead of personalized (which I can absolutely accept because I got so sick of writing those messages that I ended up not writing them), then we can get our image loading down to just one line, and our SDL_Texture* declarations to however many we can fit horizontally on the page.

The changed function will look like this:

///load in a PNG and convert it to a texture.///load in a PNG and convert it to a texture.
bool LoadTexture(std::string path, SDL_Texture** ppTextImg, SDL_Renderer* pRend)
{
SDL_Surface* pImg = NULL;
SDL_Texture* pTemp = NULL;
pImg = IMG_Load(path.c_str());
if (pImg == NULL){
printf(“PNG not loaded. IMG Error: %s\n”, IMG_GetError());
return false;
}
SDL_SetColorKey(pImg, SDL_TRUE, SDL_MapRGB(pImg->format, 255, 255, 255));
pTemp = SDL_CreateTextureFromSurface(pRend, pImg);
if (pTemp == NULL){
printf(“Surface to texture conversion failed. Error: %s\n”, SDL_GetError());
return false;
}
//we again need to free the surface
SDL_FreeSurface(pImg);

*ppTextImg = pTemp;
return true;
}

/*You’ll notice that there is no change. I only realized at the end of the project that I already had errors by filename, which made personalizing the error messages pretty much pointless.*/

And we can write our code like this:

SDL_Texture* pBlt, pHunterImg, pPCImg;
if(!LoadTexture(path+”\\boltImg.png”) return 1;
if(!LoadTexture(path+”\\hunterImg.png”) return 1;
if(!LoadTexture(path+”\\pcImg.png”) return 1;

And this will get our image loading code down to reasonable levels.

But even that may not be good enough. We might decide that, since we know exactly what texture gets what .png loaded to it, we can create one single function that will load in all the textures for us. That function will look something like this:

LoadTexturesForBattleState(SDL_Texture** im1, SDL_Texture** im2, SDL_Texture** im3, const std::string path, SDL_Renderer* pRend){
if(!LoadTexture(path+”\\boltImg.png”, *im1, pRend) return 1;
if(!LoadTexture(path+”\\hunterImg.png”, *im2, pRend) return 1;
if(!LoadTexture(path+”\\pcImg.png”, *im3, pRend) return 1;
}

And now in our main code loading in our textures becomes two lines:
SDL_Texture* pBlt, pHunterImg, pPCImg;
LoadTexturesForBattleState(&pBlt, &pHunterImg, &pPCImg, path); //path defined earlier

This is great, but there is one major problem with it: if we have a lot of textures to load in, then our function is going to be a massive space hog. BattleState.cpp has 35 textures it needs to load in, which means the above function call would be roughly 10 times as wide as it is right now. That’s not necessarily a horrible problem, since it’s very easy to understand what the line does, but it’s far from ideal. Now, like I said, this is far from a show-stopping design flaw, and it is much preferable to have code that is written horribly on one line than scroll through 272 lines of redundant information, but we can still do better. This is going to require much bigger, and much more interesting changes to our code than before. I’ll detail this later, but the short version is that we’re going to have a linked list of pointers to all of the games textures that we load up at the intro screen, and delete at the outro screen.

An example of where I had some irritating repeated code, that was wrapped up nicely is the buttons for the menus. Because I wanted the buttons to respond to the user, when the mouse if over a button we render a slightly different image then if the mouse is not over the button. This was originally done quite poorly, where I first checked where the mouse was for handling events (an example would be to check if the mouse is over a button because if the mouse is pressed down we need to know what state we’re changin to), but then also checked where the mouse was for the rendering of each button. Example code:
//handling events. Very simplified.
if(MousePressedDown){
if(MouseOverQuitButton)*pState=QUIT;
if(MouseOverPlayButton)*pState=PLAY;
if(MouseOverCreditsButton)*pMenSt=CREDITS;
}

//rendering buttons
if(MouseOverQuitButton){
Render(quitButton1);
}else{
Render(quitButton2);
}
if(MouseOverPlayButton){
Render(playButton1);
}else{
Render(playButton2);
}
if(MouseOverCreditsButton){
Render(credButton1);
}else{
Render(credButton2);
}

This all looks much better than it did before, since before I had to check the x and y coordinates nakedly, but it still is an obvious waste of space. What I ended up doing was creating a button struct:

//The SDL_Rect and tdcRct are used for rendering and logic respectively.
struct button{
bool mouseOver;
SDL_Texture* im1;
SDL_Texture* im2;
SDL_Rect rnd;
tdcRct loc;
};

I had a function for setting up the location of the button and the corresponding location on the screen, as well as a function that freed the memory stored in the textures. I also had one very simple render function. Since we already know where to render the button, and this never changes, we can just take the button and the renderer, and render the proper image.

void RenderButton(const button bt, SDL_Renderer* pFrame){
if(bt.mouseOver){
SDL_RenderCopy(pFrame, bt.im2, NULL, &bt.rnd);
}else{
SDL_RenderCopy(pFrame, bt.im1, NULL, &bt.rnd);
}
}

This is great, as it makes our rendering calls in the code:

RenderButton(quit, pFrame);
RenderButton(play, pFrame);
RenderButton(cred, pFrame);
…and others.

It’s always nice to take away 75% of the code that just shamelessly wastes vertical space and thus readability, but the button struct has one additional benefit in that it simplifies the logic code as well. We can write our event checking code as:
MakeMouseOverFalse(&quit, &play, &cred);

if(MouseOverQuitButton)quit.mouseOver=TRUE;
if(MouseOverPlayButton)play.mouseOver=TRUE;
if(MouseOverCreditsButton)cred.mouseOver=TRUE;

if(MousePressedDown){
if(quit.mouseOver)*pState=QUIT;
if(play.mouseOver)*pState=PLAY;
if(cred.mouseOver)*pMenSt=CREDITS;
}

To further better this code I would take the MouseOverQuitButton logic (which is not a function, just a simplification for example purposes), which uses the mouse coordinates, and compares them to where the button is, and instead create a mouse “collision box”, which would be 1×1 unit in size. Then we can just use our old collision detection code and it’s really as simple as:

if(CheckCollision(ms.clBx, quit.loc))quit.mouseOver=TRUE;
if(CheckCollision(ms.clBx, play.loc))play.mouseOver=TRUE;
if(CheckCollision(ms.clBx, cred.loc))cred.mouseOver=TRUE;

Which is elegant, and it makes me feel smart to repurpose collision detection code for use in the menus. It feel both like a satisfying hack, as well as a refined solution.

I’ve become a huge fan of code that shaves off lines of code, and it’s important to understand the difference between moving code, and reducing it. It would be easy for me to take the rendering code out for battlestate.cpp, and replace it all with one function call, but that would result in me jumping back and forth between the two functions trying to figure things out. Having said that, I didn’t realize it, the more I look at the rendering code, the more similar it all looks to me. It follows this pattern:

BEGIN
ForAllTheNumberOfEnemiesOfThisType
FindTheCenterOfTheImage
RenderAccordingToState //eg. ALIVE or DEAD
END

For the hunters, that looks like this:

//render hunters
if(renderHunters){        //debugging flag
for (int i=0; i<lev.wvs[wvNum].nmHnts; i++){
for (int i = 0; i < lev.wvs[wvNum].nmHnts; i++){
SDL_Point cntHnt = {hnts.rBx[i].w/2, hnts.rBx[i].h/2};
if (hnts.hlth[i] > 0){
SDL_RenderCopyEx(pFrame, pHunterImg, NULL, &hnts.rBx[i], hnts.angle[i], &cntHnt, SDL_FLIP_NONE);
} else {
SDL_RenderCopyEx(pFrame, pHntDead, NULL, &hnts.rBx[i], hnts.angle[i], &cntHnt, SDL_FLIP_NONE);
}
}
}
}

I know that there’s definitely a way to write this as cleaner, but how? The answer can be found by looking at the characters (hunters, PC, snipers, etc). They all have a bunch of repeated code. The code is:

float xVel[8], yVel[8];
float xPos[8], yPos[8];
tdcRct clBx[8];
SDL_Rect rBx[8];
float angle[8];
float hlth[8];

Hmmmmm. Why not just create a struct that contains all of that code, and then give one of them to each character controller?

struct characterBase{
float xVel, yVel, xPos, yPos, hlth, angle;
tdcRct clBx;
SDL_Rect rBx;
};

Now for the hunter struct, as an example, it’s going to look like this:

struct Hunters{
characterBase bs[8];
bool charging[8];
bool spawned[8];
…and so on.
};

This at first sounds like some good housekeeping. We prevent a ton of duplicate code from cluttering up the characters.h, but the benefits go far deeper than that. This gives us much more flexibility to wrap up our rendering. We can now recreate our rendering as follows:

if(renderHunter) RenderEnemy(&hnts.bs, numHnts, HUNTERS, pFrame);

Where RenderEnemy is a function that takes an array of characterBase type, the number of characters to render, and the type of enemy to render. That way we might write a function like:

//where enemyType is an enumerated value, more on those later.
void RenderEnemyType(characterBase* pBase, numEnemies, enemyType TYPE, SDL_Renderer* pRend){
SDL_Point imgCnt;
for(int i=0; i<numEnemies; i++){
switch(TYPE){
SDL_RenderCopyEx(pFrame, pBlt, NULL, &blts.rBx[i], blts.angle[i], &cntBlt, SDL_FLIP_NONE);
case HUNTER:
imgCnt={&pBase->rBx[i].x-(HUNTER_SIZE/2), &pBase->rBx[i].y-(HUNTER_SIZE/2);
SDL_RenderCopyEx(pRend, pHuntImg, NULL, &pBase->rBx[i], pBase->angle[i], &imgCnt, SDL_FLIP_NONE);
break;
case SNIPER:
imgCnt={&pBase->rBx[i].x-(SNIPER_SIZE/2), &pBase->rBx[i].y-(SNIPER_SIZE/2);
SDL_RenderCopyEx(pRend, pSnprImg, NULL, &pBase->rBx[i], pBase->angle[i], &imgCnt, SDL_FLIP_NONE);
break;
case PLAYER_CHAR:


break;
case BURNSPOT:


break;
//etcetera.
}
}
}

This might all seem kind of long and inelegant, but it makes the actual rendering code in the program take up a lot less space. Compare that to:

//render bolts
if (renderBolts){
for(int i=0; i<128; i++){
if (blts.isAlive[i]){
blts.angle[i] = FindAngle(blts.yVel[i], blts.xVel[i]);
SDL_Point cntBlt = {BOLT_SIZE/2*dt.scl, BOLT_SIZE/2*dt.scl};
SDL_RenderCopyEx(pFrame, pBlt, NULL, &blts.rBx[i], blts.angle[i], &cntBlt, SDL_FLIP_NONE);
}
}
}

//render healthpack(s)
if (renderHealthPacks){
for (int i = 0; i < lev.wvs[wvNum].nmPks; i++){
if (hlPks.isActivated[i] == true){
SDL_RenderCopy(pFrame, pHealthPacksImg, NULL, &hlPks.rBx[i]);
}
}
}

//render burn spots
if (burnSpots){
for (int i = 0; i < lev.wvs[wvNum].nmSpts; i++){
if (tdcSpots.isActive[i] == true){
SDL_RenderCopy(pFrame, pBurnImg, NULL, &tdcSpots.rBx[i]);
} else {
SDL_RenderCopy(pFrame, pPreBurn, NULL, &tdcSpots.rBx[i]);
}
}
}

vs:

if (renderBolts) RenderEnemyType(&blts.bs, nmBlts, BOLTS, pFrame);
if (renderHealthPacks) RenderEnemyType(&hlPks.bs, nmPks, HEALTH, pFrame);
if (renderBurnSpots) RenderEnemyType(&brnSpts.bs, nmBnSpts, BURN, pFrame);

It’s a clear win, although I can’t help but feel that I can do better. I’ve been thinking about giving each enemy an enemyType enum value, and then grouping all the enemy characterBase’s into one big linked list, where we can then pass the head into one function and have it all render there.

The biggest problem I have with doing anything like this for collision detection, is that every enemy handles collisions a little bit differently. Some objects are immovable, such as the turrets, and some are not, such as the hunters. The player character is a special case, and the hunters have special “bouncing” behaviour that I didn’t want for any of the other enemies, such as the snipers. I know that I can do this theoretically, my thoughts are just too murky right now.

Unfortunately wordpress has basically made this post unreadable, especially to the end, so here’s a more wordy example that’s lighter on code. I wrote earlier about loading in all the textures in the game at the intro screen and then passing them around to the appropriate gamestate before finally deleting them just before the program is closed. If I did that with textures I would do that with sound effects and music clips as well. To make things easier I would create a struct called Media that contains pointers to the heads of the linked lists that store the textures, sound effects, and music clips:

struct Media{
TextList* img;
SFXList* sfx;
MusicList* msc;
};

Then, instead of passing in three naked head pointers, I just pass in a const copy of the Media struct, to each of the gamestates. This saves on typing and is more safe. There are also some miscellaneous things we can put in Media, such as the visual scaling information and functions.

Likewise, I got tired of passing in a bunch of different states to functions such as:

Menu(&state, &menuState, &tutorial, &menSub, pFrame, &level, pMsc, dt, &turs, &df);

Here we have a function that takes the general state of the game, the type of menu state, the type of tutorial state, the specific menu screen state, the renderer, a pointer to a level struct (later abandoned), a pointer to the current music playing, a copy of graphical scaling information, the number of turrets we start the game with, and the difficulty of the game. There’s actually a lot of horrible coding design here, but this would be a lot better if it was:

Menu(states, media, nmTrs, &dif);

We could accomplish this by both creating the Media struct detailed above, and a struct such as:

struct States{
gameState* pGmSt;
menuState* pMnSt;
menuSubState* pMnSubSt;
tutorialState* pTutSt;
};

That we then create an instance of and pass around throughout our code.

Having to pass in the number of turrets and the difficulty level to our menu state is mostly because I was somewhat rushed finishing the game, and I would probably redesign that. The reason those need to be there is because the Menu state and the CTF state, which is where the game actually takes place, are at the same level of scope, so nothing can be passed directly from one to the other.

This entry was posted in Game Design, Monochrome, Programming and tagged , . Bookmark the permalink.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s